[SCSI] pm8001: simplify workqueue usage

pm8001 manages its own list of pending works and cancel them on device
free.  It is unnecessarily complex and has a race condition - the
works are canceled but not synced, so the work could still be running
during and after the data structures are freed.

This patch simplifies workqueue usage.

* A driver specific workqueue pm8001_wq is created to serve these
  work items.

* To avoid confusion, the "queue" suffixes are dropped from work items
  and functions.

* Delayed queueing was never used.  pm8001_work now uses work_struct
  instead.

* The driver no longer keeps track of pending works.  All pm8001_works
  are queued to pm8001_wq and the workqueue is flushed as necessary.

flush_scheduled_work() usage is removed during conversion.

Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Jack Wang <jack_wang@usish.com>
Signed-off-by: James Bottomley <James.Bottomley@suse.de>
This commit is contained in:
Tejun Heo 2011-01-24 14:57:29 +01:00 committed by James Bottomley
parent a684b8da35
commit 429305e465
3 changed files with 41 additions and 33 deletions

View File

@ -1382,53 +1382,50 @@ static u32 mpi_msg_consume(struct pm8001_hba_info *pm8001_ha,
return MPI_IO_STATUS_BUSY; return MPI_IO_STATUS_BUSY;
} }
static void pm8001_work_queue(struct work_struct *work) static void pm8001_work_fn(struct work_struct *work)
{ {
struct delayed_work *dw = container_of(work, struct delayed_work, work); struct pm8001_work *pw = container_of(work, struct pm8001_work, work);
struct pm8001_wq *wq = container_of(dw, struct pm8001_wq, work_q);
struct pm8001_device *pm8001_dev; struct pm8001_device *pm8001_dev;
struct domain_device *dev; struct domain_device *dev;
switch (wq->handler) { switch (pw->handler) {
case IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS: case IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS:
pm8001_dev = wq->data; pm8001_dev = pw->data;
dev = pm8001_dev->sas_device; dev = pm8001_dev->sas_device;
pm8001_I_T_nexus_reset(dev); pm8001_I_T_nexus_reset(dev);
break; break;
case IO_OPEN_CNX_ERROR_STP_RESOURCES_BUSY: case IO_OPEN_CNX_ERROR_STP_RESOURCES_BUSY:
pm8001_dev = wq->data; pm8001_dev = pw->data;
dev = pm8001_dev->sas_device; dev = pm8001_dev->sas_device;
pm8001_I_T_nexus_reset(dev); pm8001_I_T_nexus_reset(dev);
break; break;
case IO_DS_IN_ERROR: case IO_DS_IN_ERROR:
pm8001_dev = wq->data; pm8001_dev = pw->data;
dev = pm8001_dev->sas_device; dev = pm8001_dev->sas_device;
pm8001_I_T_nexus_reset(dev); pm8001_I_T_nexus_reset(dev);
break; break;
case IO_DS_NON_OPERATIONAL: case IO_DS_NON_OPERATIONAL:
pm8001_dev = wq->data; pm8001_dev = pw->data;
dev = pm8001_dev->sas_device; dev = pm8001_dev->sas_device;
pm8001_I_T_nexus_reset(dev); pm8001_I_T_nexus_reset(dev);
break; break;
} }
list_del(&wq->entry); kfree(pw);
kfree(wq);
} }
static int pm8001_handle_event(struct pm8001_hba_info *pm8001_ha, void *data, static int pm8001_handle_event(struct pm8001_hba_info *pm8001_ha, void *data,
int handler) int handler)
{ {
struct pm8001_wq *wq; struct pm8001_work *pw;
int ret = 0; int ret = 0;
wq = kmalloc(sizeof(struct pm8001_wq), GFP_ATOMIC); pw = kmalloc(sizeof(struct pm8001_work), GFP_ATOMIC);
if (wq) { if (pw) {
wq->pm8001_ha = pm8001_ha; pw->pm8001_ha = pm8001_ha;
wq->data = data; pw->data = data;
wq->handler = handler; pw->handler = handler;
INIT_DELAYED_WORK(&wq->work_q, pm8001_work_queue); INIT_WORK(&pw->work, pm8001_work_fn);
list_add_tail(&wq->entry, &pm8001_ha->wq_list); queue_work(pm8001_wq, &pw->work);
schedule_delayed_work(&wq->work_q, 0);
} else } else
ret = -ENOMEM; ret = -ENOMEM;

View File

@ -51,6 +51,8 @@ static int pm8001_id;
LIST_HEAD(hba_list); LIST_HEAD(hba_list);
struct workqueue_struct *pm8001_wq;
/** /**
* The main structure which LLDD must register for scsi core. * The main structure which LLDD must register for scsi core.
*/ */
@ -134,7 +136,6 @@ static void __devinit pm8001_phy_init(struct pm8001_hba_info *pm8001_ha,
static void pm8001_free(struct pm8001_hba_info *pm8001_ha) static void pm8001_free(struct pm8001_hba_info *pm8001_ha)
{ {
int i; int i;
struct pm8001_wq *wq;
if (!pm8001_ha) if (!pm8001_ha)
return; return;
@ -150,8 +151,7 @@ static void pm8001_free(struct pm8001_hba_info *pm8001_ha)
PM8001_CHIP_DISP->chip_iounmap(pm8001_ha); PM8001_CHIP_DISP->chip_iounmap(pm8001_ha);
if (pm8001_ha->shost) if (pm8001_ha->shost)
scsi_host_put(pm8001_ha->shost); scsi_host_put(pm8001_ha->shost);
list_for_each_entry(wq, &pm8001_ha->wq_list, entry) flush_workqueue(pm8001_wq);
cancel_delayed_work(&wq->work_q);
kfree(pm8001_ha->tags); kfree(pm8001_ha->tags);
kfree(pm8001_ha); kfree(pm8001_ha);
} }
@ -381,7 +381,6 @@ pm8001_pci_alloc(struct pci_dev *pdev, u32 chip_id, struct Scsi_Host *shost)
pm8001_ha->sas = sha; pm8001_ha->sas = sha;
pm8001_ha->shost = shost; pm8001_ha->shost = shost;
pm8001_ha->id = pm8001_id++; pm8001_ha->id = pm8001_id++;
INIT_LIST_HEAD(&pm8001_ha->wq_list);
pm8001_ha->logging_level = 0x01; pm8001_ha->logging_level = 0x01;
sprintf(pm8001_ha->name, "%s%d", DRV_NAME, pm8001_ha->id); sprintf(pm8001_ha->name, "%s%d", DRV_NAME, pm8001_ha->id);
#ifdef PM8001_USE_TASKLET #ifdef PM8001_USE_TASKLET
@ -758,7 +757,7 @@ static int pm8001_pci_suspend(struct pci_dev *pdev, pm_message_t state)
int i , pos; int i , pos;
u32 device_state; u32 device_state;
pm8001_ha = sha->lldd_ha; pm8001_ha = sha->lldd_ha;
flush_scheduled_work(); flush_workqueue(pm8001_wq);
scsi_block_requests(pm8001_ha->shost); scsi_block_requests(pm8001_ha->shost);
pos = pci_find_capability(pdev, PCI_CAP_ID_PM); pos = pci_find_capability(pdev, PCI_CAP_ID_PM);
if (pos == 0) { if (pos == 0) {
@ -870,17 +869,26 @@ static struct pci_driver pm8001_pci_driver = {
*/ */
static int __init pm8001_init(void) static int __init pm8001_init(void)
{ {
int rc; int rc = -ENOMEM;
pm8001_wq = alloc_workqueue("pm8001", 0, 0);
if (!pm8001_wq)
goto err;
pm8001_id = 0; pm8001_id = 0;
pm8001_stt = sas_domain_attach_transport(&pm8001_transport_ops); pm8001_stt = sas_domain_attach_transport(&pm8001_transport_ops);
if (!pm8001_stt) if (!pm8001_stt)
return -ENOMEM; goto err_wq;
rc = pci_register_driver(&pm8001_pci_driver); rc = pci_register_driver(&pm8001_pci_driver);
if (rc) if (rc)
goto err_out; goto err_tp;
return 0; return 0;
err_out:
err_tp:
sas_release_transport(pm8001_stt); sas_release_transport(pm8001_stt);
err_wq:
destroy_workqueue(pm8001_wq);
err:
return rc; return rc;
} }
@ -888,6 +896,7 @@ static void __exit pm8001_exit(void)
{ {
pci_unregister_driver(&pm8001_pci_driver); pci_unregister_driver(&pm8001_pci_driver);
sas_release_transport(pm8001_stt); sas_release_transport(pm8001_stt);
destroy_workqueue(pm8001_wq);
} }
module_init(pm8001_init); module_init(pm8001_init);

View File

@ -50,6 +50,7 @@
#include <linux/dma-mapping.h> #include <linux/dma-mapping.h>
#include <linux/pci.h> #include <linux/pci.h>
#include <linux/interrupt.h> #include <linux/interrupt.h>
#include <linux/workqueue.h>
#include <scsi/libsas.h> #include <scsi/libsas.h>
#include <scsi/scsi_tcq.h> #include <scsi/scsi_tcq.h>
#include <scsi/sas_ata.h> #include <scsi/sas_ata.h>
@ -379,18 +380,16 @@ struct pm8001_hba_info {
#ifdef PM8001_USE_TASKLET #ifdef PM8001_USE_TASKLET
struct tasklet_struct tasklet; struct tasklet_struct tasklet;
#endif #endif
struct list_head wq_list;
u32 logging_level; u32 logging_level;
u32 fw_status; u32 fw_status;
const struct firmware *fw_image; const struct firmware *fw_image;
}; };
struct pm8001_wq { struct pm8001_work {
struct delayed_work work_q; struct work_struct work;
struct pm8001_hba_info *pm8001_ha; struct pm8001_hba_info *pm8001_ha;
void *data; void *data;
int handler; int handler;
struct list_head entry;
}; };
struct pm8001_fw_image_header { struct pm8001_fw_image_header {
@ -460,6 +459,9 @@ struct fw_control_ex {
void *param3; void *param3;
}; };
/* pm8001 workqueue */
extern struct workqueue_struct *pm8001_wq;
/******************** function prototype *********************/ /******************** function prototype *********************/
int pm8001_tag_alloc(struct pm8001_hba_info *pm8001_ha, u32 *tag_out); int pm8001_tag_alloc(struct pm8001_hba_info *pm8001_ha, u32 *tag_out);
void pm8001_tag_init(struct pm8001_hba_info *pm8001_ha); void pm8001_tag_init(struct pm8001_hba_info *pm8001_ha);