Async error handling
authorBarret Rhoden <brho@cs.berkeley.edu>
Sat, 9 May 2009 05:45:01 +0000 (22:45 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Sat, 9 May 2009 06:33:20 +0000 (23:33 -0700)
Receives error return values and passes them back up the userspace
stack.  It's up to the top levels to deal with them.  cprintf currently
does not, and null does.

Also adjusted measure_ to handle an error (presumably RING_FULL), and
not try that iteration again.  The numbers may be a little odd from it
if the error doesn't happen on the first outer iteration.

Also, for any real measurements, note the final parameter of
process_generic_syscalls.

inc/lib.h
inc/measure.h
inc/null.h
lib/asynccall.c
lib/null.c
lib/printf.c
lib/syscall.c
user/hello.c
user/null.c

index 1741cf2..6854879 100644 (file)
--- a/inc/lib.h
+++ b/inc/lib.h
@@ -37,9 +37,9 @@ char* readline(const char *buf);
 
 // syscall.c
 void sys_null();
-void sys_null_async(syscall_desc_t* desc);
+error_t sys_null_async(syscall_desc_t* desc);
 void sys_cputs(const char *string, size_t len);
-void sys_cputs_async(const char *s, size_t len, syscall_desc_t* desc,
+error_t sys_cputs_async(const char *s, size_t len, syscall_desc_t* desc,
                      void (*cleanup_handler)(void*), void* cleanup_data);
 int    sys_cgetc(void);
 envid_t        sys_getenvid(void);
index 78edc87..0aac3af 100644 (file)
@@ -20,9 +20,9 @@
        uint64_t a = (1000000000LL/(iters) * ticks) / (env->env_tscfreq);          \
        if ((name))                                                                \
                cprintf("Measuring %s:\n"                                              \
-                       "    Total ticks:        %20lld\n"                             \
-                       "    Num Iterations:     %20d\n"                               \
-                       "    Time Per Iteration: %20lld\n",                            \
+                       "    Total ticks:          %20lld\n"                           \
+                       "    Num Iterations:       %20d\n"                             \
+                       "    Time Per Iteration:   %20lld\n",                          \
                name, ticks, (iters), a);                                          \
        ticks;                                                                     \
 })
        _desc_type* desc_name;                                                     \
        /* Could use an array of rsps, but we're throwing away the responses*/     \
        _rsp_type rsp;                                                             \
+       error_t err;                                                               \
+       uint32_t safe_i_iters = i_iters;                                           \
        cpuid(0, 0, 0, 0, 0);                                                      \
        ticks = read_tsc();                                                        \
        /* Run this a bunch of times to make sure its accurate */                  \
        for(int i=0; i< (o_iters); i++) {                                          \
-               for (int j = 0; j < (i_iters); j++) {                                  \
-                       func ;                                                             \
+               for (int j = 0; j < (safe_i_iters); j++) {                             \
+                       err = func ;                                                       \
+                       if (err)                                                           \
+                               (safe_i_iters) = j;                                            \
                        desc_array[j] = desc_name;                                         \
                }                                                                      \
-               for (int j = 0; j < (i_iters); j++) {                                  \
+               for (int j = 0; j < (safe_i_iters); j++) {                             \
                        wait_func(desc_array[j], &rsp);                                    \
                }                                                                      \
        }                                                                          \
        cpuid(0, 0, 0, 0, 0);                                                      \
        ticks = read_tsc() - ticks;                                                \
        /* Compute the average and print it */                                     \
-       uint64_t a = (1000000000LL/((o_iters)*(i_iters)) * ticks) /                \
+       uint64_t a = (1000000000LL/((o_iters)*(safe_i_iters)) * ticks) /           \
                     (env->env_tscfreq);                                           \
        if ((name))                                                                \
                cprintf("Measuring %s:\n"                                              \
-                       "    Total ticks:        %20lld\n"                             \
-                       "    Num Iterations:     %20d\n"                               \
-                       "    Time Per Iteration: %20lld\n",                            \
-               name, ticks, ((o_iters)*(i_iters)), a);                            \
+                       "    Total ticks:          %20lld\n"                           \
+                       "    Num Total Iterations: %20d\n"                             \
+                       "    Num Inner Iterations: %20d\n"                             \
+                       "    Time Per Iteration:   %20lld\n",                          \
+               name, ticks, ((o_iters)*(safe_i_iters)), safe_i_iters, a);         \
        ticks;                                                                     \
 })
 
index 6277484..b7987ed 100644 (file)
@@ -2,6 +2,6 @@
 #define ROS_INC_NULL_H
 
 void null();
