All of lore.kernel.org
 help / color / mirror / Atom feed
* PATCH: st.c: Fix blk_get_queue usage
@ 2013-11-14 14:48 Bodo Stroesser
  2013-11-14 19:05 ` "Kai Mäkisara (Kolumbus)"
  0 siblings, 1 reply; 7+ messages in thread
From: Bodo Stroesser @ 2013-11-14 14:48 UTC (permalink / raw)
  To: linux-scsi, bstroesser

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. */
 	if (blk_get_queue(disk->queue))
 		goto out_put_disk;
+	disk->queue = SDp->request_queue;
 	tpnt->driver = &st_template;
 
 	tpnt->device = SDp;

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: PATCH: st.c: Fix blk_get_queue usage
@ 2013-11-14 19:22 Bodo Stroesser
  2013-11-14 21:08 ` Kai Makisara
  2013-11-14 21:50 ` Joe Lawrence
  0 siblings, 2 replies; 7+ messages in thread
From: Bodo Stroesser @ 2013-11-14 19:22 UTC (permalink / raw)
  To: linux-scsi, bstroesser

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

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: PATCH: st.c: Fix blk_get_queue usage
@ 2013-11-15 11:23 Bodo Stroesser
  2013-11-15 16:59 ` Joe Lawrence
  0 siblings, 1 reply; 7+ messages in thread
From: Bodo Stroesser @ 2013-11-15 11:23 UTC (permalink / raw)
  To: joe.lawrence, kai.makisara, linux-scsi; +Cc: bstroesser

On 14.11.2013, at 22.50, Joe Lawrence <joe.lawrence@stratus.com> wrote:

> On Thu, 14 Nov 2013 20:22:40 +0100
> Bodo Stroesser <bstroesser@ts.fujitsu.com> wrote:
> 
> > 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:
> > > 

< 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@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;
> >  ??{.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@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().

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@ts.fujitsu.com>

---

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-11-15 16:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-14 14:48 PATCH: st.c: Fix blk_get_queue usage Bodo Stroesser
2013-11-14 19:05 ` "Kai Mäkisara (Kolumbus)"
  -- strict thread matches above, loose matches on Subject: below --
2013-11-14 19:22 Bodo Stroesser
2013-11-14 21:08 ` Kai Makisara
2013-11-14 21:50 ` Joe Lawrence
2013-11-15 11:23 Bodo Stroesser
2013-11-15 16:59 ` Joe Lawrence

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.