[SCSI] fix scsi process problems and clean up the target reap issues

In order to use the new execute_in_process_context() API, you have to
provide it with the work storage, which I do in SCSI in scsi_device and
scsi_target, but which also means that we can no longer queue up the
target reaps, so instead I moved the target to a state model which
allows target_alloc to detect if we've received a dying target and wait
for it to be gone.  Hopefully, this should also solve the target
namespace race.

Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index eab303d148d8b8d9d8252c2fffcd6517ee5a0504..3042520c413cc83a46ca038fe1e4f9b39f1122c9 100644 (file)
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2257,61 +2257,3 @@ scsi_target_unblock(struct device *dev)
                device_for_each_child(dev, NULL, target_unblock);
 }
 EXPORT_SYMBOL_GPL(scsi_target_unblock);
-
-
-struct work_queue_work {
-       struct work_struct      work;
-       void                    (*fn)(void *);
-       void                    *data;
-};
-
-static void execute_in_process_context_work(void *data)
-{
-       void (*fn)(void *data);
-       struct work_queue_work *wqw = data;
-
-       fn = wqw->fn;
-       data = wqw->data;
-
-       kfree(wqw);
-
-       fn(data);
-}
-
-/**
- * scsi_execute_in_process_context - reliably execute the routine with user context
- * @fn:                the function to execute
- * @data:      data to pass to the function
- *
- * Executes the function immediately if process context is available,
- * otherwise schedules the function for delayed execution.
- *
- * Returns:    0 - function was executed
- *             1 - function was scheduled for execution
- *             <0 - error
- */
-int scsi_execute_in_process_context(void (*fn)(void *data), void *data)
-{
-       struct work_queue_work *wqw;
-
-       if (!in_interrupt()) {
-               fn(data);
-               return 0;
-       }
-
-       wqw = kmalloc(sizeof(struct work_queue_work), GFP_ATOMIC);
-
-       if (unlikely(!wqw)) {
-               printk(KERN_ERR "Failed to allocate memory\n");
-               WARN_ON(1);
-               return -ENOMEM;
-       }
-
-       INIT_WORK(&wqw->work, execute_in_process_context_work, wqw);
-       wqw->fn = fn;
-       wqw->data = data;
-       schedule_work(&wqw->work);
-
-       return 1;
-}
-EXPORT_SYMBOL_GPL(scsi_execute_in_process_context);

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 94b86d5b14696d1303e716b3c723a4114f1b4f8d..84f01fd0c8fde8fbc7c5a404bfbc775c6d286280 100644 (file)
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -349,6 +349,8 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
        starget->channel = channel;
        INIT_LIST_HEAD(&starget->siblings);
        INIT_LIST_HEAD(&starget->devices);
+       starget->state = STARGET_RUNNING;
+ retry:
        spin_lock_irqsave(shost->host_lock, flags);
 
        found_target = __scsi_find_target(parent, channel, id);
@@ -381,8 +383,15 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
        found_target->reap_ref++;
        spin_unlock_irqrestore(shost->host_lock, flags);
        put_device(parent);
-       kfree(starget);
-       return found_target;
+       if (found_target->state != STARGET_DEL) {
+               kfree(starget);
+               return found_target;
+       }
+       /* Unfortunately, we found a dying target; need to
+        * wait until it's dead before we can get a new one */
+       put_device(&found_target->dev);
+       flush_scheduled_work();
+       goto retry;
 }
 
 static void scsi_target_reap_usercontext(void *data)
@@ -391,21 +400,13 @@ static void scsi_target_reap_usercontext(void *data)
        struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
        unsigned long flags;
 
+       transport_remove_device(&starget->dev);
+       device_del(&starget->dev);
+       transport_destroy_device(&starget->dev);
        spin_lock_irqsave(shost->host_lock, flags);
