2008-01-28 08:31:55

by Oliver Pinter

[permalink] [raw]
Subject: [2.6.24 REGRESSION] BUG: Soft lockup - with VFS

hi all!

in the 2.6.24 become i some soft lockups with usb-phone, when i pluged
in the mobile, then the vfs-layer crashed. am afternoon can i the
.config send, and i bisected the kernel, when i have time.

pictures from crash:
http://students.zipernowsky.hu/~oliverp/kernel/regression_2624/
--
Thanks,
Oliver


2008-02-05 05:39:26

by Andrew Morton

[permalink] [raw]
Subject: Re: [2.6.24 REGRESSION] BUG: Soft lockup - with VFS

On Mon, 28 Jan 2008 09:31:43 +0100 "Oliver Pinter (Pint?r Oliv?r)" <[email protected]> wrote:

> hi all!
>
> in the 2.6.24 become i some soft lockups with usb-phone, when i pluged
> in the mobile, then the vfs-layer crashed. am afternoon can i the
> .config send, and i bisected the kernel, when i have time.
>
> pictures from crash:
> http://students.zipernowsky.hu/~oliverp/kernel/regression_2624/

It looks like selinux's file_has_perm() is doing spin_lock() on an
uninitialised (or already locked) spinlock.

2008-02-05 10:03:44

by James Morris

[permalink] [raw]
Subject: Re: [2.6.24 REGRESSION] BUG: Soft lockup - with VFS

On Mon, 4 Feb 2008, Andrew Morton wrote:

> On Mon, 28 Jan 2008 09:31:43 +0100 "Oliver Pinter (Pint?r Oliv?r)" <[email protected]> wrote:
>
> > hi all!
> >
> > in the 2.6.24 become i some soft lockups with usb-phone, when i pluged
> > in the mobile, then the vfs-layer crashed. am afternoon can i the
> > .config send, and i bisected the kernel, when i have time.
> >
> > pictures from crash:
> > http://students.zipernowsky.hu/~oliverp/kernel/regression_2624/
>
> It looks like selinux's file_has_perm() is doing spin_lock() on an
> uninitialised (or already locked) spinlock.

Perplexing.

Do you have all of the lock debugging enabled?


- James
--
James Morris
<[email protected]>

2008-02-05 13:47:30

by Stephen Smalley

[permalink] [raw]
Subject: Re: [2.6.24 REGRESSION] BUG: Soft lockup - with VFS


On Mon, 2008-02-04 at 21:39 -0800, Andrew Morton wrote:
> On Mon, 28 Jan 2008 09:31:43 +0100 "Oliver Pinter (Pintér Olivér)" <[email protected]> wrote:
>
> > hi all!
> >
> > in the 2.6.24 become i some soft lockups with usb-phone, when i pluged
> > in the mobile, then the vfs-layer crashed. am afternoon can i the
> > .config send, and i bisected the kernel, when i have time.
> >
> > pictures from crash:
> > http://students.zipernowsky.hu/~oliverp/kernel/regression_2624/
>
> It looks like selinux's file_has_perm() is doing spin_lock() on an
> uninitialised (or already locked) spinlock.

The trace looks bogus to me - I don't see how file_has_perm() could have
been called there, and file_has_perm() doesn't directly take any spin
locks.

--
Stephen Smalley
National Security Agency

2008-02-05 21:48:43

by Oliver Pinter

[permalink] [raw]
Subject: Re: [2.6.24 REGRESSION] BUG: Soft lockup - with VFS

