Hi,
I don't understand how error paths are supposed to work with this patch.
As I see it, if we fail to get a reference, the error path tries to put a
non-existent one. moreover, error paths on lines 4188 and 4196 still goto
out_put_disk, leaking the reference.
other than that, it fixes the load/unload cycle issue for me as well
thanks,
On 04/03/13 17:14, Joe Lawrence wrote:
These changes were applied to scsi.git, branch "misc". This patch
fixes a reference count bug in the SCSI tape driver which can be
reproduced with the following:
* Boot with slub_debug=FZPU, tape drive attached
* echo 1 > /sys/devices/... tape device pci path .../remove
* Wait for device removal
* echo 1 > /sys/kernel/slab/blkdev_queue/validate
* Slub debug complains about corrupted poison pattern
In commit 523e1d39 (block: make gendisk hold a reference to its queue)
add_disk() and disk_release() were modified to get/put an additional
reference on a disk queue to fix a reference counting discrepency
between bdev release and SCSI device removal. The ST driver never
calls add_disk(), so this commit introduced an extra kref put when the
ST driver frees its struct gendisk.
Attempts were made to fix this bug at the block level [1] but later
abandoned due to floppy driver issues [2].
[1] https://lkml.org/lkml/2012/8/27/354
[2] https://lkml.org/lkml/2012/9/22/113
Regards,
-- Joe
From f209bda3ee16a70e714b55c372ed4265bfa91784 Mon Sep 17 00:00:00 2001
From: Joe Lawrence <joe.lawrence@xxxxxxxxxxx>
Date: Fri, 1 Mar 2013 17:32:30 -0500
Subject: [PATCH] st: Take additional queue ref in st_probe
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The SCSI tape driver employs a struct gendisk, calling alloc_disk() to
create an instance, but does not register it via add_disk(). When the
gendisk is torn down, disk_release() is called and expects to return a
disk queue reference that add_disk() normally would have taken out. (See
commit 523e1d39.) Fix the kref accounting by adding a blk_get_queue()
to st_probe().
Signed-off-by: Joe Lawrence <joe.lawrence@xxxxxxxxxxx>
Tested-by: Ewan Milne <emilne@xxxxxxxxxx>
Cc: Kai Mäkisara <Kai.Makisara@xxxxxxxxxxx>
Cc: James E.J. Bottomley <JBottomley@xxxxxxxxxxxxx>
Cc: Ewan Milne <emilne@xxxxxxxxxx>
Cc: linux-scsi@xxxxxxxxxxxxxxx
---
drivers/scsi/st.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 98156a9..eef8ddc 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -4112,6 +4112,10 @@ static int st_probe(struct device *dev)
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_queue;
tpnt->driver = &st_template;
tpnt->device = SDp;
@@ -4222,6 +4226,8 @@ out_put_index:
spin_lock(&st_index_lock);
idr_remove(&st_index_idr, dev_num);
spin_unlock(&st_index_lock);
+out_put_queue:
+ blk_put_queue(disk->queue);
out_put_disk:
put_disk(disk);
kfree(tpnt);
--
-----
Jan Vesely <jvesely@xxxxxxxxxx>
--
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
