linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Kristian Hogsberg <hogsberg@users.sourceforge.net>
Cc: Linus Torvalds <torvalds@transmeta.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [patch] Workqueue Abstraction, 2.5.40-H7
Date: Thu, 3 Oct 2002 20:44:09 +0200 (CEST)	[thread overview]
Message-ID: <Pine.LNX.4.44.0210032028130.2113-100000@localhost.localdomain> (raw)
In-Reply-To: <m37kh1vo9d.fsf@DK300KRH.bang-olufsen.dk>


On 1 Oct 2002, Kristian Hogsberg wrote:

> I read through your patch and I think it looks great.  I'm one of the
> ieee1394 maintainers, and we also a have a worker thread mechanism in
> the ieee1394 subsystem, which could (and will, I'm going to look into
> this) be replaced by your work queue stuff.  We use the thread for
> reading configuration information from ieee1394 devices.  This work
> isn't very performance critical, but the reason we dont just use keventd
> is that we don't want to stall keventd while reading this information.  
> So, my point is, that in this case (I'm sure there are more situations
> like this, usb has a similar worker thread, khubd), the per-cpu worker
> thread is overkill, and it would be sufficient with just one thread,
> running on all cpus.  So maybe this could be an option to
> create_workqueue()?  Either create a cpu-bound thread for each cpu, or
> create one thread that can run on all cpus.

i understand your point, and i really tried to get this problem solved
prior sending the multiple-workers patch, but it's not really doable
without major kludges. Is it really a problem on SMP boxes to have a few
more kernel threads? Those boxes are supposed to have enough RAM.

the only other way would be to introduce 1) runtime overhead [this sucks]
or 2) to split the API into per-CPU and global ones. [this isnt too good
either i think.]

there is enough flexibility internally - eg. we can in the future do
better load-balancing (if the XFS people will ever notice any problems in
this area), because right now the load-balancing is the simplest and
fastest variant: purely per-CPU. [in theory we could look at other CPU's
worker queues and queue there if they are empty or much shorter than this
CPU's worker queue.]

I also kept open the possibility of introducing multiple worker threads
per CPU in the future. But having a single-threaded an per-CPU behavior in
a single API looks quite hard.

we could perhaps do the following, add a single branch to the queue_work()
fastpath:

	if (!cwq->thread)
		cwq = wq->cpu_wq;

and the single-thread queue variant would thus fall back to using a single
queue only. Plus some sort of new variant could be used:

	struct workqueue_struct *create_single_workqueue(char *name);

[or a 'flags' argument to create_workqueue();]

The runtime thing looks slightly ugly, but i think it's acceptable. Has
anyone any better idea?

> Another minor comment: why do you kmalloc() the workqueue_t?  Wouldn't
> it be more flexible to allow the user to provide a pointer to a
> pre-allocated workqueue_t structure, e.g.:
> 
>         static workqueue_t aio_wq;
> 
>         [...]
> 
>                	create_workqueue(&aio_wq, "aio");

yes, but every creation/destruction use of workqueues is in some sort of
init/shutdown very-slow-path, so efficiency is not a factor. But clarity
of the code is a factor, and in the dynamic allocation case it's much
cleaner to have this:

	wq = create_workqueue("worker");
	if (!wq)
		goto error;

than:

	wq = kmalloc(sizeof(*wq), GFP_KERNEL);
	if (!wq)
		goto error;
	if (create_workqueue(wq, "worker")) {
		kfree(w);
		goto error;
	}

[and we might even switch to a workqueue SLAB cache internally anytime.]

	Ingo


  reply	other threads:[~2002-10-03 18:28 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-10-01 16:24 [patch] Workqueue Abstraction, 2.5.40-H7 Ingo Molnar
2002-10-01 17:55 ` Kai Germaschewski
2002-10-01 21:27   ` Ingo Molnar
2002-10-01 18:04 ` Jeff Garzik
2002-10-01 18:52   ` Ingo Molnar
2002-10-01 21:06     ` Jeff Garzik
2002-10-01 21:30       ` Ingo Molnar
2002-10-01 19:16 ` Linus Torvalds
2002-10-01 19:53   ` Linus Torvalds
2002-10-01 21:32 ` Kristian Hogsberg
2002-10-03 18:44   ` Ingo Molnar [this message]
2002-10-04 23:20     ` Kristian Hogsberg
2002-10-02  4:22 ` Christoph Hellwig
2002-10-01 21:31   ` Ingo Molnar
2002-10-02  8:22 ` Oleg Drokin
2002-10-08  3:50   ` Jeff Dike
2002-10-01 18:52 Marc-Christian Petersen
2002-10-02  3:58 ` Christoph Hellwig
2002-10-01 20:29 Ingo Molnar
2002-10-01 20:49 ` Linus Torvalds
2002-10-01 21:21   ` Ingo Molnar
2002-10-02  3:23     ` Miles Bader
2002-10-02 19:18     ` Randy.Dunlap
2002-10-01 21:09 ` Jes Sorensen
2002-10-01 21:35   ` Ingo Molnar
2002-10-03  1:38 ` Kevin O'Connor

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=Pine.LNX.4.44.0210032028130.2113-100000@localhost.localdomain \
    --to=mingo@elte.hu \
    --cc=hogsberg@users.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@transmeta.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).