-
-       if (--starget->reap_ref == 0 && list_empty(&starget->devices)) {
-               list_del_init(&starget->siblings);
-               spin_unlock_irqrestore(shost->host_lock, flags);
-               transport_remove_device(&starget->dev);
-               device_del(&starget->dev);
-               transport_destroy_device(&starget->dev);
-               put_device(&starget->dev);
-               return;
-
-       }
+       list_del_init(&starget->siblings);
        spin_unlock_irqrestore(shost->host_lock, flags);
-
-       return;
+       put_device(&starget->dev);
 }
 
 /**
@@ -419,7 +420,23 @@ static void scsi_target_reap_usercontext(void *data)
  */
 void scsi_target_reap(struct scsi_target *starget)
 {
-       scsi_execute_in_process_context(scsi_target_reap_usercontext, starget);
+       struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
+       unsigned long flags;
+
+       spin_lock_irqsave(shost->host_lock, flags);
+
+       if (--starget->reap_ref == 0 && list_empty(&starget->devices)) {
+               BUG_ON(starget->state == STARGET_DEL);
+               starget->state = STARGET_DEL;
+               spin_unlock_irqrestore(shost->host_lock, flags);
+               execute_in_process_context(scsi_target_reap_usercontext,
+                                          starget, &starget->ew);
+               return;
+
+       }
+       spin_unlock_irqrestore(shost->host_lock, flags);
+
+       return;
 }
 
 /**

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 902a5def8e62102591e1b2e4657eb560d258efc2..89055494dfee3f86a2b6ae3d7db82eab42c8cff9 100644 (file)
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -256,7 +256,9 @@ static void scsi_device_dev_release_usercontext(void *data)
 
 static void scsi_device_dev_release(struct device *dev)
 {
-       scsi_execute_in_process_context(scsi_device_dev_release_usercontext,    dev);
+       struct scsi_device *sdp = to_scsi_device(dev);
+       execute_in_process_context(scsi_device_dev_release_usercontext, dev,
+                                  &sdp->ew);
 }
 
 static struct class sdev_class = {

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 9c331258bc27a3792fdd90b23470e39cc168457d..c60b8ff2f5e4f5d7a2952a8fb0951a689ec451b1 100644 (file)
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -433,6 +433,4 @@ struct scsi_lun {
 /* Used to obtain the PCI location of a device */
 #define SCSI_IOCTL_GET_PCI             0x5387
 
-int scsi_execute_in_process_context(void (*fn)(void *data), void *data);
-
 #endif /* _SCSI_SCSI_H */

diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 8d77da932d2c33cc1794ff69e9c13c8e93e6fa4b..1ec17ee1281589ab8f955fec57d5333f8c048f20 100644 (file)
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -4,6 +4,7 @@
 #include <linux/device.h>
 #include <linux/list.h>
 #include <linux/spinlock.h>
+#include <linux/workqueue.h>
 #include <asm/atomic.h>
 
 struct request_queue;
@@ -137,6 +138,8 @@ struct scsi_device {
        struct device           sdev_gendev;
        struct class_device     sdev_classdev;
 
+       struct execute_work     ew; /* used to get process context on put */
+
        enum scsi_device_state sdev_state;
        unsigned long           sdev_data[0];
 } __attribute__((aligned(sizeof(unsigned long))));
@@ -153,6 +156,11 @@ struct scsi_device {
 #define scmd_printk(prefix, scmd, fmt, a...)   \
        dev_printk(prefix, &(scmd)->device->sdev_gendev, fmt, ##a)
 
+enum scsi_target_state {
+       STARGET_RUNNING = 1,
+       STARGET_DEL,
+};
+
 /*
  * scsi_target: representation of a scsi target, for now, this is only
  * used for single_lun devices. If no one has active IO to the target,
@@ -172,6 +180,8 @@ struct scsi_target {
                                                /* means no lun present */
 
        char                    scsi_level;
+       struct execute_work     ew;
+       enum scsi_target_state  state;
        void                    *hostdata; /* available to low-level driver */
        unsigned long           starget_data[0]; /* for the transport */
        /* starget_data must be the last element!!!! */