On Fri, Apr 02 2004, James Bottomley wrote:
> On Thu, 2004-04-01 at 18:32, James Bottomley wrote:
> > Now, the questions are, whose issue is this and how do we fix it? I can
> > see that a driver needs early notification of unplugs so it can deny all
> > access to a gone device. On the other hand, for a user land open where
> > we still have to hold resources in the driver, we'd like the driver to
> > have a notify when the device reference count drops to zero so we can
> > clean up.
>
> OK, I think this is easily fixable in sr.c by moving around the release
> code to do the right thing (and not drop the reference to sdev_gendev
> until we're completely finished).
>
> Jens, does this look OK (or have I just opened up another race window
> somewhere else)?
Doesn't quite work, how about something like this as a work-around?
diff -ur /mnt/kscratch/linux-2.6.4/drivers/cdrom/cdrom.c
linux-2.6.4-SUSE-20040402/drivers/cdrom/cdrom.c
--- /mnt/kscratch/linux-2.6.4/drivers/cdrom/cdrom.c 2004-04-02 14:53:42.000000000
+0200
+++ linux-2.6.4-SUSE-20040402/drivers/cdrom/cdrom.c 2004-04-05 15:43:01.603675727
+0200
@@ -379,6 +379,8 @@
#endif /* CONFIG_SYSCTL */
}
+ atomic_set(&cdi->all_use, 1);
+
ENSURE(drive_status, CDC_DRIVE_STATUS );
ENSURE(media_changed, CDC_MEDIA_CHANGED);
ENSURE(tray_move, CDC_CLOSE_TRAY | CDC_OPEN_TRAY);
@@ -425,6 +427,9 @@
struct cdrom_device_info *cdi, *prev;
cdinfo(CD_OPEN, "entering unregister_cdrom\n");
+ if (!atomic_dec_and_test(&unreg->all_use))
+ return -EBUSY;
+
prev = NULL;
spin_lock(&cdrom_lock);
cdi = topCdromPtr;
@@ -891,6 +896,8 @@
if (ret)
goto err;
+ atomic_inc(&cdi->all_use);
+
cdinfo(CD_OPEN, "Use count for \"/dev/%s\" now %d\n",
cdi->name, cdi->use_count);
/* Do this on open. Don't wait for mount, because they might
@@ -1096,6 +1103,7 @@
cdi->options & CDO_AUTO_EJECT && CDROM_CAN(CDC_OPEN_TRAY))
cdo->tray_move(cdi, 1);
}
+
return 0;
}
diff -ur /mnt/kscratch/linux-2.6.4/drivers/scsi/sr.c
linux-2.6.4-SUSE-20040402/drivers/scsi/sr.c
--- /mnt/kscratch/linux-2.6.4/drivers/scsi/sr.c 2004-04-02 14:53:42.000000000 +0200
+++ linux-2.6.4-SUSE-20040402/drivers/scsi/sr.c 2004-04-05 15:57:35.577714398 +0200
@@ -419,13 +419,36 @@
static int sr_block_open(struct inode *inode, struct file *file)
{
struct scsi_cd *cd = scsi_cd(inode->i_bdev->bd_disk);
- return cdrom_open(&cd->cdi, inode, file);
+ struct scsi_device *sdev = cd->device;
+ int retval;
+
+ retval = scsi_device_get(sdev);
+ if (retval)
+ return retval;
+
+ retval = cdrom_open(&cd->cdi, inode, file);
+ if (retval)
+ scsi_device_put(sdev);
+
+ return retval;
+}
+
+static void sr_cleanup(struct scsi_cd *cd)
+{
+ if (!unregister_cdrom(&cd->cdi)) {
+ scsi_device_put(cd->device);
+ kfree(cd);
+ }
}
static int sr_block_release(struct inode *inode, struct file *file)
{
struct scsi_cd *cd = scsi_cd(inode->i_bdev->bd_disk);
- return cdrom_release(&cd->cdi, file);
+ int ret;
+
+ ret = cdrom_release(&cd->cdi, file);
+ sr_cleanup(cd);
+ return ret;
}
static int sr_block_ioctl(struct inode *inode, struct file *file, unsigned cmd,
@@ -467,10 +490,6 @@
struct scsi_device *sdev = cd->device;
int retval;
- retval = scsi_device_get(sdev);
- if (retval)
- return retval;
-
/*
* If the device is in error recovery, wait until it is done.
* If the device is offline, then disallow any access to it.
@@ -499,8 +518,6 @@
if (cd->device->sector_size > 2048)
sr_set_blocklength(cd, 2048);
-
- scsi_device_put(cd->device);
}
static int sr_probe(struct device *dev)
@@ -874,8 +891,8 @@
spin_unlock(&sr_index_lock);
put_disk(cd->disk);
- unregister_cdrom(&cd->cdi);
- kfree(cd);
+
+ sr_cleanup(cd);
return 0;
}
diff -ur /mnt/kscratch/linux-2.6.4/include/linux/cdrom.h
linux-2.6.4-SUSE-20040402/include/linux/cdrom.h
--- /mnt/kscratch/linux-2.6.4/include/linux/cdrom.h 2004-04-02 14:53:42.000000000
+0200
+++ linux-2.6.4-SUSE-20040402/include/linux/cdrom.h 2004-04-05 15:10:46.347243055
+0200
@@ -916,6 +916,7 @@
/* Uniform cdrom data structures for cdrom.c */
struct cdrom_device_info {
+ atomic_t all_use;
struct cdrom_device_ops *ops; /* link to device_ops */
struct cdrom_device_info *next; /* next device_info for this major */
struct gendisk *disk; /* matching block layer disk */
--
Jens Axboe
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel