PATCH: st.c: Fix blk_get_queue usage

DISCUSSION Bodo Stroesser 2 participants 2 messages Nov 15, 2013

Subsystems (2)

SCSI SCSI tape driver

lore

Share & tools

lore

Bodo Stroesser

author

2013-11-15 11:23 UTC #0 lore raw

Quoted 8 lines · Joe Lawrence

Quoted 5 lines · On Thu, 14 Nov 2013 20:22:40 +0100 Bodo Stroesser <[email protected]>…
Quoted 3 lines · Kai M??kisara (Kolumbus)
On 14.11.2013, at 16.48, Bodo Stroesser <[email protected]> wrote:

< snip >

Quoted 63 lines

Quoted 57 lines
Quoted 5 lines
With this patch, blk_get_queue() is not called with the correct argument. Maybe change the call to blk_get_queue(SDp->request_queue) ?
Quoted 1 line
	if (blk_get_queue(disk->queue))
Yes, thank you. You are obviously right. Below is the revised patch. Sorry for the mistake.
Bodo
Quoted 4 lines
Quoted 4 lines
		goto out_put_disk;
(quoted diff) +1 -0
+	disk->queue = SDp->request_queue;
	tpnt->driver = &st_template;
Quoted 9 lines
Quoted 5 lines
	tpnt->device = SDp--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
;
Thanks,
Kai
From: Bodo Stroesser <[email protected]>
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 <[email protected]>
---
drivers/scsi/st.c +2 -2
@@ -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;
Quoted 1 line
??{.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?

Quoted 8 lines

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 <[email protected]>
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 <[email protected]>

---

drivers/scsi/st.c +2 -2

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

Joe Lawrence

2013-11-15 16:59 UTC #1 lore raw

Quoted 139 lines · On Fri, 15 Nov 2013 12:23:06 +0100 Bodo Stroesser <[email protected]>…

Quoted 8 lines · Joe Lawrence
Quoted 5 lines · On Thu, 14 Nov 2013 20:22:40 +0100 Bodo Stroesser <[email protected]>…
Quoted 3 lines · Kai M??kisara (Kolumbus)
On 14.11.2013, at 16.48, Bodo Stroesser <[email protected]> wrote:
< snip >
Quoted 63 lines
Quoted 57 lines
Quoted 5 lines
With this patch, blk_get_queue() is not called with the correct argument. Maybe change the call to blk_get_queue(SDp->request_queue) ?
Quoted 1 line
	if (blk_get_queue(disk->queue))
Yes, thank you. You are obviously right. Below is the revised patch. Sorry for the mistake.
Bodo
Quoted 4 lines
Quoted 4 lines
		goto out_put_disk;
drivers/scsi/st.c +1 -0
+	disk->queue = SDp->request_queue;
	tpnt->driver = &st_template;
Quoted 9 lines
Quoted 5 lines
	tpnt->device = SDp--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
;
Thanks,
Kai
From: Bodo Stroesser <[email protected]>
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 <[email protected]>
---
drivers/scsi/st.c +2 -2
@@ -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;
Quoted 1 line
??{.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?
Quoted 8 lines
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 <[email protected]>
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 <[email protected]>
---
drivers/scsi/st.c +2 -2
@@ -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;

Quoted 1 line

?????

?&?~?&???+-??????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