update sr to use kref and fix open/release race
PATCH 1/1 update sr to use kref and fix open/release race
The previous patch actually didn't null out the disk->private_data pointer in it's release mechanism. This does that and converts sr to use the lightweight kref object instead of a kobject.
James
===== drivers/scsi/sr.c 1.104 vs edited =====
drivers/scsi/sr.c +29 -13
@@ -86,6 +86,7 @@static unsigned long sr_index_bits[SR_DISKS /BITS_PER_LONG ];static spinlock_t sr_index_lock =SPIN_LOCK_UNLOCKED ; +static DECLARE_MUTEX (sr_ref_sem);static int sr_open (struct cdrom_device_info *,int );static void sr_release (struct cdrom_device_info *);
@@ -119,20 +120,35 @@ .release = sr_kobject_release, }; +static inline struct scsi_cd *scsi_cd (struct gendisk *disk) +{ + return container_of(disk->private_data, struct scsi_cd, driver); +} +/* * The get and put routines for the struct scsi_cd. Note this entity * has a scsi_device pointer and owns a reference to this. */ -static inline int scsi_cd_get (struct scsi_cd *cd) +static inline struct scsi_cd *scsi_cd_get (struct gendisk *disk) { +struct scsi_cd *cd =NULL ; + +down (&sr_ref_sem); +if (disk->private_data ==NULL ) + goto out; + cd =scsi_cd (disk);if (!kobject_get (&cd->kobj)) -return -ENODEV ; -return 0 ; + cd =NULL ; + out: +up (&sr_ref_sem); +return cd; }static inline void scsi_cd_put (struct scsi_cd *cd) { +down (&sr_ref_sem);kobject_put (&cd->kobj); +up (&sr_ref_sem); }/*
@@ -187,11 +203,6 @@return retval; } -static inline struct scsi_cd *scsi_cd (struct gendisk *disk) -{ - return container_of(disk->private_data, struct scsi_cd, driver); -} -/* * rw_intr is the interrupt routine for the device driver. *
@@ -440,8 +451,17 @@static int sr_block_open (struct inode *inode,struct file *file) { +struct gendisk *disk = inode->i_bdev->bd_disk;struct scsi_cd *cd =scsi_cd (inode->i_bdev->bd_disk); -return cdrom_open (&cd->cdi, inode, file); +int ret =0 ; + +if (!(cd =scsi_cd_get (disk))) +return -ENXIO ; + +if ((ret =cdrom_open (&cd->cdi, inode, file)) !=0 ) +scsi_cd_put (cd); + +return ret; }static int sr_block_release (struct inode *inode,struct file *file)
@@ -496,10 +516,6 @@struct scsi_device *sdev = cd->device;int retval; - retval =scsi_cd_get (cd); -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.
And, er, with the correct patch this time.
James
===== sr.c 1.104 vs edited =====
sr.c renamed from drivers/scsi/sr.c +39 -25
@@ -86,6 +86,7 @@static unsigned long sr_index_bits[SR_DISKS /BITS_PER_LONG ];static spinlock_t sr_index_lock =SPIN_LOCK_UNLOCKED ; +static DECLARE_MUTEX (sr_ref_sem);static int sr_open (struct cdrom_device_info *,int );static void sr_release (struct cdrom_device_info *);
@@ -113,26 +114,37 @@ .generic_packet = sr_packet, }; -static void sr_kobject_release (struct kobject *kobj); +static void sr_kref_release (struct kref *kref); -static struct kobj_type scsi_cdrom_kobj_type = { - .release = sr_kobject_release, -}; +static inline struct scsi_cd *scsi_cd (struct gendisk *disk) +{ + return container_of(disk->private_data, struct scsi_cd, driver); +}/* * The get and put routines for the struct scsi_cd. Note this entity * has a scsi_device pointer and owns a reference to this. */ -static inline int scsi_cd_get (struct scsi_cd *cd) +static inline struct scsi_cd *scsi_cd_get (struct gendisk *disk) { -if (!kobject_get (&cd->kobj)) -return -ENODEV ; -return 0 ; +struct scsi_cd *cd =NULL ; + +down (&sr_ref_sem); +if (disk->private_data ==NULL ) + goto out; + cd =scsi_cd (disk); +if (!kref_get (&cd->kref)) + cd =NULL ; + out: +up (&sr_ref_sem); +return cd; }static inline void scsi_cd_put (struct scsi_cd *cd) { -kobject_put (&cd->kobj); +down (&sr_ref_sem); +kref_put (&cd->kref); +up (&sr_ref_sem); }/*
@@ -187,11 +199,6 @@return retval; } -static inline struct scsi_cd *scsi_cd (struct gendisk *disk) -{ - return container_of(disk->private_data, struct scsi_cd, driver); -} -/* * rw_intr is the interrupt routine for the device driver. *
@@ -440,8 +447,17 @@static int sr_block_open (struct inode *inode,struct file *file) { +struct gendisk *disk = inode->i_bdev->bd_disk;struct scsi_cd *cd =scsi_cd (inode->i_bdev->bd_disk); -return cdrom_open (&cd->cdi, inode, file); +int ret =0 ; + +if (!(cd =scsi_cd_get (disk))) +return -ENXIO ; + +if ((ret =cdrom_open (&cd->cdi, inode, file)) !=0 ) +scsi_cd_put (cd); + +return ret; }static int sr_block_release (struct inode *inode,struct file *file)
@@ -496,10 +512,6 @@struct scsi_device *sdev = cd->device;int retval; - retval =scsi_cd_get (cd); -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.
@@ -551,8 +563,7 @@ goto fail_put_sdev;memset (cd,0 ,sizeof (*cd)); -kobject_init (&cd->kobj); - cd->kobj.ktype = &scsi_cdrom_kobj_type; +kref_init (&cd->kref, sr_kref_release); disk =alloc_disk (1 );if (!disk)
@@ -899,18 +910,21 @@return cgc->stat; } -static void sr_kobject_release (struct kobject *kobj) +static void sr_kref_release (struct kref *kref) { - struct scsi_cd *cd = container_of(kobj, struct scsi_cd, kobj); + struct scsi_cd *cd = container_of(kref, struct scsi_cd, kref);struct scsi_device *sdev = cd->device; +struct gendisk *disk = cd->disk;spin_lock (&sr_index_lock); -clear_bit (cd->disk->first_minor, sr_index_bits); +clear_bit (disk->first_minor, sr_index_bits);spin_unlock (&sr_index_lock);unregister_cdrom (&cd->cdi); -put_disk (cd->disk); + disk->private_data =NULL ; + +put_disk (disk);kfree (cd);
===== sr.h 1.11 vs edited =====
sr.h renamed from drivers/scsi/sr.h +3 -2
@@ -19,6 +19,7 @@#include "scsi.h" #include <linux/genhd.h> +#include <linux/kref.h> /* The CDROM is fairly slow, so we need a little extra time */ /* In fact, it is very slow if it has to spin up first */
@@ -37,8 +38,8 @@ unsigned readcd_cdda:1; /* reading audio data using READ_CD */struct cdrom_device_info cdi;/* We hold gendisk and scsi_device references on probe and use -* the refs on this kobj to decide when to release them */ -struct kobject kobj; +* the refs on this kref to decide when to release them */ +struct kref kref;struct gendisk *disk; } Scsi_CD;