-void null_async(async_desc_t** desc);
+error_t null_async(async_desc_t** desc);
 
 #endif // ROS_INC_NULL_H
index de5bbb2..dcbdb74 100644 (file)
@@ -15,9 +15,12 @@ error_t waiton_async_call(async_desc_t* desc, async_rsp_t* rsp)
 {
        syscall_rsp_t syscall_rsp;
        syscall_desc_t* d;
+       error_t err = 0;
+       if (!desc)
+               return -E_INVAL;
        while (!(LIST_EMPTY(&desc->syslist))) {
                d = LIST_FIRST(&desc->syslist);
-               waiton_syscall(d, &syscall_rsp);
+               err = MIN(waiton_syscall(d, &syscall_rsp), err);
                // TODO: processing the retval out of rsp here.  might be specific to
                // the async call.  do we want to accumulate?  return any negative
                // values?  depends what we want from the return value, so we might
@@ -34,7 +37,7 @@ error_t waiton_async_call(async_desc_t* desc, async_rsp_t* rsp)
                desc->cleanup(desc->data);
        // free the asynccall desc
        POOL_PUT(&async_desc_pool, desc);
-       return 0;
+       return err;
 }
 
 syscall_desc_t* get_sys_desc(async_desc_t* desc)
index 00a2aa5..b6df173 100644 (file)
@@ -11,9 +11,10 @@ void null()
        sys_null();
 }
 
-void null_async(async_desc_t** desc)
+error_t null_async(async_desc_t** desc)
 {
-       current_async_desc = get_async_desc();
+       if ((current_async_desc = get_async_desc()) == NULL)
+               return E_BUSY;
        *desc = current_async_desc;
-       sys_null_async(get_sys_desc(current_async_desc));
+       return sys_null_async(get_sys_desc(current_async_desc));
 }
index f64ecea..97a1bff 100644 (file)
@@ -81,14 +81,14 @@ static void cputs_async_cleanup(void* data)
        POOL_PUT(&print_buf_pool, (printbuf_t*)data);
 }
 
+// TODO: its a little difficult to pass back an error through vprintfmt
 static void putch_async(int ch, printbuf_t **b)
 {
        (*b)->buf[(*b)->idx++] = ch;
        if ((*b)->idx == BUF_SIZE) {
-               // will need some way to track the result of the syscall
+               // TODO - should check for a return value for sys_ and get_sys
                sys_cputs_async((*b)->buf, (*b)->idx, get_sys_desc(current_async_desc),
                                cputs_async_cleanup, *b);
-               // TODO - this isn't getting passed back properly
                // TODO - should check for a return value
                *b = get_free_buffer();
                (*b)->idx = 0;
@@ -98,16 +98,17 @@ static void putch_async(int ch, printbuf_t **b)
 
 static int vcprintf_async(const char *fmt, va_list ap)
 {
-       // start with an available buffer
+       // start with an available buffer.  TODO: check return value
        printbuf_t* b = get_free_buffer();
 
        b->idx = 0;
        b->cnt = 0;
        vprintfmt((void*)putch_async, (void**)&b, fmt, ap);
+       // TODO - should check for a return value for sys_
        sys_cputs_async(b->buf, b->idx, get_sys_desc(current_async_desc),
                        cputs_async_cleanup, b);
 
-       return b->cnt; // this is lying if we used more than one buffer
+       return b->cnt; // this is lying if we used more than one buffer (TODO)
 }
 
 int cprintf_async(async_desc_t** desc, const char *fmt, ...)
@@ -123,6 +124,7 @@ int cprintf_async(async_desc_t** desc, const char *fmt, ...)
        }
        // get a free async_desc for this async call, and save it in the per-thread
        // tracking variable (current_async_desc).  then pass it back out.
+       // TODO: check return value, return error_t
        current_async_desc = get_async_desc();
        *desc = current_async_desc;
        // This is the traditional (sync) cprintf code
@@ -130,6 +132,7 @@ int cprintf_async(async_desc_t** desc, const char *fmt, ...)
        cnt = vcprintf_async(fmt, ap);
        va_end(ap);
 
+       // TODO: return cnt via a parameter, and return an error_t?
        return cnt;
 }
 
index 7f1faf8..2de1a5c 100644 (file)
@@ -35,10 +35,12 @@ syscall(int num, uint32_t a1, uint32_t a2, uint32_t a3, uint32_t a4, uint32_t a5
        return ret;
 }
 
