On Fri, 15 Nov 2013 12:23:06 +0100 Bodo Stroesser <bstroesser@xxxxxxxxxxxxxx> wrote: > On 14.11.2013, at 22.50, Joe Lawrence <joe.lawrence@xxxxxxxxxxx> wrote: > > > On Thu, 14 Nov 2013 20:22:40 +0100 > > Bodo Stroesser <bstroesser@xxxxxxxxxxxxxx> wrote: > > > > > On 14.11.2013, at 20.05, Kai M??kisara (Kolumbus) <kai.makisara@xxxxxxxxxxx> wrote: > > > > > > > > > > > On 14.11.2013, at 16.48, Bodo Stroesser <bstroesser@xxxxxxxxxxxxxx> wrote: > > > > > > < snip > > > > > > > > > > With this patch, blk_get_queue() is not called with the correct argument. > > > > Maybe change the call to blk_get_queue(SDp->request_queue) ? > > > > > > > > > if (blk_get_queue(disk->queue)) > > > > > > Yes, thank you. You are obviously right. Below is the revised patch. > > > Sorry for the mistake. > > > > > > Bodo > > > > > > > > goto out_put_disk; > > > > > + disk->queue = SDp->request_queue; > > > > > tpnt->driver = &st_template; > > > > > > > > > > tpnt->device = SDp-- > > > > > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > > > > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > > ; > > > > > > > > Thanks, > > > > Kai > > > > > > > > > > From: Bodo Stroesser <bstroesser@xxxxxxxxxxxxxx> > > > Date: Thu, 14 Nov 2013 14:35:00 +0100 > > > Subject: [PATCH] sg: fix blk_get_queue usage > > > > > > If blk_queue_get() in st_probe fails, disk->queue must not > > > be set to SDp->request_queue, as that would result in > > > put_disk() dropping a not taken reference. > > > > > > Thus, disk->queue should be set only after a successful > > > blk_queue_get(). > > > Revised patch due to a hint from Kai Makisara. > > > > > > Signed-off-by: Bodo Stroesser <bstroesser@xxxxxxxxxxxxxx> > > > > > > --- > > > > > > --- a/drivers/scsi/st.c 2013-11-14 14:10:40.000000000 +0100 > > > +++ b/drivers/scsi/st.c 2013-11-14 14:10:57.000000000 +0100 > > > @@ -4107,11 +4107,11 @@ > > > kref_init(&tpnt->kref); > > > tpnt->disk = disk; > > > disk->private_data = &tpnt->driver; > > > - disk->queue = SDp->request_queue; > > > /* SCSI tape doesn't register this gendisk via add_disk(). Manually > > > * take queue reference that release_disk() expects. */ > > > - if (blk_get_queue(disk->queue)) > > > + if (blk_get_queue(SDp->request_queue)) > > > goto out_put_disk; > > > + disk->queue = SDp->request_queue; > > > tpnt->driver = &st_template; > > > > > > tpnt->device = SDp; > > > ??{.n?+???????+%??lzwm??b??????r??zX?? ?(?? ????}?????z?&j:+v???????zZ+??+zf???h???~????i???z? ?w?????????&?)?? f > > > > Hi Bodo, > > > > Minor nit, wasn't blk_get_queue modified to return false on failure? > > > > 09ac46c42946 "block: misc updates to blk_get_queue()" > > > > Just curious what tree this patch was tested against. > > > > Thanks, > > > > -- Joe > > > > Hi Joe, > > we are intensively using SuSE SLES11 (currently kernel 3.0.80-xxxxx). At the moment > I'm not involved with mainline. > > The patch resulted from the hunt for another problem and was accepted by SuSE. They > told me that this would be relevant for mainline also. So I posted it but didn't > check mainline for changes :-( > > Thus, here is the third revision of the patch that now is suited for mainline. > Sorry for the noise. > > Thank you. > > Bodo > > ------------------------------- > > From: Bodo Stroesser <bstroesser@xxxxxxxxxxxxxx> > Date: Thu, 14 Nov 2013 14:35:00 +0100 > Subject: [PATCH] sg: fix blk_get_queue usage > > If blk_queue_get() in st_probe fails, disk->queue must not > be set to SDp->request_queue, as that would result in > put_disk() dropping a not taken reference. > > Thus, disk->queue should be set only after a successful > blk_queue_get(). > > 2nd revised patch due to hints from Kai Makisara and Joe Lawrence. > > Now this should be suited for mainline. > > Signed-off-by: Bodo Stroesser <bstroesser@xxxxxxxxxxxxxx> > > --- > > --- a/drivers/scsi/st.c 2013-11-15 11:58:39.000000000 +0100 > +++ b/drivers/scsi/st.c 2013-11-15 11:59:10.000000000 +0100 > @@ -4111,11 +4111,11 @@ static int st_probe(struct device *dev) > kref_init(&tpnt->kref); > tpnt->disk = disk; > disk->private_data = &tpnt->driver; > - disk->queue = SDp->request_queue; > /* SCSI tape doesn't register this gendisk via add_disk(). Manually > * take queue reference that release_disk() expects. */ > - if (!blk_get_queue(disk->queue)) > + if (!blk_get_queue(SDp->request_queue)) > goto out_put_disk; > + disk->queue = SDp->request_queue; > tpnt->driver = &st_template; > > tpnt->device = SDp; > > ????? ?&?~?&???+-??????w???????m?b??lr????^n?r???z???h?????&?? ?G???h?(???????j"???m??????z??????f???h???~?m? Hi Bodo, Thanks for updated patch. I just noticed an error in the original comment as well: "release_disk" should have been "disk_release". No wonder cscope had problems finding that function. Thanks, -- Joe -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
