Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

From: Paul Mackerras
Date: Wed Aug 15 2007 - 23:49:00 EST


Herbert Xu writes:

> If you're referring to the code in sk_stream_mem_schedule
> then it's working as intended. The atomicity guarantees

You mean it's intended that *sk->sk_prot->memory_pressure can end up
as 1 when sk->sk_prot->memory_allocated is small (less than
->sysctl_mem[0]), or as 0 when ->memory_allocated is large (greater
than ->sysctl_mem[2])? Because that's the effect of the current code.
If so I wonder why you bother computing it.

> that the atomic_add/atomic_sub won't be seen in parts by
> other readers.
>
> We certainly do not need to see other atomic_add/atomic_sub
> operations immediately.
>
> If you're referring to another code snippet please cite.
>
> > I'd go so far as to say that anywhere where you want a non-"volatile"
> > atomic_read, either your code is buggy, or else an int would work just
> > as well.
>
> An int won't work here because += and -= do not have the
> atomicity guarantees that atomic_add/atomic_sub do. In
> particular, this may cause an atomic_read on another CPU
> to give a bogus reading.

The point is that guaranteeing the atomicity of the increment or
decrement does not suffice to make the code race-free. In this case
the race arises from the fact that reading ->memory_allocated and
setting *->memory_pressure are separate operations. To make that code
work properly you need a lock. And once you have the lock an ordinary
int would suffice for ->memory_allocated.

Paul.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/