vfs: i_state needs to be 'unsigned long' for now
authorLinus Torvalds <torvalds@linux-foundation.org>
Tue, 21 Jun 2011 03:13:49 +0000 (20:13 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Tue, 21 Jun 2011 03:13:49 +0000 (20:13 -0700)
Commit 13e12d14e2dc ("vfs: reorganize 'struct inode' layout a bit")
moved things around a bit changed i_state to be unsigned int instead of
unsigned long.  That was to help structure layout for the 64-bit case,
and shrink 'struct inode' a bit (admittedly that only happened when
spinlock debugging was on and i_flags didn't pack with i_lock).

However, Meelis Roos reports that this results in unaligned exceptions
on sprc, and it turns out that the bit-locking primitives that we use
for the I_NEW bit want to use the bitops.  Which want 'unsigned long',
not 'unsigned int'.

We really should fix the bit locking code to not have that kind of
requirement, but that's a much bigger change.  So for now, revert that
field back to 'unsigned long' (but keep the other re-ordering changes
from the commit that caused this).

Andi points out that we have played games with this in 'struct page', so
it's solvable with other hacks too, but since right now the struct inode
size advantage only happens with some rare config options, it's not
worth fighting.

It _would_ be worth fixing the bitlocking code, though.  Especially
since there is no type safety in the bitlocking code (this never caused
any warnings, and worked fine on x86-64, because the bitlocks take a
'void *' and x86-64 doesn't care that deeply about alignment).  So it's
currently a very easy problem to trigger by mistake and never notice.

Reported-by: Meelis Roos <mroos@linux.ee>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
include/linux/fs.h

index 1c777878f1ea5d375be6f04f3364ac2fb9d6c8ba..6e73e2e9ae33c5b7719f11fb303cee693dc1b043 100644 (file)
@@ -744,7 +744,7 @@ struct inode {
 
        spinlock_t              i_lock; /* i_blocks, i_bytes, maybe i_size */
        unsigned int            i_flags;
-       unsigned int            i_state;
+       unsigned long           i_state;
 #ifdef CONFIG_SECURITY
        void                    *i_security;
 #endif