-static inline error_t async_syscall(syscall_req_t* req, syscall_desc_t* desc)
+static error_t async_syscall(syscall_req_t* req, syscall_desc_t* desc)
 {
        // Note that this assumes one global frontring (TODO)
-       // spin til there is room for our request.  ring size is currently 64.
+       // abort if there is no room for our request.  ring size is currently 64.
+       // we could spin til it's free, but that could deadlock if this same thread
+       // is supposed to consume the requests it is waiting on later.
        if (RING_FULL(&sysfrontring))
                return E_BUSY;
        // req_prod_pvt comes in as the previously produced item.  need to
@@ -58,7 +60,11 @@ static inline error_t async_syscall(syscall_req_t* req, syscall_desc_t* desc)
 // consider a timeout too
 error_t waiton_syscall(syscall_desc_t* desc, syscall_rsp_t* rsp)
 {
-       // this forces us to call wait in the order in which they are called
+       // Make sure we were given a desc with a non-NULL frontring.  This could
+       // happen if someone forgot to check the error code on the paired syscall.
+       if (!desc->sysfr)
+               return E_FAIL;
+       // this forces us to call wait in the order in which the syscalls are made.
        if (desc->idx != desc->sysfr->rsp_cons + 1)
                return E_DEADLOCK;
        while (!(RING_HAS_UNCONSUMED_RESPONSES(desc->sysfr)))
@@ -71,22 +77,22 @@ error_t waiton_syscall(syscall_desc_t* desc, syscall_rsp_t* rsp)
        return 0;
 }
 
-void sys_null_async(syscall_desc_t* desc)
+error_t sys_null_async(syscall_desc_t* desc)
 {
        syscall_req_t syscall = {SYS_null, 0, {[0 ... (NUM_SYS_ARGS-1)] 0}};
        desc->cleanup = NULL;
        desc->data = NULL;
-       async_syscall(&syscall, desc);
+       return async_syscall(&syscall, desc);
 }
 
-void sys_cputs_async(const char *s, size_t len, syscall_desc_t* desc,
+error_t sys_cputs_async(const char *s, size_t len, syscall_desc_t* desc,
                      void (*cleanup_handler)(void*), void* cleanup_data)
 {
        // could just hardcode 4 0's, will eventually wrap this marshaller anyway
        syscall_req_t syscall = {SYS_cputs, 0, {(uint32_t)s, len, [2 ... (NUM_SYS_ARGS-1)] 0} };
        desc->cleanup = cleanup_handler;
        desc->data = cleanup_data;
-       async_syscall(&syscall, desc);
+       return async_syscall(&syscall, desc);
 }
 
 void sys_null()
index e79c45e..6d7a6e6 100644 (file)
@@ -1,4 +1,3 @@
-// hello, world
 #include <inc/lib.h>
 
 #ifdef __DEPUTY__
@@ -8,17 +7,11 @@
 void umain(void)
 {
        cprintf("goodbye, world!\n");
-       // this is just their way of generating a pagefault..., until now!
        cprintf("i am environment %08x\n", env->env_id);
 
-       // async via shared mem
        cprintf("about to write to shared mem.  hope it gets printed.  blimey! \n");
-       // note that when using the cprintf family, we can't currently call again,
-       // since the library functions use the same buffer.  the last used string
-       // will be the one printed when the syscall is serviced, regardless of
-       // whether the actual syscall can handle multiples in flight.
        async_desc_t *desc1, *desc2, *desc3;
-       async_desc_t rsp1, rsp2, rsp3;
+       async_rsp_t rsp1, rsp2, rsp3;
        cprintf_async(&desc1, "Cross-Core call 1, coming from env %08x\n", env->env_id);
        cprintf("Call 1 is sent!\n");
        //cprintf_async(&desc2, "Cross-Core call 2, coming from env %08x\n", env->env_id);
@@ -34,6 +27,4 @@ void umain(void)
        // might as well spin, just to make sure nothing gets deallocated
        // while we're waiting to test the async call
        while (1);
-
-       //check that my shit is done
 }
index b91186e..7da24d9 100644 (file)
@@ -30,7 +30,9 @@ void umain(void)
        //measure_function(cprintf("Reg Sync call  \n"), 10, "printf");
        //measure_function_async(cprintf_async(&desc, "Cross-Core call\n"), desc, 10,\
        //                       1, "Async Printf");
-       measure_async_call(null_async(&desc), desc, 10, 1, "Async Null");
+       // Note this won't actually do 100 inner runs (first parameter).  will stop
+       // making calls beyond the ring size and won't wait on any extra calls.
+       measure_async_call(null_async(&desc), desc, 100, 100, "Async Null");
 
        // Spin to make sure we don't have any resources deallocated before done
        while(1);