Subject: Re: linux-next: next-20090609 hangs in early user mode

From: Al Viro <[email-masked]>
Date: Wed, 10 Jun 2009 16:08:39 +0100

On Wed, Jun 10, 2009 at 09:19:09AM -0400, Theodore Tso wrote:
> On Wed, Jun 10, 2009 at 02:00:54PM +0100, Al Viro wrote:
> > 
> > Yes, it's obviously bogus.  Dropped from the tree; I don't think it's
> > really salvagable - even merging into one unsigned long will not be
> > enough, since we will end up with different locking for different bits.
> 
> Oops, sorry, I didn't realize we were using bitops for i_state.  As
> far as I can tell we're not using the bitops functions for i_flags,
> though.  Is that right?  So we can convert i_flags to be a unsigned
> short, but we can't do anything with i_state.

We can, but... it's again a matter of combining things with different
locking.  i_flags is protected by i_mutex, so if you put another
unsigned short next to it, you'd better make sure that i_mutex
is necessary and sufficient for modifying it.

Depending on the target, gcc may turn 16bit read-modify-store into 32bit one,
so if you have two 16bit fields next to each other, you can run into

CPU1:                     CPU2:
r1 = *(u32 *)p;           r2 = *(u32 *)p;
r1 |= 1;                  r2 |= 1 << 16;
*(u32 *)p = r1;           *(u32 *)p = r2;

with obvious results.  So we need the same locking for both such fields...