OK, fixed. And rebased onto master. On Fri, Nov 13, 2015 at 3:30 PM, Kevin Klues <[email protected]> wrote: > On Fri, Nov 13, 2015 at 3:25 PM, Kevin Klues <[email protected]> wrote: >> Comments below based on your comments. I also patched up a bug >> related to sigprocmask() that I noticed independent of your comments. >> This change is reflected in: >> >> e11dfb4 Fix bug in pthread_sigmask() semantics. >> 5c2be9f Migrate signal code from pthread.c to signal.c > > I lied, I reintroduced an old bug with this "fix". I'll revert and > repush in a sec. > >> >>>> --- a/user/parlib/signal.c >>>> +++ b/user/parlib/signal.c >>>> @@ -234,36 +129,27 @@ void init_posix_signals(void) >>>> posix_sig_ev_q->ev_flags = EVENT_IPI | EVENT_INDIR | >>>> EVENT_SPAM_INDIR | >>>> EVENT_WAKEUP; >>>> register_kevent_q(posix_sig_ev_q, EV_POSIX_SIGNAL); >>>> + signal_ops = &default_signal_ops; >>> >>> Minor race: probably need to set signal_ops before registering the kevent_q. >>> >>>> wfl_init(&sigdata_list); >>>> } >> >> Fixed >> >>>> From d38bf552fecffadbc5336db29e3d101a0401be2c Mon Sep 17 00:00:00 2001 >>>> From: Kevin Klues <[email protected]> >>>> Date: Tue, 10 Nov 2015 13:50:52 -0800 >>>> Subject: Add arch independent accessor for user ctx stack >>> >>>> --- a/user/parlib/include/x86/vcore32.h >>>> +++ b/user/parlib/include/x86/vcore32.h >>>> @@ -315,6 +315,11 @@ static inline void init_user_ctx(struct user_context >>>> *ctx, uint32_t entry_pt, >>>> sw_tf->tf_fpucw = 0x037f; /* x86 default FP CW */ >>>> } >>>> >>>> +static inline uintptr_t get_user_ctx_stack(struct user_context *ctx) >>>> +{ >>>> + return ctx->tf.sw_tf.tf_esp; >>>> +} >>>> + >>> >>> You'll need to switch on the context type for all of these. It could be a >>> HW >>> TF, for example. >> >> Fixed >> >>>> From f080545ec5b79bbb4563b5b3123b90e5932e2f8e Mon Sep 17 00:00:00 2001 >>>> From: Kevin Klues <[email protected]> >>>> Date: Tue, 10 Nov 2015 18:10:27 -0800 >>>> Subject: Add uthread_paused() API call >>> >>>> +/* Function indicating an external event has temporarily paused a >>>> uthread, but >>>> + * it is ok to resume it if possible. */ >>>> +void uthread_paused(struct uthread *uthread) >>>> +{ >>>> + uthread->state = UT_NOT_RUNNING; >>> >>> I think this should be an assert(uthread->state == UT_NOT_RUNNING). This is >>> called from a uthread_yield() callback, right? In that case, NOT_RUNNING >>> should >>> already be set. And if not, we'll catch buggy callers. >> >> Agreed. Fixed. >> >>>> From b133811ff974d66a61fe007034ee981deb3abd94 Mon Sep 17 00:00:00 2001 >>>> From: Kevin Klues <[email protected]> >>>> Date: Tue, 10 Nov 2015 19:00:04 -0800 >>>> Subject: Migrate signal code from pthread.c to signal.c >>> >>>> For 2LSs that don't require posix signals, the only waste is a few words in >>>> the uthread struct, and a few unnecessary instructions to clear these >>>> fields >>>> when a uthread is first initialized. There is no actual performance hit >>>> unless the APIs from parlib/signal.h are actually called (as they are in >>>> pthread.c now). >>> >>> Sounds good. IIUC, for another 2LS to use signal handling, it'll need to do >>> something like this: >>> >>>> @@ -250,7 +114,7 @@ static void __attribute__((noreturn)) >>>> pth_sched_entry(void) >>>> } while (1); >>>> /* Prep the pthread to run any pending posix signal handlers >>>> registered >>>> * via pthread_kill once it is restored. */ >>>> - __pthread_prep_for_pending_posix_signals(new_thread); >>>> + uthread_prep_pending_signals((struct uthread*)new_thread); >>>> /* Run the thread itself */ >>>> run_uthread((struct uthread*)new_thread); >>> >>> and: >>> >>>> +static void __signal_and_restart(struct uthread *uthread, >>>> + int signo, int code, void *addr) >>>> +{ >>>> + uthread_prep_signal_from_fault(uthread, signo, code, addr); >>>> + pth_thread_runnable(uthread); >>>> +} >>> >>> Which sounds good. >> >> That's right. Take a look at user/parlib/thread0_sched.c It does this >> exact thing to add signal support for the dumb thread0 scheduler. >> >>>> - /* Publish our signal_ops. Sched ops is set by 2ls_init */ >>>> - signal_ops = &pthread_signal_ops; >>>> + /* Sched ops is set by 2ls_init */ >>>> uthread_2ls_init((struct uthread*)t, &pthread_sched_ops); >>> >>> How does pthread (or any 2LS) go about mucking with the signal ops? I >>> imagine >>> it's just "overwrite signal_ops->whatever = my_whatever_func_ptr"? If so, >>> that >>> sounds good. >> >> Yes that's right. It just so happens that the pthread scheduler >> doesn't need to do anything different than the default anymore, so it >> doesn't overwrite any of the function pointers. If your scheduler >> needs to do something custom, just overwrite the calls you care about. > > > > -- > ~Kevin
-- ~Kevin -- You received this message because you are subscribed to the Google Groups "Akaros" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. For more options, visit https://groups.google.com/d/optout.
