----- On Nov 14, 2017, at 3:49 PM, Ben Maurer bmaurer@xxxxxx 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 -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html