Re: bit fields && data tearing

[Paul E. McKenney]

On Thu, Sep 04, 2014 at 10:47:24PM -0400, Peter Hurley wrote:
> Hi James,
> 
> On 09/04/2014 10:11 PM, James Bottomley wrote:
> > On Thu, 2014-09-04 at 17:17 -0700, Paul E. McKenney wrote:
> >> +And there are anti-guarantees:
> >> +
> >> + (*) These guarantees do not apply to bitfields, because compilers often
> >> +     generate code to modify these using non-atomic read-modify-write
> >> +     sequences.  Do not attempt to use bitfields to synchronize parallel
> >> +     algorithms.
> >> +
> >> + (*) Even in cases where bitfields are protected by locks, all fields
> >> +     in a given bitfield must be protected by one lock.  If two fields
> >> +     in a given bitfield are protected by different locks, the compiler's
> >> +     non-atomic read-modify-write sequences can cause an update to one
> >> +     field to corrupt the value of an adjacent field.
> >> +
> >> + (*) These guarantees apply only to properly aligned and sized scalar
> >> +     variables.  "Properly sized" currently means "int" and "long",
> >> +     because some CPU families do not support loads and stores of
> >> +     other sizes.  ("Some CPU families" is currently believed to
> >> +     be only Alpha 21064.  If this is actually the case, a different
> >> +     non-guarantee is likely to be formulated.)
> > 
> > This is a bit unclear.  Presumably you're talking about definiteness of
> > the outcome (as in what's seen after multiple stores to the same
> > variable).
> 
> No, the last conditions refers to adjacent byte stores from different
> cpu contexts (either interrupt or SMP).
> 
> > The guarantees are only for natural width on Parisc as well,
> > so you would get a mess if you did byte stores to adjacent memory
> > locations.
> 
> For a simple test like:
> 
> struct x {
> 	long a;
> 	char b;
> 	char c;
> 	char d;
> 	char e;
> };
> 
> void store_bc(struct x *p) {
> 	p->b = 1;
> 	p->c = 2;
> }
> 
> on parisc, gcc generates separate byte stores
> 
> void store_bc(struct x *p) {
>    0:	34 1c 00 02 	ldi 1,ret0
>    4:	0f 5c 12 08 	stb ret0,4(r26)
>    8:	34 1c 00 04 	ldi 2,ret0
>    c:	e8 40 c0 00 	bv r0(rp)
>   10:	0f 5c 12 0a 	stb ret0,5(r26)
> 
> which appears to confirm that on parisc adjacent byte data
> is safe from corruption by concurrent cpu updates; that is,
> 
> CPU 0                | CPU 1
>                      |
> p->b = 1             | p->c = 2
>                      |
> 
> will result in p->b == 1 && p->c == 2 (assume both values
> were 0 before the call to store_bc()).

What Peter said.  I would ask for suggestions for better wording, but
I would much rather be able to say that single-byte reads and writes
are atomic and that aligned-short reads and writes are also atomic.

Thus far, it looks like we lose only very old Alpha systems, so unless
I hear otherwise, I update my patch to outlaw these very old systems.

						Thanx, 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/