From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Garzik Subject: Re: [PATCH] libata: use single threaded work queue Date: Wed, 19 Aug 2009 07:59:46 -0400 Message-ID: <4A8BE932.5090300@garzik.org> References: <20090819112554.GY12579@kernel.dk> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from srv5.dvmed.net ([207.36.208.214]:55035 "EHLO mail.dvmed.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751533AbZHSL7t (ORCPT ); Wed, 19 Aug 2009 07:59:49 -0400 In-Reply-To: <20090819112554.GY12579@kernel.dk> Sender: linux-ide-owner@vger.kernel.org List-Id: linux-ide@vger.kernel.org To: Jens Axboe Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org, htejun@gmail.com, Benjamin Herrenschmidt On 08/19/2009 07:25 AM, Jens Axboe wrote: > Hi, > > On boxes with lots of CPUs, we have so many kernel threads it's not > funny. The basic problem is that create_workqueue() creates a per-cpu > thread, where we could easily get by with a single thread for a lot of > cases. > > One such case appears to be ata_wq. You want at most one per pio drive, > not one per CPU. I'd suggest just dropping it to a single threaded wq. > > Signed-off-by: Jens Axboe > > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > index 072ba5e..0d78628 100644 > --- a/drivers/ata/libata-core.c > +++ b/drivers/ata/libata-core.c > @@ -6580,7 +6580,7 @@ static int __init ata_init(void) > { > ata_parse_force_param(); > > - ata_wq = create_workqueue("ata"); > + ata_wq = create_singlethread_workqueue("ata"); > if (!ata_wq) > goto free_force_tbl; I agree with one-thread-per-cpu is too much, in these modern multi-core times, but one thread is too little. You have essentially re-created simplex DMA -- blocking and waiting such that one drive out of ~4 can be used at any one time. ata_pio_task() is in a workqueue so that it can sleep and/or spend a long time polling ATA registers. That means an active task can definitely starve all other tasks in the workqueue, if only one thread is available. If starvation occurs, it will potentially pause the unrelated task for several seconds. The proposed patch actually expands an existing problem -- uniprocessor case, where there is only one workqueue thread. For the reasons outlined above, we actually want multiple threads even in the UP case. If you have more than one PIO device, latency is bloody awful, with occasional multi-second "hiccups" as one PIO devices waits for another. It's an ugly wart that users DO occasionally complain about; luckily most users have at most one PIO polled device. It would be nice if we could replace this workqueue with a thread pool, where thread count inside the pool ranges from zero to $N depending on level of thread pool activity. Our common case in libata would be _zero_ threads, if so... Jeff