[PATCH] firewire: sbp2: parallelize login/inquiry, reconnect, andshutdown

From: Stefan Richter
Date: Sun Oct 10 2010 - 10:58:18 EST


firewire-sbp2 used to perform management tasks (and SCSI Inquiry, sd
probe etc.) in one single-threaded workqueue for all targets. Some of
these jobs may take very long, e.g. when the target needs to spin up a
disk to perform the task.

We should have had per-target workqueues for that but didn't because
that would have created respectively many kernel threads which almost
always sit idle. Now that we have concurrency managed workqueues,
correct that lack of parallelism:

Simply swap out the driver workqueue by system_nrt_wq. It provides all
the parallelism we will ever need, accepts long-running work, and
provides the here necessary guarantee of non-reentrance across all CPUs.
(The work items are scheduled from firewire-core's fw_device work items
which themselves may change CPUs.)

This simple approach requires though that targets with multiple logical
units accept multiple outstanding management requests. The SBP-2 spec
implicitly requires this capability, but who knows how real firmwares
react.

I only have access to two dual-LU targets: A OXUF924DSB based one from
MacPower and a INIC-2430 based one from IOI. They both work fine with
this change, as do several single-LU targets of course.

Signed-off-by: Stefan Richter <stefanr@xxxxxxxxxxxxxxxxx>
---
drivers/firewire/sbp2.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)

Index: b/drivers/firewire/sbp2.c
===================================================================
--- a/drivers/firewire/sbp2.c
+++ b/drivers/firewire/sbp2.c
@@ -835,8 +835,6 @@ static void sbp2_target_put(struct sbp2_
kref_put(&tgt->kref, sbp2_release_target);
}

-static struct workqueue_struct *sbp2_wq;
-
/*
* Always get the target's kref when scheduling work on one its units.
* Each workqueue job is responsible to call sbp2_target_put() upon return.
@@ -844,7 +842,7 @@ static struct workqueue_struct *sbp2_wq;
static void sbp2_queue_work(struct sbp2_logical_unit *lu, unsigned long delay)
{
sbp2_target_get(lu->tgt);
- if (!queue_delayed_work(sbp2_wq, &lu->work, delay))
+ if (!queue_delayed_work(system_nrt_wq, &lu->work, delay))
sbp2_target_put(lu->tgt);
}

@@ -1656,17 +1654,12 @@ MODULE_ALIAS("sbp2");

static int __init sbp2_init(void)
{
- sbp2_wq = create_singlethread_workqueue(KBUILD_MODNAME);
- if (!sbp2_wq)
- return -ENOMEM;
-
return driver_register(&sbp2_driver.driver);
}

static void __exit sbp2_cleanup(void)
{
driver_unregister(&sbp2_driver.driver);
- destroy_workqueue(sbp2_wq);
}

module_init(sbp2_init);

--
Stefan Richter
-=====-==-=- =-=- -=-=-
http://arcgraph.de/sr/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/