* linux-next: next-20090609 hangs in early user mode
@ 2009-06-10 8:00 Stephen Rothwell
2009-06-10 8:31 ` Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Stephen Rothwell @ 2009-06-10 8:00 UTC (permalink / raw)
To: LKML; +Cc: linux-next, Theodore Ts'o, Al Viro, linux-fsdevel
[-- Attachment #1: Type: text/plain, Size: 2661 bytes --]
Hi all,
My boot tests last night produced this during boot:
Loading, please wait...
exe used greatest stack depth: 11120 bytes left
async/0 used greatest stack depth: 10928 bytes left
-- 0:conmux-control -- time-stamp -- Jun/09/09 21:02:12 --
INFO: task init:1 blocked for more than 120 seconds.
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
init D 0000000010078228 8192 1 0 0x00008010
Call Trace:
[c0000000be67f380] [c0000000be67f430] 0xc0000000be67f430 (unreliable)
[c0000000be67f550] [c00000000001153c] .__switch_to+0xfc/0x1b0
[c0000000be67f5e0] [c00000000055dc70] .schedule+0x2b0/0x980
[c0000000be67f700] [c000000000138850] .inode_wait+0x10/0x30
[c0000000be67f780] [c00000000055ebd8] .__wait_on_bit+0xd8/0x160
[c0000000be67f830] [c00000000055ece8] .out_of_line_wait_on_bit+0x88/0xb0
[c0000000be67f910] [c000000000139060] .clear_inode+0x120/0x150
[c0000000be67f990] [c0000000001395c8] .generic_drop_inode+0x198/0x330
[c0000000be67fa20] [c0000000001388f8] .iput+0x88/0xc0
[c0000000be67faa0] [c000000000134308] .dentry_iput+0x108/0x220
[c0000000be67fb30] [c0000000001345c8] .d_kill+0x68/0xb0
[c0000000be67fbc0] [c000000000135104] .dput+0x154/0x2c0
[c0000000be67fc60] [c00000000011f9a4] .__fput+0x1b4/0x290
[c0000000be67fd00] [c00000000011b760] .filp_close+0xa0/0x100
[c0000000be67fd90] [c00000000011b870] .SyS_close+0xb0/0x180
[c0000000be67fe30] [c0000000000084ac] syscall_exit+0x0/0x40
And it never recovers. Sometimes during the bisection run, it was a
different process - but it was the same backtrace.
Bisection lead me to this:
d4fdcb2068eef29c03d6027aa219fa60171c6b87 is first bad commit
commit d4fdcb2068eef29c03d6027aa219fa60171c6b87
Author: Theodore Ts'o <tytso@mit.edu>
Date: Thu May 21 16:00:59 2009 -0400
fs: i_flags and i_state in struct inode only need to be unsigned short
Currently i_flags and i_state do not need to be an unsigned int and an
unsigned long, respectively. (We currently use 9 i_flags bits, and 8
i_state bits.) Changing them to be an unsigned short saves 4 bytes
per inode on an x86 platform, and 8 bytes on an x86_64 platform.
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Reverting that commit from next-20090609 gets me a bootable kernel.
The machines are PowerPC64 (so big endian). It looks to me like we use
bitops on i_state - don't our bitops only apply to unsigned longs?
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: linux-next: next-20090609 hangs in early user mode
2009-06-10 8:00 linux-next: next-20090609 hangs in early user mode Stephen Rothwell
@ 2009-06-10 8:31 ` Christoph Hellwig
2009-06-10 8:44 ` Stephen Rothwell
2009-06-10 8:46 ` Stephen Rothwell
2009-06-10 13:00 ` Al Viro
2 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2009-06-10 8:31 UTC (permalink / raw)
To: Stephen Rothwell
Cc: LKML, linux-next, Theodore Ts'o, Al Viro, linux-fsdevel
On Wed, Jun 10, 2009 at 06:00:58PM +1000, Stephen Rothwell wrote:
> The machines are PowerPC64 (so big endian). It looks to me like we use
> bitops on i_state - don't our bitops only apply to unsigned longs?
Yes, bitops need unsigned long. And on most plaforms should give you
warnings when using them on the wrong type.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: linux-next: next-20090609 hangs in early user mode
2009-06-10 8:31 ` Christoph Hellwig
@ 2009-06-10 8:44 ` Stephen Rothwell
0 siblings, 0 replies; 9+ messages in thread
From: Stephen Rothwell @ 2009-06-10 8:44 UTC (permalink / raw)
To: Christoph Hellwig
Cc: LKML, linux-next, Theodore Ts'o, Al Viro, linux-fsdevel
[-- Attachment #1: Type: text/plain, Size: 671 bytes --]
Hi Christoph,
On Wed, 10 Jun 2009 04:31:39 -0400 Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Jun 10, 2009 at 06:00:58PM +1000, Stephen Rothwell wrote:
> > The machines are PowerPC64 (so big endian). It looks to me like we use
> > bitops on i_state - don't our bitops only apply to unsigned longs?
>
> Yes, bitops need unsigned long. And on most plaforms should give you
> warnings when using them on the wrong type.
In this case the "word" is being passed through wait_on_bit which takes a
"void *", so we lose the type information.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: linux-next: next-20090609 hangs in early user mode
2009-06-10 8:00 linux-next: next-20090609 hangs in early user mode Stephen Rothwell
2009-06-10 8:31 ` Christoph Hellwig
@ 2009-06-10 8:46 ` Stephen Rothwell
2009-06-10 13:00 ` Al Viro
2 siblings, 0 replies; 9+ messages in thread
From: Stephen Rothwell @ 2009-06-10 8:46 UTC (permalink / raw)
To: LKML; +Cc: linux-next, Theodore Ts'o, Al Viro, linux-fsdevel
[-- Attachment #1: Type: text/plain, Size: 463 bytes --]
On Wed, 10 Jun 2009 18:00:58 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> commit d4fdcb2068eef29c03d6027aa219fa60171c6b87
> Author: Theodore Ts'o <tytso@mit.edu>
> Date: Thu May 21 16:00:59 2009 -0400
>
> fs: i_flags and i_state in struct inode only need to be unsigned short
I will revert that commit from linux-next today.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: linux-next: next-20090609 hangs in early user mode
2009-06-10 8:00 linux-next: next-20090609 hangs in early user mode Stephen Rothwell
2009-06-10 8:31 ` Christoph Hellwig
2009-06-10 8:46 ` Stephen Rothwell
@ 2009-06-10 13:00 ` Al Viro
2009-06-10 13:07 ` Stephen Rothwell
2009-06-10 13:19 ` Theodore Tso
2 siblings, 2 replies; 9+ messages in thread
From: Al Viro @ 2009-06-10 13:00 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: LKML, linux-next, Theodore Ts'o, linux-fsdevel
On Wed, Jun 10, 2009 at 06:00:58PM +1000, Stephen Rothwell wrote:
> d4fdcb2068eef29c03d6027aa219fa60171c6b87 is first bad commit
> commit d4fdcb2068eef29c03d6027aa219fa60171c6b87
> Author: Theodore Ts'o <tytso@mit.edu>
> Date: Thu May 21 16:00:59 2009 -0400
>
> fs: i_flags and i_state in struct inode only need to be unsigned short
>
> Currently i_flags and i_state do not need to be an unsigned int and an
> unsigned long, respectively. (We currently use 9 i_flags bits, and 8
> i_state bits.) Changing them to be an unsigned short saves 4 bytes
> per inode on an x86 platform, and 8 bytes on an x86_64 platform.
>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> Cc: linux-fsdevel@vger.kernel.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
*gyah*
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: linux-next: next-20090609 hangs in early user mode
2009-06-10 13:00 ` Al Viro
@ 2009-06-10 13:07 ` Stephen Rothwell
2009-06-10 13:19 ` Theodore Tso
1 sibling, 0 replies; 9+ messages in thread
From: Stephen Rothwell @ 2009-06-10 13:07 UTC (permalink / raw)
To: Al Viro; +Cc: LKML, linux-next, Theodore Ts'o, linux-fsdevel
[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]
Hi Al,
On Wed, 10 Jun 2009 14:00:54 +0100 Al Viro <viro@ZenIV.linux.org.uk> wrote:
>
> On Wed, Jun 10, 2009 at 06:00:58PM +1000, Stephen Rothwell wrote:
>
> > d4fdcb2068eef29c03d6027aa219fa60171c6b87 is first bad commit
> > commit d4fdcb2068eef29c03d6027aa219fa60171c6b87
> > Author: Theodore Ts'o <tytso@mit.edu>
> > Date: Thu May 21 16:00:59 2009 -0400
> >
> > fs: i_flags and i_state in struct inode only need to be unsigned short
> >
> > Currently i_flags and i_state do not need to be an unsigned int and an
> > unsigned long, respectively. (We currently use 9 i_flags bits, and 8
> > i_state bits.) Changing them to be an unsigned short saves 4 bytes
> > per inode on an x86 platform, and 8 bytes on an x86_64 platform.
> >
> > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> > Cc: linux-fsdevel@vger.kernel.org
> > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
>
> *gyah*
>
> 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.
OK, thanks.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: linux-next: next-20090609 hangs in early user mode
2009-06-10 13:00 ` Al Viro
2009-06-10 13:07 ` Stephen Rothwell
@ 2009-06-10 13:19 ` Theodore Tso
2009-06-10 15:08 ` Al Viro
1 sibling, 1 reply; 9+ messages in thread
From: Theodore Tso @ 2009-06-10 13:19 UTC (permalink / raw)
To: Al Viro; +Cc: Stephen Rothwell, LKML, linux-next, linux-fsdevel
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.
- Ted
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: linux-next: next-20090609 hangs in early user mode
2009-06-10 13:19 ` Theodore Tso
@ 2009-06-10 15:08 ` Al Viro
2009-06-10 16:09 ` Theodore Tso
0 siblings, 1 reply; 9+ messages in thread
From: Al Viro @ 2009-06-10 15:08 UTC (permalink / raw)
To: Theodore Tso, Stephen Rothwell, LKML, linux-next, linux-fsdevel
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...
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: linux-next: next-20090609 hangs in early user mode
2009-06-10 15:08 ` Al Viro
@ 2009-06-10 16:09 ` Theodore Tso
0 siblings, 0 replies; 9+ messages in thread
From: Theodore Tso @ 2009-06-10 16:09 UTC (permalink / raw)
To: Al Viro; +Cc: Stephen Rothwell, LKML, linux-next, linux-fsdevel
On Wed, Jun 10, 2009 at 04:08:39PM +0100, Al Viro wrote:
> 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...
Yelch.... good point. I'll look and see if there's some other 8 or
16-bit type to combine it with, but we may have started to hit
diminishing returns with this this approach to sliming the inode slab
caches. I'm beginning to think if I want to make the inodes smaller,
I'm going to have to create a separate substructure for fields only
used when a file descriptor is opened on that inode, both in struct
inode and in struct ext4_inode_info. (Lifetime management of the
substructure is going to be non-trivial, though.)
- Ted
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-06-10 16:09 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-10 8:00 linux-next: next-20090609 hangs in early user mode Stephen Rothwell
2009-06-10 8:31 ` Christoph Hellwig
2009-06-10 8:44 ` Stephen Rothwell
2009-06-10 8:46 ` Stephen Rothwell
2009-06-10 13:00 ` Al Viro
2009-06-10 13:07 ` Stephen Rothwell
2009-06-10 13:19 ` Theodore Tso
2009-06-10 15:08 ` Al Viro
2009-06-10 16:09 ` Theodore Tso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).