Messages in this thread | | | Date | Tue, 14 Nov 2017 21:03:24 +0000 (UTC) | From | Mathieu Desnoyers <> | Subject | Re: [RFC PATCH v11 for 4.15 01/24] Restartable sequences system call |
| |
----- On Nov 14, 2017, at 3:49 PM, Ben Maurer bmaurer@fb.com wrote:
> (apologies for the duplicate email, the previous one bounced as it was > accidentally using HTML formatting) > > If I understand correctly this is run on every context switch so we probably > want to make it really fast
Yes, more precisely, it runs on return to user-space, after every context switch going back to a registered rseq thread.
> >> +static int rseq_need_restart(struct task_struct *t, uint32_t cs_flags) >> +{ >> + bool need_restart = false; >> + uint32_t flags; >> + >> + /* Get thread flags. */ >> + if (__get_user(flags, &t->rseq->flags)) >> + return -EFAULT; >> + >> + /* Take into account critical section flags. */ >> + flags |= cs_flags; >> + >> + /* >> + * Restart on signal can only be inhibited when restart on >> + * preempt and restart on migrate are inhibited too. Otherwise, >> + * a preempted signal handler could fail to restart the prior >> + * execution context on sigreturn. >> + */ >> + if (flags & RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL) { >> + if (!(flags & RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE)) >> + return -EINVAL; >> + if (!(flags & RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT)) >> + return -EINVAL; >> + } > > How does this error even get to userspace? Is it worth doing this switch on > every execution?
If we detect this situation, the rseq_need_restart caller will end up sending a SIGSEGV signal to user-space. Note that the two nested if() checks are only executing in the unlikely case where the NO_RESTART_ON_SIGNAL flag is set.
> > >> + if (t->rseq_migrate >> + && !(flags & RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE)) >> + need_restart = true; >> + else if (t->rseq_preempt >> + && !(flags & RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT)) >> + need_restart = true; >> + else if (t->rseq_signal >> + && !(flags & RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL)) >> + need_restart = true; > > This could potentially be sped up by having the rseq_* fields in t use a single > bitmask with the same bit offsets as RSEQ_CS_FLAG_NO_* then using bit > operations to check the appropriate overlap.
Given that those are not requests impacting the ABI presented to user-space, I'm tempted to keep these optimizations for the following 4.16 merge window. Is that ok with you ?
Thanks,
Mathieu
-- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
| |