From: Ming Lei <ming.lei@xxxxxxxxxx>
Fix the race conditions between SCSI LLD kernel module unloading and SCSI
device and target removal by making sure that SCSI hosts are destroyed after
all associated target and device objects have been freed.
Cc: Christoph Hellwig <hch@xxxxxx>
Cc: Ming Lei <ming.lei@xxxxxxxxxx>
Cc: Hannes Reinecke <hare@xxxxxxx>
Cc: John Garry <john.garry@xxxxxxxxxx>
Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
[ bvanassche: Reworked Ming's patch ]
---
drivers/scsi/hosts.c | 9 +++++++++
drivers/scsi/scsi.c | 9 ++++++---
drivers/scsi/scsi_scan.c | 7 +++++++
drivers/scsi/scsi_sysfs.c | 9 ---------
include/scsi/scsi_host.h | 3 +++
5 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index ef6c0e37acce..e0a56a8f1f74 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -168,6 +168,7 @@ void scsi_remove_host(struct Scsi_Host *shost)
mutex_lock(&shost->scan_mutex);
spin_lock_irqsave(shost->host_lock, flags);
+ /* Prevent that new SCSI targets or SCSI devices are added. */
if (scsi_host_set_state(shost, SHOST_CANCEL))
if (scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY)) {
spin_unlock_irqrestore(shost->host_lock, flags);
@@ -190,6 +191,13 @@ void scsi_remove_host(struct Scsi_Host *shost)
transport_unregister_device(&shost->shost_gendev);
device_unregister(&shost->shost_dev);
device_del(&shost->shost_gendev);
+
+ /*
+ * After scsi_remove_host() has returned the scsi LLD module can be
+ * unloaded and/or the host resources can be released. Hence wait until
+ * the dependent SCSI targets and devices are gone before returning.
+ */
+ wait_event(shost->targets_wq, atomic_read(&shost->target_count) == 0);
}
EXPORT_SYMBOL(scsi_remove_host);
@@ -394,6 +402,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
INIT_LIST_HEAD(&shost->starved_list);
init_waitqueue_head(&shost->host_wait);
mutex_init(&shost->scan_mutex);
+ init_waitqueue_head(&shost->targets_wq);
index = ida_alloc(&host_index_ida, GFP_KERNEL);
if (index < 0) {
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index c59eac7a32f2..086ec5b5862d 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -586,10 +586,13 @@ EXPORT_SYMBOL(scsi_device_get);
*/
void scsi_device_put(struct scsi_device *sdev)
{
- struct module *mod = sdev->host->hostt->module;
-
+ /*
+ * Decreasing the module reference count before the device reference
+ * count is safe since scsi_remove_host() only returns after all
+ * devices have been removed.
+ */
+ module_put(sdev->host->hostt->module);
put_device(&sdev->sdev_gendev);
- module_put(mod);
}
EXPORT_SYMBOL(scsi_device_put);
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 91ac901a6682..00c5754f0081 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -406,9 +406,14 @@ static void scsi_target_destroy(struct scsi_target *starget)
static void scsi_target_dev_release(struct device *dev)
{
struct device *parent = dev->parent;
+ struct Scsi_Host *shost = dev_to_shost(parent);
struct scsi_target *starget = to_scsi_target(dev);
kfree(starget);
+
+ if (atomic_dec_return(&shost->target_count) == 0)
+ wake_up(&shost->targets_wq);
+
put_device(parent);
}
@@ -521,6 +526,8 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
starget->state = STARGET_CREATED;
starget->scsi_level = SCSI_2;
starget->max_target_blocked = SCSI_DEFAULT_TARGET_BLOCKED;
+ atomic_inc(&shost->target_count);
+
retry:
spin_lock_irqsave(shost->host_lock, flags);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 43949798a2e4..72bf5131e9d8 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -450,12 +450,9 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
struct scsi_vpd *vpd_pg0 = NULL, *vpd_pg89 = NULL;
struct scsi_vpd *vpd_pgb0 = NULL, *vpd_pgb1 = NULL, *vpd_pgb2 = NULL;
unsigned long flags;
- struct module *mod;
sdev = container_of(work, struct scsi_device, ew.work);
- mod = sdev->host->hostt->module;
-
scsi_dh_release_device(sdev);
parent = sdev->sdev_gendev.parent;
@@ -518,17 +515,11 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
if (parent)
put_device(parent);
- module_put(mod);
}
static void scsi_device_dev_release(struct device *dev)
{
struct scsi_device *sdp = to_scsi_device(dev);
-
- /* Set module pointer as NULL in case of module unloading */
- if (!try_module_get(sdp->host->hostt->module))
- sdp->host->hostt->module = NULL;
-
execute_in_process_context(scsi_device_dev_release_usercontext,
&sdp->ew);
}
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 667d889b92b5..339f975d356e 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -689,6 +689,9 @@ struct Scsi_Host {
/* ldm bits */
struct device shost_gendev, shost_dev;
+ atomic_t target_count;
+ wait_queue_head_t targets_wq;
+
/*
* Points to the transport data (if any) which is allocated
* separately