On 2/5/08, Oliver Pinter <[email protected]> wrote:
> http://students.zipernowsky.hu/~oliverp/kernel/regression_2624/
>
> uploaded:
> kernel image
> .config
> new pictures
> lspci
> lsusb
>
> -----
>
> when read for /dev/uba then crashed the kernel, the read is egal, thet
> dd or mount is ...
>
> On 2/5/08, Oliver Pinter <[email protected]> wrote:
> > yes, but auch too with latest git ... my top is on:
> > 9ef9dc69d4167276c04590d67ee55de8380bc1ad
> >
> > then i complie the new kernel
> >
> > On 2/5/08, Andrew Morton <[email protected]> wrote:
> > > On Tue, 05 Feb 2008 08:46:56 -0500 Stephen Smalley <[email protected]>
> > > wrote:
> > >
> > > >
> > > > On Mon, 2008-02-04 at 21:39 -0800, Andrew Morton wrote:
> > > > > On Mon, 28 Jan 2008 09:31:43 +0100 "Oliver Pinter (Pint?r Oliv?r)"
> > > <[email protected]> wrote:
> > > > >
> > > > > > hi all!
> > > > > >
> > > > > > in the 2.6.24 become i some soft lockups with usb-phone, when i
> > pluged
> > > > > > in the mobile, then the vfs-layer crashed. am afternoon can i the
> > > > > > .config send, and i bisected the kernel, when i have time.
> > > > > >
> > > > > > pictures from crash:
> > > > > > http://students.zipernowsky.hu/~oliverp/kernel/regression_2624/
> > > > >
> > > > > It looks like selinux's file_has_perm() is doing spin_lock() on an
> > > > > uninitialised (or already locked) spinlock.
> > > >
> > > > The trace looks bogus to me - I don't see how file_has_perm() could
> > have
> > > > been called there, and file_has_perm() doesn't directly take any spin
> > > > locks.
> > > >
> > >
> > > Oliver, could you please set CONFIG_FRAME_POINTER=y (which might get a
> > > better trace), and perhaps try Linus's latest tree from
> > > ftp://ftp.kernel.org/pub/linux/kernel/v2.6/snapshots/ (which is a bit
> > more
> > > careful about telling us about possibly-bogus backtrace entries)?
> > >
> > > Thanks.
> > >
> >
> >
> > --
> > Thanks,
> > Oliver
> >
>
>
> --
> Thanks,
> Oliver
>


--
Thanks,
Oliver

2008-02-05 22:06:33

by Andrew Morton

[permalink] [raw]
Subject: Re: [2.6.24 REGRESSION] BUG: Soft lockup - with VFS

On Tue, 5 Feb 2008 22:48:29 +0100
"Oliver Pinter" <[email protected]> wrote:

> On 2/5/08, Oliver Pinter <[email protected]> wrote:
> > http://students.zipernowsky.hu/~oliverp/kernel/regression_2624/
> >
> > uploaded:
> > kernel image
> > .config
> > new pictures
> > lspci
> > lsusb
> >
> > -----
> >
> > when read for /dev/uba then crashed the kernel, the read is egal, thet
> > dd or mount is ...

Looks like you deadlocked in ub_request_fn(). I assume that you were using
ub.c in 2.6.23 and that it worked OK? If so, we broke it, possibly via
changes to the core block layer.

I think ub.c is basically abandoned in favour of usb-storage. If so,
perhaps we should remove or disble ub.c?

2008-02-05 22:22:27

by Pete Zaitcev

[permalink] [raw]
Subject: Re: [2.6.24 REGRESSION] BUG: Soft lockup - with VFS

On Tue, 5 Feb 2008 14:05:06 -0800, Andrew Morton <[email protected]> wrote:

> Looks like you deadlocked in ub_request_fn(). I assume that you were using
> ub.c in 2.6.23 and that it worked OK? If so, we broke it, possibly via
> changes to the core block layer.
>
> I think ub.c is basically abandoned in favour of usb-storage. If so,
> perhaps we should remove or disble ub.c?

Actually I think it may be an argument for keeping ub, if ub exposes
a bug in the __blk_end_request. I'll look at the head of the thread
and see if Mr. Pinter has hit anything related to Mr. Ueda's work.

-- Pete

2008-02-05 22:29:48

by Oliver Pinter

