Re: [RFC] [PATCH 2/7 v2] memcg: add memory barrier for checking account move.

[Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org>]

On Tue 24-01-12 11:04:16, Ying Han wrote:
> On Mon, Jan 23, 2012 at 1:04 AM, Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org> wrote:
> > On Fri 20-01-12 10:08:44, Ying Han wrote:
> >> On Wed, Jan 18, 2012 at 6:17 PM, KAMEZAWA Hiroyuki
> >> <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org> wrote:
[...]
> >> > 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