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 -- 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.