[permalink] [raw]
Subject: Re: [2.6.24 REGRESSION] BUG: Soft lockup - with VFS

i reverted this commit 7d699bafe258ebd8f9b4ec182c554200b369a504 , and
now compile ...

On 2/5/08, Pete Zaitcev <[email protected]> wrote:
> On Tue, 5 Feb 2008 14:05:06 -0800, Andrew Morton <[email protected]>
> wrote:
>
> > Looks like you deadlocked in ub_request_fn(). I assume that you were
> using
> > ub.c in 2.6.23 and that it worked OK? If so, we broke it, possibly via
> > changes to the core block layer.
> >
> > I think ub.c is basically abandoned in favour of usb-storage. If so,
> > perhaps we should remove or disble ub.c?
>
> Actually I think it may be an argument for keeping ub, if ub exposes
> a bug in the __blk_end_request. I'll look at the head of the thread
> and see if Mr. Pinter has hit anything related to Mr. Ueda's work.
>
> -- Pete
>


--
Thanks,
Oliver

2008-02-05 22:35:58

by Oliver Pinter

[permalink] [raw]
Subject: Re: [2.6.24 REGRESSION] BUG: Soft lockup - with VFS

/usr/data/source/git/linux-2.6/drivers/block/ub.c: In function 'ub_end_rq':
/usr/data/source/git/linux-2.6/drivers/block/ub.c:819: error: implicit
declaration of function 'end_that_request_first'
/usr/data/source/git/linux-2.6/drivers/block/ub.c:820: error: implicit
declaration of function 'end_that_request_last'
make[7]: *** [drivers/block/ub.o] Error 1
make[6]: *** [drivers/block] Error 2
make[5]: *** [drivers] Error 2
make[5]: *** Waiting for unfinished jobs....


On 2/5/08, Oliver Pinter <[email protected]> wrote:
> i reverted this commit 7d699bafe258ebd8f9b4ec182c554200b369a504 , and
> now compile ...
>
> On 2/5/08, Pete Zaitcev <[email protected]> wrote:
> > On Tue, 5 Feb 2008 14:05:06 -0800, Andrew Morton
> <[email protected]>
> > wrote:
> >
> > > Looks like you deadlocked in ub_request_fn(). I assume that you were
> > using
> > > ub.c in 2.6.23 and that it worked OK? If so, we broke it, possibly via
> > > changes to the core block layer.
> > >
> > > I think ub.c is basically abandoned in favour of usb-storage. If so,
> > > perhaps we should remove or disble ub.c?
> >
> > Actually I think it may be an argument for keeping ub, if ub exposes
> > a bug in the __blk_end_request. I'll look at the head of the thread
> > and see if Mr. Pinter has hit anything related to Mr. Ueda's work.
> >
> > -- Pete
> >
>
>
> --
> Thanks,
> Oliver
>


--
Thanks,
Oliver

2008-02-09 07:51:41

by Pete Zaitcev

[permalink] [raw]
Subject: Re: [2.6.24 REGRESSION] BUG: Soft lockup - with VFS

On Tue, 5 Feb 2008 14:05:06 -0800, Andrew Morton <[email protected]> wrote:

> > > http://students.zipernowsky.hu/~oliverp/kernel/regression_2624/

> I think ub.c is basically abandoned in favour of usb-storage.
> If so, perhaps we should remove or disble ub.c?

Looks like it's just Tomo or Jens made a mistake when converting to
the new s/g API. Nothing to be too concerned about. I know I should've
reviewed their patch closer, but it seemed too simple...

-- Pete

Fix up the conversion to sg_init_table().

Signed-off-by: Pete Zaitcev <[email protected]>

--- a/drivers/block/ub.c
+++ b/drivers/block/ub.c
@@ -657,7 +657,6 @@ static int ub_request_fn_1(struct ub_lun *lun, struct request *rq)
if ((cmd = ub_get_cmd(lun)) == NULL)
return -1;
memset(cmd, 0, sizeof(struct ub_scsi_cmd));
- sg_init_table(cmd->sgv, UB_MAX_REQ_SG);

