lkml.org 
[lkml]   [2019]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 5/5] psi: introduce psi monitor
On Mon, Jan 14, 2019 at 11:30:12AM -0800, Suren Baghdasaryan wrote:
> For memory ordering (which Johannes also pointed out) the critical point is:
>
> times[cpu] += delta | if g->polling:
> smp_wmb() | g->polling = polling = 0
> cmpxchg(g->polling, 0, 1) | smp_rmb()
> | delta = times[*] (through goto SLOWPATH)
>
> So that hotpath writes to times[] then g->polling and slowpath reads
> g->polling then times[]. cmpxchg() implies a full barrier, so we can
> drop smp_wmb(). Something like this:
>
> times[cpu] += delta | if g->polling:
> cmpxchg(g->polling, 0, 1) | g->polling = polling = 0
> | smp_rmb()
> | delta = times[*] (through goto SLOWPATH)
>
> Would that address your concern about ordering?

cmpxchg() implies smp_mb() before and after, so the smp_wmb() on the
left column is superfluous.

The right hand column is actively wrong; because that reads like it
wants to order a store (g->polling = 0) and a load (d = times[]), and
therefore requires smp_mb().

Also, you probably want to use atomic_t for g->polling, because we
(sadly) have architectures where regular stores and atomic ops don't
work 'right'.

\
 
 \ /
  Last update: 2019-01-16 14:25    [W:0.102 / U:11.888 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site