Improved naming & cleanup in copy capop.
authorMark Nevill <nevillm@ethz.ch>
Sun, 3 Jun 2012 19:19:37 +0000 (21:19 +0200)
committerSimon Gerber <simon.gerber@inf.ethz.ch>
Thu, 18 Jul 2013 12:58:34 +0000 (14:58 +0200)
usr/monitor/capops/copy.c
usr/monitor/capops/init.c
usr/monitor/capops/internal.h

index 6526a5c..f5a9f6a 100644 (file)
  */
 
 struct cap_copy_rpc_st {
-    // caller/sender st
+    /// caller/sender st
     genvaddr_t st;
-    // sender if acting as intermediary
+    /// sender if acting as intermediary
     coreid_t from;
-    // cap that is being copied out
+    /// cap that is being copied out
     struct capref cap;
-    // result handler if being called directly
+    /// result handler if being called directly
     copy_result_handler_t result_handler;
-    // whether the local cap should be deleted when the rpc is complete
+    /// whether the local cap should be deleted when the rpc is complete
     bool delete_after;
-    // cap was last copy on request source (only relevant if delete_after is true)
+    /// cap was last copy on request source (only relevant if delete_after is true)
     bool is_last;
 };
 
-/*
- * Send operations {{{1
- */
 
 /*
- * Copy result {{{2
+ * Copy result {{{1
  */
 
