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

Reply via email to