Messages in this thread | | | Date | Wed, 22 Apr 2015 20:36:04 +0000 (UTC) | From | Mathieu Desnoyers <> | Subject | Re: [PATCH v16] sys_membarrier(): system-wide memory barrier (generic, x86) |
| |
----- Original Message ----- > On Fri, 17 Apr 2015, Mathieu Desnoyers wrote: > > +/** > > + * enum membarrier_cmd - membarrier system call command > > + * > > * @MEMBARRIER_CMD_QUERY: Explanatory blurb...... > > Hint: Create a kerneldoc document and build it.
Oops, fixed, and figured out how to build it. Next version should be ok.
> > > + * Command to be passed to the membarrier system call. > > Please epxlain, that the commands need to be a single bit each.
OK
> > > +config MEMBARRIER > > + bool "Enable membarrier() system call" if EXPERT > > + default y > > + depends on SMP > > + help > > + Enable the membarrier() system call that allows issuing memory > > + barriers across all running threads, which can be used to distribute > > + the cost of user-space memory barriers asymmetrically by transforming > > + pairs of memory barriers into pairs consisting of membarrier() and a > > + compiler barrier. > > + > > + If unsure, say Y. > > Is it really worth to make this configurable?
Josh Triplett asked for it to be made configurable. I don't have any strong opinion one way or the other.
> > > +/** > > + * sys_membarrier - issue memory barriers on a set of threads > > + * @cmd: MEMBARRIER_CMD_QUERY: > > + * Query the set of supported commands. It returns a bitmask > > of > > + * supported commands. > > + * MEMBARRIER_CMD_SHARED: > > + * Execute a memory barrier on all running threads. Upon > > + * return from system call, the caller thread is ensured that > > + * all running threads have passed through a state where all > > + * memory accesses to user-space addresses match program order > > + * between entry to and return from the system call > > (non-running > > + * threads are de facto in such a state). This covers threads > > + * from all processes running on the system. This command > > + * returns 0. > > I think the explanation for the commands should be in the enum > documentation. This here should explain that @cmd takes command values > defined in the enum.
OK
> > > +SYSCALL_DEFINE2(membarrier, int, cmd, int, flags) > > +{ > > + switch (cmd) { > > + case MEMBARRIER_CMD_QUERY: > > + return MEMBARRIER_CMD_BITMASK; > > + case MEMBARRIER_CMD_SHARED: > > + if (num_online_cpus() > 1) > > + synchronize_sched(); > > + return 0; > > + default: > > + return -EINVAL; > > + } > > +} > > This looks way cleaner now :)
Much smaller too :)
Thanks,
Mathieu
> > Thanks, > > tglx >
-- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
| |