-// send state struct for recv_copy_result
+/**
+ * \brief Send state struct for recv_copy_result
+ */
 struct recv_copy_result_msg_st {
     struct intermon_msg_queue_elem queue_elem;
     errval_t status;
@@ -54,22 +53,30 @@ struct recv_copy_result_msg_st {
     genvaddr_t st;
 };
 
-// send queue continuation for recv_copy_result
+/**
+ * \brief Intermon is ready to send recv_copy_result
+ */
 static void
-recv_copy_result_send_cont(struct intermon_binding *b, struct intermon_msg_queue_elem *e)
+recv_copy_result_send__rdy(struct intermon_binding *b,
+                           struct intermon_msg_queue_elem *e)
 {
     errval_t err;
     struct recv_copy_result_msg_st *msg_st = (struct recv_copy_result_msg_st*)e;
-    err = intermon_capops_recv_copy_result__tx(b, NOP_CONT, msg_st->status, msg_st->capaddr, msg_st->vbits, msg_st->slot, msg_st->st);
+    err = intermon_capops_recv_copy_result__tx(b, NOP_CONT, msg_st->status,
+                                               msg_st->capaddr, msg_st->vbits,
+                                               msg_st->slot, msg_st->st);
     if (err_is_fail(err)) {
         USER_PANIC_ERR(err, "failed to send recv_copy_result");
     }
     free(msg_st);
 }
 
-// enqueueing function for recv_copy_result
+/**
+ * \brief A recv_copy_result needs to be enqueued
+ */
 static errval_t
-recv_copy_result(coreid_t dest, errval_t status, capaddr_t capaddr, uint8_t vbits, cslot_t slot, genvaddr_t st)
+recv_copy_result__enq(coreid_t dest, errval_t status, capaddr_t capaddr,
+                      uint8_t vbits, cslot_t slot, genvaddr_t st)
 {
     errval_t err;
 
@@ -78,7 +85,7 @@ recv_copy_result(coreid_t dest, errval_t status, capaddr_t capaddr, uint8_t vbit
     if (!msg_st) {
         return LIB_ERR_MALLOC_FAIL;
     }
-    msg_st->queue_elem.cont = recv_copy_result_send_cont;
+    msg_st->queue_elem.cont = recv_copy_result_send__rdy;
     msg_st->status = status;
     msg_st->capaddr = capaddr;
     msg_st->vbits = vbits;
@@ -95,44 +102,45 @@ recv_copy_result(coreid_t dest, errval_t status, capaddr_t capaddr, uint8_t vbit
     return SYS_ERR_OK;
 }
 
+/**
+ * \brief Shortcut for when a error recv_copy_result needs to be enqueued
+ */
 inline static errval_t
-recv_copy_error_result(coreid_t dest, errval_t status, genvaddr_t st)
+recv_copy_error_result__enq(coreid_t dest, errval_t status, genvaddr_t st)
 {
-    return recv_copy_result(dest, status, 0, 0, 0, st);
+    return recv_copy_result__enq(dest, status, 0, 0, 0, st);
 }
 
 /*
- * Copy from owner to dest (possibly as intermediary) {{{2
+ * Copy from owner to dest (possibly as intermediary) {{{1
  */
 
-// send state struct for owner_copy
+/**
+ * \brief Send state struct for owner_copy
+ */
 struct owner_copy_msg_st {
     struct intermon_msg_queue_elem queue_elem;
-    struct capref capref;
     intermon_caprep_t caprep;
     uint8_t owner_relations;
     genvaddr_t st;
 };
 
-// send queue continuation for owner_copy
+/**
+ * \brief Intermon is ready to send owner_copy
+ */
 static void
-owner_copy_send_cont(struct intermon_binding *b, struct intermon_msg_queue_elem *e)
+owner_copy_send__rdy(struct intermon_binding *b,
+                     struct intermon_msg_queue_elem *e)
 {
     struct owner_copy_msg_st *msg_st = (struct owner_copy_msg_st*)e;
     struct cap_copy_rpc_st *rpc_st = (struct cap_copy_rpc_st*)(msg_st->st);
     assert(rpc_st);
-    errval_t err, cleanup_err;
+    errval_t err;
 
     err = intermon_capops_recv_copy__tx(b, NOP_CONT, msg_st->caprep,
                                         msg_st->owner_relations, msg_st->st);
 
-    if (rpc_st->from != my_core_id) {
-        // need to cleanup cap allocated in request_copy__rx_handler
-        cleanup_err = cap_destroy(msg_st->capref);
-        if (err_is_fail(cleanup_err)) {
-            DEBUG_ERR(err, "cleaning up send state");
-        }
-    }
+    // message sent or failed, cleanup message state
     free(msg_st);
     msg_st = NULL;
 
@@ -140,30 +148,19 @@ owner_copy_send_cont(struct intermon_binding *b, struct intermon_msg_queue_elem
         // skip rpc cleanup if message sent successfully
         return;
     }
-
-    if (rpc_st->from != my_core_id) {
-        // source is another core (we're acting as intermediary)
-        assert(!rpc_st->result_handler);
-        cleanup_err = recv_copy_error_result(rpc_st->from, err, rpc_st->st);
-        if (err_is_fail(cleanup_err)) {
-            DEBUG_ERR(err, "sending recv_copy from owner");
-            USER_PANIC_ERR(cleanup_err, "failed to send recv_copy_result for recv_copy error");
-        }
-    }
     else {
-        // source is this core, call result handler directly
-        if (rpc_st->result_handler) {
-            rpc_st->result_handler(err, 0, 0, 0, (void*)rpc_st->st);
-        }
+        // Send failed, report result
+        recv_copy_result__rx(b, err, 0, 0, 0, (genvaddr_t)rpc_st);
     }
-    free(rpc_st);
 }
 
-// enqueueing function for owner_copy
+/**
+ * \brief An owner_copy is to be enqueued
+ */
 static errval_t
-owner_copy(struct capref capref, struct capability *cap, coreid_t from,
-           coreid_t dest, bool give_away, copy_result_handler_t result_handler,
-           genvaddr_t st)
+owner_copy__enq(struct capref capref, struct capability *cap, coreid_t from,
+                coreid_t dest, bool give_away,
+                copy_result_handler_t result_handler, genvaddr_t st)
 {
     errval_t err;
 
@@ -203,6 +200,14 @@ owner_copy(struct capref capref, struct capability *cap, coreid_t from,
         }
     }
 
+    /*
+     * Here, we're handling the give_away parameter. If set, the copy operation
+     * is also responsible for deleting the input cap. In the case when the
+     * input cap has no other copies, this allows the copy operation to
+     * optimize transfer across cores by simultaneously passing over ownership
+     * (this is called a "true" give away below).
+     */
+
     // unless we're performing a "true" give_away, set the remote relations
     // copy bit
     if (!(rpc_st->delete_after && rpc_st->is_last)) {
@@ -231,7 +236,7 @@ owner_copy(struct capref capref, struct capability *cap, coreid_t from,
         free(rpc_st);
         return LIB_ERR_MALLOC_FAIL;
     }
-    msg_st->queue_elem.cont = owner_copy_send_cont;
+    msg_st->queue_elem.cont = owner_copy_send__rdy;
     capability_to_caprep(cap, &msg_st->caprep);
     msg_st->owner_relations = remote_relations;
     msg_st->st = (genvaddr_t)rpc_st;
@@ -248,10 +253,12 @@ owner_copy(struct capref capref, struct capability *cap, coreid_t from,
 }
 
 /*
- * Copy request from non-owner to owner {{{2
+ * Copy request from non-owner to owner {{{1
  */
 
-// send state struct for request_copy
+/**
+ * \brief Send state struct for request_copy
+ */
 struct request_copy_msg_st {
     struct intermon_msg_queue_elem queue_elem;
     intermon_caprep_t caprep;
@@ -259,9 +266,12 @@ struct request_copy_msg_st {
     struct cap_copy_rpc_st *st;
 };
 
-// send queue continuation for request_copy
+/**
+ * \brief Intermon is ready to send request_copy
+ */
 static void
-request_copy_send_cont(struct intermon_binding *b, struct intermon_msg_queue_elem *e)
+request_copy_send__rdy(struct intermon_binding *b,
+                       struct intermon_msg_queue_elem *e)
 {
     errval_t err;
     struct request_copy_msg_st *msg_st = (struct request_copy_msg_st*)e;
@@ -277,10 +287,12 @@ request_copy_send_cont(struct intermon_binding *b, struct intermon_msg_queue_ele
     free(msg_st);
 }
 
-// enqueueing function for request_copy
+/**
+ * \brief A copy request is to be enqueued
+ */
 static errval_t
-request_copy(struct capref capref, coreid_t dest, bool give_away,
-             copy_result_handler_t result_handler, genvaddr_t st)
+request_copy__enq(struct capref capref, coreid_t dest, bool give_away,
+                  copy_result_handler_t result_handler, genvaddr_t st)
 {
     errval_t err;
     struct capability cap;
@@ -304,12 +316,13 @@ request_copy(struct capref capref, coreid_t dest, bool give_away,
     rpc_st->result_handler = result_handler;
 
     // create send state
-    struct request_copy_msg_st *msg_st = calloc(1, sizeof(struct request_copy_msg_st));
+    struct request_copy_msg_st *msg_st
+        = calloc(1, sizeof(struct request_copy_msg_st));
     if (!msg_st) {
         free(rpc_st);
         return LIB_ERR_MALLOC_FAIL;
     }
-    msg_st->queue_elem.cont = request_copy_send_cont;
+    msg_st->queue_elem.cont = request_copy_send__rdy;
     msg_st->dest = dest;
     capability_to_caprep(&cap, &msg_st->caprep);
     msg_st->st = rpc_st;
@@ -326,13 +339,16 @@ request_copy(struct capref capref, coreid_t dest, bool give_away,
 }
 
 /*
- * Receive handlers {{{1
+ * Receive copy result {{{1
  */
 
+/**
+ * \brief Result from sending copy to dest has been received
+ */
 void
-recv_copy_result__rx_handler(struct intermon_binding *b, errval_t status,
-                             capaddr_t capaddr, uint8_t vbits, cslot_t slot,
-                             genvaddr_t st)
+recv_copy_result__rx(struct intermon_binding *b, errval_t status,
+                     capaddr_t capaddr, uint8_t vbits, cslot_t slot,
+                     genvaddr_t st)
 {
     assert(st);
     errval_t err;
@@ -348,7 +364,12 @@ recv_copy_result__rx_handler(struct intermon_binding *b, errval_t status,
         if (err_is_fail(err)) {
             USER_PANIC_ERR(err, "destroying temp cap for f-to-f copy");
         }
-        recv_copy_result(rpc_st->from, status, capaddr, vbits, slot, rpc_st->st);
+        err = recv_copy_result__enq(rpc_st->from, status, capaddr, vbits, slot,
+                                    rpc_st->st);
+        if (err_is_fail(err)) {
+            DEBUG_ERR(status, "sending copy from owner");
+            USER_PANIC_ERR(err, "failed to send recv_copy_result");
+        }
     }
     else {
         // origin of copy
@@ -385,11 +406,18 @@ recv_copy_result__rx_handler(struct intermon_binding *b, errval_t status,
     free(rpc_st);
 }
 
+/*
+ * Receive cap copy {{1
+ */
+
+/**
+ * \brief A cap has been received
+ */
 void
-recv_copy__rx_handler(struct intermon_binding *b, intermon_caprep_t caprep,
-                      uint8_t owner_relations, genvaddr_t st)
+recv_copy__rx(struct intermon_binding *b, intermon_caprep_t caprep,
+              uint8_t owner_relations, genvaddr_t st)
 {
-    errval_t err;
+    errval_t err, err2;
     struct intermon_state *inter_st = (struct intermon_state*)b->st;
     coreid_t from = inter_st->core_id;
     assert(from != my_core_id);
@@ -398,6 +426,7 @@ recv_copy__rx_handler(struct intermon_binding *b, intermon_caprep_t caprep,
 
     caprep_to_capability(&caprep, &cap);
 
+    // get a slot to put the cap
     err = slot_alloc(&dest);
     if (err_is_fail(err)) {
         dest = NULL_CAP;
@@ -428,6 +457,8 @@ recv_copy__rx_handler(struct intermon_binding *b, intermon_caprep_t caprep,
         // may fail if given owner does not match owner of existing copies
         goto free_slot;
     }
+
+    // if we are now owner we need to set up the relations
     if (owner == my_core_id && owner_relations) {
         err = monitor_remote_relations(dest, owner_relations, ~(uint8_t)0, NULL);
         if (err_is_fail(err)) {
@@ -438,22 +469,32 @@ recv_copy__rx_handler(struct intermon_binding *b, intermon_caprep_t caprep,
     goto send_result;
 
 free_slot:
-    slot_free(dest);
+    err2 = slot_free(dest);
+    if (err_is_fail(err2)) {
+        DEBUG_ERR(err2, "slot_free failed, slot will leak");
+    }
     dest = NULL_CAP;
 
 send_result:
-    recv_copy_result(from, err, get_cnode_addr(dest), get_cnode_valid_bits(dest), dest.slot, st);
+    err2 = recv_copy_result__enq(from, err, get_cnode_addr(dest),
+                                 get_cnode_valid_bits(dest), dest.slot, st);
+    if (err_is_fail(err2)) {
+        USER_PANIC_ERR(err2, "recv_copy_result enque failed, cap will leak");
+    }
 }
 
+/**
+ * \brief A copy request has been received
+ */
 void
-request_copy__rx_handler(struct intermon_binding *b, coreid_t dest,
-                         intermon_caprep_t caprep, genvaddr_t st)
+request_copy__rx(struct intermon_binding *b, coreid_t dest,
+                 intermon_caprep_t caprep, genvaddr_t st)
 {
-    errval_t err, send_err;
+    errval_t err, err2;
     struct intermon_state *inter_st = (struct intermon_state*)b->st;
     coreid_t from = inter_st->core_id;
     assert(from != my_core_id);
-    struct capref capref;
+    struct capref capref = NULL_CAP;
     memset(&capref, 0, sizeof(capref));
     struct capability cap;
     caprep_to_capability(&caprep, &cap);
@@ -471,53 +512,60 @@ request_copy__rx_handler(struct intermon_binding *b, coreid_t dest,
     }
     err = monitor_copy_if_exists(&cap, capref);
     if (err_is_fail(err)) {
-        DEBUG_ERR(err, "while finding copy in request_copy");
-        goto send_err;
+        goto free_slot;
     }
     err = cap_get_state(capref, &state);
     if (err_is_fail(err)) {
-        goto send_err;
+        goto destroy_cap;
     }
     if (distcap_state_is_foreign(state)) {
         err = MON_ERR_CAP_FOREIGN;
-        goto send_err;
+        goto destroy_cap;
     }
     if (distcap_state_is_busy(state)) {
         err = MON_ERR_REMOTE_CAP_RETRY;
-        goto send_err;
+        goto destroy_cap;
     }
 
     if (dest == my_core_id) {
         // tried to send copy to owning core, success!
-        recv_copy_result(from, SYS_ERR_OK, get_cnode_addr(capref),
-                         get_cnode_valid_bits(capref), capref.slot, st);
-    }
-    else {
-        // mark cap as having remote copies
-        err = monitor_remote_relations(capref, RRELS_COPY_BIT, RRELS_COPY_BIT, NULL);
+        err = recv_copy_result__enq(from, SYS_ERR_OK, get_cnode_addr(capref),
+                                    get_cnode_valid_bits(capref), capref.slot,
+                                    st);
         if (err_is_fail(err)) {
-            goto send_err;
+            USER_PANIC_ERR(err, "sending result to request_copy sender");
         }
-
+    }
+    else {
         // forward copy to destination core
-        err = owner_copy(capref, &cap, from, dest, true, NULL, st);
+        err = owner_copy__enq(capref, &cap, from, dest, true, NULL, st);
         if (err_is_fail(err)) {
-            goto send_err;
+            goto destroy_cap;
         }
     }
 
-    goto end;
+    return;
 
-send_err:
-    send_err = recv_copy_error_result(from, err, st);
-    if (err_is_fail(send_err)) {
-        err_push(send_err, err);
-        USER_PANIC_ERR(err, "failed to send error to request_copy sender");
+destroy_cap:
+    err2 = cap_delete(capref);
+    if (err_is_fail(err2)) {
+        DEBUG_ERR(err, "handling copy request");
+        USER_PANIC_ERR(err2, "deleting received cap");
+    }
+
+free_slot:
+    err2 = slot_free(capref);
+    if (err_is_fail(err2)) {
+        DEBUG_ERR(err, "handling copy request");
+        USER_PANIC_ERR(err2, "freeing slot for cap recv");
     }
 
-end:
-    // cleanup temporary copy of cap
-    cap_destroy(capref);
+send_err:
+    err2 = recv_copy_error_result__enq(from, err, st);
+    if (err_is_fail(err2)) {
+        DEBUG_ERR(err, "handling copy request");
+        USER_PANIC_ERR(err2, "sending error to request_copy sender");
+    }
 }
 
 /*
@@ -568,8 +616,8 @@ capops_copy(struct capref capref, coreid_t dest, bool give_away,
 
     if (distcap_state_is_foreign(state)) {
         // sending copy from non-owner, send copy request to owner
-        return request_copy(capref, dest, give_away, result_handler,
-                            (genvaddr_t)st);
+        return request_copy__enq(capref, dest, give_away, result_handler,
+                                 (genvaddr_t)st);
     }
     else {
         // sending copy from owner
@@ -577,7 +625,7 @@ capops_copy(struct capref capref, coreid_t dest, bool give_away,
         if (err_is_fail(err)) {
             return err;
         }
-        return owner_copy(capref, &cap, my_core_id, dest, give_away,
-                          result_handler, (genvaddr_t)st);
+        return owner_copy__enq(capref, &cap, my_core_id, dest, give_away,
+                               result_handler, (genvaddr_t)st);
     }
 }
index 7d18047..4f75bb9 100644 (file)
@@ -4,9 +4,9 @@
 
 errval_t capops_intermon_init(struct intermon_binding *b)
 {
-    b->rx_vtbl.capops_request_copy            = request_copy__rx_handler;
-    b->rx_vtbl.capops_recv_copy               = recv_copy__rx_handler;
-    b->rx_vtbl.capops_recv_copy_result        = recv_copy_result__rx_handler;
+    b->rx_vtbl.capops_request_copy            = request_copy__rx;
+    b->rx_vtbl.capops_recv_copy               = recv_copy__rx;
+    b->rx_vtbl.capops_recv_copy_result        = recv_copy_result__rx;
     b->rx_vtbl.capops_move_request            = move_request__rx_handler;
     b->rx_vtbl.capops_move_result             = move_result__rx_handler;
     b->rx_vtbl.capops_delete_remote           = delete_remote__rx_handler;
index b066131..edde7e7 100644 (file)
@@ -10,14 +10,13 @@ void find_descendants_result__rx_handler(struct intermon_binding *b,
 void owner_updated__rx_handler(struct intermon_binding *b, genvaddr_t st);
 void update_owner__rx_handler(struct intermon_binding *b,
                               intermon_caprep_t caprep, genvaddr_t st);
-void recv_copy_result__rx_handler(struct intermon_binding *b, errval_t status,
-                                  capaddr_t capaddr, uint8_t vbits,
-                                  cslot_t slot, genvaddr_t st);
-void recv_copy__rx_handler(struct intermon_binding *b,
-                           intermon_caprep_t caprep, uint8_t owner_relations,
-                           genvaddr_t st);
-void request_copy__rx_handler(struct intermon_binding *b, coreid_t dest,
-                              intermon_caprep_t caprep, genvaddr_t st);
+void recv_copy_result__rx(struct intermon_binding *b, errval_t status,
+                          capaddr_t capaddr, uint8_t vbits, cslot_t slot,
+                          genvaddr_t st);
+void recv_copy__rx(struct intermon_binding *b, intermon_caprep_t caprep,
+                   uint8_t owner_relations, genvaddr_t st);
+void request_copy__rx(struct intermon_binding *b, coreid_t dest,
+                      intermon_caprep_t caprep, genvaddr_t st);
 void delete_remote__rx_handler(struct intermon_binding *b,
                                intermon_caprep_t caprep, genvaddr_t st);
 void delete_remote_result__rx_handler(struct intermon_binding *b,