Re: PATCH: st.c: Fix blk_get_queue usage

[Bodo Stroesser <bstroesser@ts.fujitsu.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=us-ascii, Size: 3097 bytes --]

On 14.11.2013, at 20.05, Kai Mäkisara (Kolumbus) <kai.makisara@kolumbus.fi> wrote:

> 
> On 14.11.2013, at 16.48, Bodo Stroesser <bstroesser@ts.fujitsu.com> wrote:
> 
> > Hi,
> > 
> > in st_probe(), st.c I stumbled across what I'd call a minor problem.
> > 
> > So I'd like to suggest the following patch.
> > 
> > Best Regards,
> > Bodo
> > 
> > P.S.: Please CC me, I'm not on the list.
> > 
> > ---------------------------------------------------------------------
> > 
> > 
> > From: Bodo Stroesser <bstroesser@ts.fujitsu.com>
> > 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().
> > 
> > Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
> > 
> > ---
> > 
> > --- 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. */
> 
> 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@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > ;
> 
> Thanks,
> Kai
> 

From: Bodo Stroesser <bstroesser@ts.fujitsu.com>
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@ts.fujitsu.com>

---

--- 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;