lkml.org 
[lkml]   [2015]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2015-04-21 14:21    [W:0.151 / U:1.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site