monitor: capops: ownership xfer, delete, retype, revoke: delete temp capabilities...

The monitor domain did not cleanup root cnode capabilities which it receives
on a temporary basis during some of the distributed capability operations.
Depending on operation order this led to a steadily growing number of
capabilities on some cores.

Signed-off-by: Simon Gerber <simon.gerber@inf.ethz.ch>

diff --git a/usr/monitor/capops/capsend.c b/usr/monitor/capops/capsend.c
index fe365d6..edcebca 100644 (file)
--- a/usr/monitor/capops/capsend.c
+++ b/usr/monitor/capops/capsend.c
@@ -8,6 +8,7 @@
  */
 
 #include <barrelfish/barrelfish.h>
+#include <barrelfish/deferred.h>
 #include "capsend.h"
 #include "monitor.h"
 #include "capops.h"
@@ -823,6 +824,39 @@ owner_updated__rx_handler(struct intermon_binding *b, genvaddr_t st)
     free(uo_bc_st);
 }
 
+struct delayed_cleanup_st {
+    struct deferred_event d;
+    struct event_closure ev;
+    struct capref capref;
+    delayus_t delay;
+};
+
+static void defer_free_owner_rx_cap(struct delayed_cleanup_st *st)
+{
+    errval_t err;
+    deferred_event_init(&st->d);
+    err = deferred_event_register(&st->d, get_default_waitset(), st->delay, st->ev);
+    if (err_is_fail(err)) {
+        DEBUG_ERR(err, "unable to register deferred event, leaking cap");
+        free(st);
+    }
+}
+
+static void free_owner_rx_cap(void *st_)
+{
+    errval_t err;
+    struct delayed_cleanup_st *st = st_;
+    err = cap_destroy(st->capref);
+    if (err_no(err) == SYS_ERR_CAP_LOCKED) {
+        // exponential backoff
+        st->delay *= 2;
+        defer_free_owner_rx_cap(st);
+        return;
+    }
+    PANIC_IF_ERR(err, "cap cleanup after update_owner_rx");
+    free(st);
+}
+
 void
 update_owner__rx_handler(struct intermon_binding *b, intermon_caprep_t caprep, genvaddr_t st)
 {
@@ -845,14 +879,26 @@ update_owner__rx_handler(struct intermon_binding *b, intermon_caprep_t caprep, g
     }
     if (err_no(err) == SYS_ERR_CAP_NOT_FOUND) {
         err = SYS_ERR_OK;
+        slot_free(capref);
+        goto reply;
     }
 
     if (err_is_fail(err)) {
         USER_PANIC_ERR(err, "failed to update cap ownership");
     }
 
-    cap_destroy(capref);
+    err = cap_destroy(capref);
+    if (err_no(err) == SYS_ERR_CAP_LOCKED) {
+        // ownership updates still in flight, delete cap later
+        struct delayed_cleanup_st *dst = malloc(sizeof(*dst));
+        assert(dst);
+        dst->capref = capref;
+        dst->ev = MKCLOSURE(free_owner_rx_cap, dst);
+        dst->delay = 1000; // 1ms delay
+        defer_free_owner_rx_cap(dst);
+    }
 
+reply:
     err = owner_updated(from, st);
     if (err_is_fail(err)) {
         USER_PANIC_ERR(err, "failed to send ownership update response");

diff --git a/usr/monitor/capops/delete.c b/usr/monitor/capops/delete.c
index 1ff27f7..65028b1 100644 (file)
--- a/usr/monitor/capops/delete.c
+++ b/usr/monitor/capops/delete.c
@@ -52,6 +52,9 @@ delete_result__rx(errval_t status, struct delete_st *del_st, bool locked)
     delete_result_handler_t handler = del_st->result_handler;
     void *st = del_st->st;
     free(del_st);
+    // Delete our copy of domain's rootcn
+    err = cap_destroy(del_st->capref.croot);
+    PANIC_IF_ERR(err, "cleaning up domain's rootcn");
     handler(status, st);
 }
 

diff --git a/usr/monitor/capops/move.c b/usr/monitor/capops/move.c
index 5e03b75..9b6bf17 100644 (file)
--- a/usr/monitor/capops/move.c
+++ b/usr/monitor/capops/move.c
@@ -217,6 +217,8 @@ move_request__rx_handler(struct intermon_binding *b, intermon_caprep_t caprep, u
         goto reset_owner;
     }
 
+    // If broadcast doesn't fail, unlock cap only after all nodes have
+    // acknowledged ownership change, see free_owner_recv_cap().
     err = capsend_update_owner(domcapref, MKCONT(free_owner_recv_cap, capref));
     if (err_is_fail(err)) {
         goto reset_owner;

diff --git a/usr/monitor/capops/retype.c b/usr/monitor/capops/retype.c
index f84bcd0..f383f90 100644 (file)
--- a/usr/monitor/capops/retype.c
+++ b/usr/monitor/capops/retype.c
@@ -367,12 +367,12 @@ retype_check__rx(errval_t status, struct retype_check_st* check,
                  struct retype_output_st *output, void *to_free)
 {
     DEBUG_CAPOPS("%s\n", __FUNCTION__);
-    errval_t err = status;
-    if (err_is_ok(err)) {
+    errval_t err;
+    struct domcapref *src = &check->src;
+    struct domcapref *destcn = &output->destcn;
+    if (err_is_ok(status)) {
         // the retype may procede
-        struct domcapref *src = &check->src;
-        struct domcapref *destcn = &output->destcn;
-        err = monitor_create_caps(src->croot, destcn->croot, check->type,
+        status = monitor_create_caps(src->croot, destcn->croot, check->type,
                                   check->objsize, check->count, src->cptr,
                                   src->level, check->offset, destcn->cptr,
                                   destcn->level, output->start_slot);
@@ -380,7 +380,12 @@ retype_check__rx(errval_t status, struct retype_check_st* check,
     struct result_closure cont = output->cont;
     assert(cont.handler);
     free(to_free);
-    CALLRESCONT(cont, err);
+    // Delete copies of domain's src/dest root cnodes
+    err = cap_destroy(src->croot);
+    PANIC_IF_ERR(err, "deleting monitor's copy of src rootcn");
+    err = cap_destroy(destcn->croot);
+    PANIC_IF_ERR(err, "deleting monitor's copy dest rootcn");
+    CALLRESCONT(cont, status);
 }
 
 /**

diff --git a/usr/monitor/capops/revoke.c b/usr/monitor/capops/revoke.c
index fdd2df8..c0d5ce9 100644 (file)
--- a/usr/monitor/capops/revoke.c
+++ b/usr/monitor/capops/revoke.c
@@ -139,6 +139,8 @@ revoke_result__rx(errval_t result,
     DEBUG_CAPOPS("%s ## revocation completed, calling %p\n", __FUNCTION__,
                  st->result_handler);
 
+    err = cap_destroy(st->cap.croot);
+    PANIC_IF_ERR(err, "deleting monitor's copy of rootcn");
     st->result_handler(result, st->st);
     free(st);
 }