Messages in this thread | | | From | Peter Oskolkov <> | Date | Tue, 11 Aug 2020 14:08:22 -0700 | Subject | Re: [PATCH 1/2 v3] rseq/membarrier: add MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ |
| |
On Mon, Aug 10, 2020 at 11:27 PM Peter Zijlstra <peterz@infradead.org> wrote: > > 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.
Ack. Will do in the next version - for now waiting for the rest to be worked out.
[...]
> > 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;
@flags is not used now; maybe just rename it to something more generic? @param? Or @options? Or maybe more specific, like @cpu_id?
> - he wondered if we should support SYNC_CORE + RSEQ.
It seems to me that CMD_PRIVATE_EXPEDITED_RSEQ is basically CMD_PRIVATE_EXPEDITED_SYNC_CORE with the extra "restart RSEQ CSs" behavior. Am I missing something? If not, what is the point of complicating the code as suggested below? Maybe just renaming CMD_PRIVATE_EXPEDITED_RSEQ to CMD_PRIVATE_EXPEDITED_SYNC_CORE_RSEQ will do?
> > > 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; > } >
| |