blkdev_dequeue_request(rq);

@@ -668,6 +667,7 @@ static int ub_request_fn_1(struct ub_lun *lun, struct request *rq)
/*
* get scatterlist from block layer
*/
+ sg_init_table(&urq->sgv[0], UB_MAX_REQ_SG);
n_elem = blk_rq_map_sg(lun->disk->queue, rq, &urq->sgv[0]);
if (n_elem < 0) {
/* Impossible, because blk_rq_map_sg should not hit ENOMEM. */

2008-02-12 01:55:18

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [2.6.24 REGRESSION] BUG: Soft lockup - with VFS

On Fri, 8 Feb 2008 23:46:19 -0800
Pete Zaitcev <[email protected]> wrote:

> On Tue, 5 Feb 2008 14:05:06 -0800, Andrew Morton <[email protected]> wrote:
>
> > > > http://students.zipernowsky.hu/~oliverp/kernel/regression_2624/
>
> > I think ub.c is basically abandoned in favour of usb-storage.
> > If so, perhaps we should remove or disble ub.c?
>
> Looks like it's just Tomo or Jens made a mistake when converting to
> the new s/g API. Nothing to be too concerned about. I know I should've
> reviewed their patch closer, but it seemed too simple...

I guess I can put the blame for this on Jens' commit (45711f1a) ;)

On a serious note, it seems that two scatter lists per request leaded
to this bug. Can the scatter list in struct ub_request be removed?

Thanks,

> -- Pete
>
> Fix up the conversion to sg_init_table().
>
> Signed-off-by: Pete Zaitcev <[email protected]>
>
> --- a/drivers/block/ub.c
> +++ b/drivers/block/ub.c
> @@ -657,7 +657,6 @@ static int ub_request_fn_1(struct ub_lun *lun, struct request *rq)
> if ((cmd = ub_get_cmd(lun)) == NULL)
> return -1;
> memset(cmd, 0, sizeof(struct ub_scsi_cmd));
> - sg_init_table(cmd->sgv, UB_MAX_REQ_SG);
>
> blkdev_dequeue_request(rq);
>
> @@ -668,6 +667,7 @@ static int ub_request_fn_1(struct ub_lun *lun, struct request *rq)
> /*
> * get scatterlist from block layer
> */
> + sg_init_table(&urq->sgv[0], UB_MAX_REQ_SG);
> n_elem = blk_rq_map_sg(lun->disk->queue, rq, &urq->sgv[0]);
> if (n_elem < 0) {
> /* Impossible, because blk_rq_map_sg should not hit ENOMEM. */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2008-02-12 02:18:15

by Pete Zaitcev

[permalink] [raw]
Subject: Re: [2.6.24 REGRESSION] BUG: Soft lockup - with VFS

On Tue, 12 Feb 2008 10:46:12 +0900, FUJITA Tomonori <[email protected]> wrote:

> On a serious note, it seems that two scatter lists per request leaded
> to this bug. Can the scatter list in struct ub_request be removed?

Good question. It's an eyesore to be sure. The duplication exists
for the sake of retries combined with the separation of requests
from commands.

Please bear with me, if you're curious: commands can be launched
without requests (at probe time, for instance, or when sense is
requested). So, they need an s/g table. But then, the lifetime of
a request is greater than than of a command, in case of a retry
especially. Therefore a request needs the s/g table too.

So, one way to kill this duplication is to mandate that a
request existed for every command. It seemed like way more code
than just one memcpy() when I wrote it.

Another way would be to make commands flexible, e.g. sometimes with
just a virtual address and size, sometimes with an s/g table.
If you guys make struct scatterlist illegal to copy with memcpy
one day, this is probably what I'll do.

-- Pete