Messages in this thread | | | Date | Tue, 21 Apr 2015 13:47:38 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH v16] sys_membarrier(): system-wide memory barrier (generic, x86) |
| |
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.
> + * Command to be passed to the membarrier system call.
Please epxlain, that the commands need to be a single bit each.
> +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?
> +/** > + * 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.
> +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 :)
Thanks,
tglx
| |