Skip to contentSkip to conversation

PATCH: st.c: Fix blk_get_queue usage

DISCUSSION Bodo Stroesser 3 participants 3 messages Nov 14, 2013

Subsystems (2)
SCSI SCSI tape driver
lore
Share & tools
lore

Bodo Stroesser

author
#0 lore raw
Quoted 32 lines · Kai Mäkisara (Kolumbus)
Quoted 29 lines · Bodo Stroesser

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 <[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().

Signed-off-by Bodo Stroesser <[email protected]>

---

Quoted 14 lines

drivers/scsi/st.c line 4111

Quoted 9 lines
drivers/scsi/st.c +0 -1
@@ -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) ?

Quoted 1 line
drivers/scsi/st.c +0 -0
 	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
drivers/scsi/st.c +1 -0
 		goto out_put_disk;
+	disk->queue = SDp->request_queue;
 	tpnt->driver = &st_template;
Quoted 9 lines
Quoted 5 lines
drivers/scsi/st.c +0 -0
 	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;
Acked-by
#1 lore raw

On Thu, 14 Nov 2013, Bodo Stroesser wrote:

...

Quoted 8 lines

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.

Quoted 5 lines

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]>
Acked-by Kai Mäkisara <[email protected]>
Quoted 9 lines

---

drivers/scsi/st.c +0 -0
@@ -4107,11 +4107,11 @@
 	kref_init(&tpnt->kref);
 	tpnt->disk = disk;
 	disk->private_data = &tpnt->driver;

drivers/scsi/st.c line 4117

Quoted 10 lines
drivers/scsi/st.c +2 -2
-	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;

Thanks for the fix,
Kai

#2 lore raw
In reply to Bodo Stroesser
Quoted 100 lines · On Thu, 14 Nov 2013 20:22:40 +0100 Bodo Stroesser <[email protected]>…
Quoted 32 lines · Kai M??kisara (Kolumbus)
Quoted 29 lines · Bodo Stroesser

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 <[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().

Signed-off-by Bodo Stroesser <[email protected]>

---

Quoted 14 lines

drivers/scsi/st.c line 4111

Quoted 9 lines
drivers/scsi/st.c +0 -1
@@ -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) ?

Quoted 1 line
drivers/scsi/st.c +0 -0
 	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
drivers/scsi/st.c +1 -0
 		goto out_put_disk;
+	disk->queue = SDp->request_queue;
 	tpnt->driver = &st_template;
Quoted 9 lines
Quoted 5 lines
drivers/scsi/st.c +0 -0
 	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?

09ac46c42946 "block: misc updates to blk_get_queue()"

Just curious what tree this patch was tested against.

Thanks,

-- Joe