From ffedb4522571ac170f941678d138a31bc0884ab4 Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Thu, 23 Feb 2006 14:27:18 -0600 Subject: [PATCH] [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 --- drivers/scsi/scsi_lib.c | 58 -------------------------------------- drivers/scsi/scsi_scan.c | 49 +++++++++++++++++++++----------- drivers/scsi/scsi_sysfs.c | 4 ++- include/scsi/scsi.h | 2 -- include/scsi/scsi_device.h | 10 +++++++ 5 files changed, 46 insertions(+), 77 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index eab303d148d8..3042520c413c 100644 --- 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 94b86d5b1469..84f01fd0c8fd 100644 --- 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 902a5def8e62..89055494dfee 100644 --- 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 9c331258bc27..c60b8ff2f5e4 100644 --- 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 8d77da932d2c..1ec17ee12815 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -4,6 +4,7 @@ #include #include #include +#include #include 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!!!! */