This is my first attempt at submitting a patch, so I hope I'm not making any
mistakes...
This patch fixes two problems I came across in sg, both of which occur when
sg_remove is called on a disk which hasn't yet been sg_release'd:
1. I got the following Oops in sg_remove:
--
Unable to handle kernel paging request at virtual address e0a58020
printing eip:
e0a7ceba
*pde = 1c245c8b
Oops: 0000 [#1]
SMP
CPU: 3
EIP: 0060:[<e0a7ceba>] Tainted: G U
EFLAGS: 00210246 (2.6.5-7.139-bigsmp )
EIP is at sg_remove+0xba/0x240 [sg]
eax: dfee0500 ebx: 00000000 ecx: dfeefa50 edx: dfee05a8
esi: 00000000 edi: c7beb000 ebp: e0a58000 esp: daf4bec0
ds: 007b es: 007b ss: 0068
Process bash (pid: 27436, threadinfo=daf4a000 task=dbcd3350)
Stack: c10c1840 000000d0 00000001 01500002 00000000 00200246 df844000
e0a84d08
df844240 e0857ec0 e0857f24 c0263dff e0857f0c df844240 df9d3028
df9d3000
fffffffa c0263e78 df844000 e08449be df9d3000 df844000 00000000
fffffffa
Call Trace:
[<c0263dff>] class_device_del+0x5f/0xd0
[<c0263e78>] class_device_unregister+0x8/0x10
[<e08449be>] scsi_remove_device+0x3e/0xc0 [scsi_mod]
[<e0845b59>] proc_scsi_write+0x269/0x2a0 [scsi_mod]
[<c0176136>] vfs_write+0xc6/0x160
[<c011057d>] do_mmap2+0xdd/0x170
[<c01763e1>] sys_write+0x91/0xf0
[<c0109199>] sysenter_past_esp+0x52/0x71
Code: 8b 45 20 e8 ce 2a 70 df c7 45 20 00 00 00 00 8b 45 1c e8 ef
--
It looks like sg_remove needs to hold the sg_dev_arr_lock a little longer.
After sg_remove dropped the lock, sg_release/sg_remove_sfp came along and
freed the sdp... and then sg_remove tried to do cdev_del(sdp->cdev). I made
a change to get what we need from the sdp while holding the lock, and it
fixed the problem for me.
2. Scsi_device references are never released: after sg_remove sets the
detached flag, sg_release won't call scsi_device_put. But someone needs to
release the reference obtained in sg_open. A change also needs to be made in
sg_remove_sfp to call scsi_device_put if freeing sdp--since sg_release
wouldn't be able to do it in that case. I verified (by sticking a printk in
scsi_device_dev_release) that the scsi_device wasn't freed in this case,
without my change in place.
To provoke these, I used a few 'dd if=/dev/sg2 of=/dev/null' commands to
hold the device open. I then did 'echo "scsi remove-single-device 1 0 0 0" >
/proc/scsi/scsi' to trigger the sg_remove.
This patch is against 2.6.12-rc2, though I actually found/fixed the problems
in 2.6.5-7.139 (SLES9 SP1).
Do these look okay?
Nate Dailey
Stratus Technologies
Signed-off-by: Nate Dailey <nate.dailey@xxxxxxxxxxx>
--- linux-2.6.12-rc2/drivers/scsi/sg.c.orig 2005-04-06
14:17:51.000000000 -0400
+++ linux-2.6.12-rc2/drivers/scsi/sg.c 2005-04-06 15:25:08.000000000 -0400
@@ -318,9 +318,7 @@ sg_release(struct inode *inode, struct f
SCSI_LOG_TIMEOUT(3, printk("sg_release: %s\n",
sdp->disk->disk_name));
sg_fasync(-1, filp, 0); /* remove filp from async notification list
*/
if (0 == sg_remove_sfp(sdp, sfp)) { /* Returns 1 when sdp gone
*/
- if (!sdp->detached) {
- scsi_device_put(sdp->device);
- }
+ scsi_device_put(sdp->device);
sdp->exclude = 0;
wake_up_interruptible(&sdp->o_excl_wait);
}
@@ -1574,19 +1572,28 @@ sg_remove(struct class_device *cl_dev)
sg_nr_dev--;
break;
}
- write_unlock_irqrestore(&sg_dev_arr_lock, iflags);
if (sdp) {
+ struct cdev *tmp_cdev;
+ struct gendisk *tmp_disk;
+ int free_sdp = 0;
+
+ tmp_cdev = sdp->cdev;
+ sdp->cdev = NULL;
+ tmp_disk = sdp->disk;
+ sdp->disk = NULL;
+ if (NULL == sdp->headfp) free_sdp = 1;
+ write_unlock_irqrestore(&sg_dev_arr_lock, iflags);
+
sysfs_remove_link(&scsidp->sdev_gendev.kobj, "generic");
class_simple_device_remove(MKDEV(SCSI_GENERIC_MAJOR, k));
- cdev_del(sdp->cdev);
- sdp->cdev = NULL;
+ cdev_del(tmp_cdev);
devfs_remove("%s/generic", scsidp->devfs_name);
- put_disk(sdp->disk);
- sdp->disk = NULL;
- if (NULL == sdp->headfp)
+ put_disk(tmp_disk);
+ if (free_sdp)
kfree((char *) sdp);
}
+ else write_unlock_irqrestore(&sg_dev_arr_lock, iflags);
if (delay)
msleep(10); /* dirty detach so delay device destruction
*/
@@ -2550,6 +2557,7 @@ sg_remove_sfp(Sg_device * sdp, Sg_fd * s
}
if (k < maxd)
sg_dev_arr[k] = NULL;
+ scsi_device_put(sdp->device);
kfree((char *) sdp);
res = 1;
}
-
: 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
