Commit 70e28c9d authored by Christoph Hellwig's avatar Christoph Hellwig Committed by James Bottomley

[PATCH] change sdev.access_count to an atomic_t

For various reasons (e.g. the .my_devices lockdown) it's undesirable
to have scsi_device_{get,put} blocking.  Change .access_count to an
atomic_t to avoid the driver model r/w semaphore.

Sideeffects:

 - possible module reference leak in scsi_device_get when get_device
   fails fixes.
 - the access_count attribute is gone.  We already have this exposed
   to userspace in too many places given that this whole second
   refcount will go away once Al fixes the block layer.
parent 47f64403
...@@ -889,21 +889,24 @@ int scsi_track_queue_full(struct scsi_device *sdev, int depth) ...@@ -889,21 +889,24 @@ int scsi_track_queue_full(struct scsi_device *sdev, int depth)
int scsi_device_get(struct scsi_device *sdev) int scsi_device_get(struct scsi_device *sdev)
{ {
struct class *class = class_get(&sdev_class); struct class *class = class_get(&sdev_class);
int error = -ENXIO;
if (class) {
down_write(&class->subsys.rwsem);
if (!test_bit(SDEV_DEL, &sdev->sdev_state))
if (try_module_get(sdev->host->hostt->module))
if (get_device(&sdev->sdev_gendev)) {
sdev->access_count++;
error = 0;
}
up_write(&class->subsys.rwsem);
class_put(&sdev_class);
}
return error; if (!class)
goto out;
if (test_bit(SDEV_DEL, &sdev->sdev_state))
goto out;
if (!try_module_get(sdev->host->hostt->module))
goto out;
if (!get_device(&sdev->sdev_gendev))
goto out_put_module;
atomic_inc(&sdev->access_count);
class_put(&sdev_class);
return 0;
out_put_module:
module_put(sdev->host->hostt->module);
out:
class_put(&sdev_class);
return -ENXIO;
} }
void scsi_device_put(struct scsi_device *sdev) void scsi_device_put(struct scsi_device *sdev)
...@@ -913,15 +916,11 @@ void scsi_device_put(struct scsi_device *sdev) ...@@ -913,15 +916,11 @@ void scsi_device_put(struct scsi_device *sdev)
if (!class) if (!class)
return; return;
down_write(&class->subsys.rwsem);
module_put(sdev->host->hostt->module); module_put(sdev->host->hostt->module);
if (--sdev->access_count == 0) { if (atomic_dec_and_test(&sdev->access_count))
if (test_bit(SDEV_DEL, &sdev->sdev_state)) if (test_bit(SDEV_DEL, &sdev->sdev_state))
device_del(&sdev->sdev_gendev); device_del(&sdev->sdev_gendev);
}
put_device(&sdev->sdev_gendev); put_device(&sdev->sdev_gendev);
up_write(&class->subsys.rwsem);
class_put(&sdev_class); class_put(&sdev_class);
} }
......
...@@ -215,7 +215,7 @@ static int scsi_remove_single_device(uint host, uint channel, uint id, uint lun) ...@@ -215,7 +215,7 @@ static int scsi_remove_single_device(uint host, uint channel, uint id, uint lun)
sdev = scsi_find_device(shost, channel, id, lun); sdev = scsi_find_device(shost, channel, id, lun);
if (!sdev) if (!sdev)
goto out; goto out;
if (sdev->access_count) if (atomic_read(&sdev->access_count))
goto out; goto out;
scsi_remove_device(sdev); scsi_remove_device(sdev);
......
...@@ -196,6 +196,7 @@ static struct scsi_device *scsi_alloc_sdev(struct Scsi_Host *shost, ...@@ -196,6 +196,7 @@ static struct scsi_device *scsi_alloc_sdev(struct Scsi_Host *shost,
goto out; goto out;
memset(sdev, 0, sizeof(*sdev)); memset(sdev, 0, sizeof(*sdev));
atomic_set(&sdev->access_count, 0);
sdev->vendor = scsi_null_device_strs; sdev->vendor = scsi_null_device_strs;
sdev->model = scsi_null_device_strs; sdev->model = scsi_null_device_strs;
sdev->rev = scsi_null_device_strs; sdev->rev = scsi_null_device_strs;
......
...@@ -246,7 +246,6 @@ sdev_rd_attr (device_blocked, "%d\n"); ...@@ -246,7 +246,6 @@ sdev_rd_attr (device_blocked, "%d\n");
sdev_rd_attr (queue_depth, "%d\n"); sdev_rd_attr (queue_depth, "%d\n");
sdev_rd_attr (type, "%d\n"); sdev_rd_attr (type, "%d\n");
sdev_rd_attr (scsi_level, "%d\n"); sdev_rd_attr (scsi_level, "%d\n");
sdev_rd_attr (access_count, "%d\n");
sdev_rd_attr (vendor, "%.8s\n"); sdev_rd_attr (vendor, "%.8s\n");
sdev_rd_attr (model, "%.16s\n"); sdev_rd_attr (model, "%.16s\n");
sdev_rd_attr (rev, "%.4s\n"); sdev_rd_attr (rev, "%.4s\n");
...@@ -265,17 +264,14 @@ static ssize_t sdev_store_delete(struct device *dev, const char *buf, ...@@ -265,17 +264,14 @@ static ssize_t sdev_store_delete(struct device *dev, const char *buf,
size_t count) size_t count)
{ {
struct scsi_device *sdev = to_scsi_device(dev); struct scsi_device *sdev = to_scsi_device(dev);
int res = count;
if (sdev->access_count) /*
/* * FIXME and scsi_proc.c: racey use of access_count,
* FIXME and scsi_proc.c: racey use of access_count, */
* possibly add a new arg to scsi_remove_device. if (atomic_read(&sdev->access_count))
*/ return -EBUSY;
res = -EBUSY; scsi_remove_device(sdev);
else return count;
scsi_remove_device(sdev);
return res;
}; };
static DEVICE_ATTR(delete, S_IWUSR, NULL, sdev_store_delete); static DEVICE_ATTR(delete, S_IWUSR, NULL, sdev_store_delete);
...@@ -285,7 +281,6 @@ static struct device_attribute *scsi_sysfs_sdev_attrs[] = { ...@@ -285,7 +281,6 @@ static struct device_attribute *scsi_sysfs_sdev_attrs[] = {
&dev_attr_queue_depth, &dev_attr_queue_depth,
&dev_attr_type, &dev_attr_type,
&dev_attr_scsi_level, &dev_attr_scsi_level,
&dev_attr_access_count,
&dev_attr_vendor, &dev_attr_vendor,
&dev_attr_model, &dev_attr_model,
&dev_attr_rev, &dev_attr_rev,
...@@ -415,7 +410,7 @@ void scsi_remove_device(struct scsi_device *sdev) ...@@ -415,7 +410,7 @@ void scsi_remove_device(struct scsi_device *sdev)
if (class) { if (class) {
down_write(&class->subsys.rwsem); down_write(&class->subsys.rwsem);
set_bit(SDEV_DEL, &sdev->sdev_state); set_bit(SDEV_DEL, &sdev->sdev_state);
if (sdev->access_count == 0) if (atomic_read(&sdev->access_count))
device_del(&sdev->sdev_gendev); device_del(&sdev->sdev_gendev);
up_write(&class->subsys.rwsem); up_write(&class->subsys.rwsem);
} }
......
...@@ -912,7 +912,7 @@ sg_ioctl(struct inode *inode, struct file *filp, ...@@ -912,7 +912,7 @@ sg_ioctl(struct inode *inode, struct file *filp,
case SG_GET_VERSION_NUM: case SG_GET_VERSION_NUM:
return put_user(sg_version_num, (int *) arg); return put_user(sg_version_num, (int *) arg);
case SG_GET_ACCESS_COUNT: case SG_GET_ACCESS_COUNT:
val = (sdp->device ? sdp->device->access_count : 0); val = (sdp->device ? atomic_read(&sdp->device->access_count) : 0);
return put_user(val, (int *) arg); return put_user(val, (int *) arg);
case SG_GET_REQUEST_TABLE: case SG_GET_REQUEST_TABLE:
result = verify_area(VERIFY_WRITE, (void *) arg, result = verify_area(VERIFY_WRITE, (void *) arg,
...@@ -2901,7 +2901,7 @@ sg_proc_dev_info(char *buffer, int *len, off_t * begin, off_t offset, int size) ...@@ -2901,7 +2901,7 @@ sg_proc_dev_info(char *buffer, int *len, off_t * begin, off_t offset, int size)
PRINT_PROC("%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\n", PRINT_PROC("%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\n",
scsidp->host->host_no, scsidp->channel, scsidp->host->host_no, scsidp->channel,
scsidp->id, scsidp->lun, (int) scsidp->type, scsidp->id, scsidp->lun, (int) scsidp->type,
(int) scsidp->access_count, (int) atomic_read(&scsidp->access_count),
(int) scsidp->queue_depth, (int) scsidp->queue_depth,
(int) scsidp->device_busy, (int) scsidp->device_busy,
(int) scsidp->online); (int) scsidp->online);
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include <linux/device.h> #include <linux/device.h>
#include <linux/list.h> #include <linux/list.h>
#include <linux/spinlock.h> #include <linux/spinlock.h>
#include <asm/atomic.h>
struct request_queue; struct request_queue;
struct scsi_cmnd; struct scsi_cmnd;
...@@ -44,7 +45,7 @@ struct scsi_device { ...@@ -44,7 +45,7 @@ struct scsi_device {
* vendor-specific cmd's */ * vendor-specific cmd's */
unsigned sector_size; /* size in bytes */ unsigned sector_size; /* size in bytes */
int access_count; /* Count of open channels/mounts */ atomic_t access_count; /* Count of open channels/mounts */
void *hostdata; /* available to low-level driver */ void *hostdata; /* available to low-level driver */
char devfs_name[256]; /* devfs junk */ char devfs_name[256]; /* devfs junk */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment