On Tue 24-01-12 11:04:16, Ying Han wrote:

On Mon, Jan 23, 2012 at 1:04 AM, Michal Hocko [off-list ref] wrote:

quoted

On Fri 20-01-12 10:08:44, Ying Han wrote:

quoted

On Wed, Jan 18, 2012 at 6:17 PM, KAMEZAWA Hiroyuki
[off-list ref] wrote:

[...]

quoted

quoted

quoted

I doubt .... If no barrier, this case happens

==
       update                  reference
       CPU A                   CPU B
       set value
       synchronize_rcu()       rcu_read_lock()
                               read_value <= find old value
                               rcu_read_unlock()
                               do no lock
==

Hi Kame,

Can you help to clarify a bit more on the example above? Why
read_value got the old value after synchronize_rcu().

AFAIU it is because rcu_read_unlock doesn't force any memory barrier
and we synchronize only the updater (with synchronize_rcu), so nothing
guarantees that the value set on CPUA is visible to CPUB.

Thanks, and i might have found similar comment on the
documentation/rcu/checklist.txt:
"
The various RCU read-side primitives do -not- necessarily contain
memory barriers.
"

So, the read barrier here is to make sure no reordering between the
reader and the rcu_read_lock. The same for the write barrier which
makes sure no reordering between the updater and synchronize_rcu. The
the rcu here is to synchronize between the updater and reader. If so,
why not the change like :

       for_each_online_cpu(cpu)
               per_cpu(memcg->stat->count[MEM_CGROUP_ON_MOVE], cpu) += 1;
+      smp_wmb();

Threre is a data dependency between per_cpu update (the above for look)
and local read of the per-cpu on the read-side and IIUC we need to pair
write barrier with read one before we read the value.

But I might be wrong here (see the SMP BARRIER PAIRING section in
Documentation/memory-barriers.txt).

Sorry, the use of per-cpu variable MEM_CGROUP_ON_MOVE does confuse me.

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic