Messages in this thread Patch in this message | | | Date | Tue, 11 Aug 2020 08:27:33 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 1/2 v3] rseq/membarrier: add MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ |
| |
On Mon, Aug 10, 2020 at 05:09:58PM -0700, Peter Oskolkov wrote: > @@ -27,6 +35,12 @@ > > static void ipi_mb(void *info) > {
The #ifdef wants to behere, otherwise you'll get a compile warning for !RSEQ builds.
> + int *flags = info; > + > +#ifdef CONFIG_RSEQ > + if (flags && (*flags == MEMBARRIER_FLAG_RSEQ)) > + rseq_preempt(current); > +#endif > smp_mb(); /* IPIs should be serializing but paranoid. */ > }
But yes, this looks much better.
Mathieu did mention a few other points that I didn't see addressed:
- he didn't like abusing the @flags syscall argument for a CPUid; - he wondered if we should support SYNC_CORE + RSEQ.
Not sure we can easily change the syscall at this point, but the latter point could be addressed with something like this.
--- Index: linux-2.6/kernel/sched/membarrier.c =================================================================== --- linux-2.6.orig/kernel/sched/membarrier.c +++ linux-2.6/kernel/sched/membarrier.c @@ -374,8 +374,26 @@ static int membarrier_register_private_e */ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags) { + int cflags = 0, int cpuid = -1; + if (unlikely(flags) && cmd != MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ) return -EINVAL; + + if (cmd & (MEMBARRIER_CMD_PRIVATE_EXPEDITED | + MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE | + MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ)) { + + if (cmd & MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ) + cflags |= MEMBARRIER_FLAG_RSEQ; + + if (cmd & MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE) { + cflags |= MEMBARRIER_FLAG_SYNC_CORE; + cpuid = flags; + } + + cmd = MEMBARRIER_CMD_PRIVATE_EXPEDITED; + } + switch (cmd) { case MEMBARRIER_CMD_QUERY: { @@ -396,18 +414,16 @@ SYSCALL_DEFINE2(membarrier, int, cmd, in return membarrier_global_expedited(); case MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED: return membarrier_register_global_expedited(); - case MEMBARRIER_CMD_PRIVATE_EXPEDITED: - return membarrier_private_expedited(0, -1); case MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED: return membarrier_register_private_expedited(0); - case MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE: - return membarrier_private_expedited(MEMBARRIER_FLAG_SYNC_CORE, -1); case MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE: return membarrier_register_private_expedited(MEMBARRIER_FLAG_SYNC_CORE); - case MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ: - return membarrier_private_expedited(MEMBARRIER_FLAG_RSEQ, flags); case MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_RSEQ: return membarrier_register_private_expedited(MEMBARRIER_FLAG_RSEQ); + + case MEMBARRIER_CMD_PRIVATE_EXPEDITED: + return membarrier_private_expedited(cflags, cpuid); + default: return -EINVAL; }
| |