From: Tejun Heo <tj@kernel.org>
To: torvalds@linux-foundation.org, mingo@elte.hu,
linux-kernel@vger.kernel.org, jeff@garzik.org,
akpm@linux-foundation.org, rusty@rustcorp.com.au,
cl@linux-foundation.org, dhowells@redhat.com,
arjan@linux.intel.com, oleg@redhat.com, axboe@kernel.dk,
fweisbec@gmail.com, dwalker@codeaurora.org,
stefanr@s5r6.in-berlin.de, florian@mickler.org,
andi@firstfloor.org, mst@redhat.com, randy.dunlap@oracle.com
Cc: Tejun Heo <tj@kernel.org>, Jeff Garzik <jgarzik@pobox.com>
Subject: [PATCH 33/35] libata: take advantage of cmwq and remove concurrency limitations
Date: Mon, 28 Jun 2010 23:04:21 +0200 [thread overview]
Message-ID: <1277759063-24607-34-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1277759063-24607-1-git-send-email-tj@kernel.org>
libata has two concurrency related limitations.
a. ata_wq which is used for polling PIO has single thread per CPU. If
there are multiple devices doing polling PIO on the same CPU, they
can't be executed simultaneously.
b. ata_aux_wq which is used for SCSI probing has single thread. In
cases where SCSI probing is stalled for extended period of time
which is possible for ATAPI devices, this will stall all probing.
#a is solved by increasing maximum concurrency of ata_wq. Please note
that polling PIO might be used under allocation path and thus needs to
be served by a separate wq with a rescuer.
#b is solved by using the default wq instead and achieving exclusion
via per-port mutex.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jeff Garzik <jgarzik@pobox.com>
---
drivers/ata/libata-core.c | 20 +++++---------------
drivers/ata/libata-eh.c | 4 ++--
drivers/ata/libata-scsi.c | 10 ++++++----
drivers/ata/libata-sff.c | 9 +--------
drivers/ata/libata.h | 1 -
include/linux/libata.h | 1 +
6 files changed, 15 insertions(+), 30 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index ddf8e48..4f78741 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -98,8 +98,6 @@ static unsigned long ata_dev_blacklisted(const struct ata_device *dev);
unsigned int ata_print_id = 1;
-struct workqueue_struct *ata_aux_wq;
-
struct ata_force_param {
const char *name;
unsigned int cbl;
@@ -5611,6 +5609,7 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
ap->msg_enable = ATA_MSG_DRV | ATA_MSG_ERR | ATA_MSG_WARN;
#endif
+ mutex_init(&ap->scsi_scan_mutex);
INIT_DELAYED_WORK(&ap->hotplug_task, ata_scsi_hotplug);
INIT_WORK(&ap->scsi_rescan_task, ata_scsi_dev_rescan);
INIT_LIST_HEAD(&ap->eh_done_q);
@@ -6549,29 +6548,20 @@ static int __init ata_init(void)
ata_parse_force_param();
- ata_aux_wq = create_singlethread_workqueue("ata_aux");
- if (!ata_aux_wq)
- goto fail;
-
rc = ata_sff_init();
- if (rc)
- goto fail;
+ if (rc) {
+ kfree(ata_force_tbl);
+ return rc;
+ }
printk(KERN_DEBUG "libata version " DRV_VERSION " loaded.\n");
return 0;
-
-fail:
- kfree(ata_force_tbl);
- if (ata_aux_wq)
- destroy_workqueue(ata_aux_wq);
- return rc;
}
static void __exit ata_exit(void)
{
ata_sff_exit();
kfree(ata_force_tbl);
- destroy_workqueue(ata_aux_wq);
}
subsys_initcall(ata_init);
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index f77a673..4d2af82 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -727,7 +727,7 @@ void ata_scsi_error(struct Scsi_Host *host)
if (ap->pflags & ATA_PFLAG_LOADING)
ap->pflags &= ~ATA_PFLAG_LOADING;
else if (ap->pflags & ATA_PFLAG_SCSI_HOTPLUG)
- queue_delayed_work(ata_aux_wq, &ap->hotplug_task, 0);
+ schedule_delayed_work(&ap->hotplug_task, 0);
if (ap->pflags & ATA_PFLAG_RECOVERED)
ata_port_printk(ap, KERN_INFO, "EH complete\n");
@@ -2944,7 +2944,7 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
ehc->i.flags |= ATA_EHI_SETMODE;
/* schedule the scsi_rescan_device() here */
- queue_work(ata_aux_wq, &(ap->scsi_rescan_task));
+ schedule_work(&(ap->scsi_rescan_task));
} else if (dev->class == ATA_DEV_UNKNOWN &&
ehc->tries[dev->devno] &&
ata_class_enabled(ehc->classes[dev->devno])) {
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index a54273d..d75c9c4 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3435,7 +3435,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
" switching to async\n");
}
- queue_delayed_work(ata_aux_wq, &ap->hotplug_task,
+ queue_delayed_work(system_long_wq, &ap->hotplug_task,
round_jiffies_relative(HZ));
}
@@ -3582,6 +3582,7 @@ void ata_scsi_hotplug(struct work_struct *work)
}
DPRINTK("ENTER\n");
+ mutex_lock(&ap->scsi_scan_mutex);
/* Unplug detached devices. We cannot use link iterator here
* because PMP links have to be scanned even if PMP is
@@ -3595,6 +3596,7 @@ void ata_scsi_hotplug(struct work_struct *work)
/* scan for new ones */
ata_scsi_scan_host(ap, 0);
+ mutex_unlock(&ap->scsi_scan_mutex);
DPRINTK("EXIT\n");
}
@@ -3673,9 +3675,7 @@ static int ata_scsi_user_scan(struct Scsi_Host *shost, unsigned int channel,
* @work: Pointer to ATA port to perform scsi_rescan_device()
*
* After ATA pass thru (SAT) commands are executed successfully,
- * libata need to propagate the changes to SCSI layer. This
- * function must be executed from ata_aux_wq such that sdev
- * attach/detach don't race with rescan.
+ * libata need to propagate the changes to SCSI layer.
*
* LOCKING:
* Kernel thread context (may sleep).
@@ -3688,6 +3688,7 @@ void ata_scsi_dev_rescan(struct work_struct *work)
struct ata_device *dev;
unsigned long flags;
+ mutex_lock(&ap->scsi_scan_mutex);
spin_lock_irqsave(ap->lock, flags);
ata_for_each_link(link, ap, EDGE) {
@@ -3707,6 +3708,7 @@ void ata_scsi_dev_rescan(struct work_struct *work)
}
spin_unlock_irqrestore(ap->lock, flags);
+ mutex_unlock(&ap->scsi_scan_mutex);
}
/**
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index efa4a18..674c143 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -3318,14 +3318,7 @@ void ata_sff_port_init(struct ata_port *ap)
int __init ata_sff_init(void)
{
- /*
- * FIXME: In UP case, there is only one workqueue thread and if you
- * have more than one PIO device, latency is bloody awful, with
- * occasional multi-second "hiccups" as one PIO device waits for
- * another. It's an ugly wart that users DO occasionally complain
- * about; luckily most users have at most one PIO polled device.
- */
- ata_sff_wq = create_workqueue("ata_sff");
+ ata_sff_wq = alloc_workqueue("ata_sff", WQ_RESCUER, WQ_MAX_ACTIVE);
if (!ata_sff_wq)
return -ENOMEM;
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 4b84ed6..9ce1ecc 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -54,7 +54,6 @@ enum {
};
extern unsigned int ata_print_id;
-extern struct workqueue_struct *ata_aux_wq;
extern int atapi_passthru16;
extern int libata_fua;
extern int libata_noacpi;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index b85f3ff..f010f18 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -751,6 +751,7 @@ struct ata_port {
struct ata_host *host;
struct device *dev;
+ struct mutex scsi_scan_mutex;
struct delayed_work hotplug_task;
struct work_struct scsi_rescan_task;
--
1.6.4.2
next prev parent reply other threads:[~2010-06-28 21:09 UTC|newest]
Thread overview: 98+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-06-28 21:03 [PATCHSET] workqueue: concurrency managed workqueue, take#6 Tejun Heo
2010-06-28 21:03 ` [PATCH 01/35] kthread: implement kthread_worker Tejun Heo
2010-06-28 21:03 ` [PATCH 02/35] ivtv: use kthread_worker instead of workqueue Tejun Heo
2010-07-05 17:11 ` Andy Walls
2010-07-06 7:01 ` Tejun Heo
2010-07-09 13:15 ` Andy Walls
2010-06-28 21:03 ` [PATCH 03/35] kthread: implement kthread_data() Tejun Heo
2010-06-28 21:03 ` [PATCH 04/35] acpi: use queue_work_on() instead of binding workqueue worker to cpu0 Tejun Heo
2010-06-28 21:03 ` [PATCH 05/35] workqueue: kill RT workqueue Tejun Heo
2010-06-28 21:03 ` [PATCH 06/35] workqueue: misc/cosmetic updates Tejun Heo
2010-06-28 21:03 ` [PATCH 07/35] workqueue: merge feature parameters into flags Tejun Heo
2010-06-28 21:03 ` [PATCH 08/35] workqueue: define masks for work flags and conditionalize STATIC flags Tejun Heo
2010-06-28 21:03 ` [PATCH 09/35] workqueue: separate out process_one_work() Tejun Heo
2010-06-28 21:03 ` [PATCH 10/35] workqueue: temporarily remove workqueue tracing Tejun Heo
2010-06-28 21:03 ` [PATCH 11/35] workqueue: kill cpu_populated_map Tejun Heo
2010-06-28 21:04 ` [PATCH 12/35] workqueue: update cwq alignement Tejun Heo
2010-06-28 22:47 ` Frederic Weisbecker
2010-06-29 7:39 ` Tejun Heo
2010-06-29 12:36 ` Frederic Weisbecker
2010-06-29 15:42 ` Tejun Heo
2010-06-29 15:47 ` Frederic Weisbecker
2010-06-29 15:51 ` Tejun Heo
2010-06-29 16:01 ` Frederic Weisbecker
2010-06-29 16:09 ` Tejun Heo
2010-06-29 16:17 ` Frederic Weisbecker
2010-07-06 14:22 ` Christoph Lameter
2010-07-06 14:26 ` Tejun Heo
2010-06-29 8:12 ` [PATCH UPDATED " Tejun Heo
2010-06-29 13:39 ` Frederic Weisbecker
2010-06-28 21:04 ` [PATCH 13/35] workqueue: reimplement workqueue flushing using color coded works Tejun Heo
2010-06-28 21:04 ` [PATCH 14/35] workqueue: introduce worker Tejun Heo
2010-06-28 21:04 ` [PATCH 15/35] workqueue: reimplement work flushing using linked works Tejun Heo
2010-06-28 21:04 ` [PATCH 16/35] workqueue: implement per-cwq active work limit Tejun Heo
2010-06-28 21:04 ` [PATCH 17/35] workqueue: reimplement workqueue freeze using max_active Tejun Heo
2010-06-28 21:04 ` [PATCH 18/35] workqueue: introduce global cwq and unify cwq locks Tejun Heo
2010-06-28 21:04 ` [PATCH 19/35] workqueue: implement worker states Tejun Heo
2010-06-28 21:04 ` [PATCH 20/35] workqueue: reimplement CPU hotplugging support using trustee Tejun Heo
2010-06-28 21:04 ` [PATCH 21/35] workqueue: make single thread workqueue shared worker pool friendly Tejun Heo
2010-06-28 21:04 ` [PATCH 22/35] workqueue: add find_worker_executing_work() and track current_cwq Tejun Heo
2010-06-28 21:04 ` [PATCH 23/35] workqueue: carry cpu number in work data once execution starts Tejun Heo
2010-06-28 21:04 ` [PATCH 24/35] workqueue: implement WQ_NON_REENTRANT Tejun Heo
2010-06-28 21:04 ` [PATCH 25/35] workqueue: use shared worklist and pool all workers per cpu Tejun Heo
2010-06-28 21:04 ` [PATCH 26/35] workqueue: implement worker_{set|clr}_flags() Tejun Heo
2010-06-28 21:04 ` [PATCH 27/35] workqueue: implement concurrency managed dynamic worker pool Tejun Heo
2010-07-09 9:11 ` Yong Zhang
2010-07-12 8:53 ` [PATCH] workqueue: fix locking in retry path of maybe_create_worker() Tejun Heo
2010-07-12 13:23 ` Yong Zhang
2010-07-14 9:37 ` Tejun Heo
2010-06-28 21:04 ` [PATCH 28/35] workqueue: increase max_active of keventd and kill current_is_keventd() Tejun Heo
2010-06-28 21:04 ` [PATCH 29/35] workqueue: s/__create_workqueue()/alloc_workqueue()/, and add system workqueues Tejun Heo
2010-06-28 21:04 ` [PATCH 30/35] workqueue: implement several utility APIs Tejun Heo
2010-06-28 21:04 ` [PATCH 31/35] workqueue: implement high priority workqueue Tejun Heo
2010-06-28 21:04 ` [PATCH 32/35] workqueue: implement cpu intensive workqueue Tejun Heo
2010-06-28 21:04 ` Tejun Heo [this message]
2010-06-28 22:32 ` [PATCH 33/35] libata: take advantage of cmwq and remove concurrency limitations Jeff Garzik
2010-06-29 7:00 ` Tejun Heo
2010-06-28 21:04 ` [PATCH 34/35] async: use workqueue for worker pool Tejun Heo
2010-06-28 22:55 ` Frederic Weisbecker
2010-06-29 7:25 ` Tejun Heo
2010-06-29 12:18 ` Frederic Weisbecker
2010-06-29 15:46 ` Tejun Heo
2010-06-29 15:52 ` Frederic Weisbecker
2010-06-29 15:55 ` Tejun Heo
2010-06-29 16:40 ` Arjan van de Ven
2010-06-29 16:59 ` Tejun Heo
2010-06-29 17:12 ` Tejun Heo
2010-06-29 18:08 ` Arjan van de Ven
2010-06-29 18:07 ` Arjan van de Ven
2010-06-29 18:15 ` Tejun Heo
2010-06-29 18:22 ` Arjan van de Ven
2010-06-29 18:34 ` Tejun Heo
2010-06-29 18:41 ` Arjan van de Ven
2010-06-29 18:59 ` Tejun Heo
2010-06-29 21:37 ` David Howells
2010-07-02 9:17 ` [PATCHSET] workqueue: implement and use WQ_UNBOUND Tejun Heo
2010-07-02 9:19 ` [PATCH 1/4] workqueue: prepare for WQ_UNBOUND implementation Tejun Heo
2010-07-02 9:24 ` [PATCH 3/4] workqueue: remove WQ_SINGLE_CPU and use WQ_UNBOUND instead Tejun Heo
2010-07-02 9:25 ` [PATCH 4/4] async: use workqueue for worker pool Tejun Heo
2010-07-02 15:09 ` Stefan Richter
2010-07-02 16:26 ` Tejun Heo
2010-07-02 16:25 ` [PATCH UPDATED " Tejun Heo
2010-07-02 9:28 ` [PATCH 2/4] workqueue: implement unbound workqueue Tejun Heo
2010-07-02 9:32 ` [PATCHSET] workqueue: implement and use WQ_UNBOUND Tejun Heo
2010-07-07 5:41 ` Tejun Heo
2010-07-14 9:39 ` Tejun Heo
2010-07-20 22:01 ` David Howells
2010-07-02 9:20 ` [PATCH 2/4] workqueue: implement unbound workqueue Tejun Heo
2010-06-28 21:04 ` [PATCH 35/35] pcrypt: use HIGHPRI and CPU_INTENSIVE workqueues for padata Tejun Heo
2010-06-28 23:18 ` [PATCHSET] workqueue: concurrency managed workqueue, take#6 Frederic Weisbecker
2010-06-29 7:05 ` Tejun Heo
2010-07-02 8:32 ` [PATCHSET] workqueue: fixes on top of cmwq take#6 Tejun Heo
2010-07-02 8:33 ` [PATCH 1/4] workqueue: use worker_set/clr_flags() only from worker itself Tejun Heo
2010-07-02 8:34 ` [PATCH 2/4] workqueue: fix race condition in flush_workqueue() Tejun Heo
2010-07-02 8:35 ` [PATCH 3/4] workqueue: fix incorrect cpu number BUG_ON() in get_work_gcwq() Tejun Heo
2010-07-02 8:35 ` [PATCH 4/4] workqueue: fix worker management invocation without pending works Tejun Heo
2010-07-19 14:51 ` [PATCHSET] workqueue: concurrency managed workqueue, take#6 Tejun Heo
2010-07-21 13:23 ` David Howells
2010-07-21 14:52 ` Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1277759063-24607-34-git-send-email-tj@kernel.org \
--to=tj@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=andi@firstfloor.org \
--cc=arjan@linux.intel.com \
--cc=axboe@kernel.dk \
--cc=cl@linux-foundation.org \
--cc=dhowells@redhat.com \
--cc=dwalker@codeaurora.org \
--cc=florian@mickler.org \
--cc=fweisbec@gmail.com \
--cc=jeff@garzik.org \
--cc=jgarzik@pobox.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=mst@redhat.com \
--cc=oleg@redhat.com \
--cc=randy.dunlap@oracle.com \
--cc=rusty@rustcorp.com.au \
--cc=stefanr@s5r6.in-berlin.de \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.