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/ - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
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 - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
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. */ - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
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 - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
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 - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
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 - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
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? - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html