All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Tejun Heo <tj@kernel.org>
Cc: tglx@linutronix.de, mingo@elte.hu, avi@redhat.com, efault@gmx.de,
	rusty@rustcorp.com.au, linux-kernel@vger.kernel.org,
	Gautham R Shenoy <ego@in.ibm.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH 4/7] sched: implement force_cpus_allowed()
Date: Tue, 08 Dec 2009 14:35:51 +0100	[thread overview]
Message-ID: <1260279351.3935.1375.camel@laptop> (raw)
In-Reply-To: <4B1E4548.5040408@kernel.org>

On Tue, 2009-12-08 at 21:23 +0900, Tejun Heo wrote:
> On 12/08/2009 09:10 PM, Peter Zijlstra wrote:
> > Hotplug and deterministic are not to be used in the same sentence, its
> > an utter slow path and I'd much rather have simple code than clever code
> > there -- there's been way too many 'interesting' hotplug problems.
> 
> Slow and indeterminism comes in different magnitudes.

Determinism does _not_ come in magnitudes, its a very binary property,
either it is or it is not.

As to the order of slowness for unplug, that is about maximal, its _the_
slowest path in the whole kernel.

> > Furthermore if it's objective is to cater to generic thread pools then I
> > think its an utter fail simply because it mandates strict cpu affinity,
> > that basically requires you to write a work scheduler to balance work
> > load etc.. Much easier is a simple unbounded thread pool that gets
> > balanced by the regular scheduler.
> 
> The observation was that for most long running async jobs, most time
> is spent sleeping instead of burning cpu cycles and long running ones
> are relatively few compared to short ones so the strict affinity would
> be more helpful.  That is the basis of whole design and why it has
> scheduler callbacks to regulate concurrency instead of creating a
> bunch of active workers and letting the scheduler take care of it.
> Works wouldn't be competing for cpu cycles.
> 
> In short, the target workload is the current short works + long
> running mostly sleeping async works, which cover most of worker pools
> we have in kernel.

Ok, maybe, but that is not what I would call a generic thread pool.

So the reason we have tons of idle workqueues around are purely because
of deadlock scenarios? Or is there other crap about?

So why not start simple and only have one thread per cpu (lets call it
events/#) and run all works there. Then when you enqueue a work and
events/# is already busy with a work from anther wq, hand the work to a
global event thread which will spawn a special single shot kthread for
it, with a second exception for those reclaim wq's, for which you'll
have this rescue thread which you'll bind to the right cpu for that
work.

That should get rid of all these gazillion threads we have, preserve the
queue property and not be as invasive as your current thing.

If they're really as idle as reported you'll never need the fork-fest
you currently propose, simply because there's not enough work.

So basically, have events/# service the first non-empty cwq, when
there's more non empty cwqs spawn them single shot threads, or use a
rescue thread.

>   I thought about adding an unbound pool of workers
> for cpu intensive works for completeness but I really couldn't find
> much use for that.  If enough number of users would need something
> like that, we can add an anonymous pool but for now I really don't see
> the need to worry about that.

And I though I'd heard multiple parties express interesting in exactly
that, btrfs, bdi and pohmelfs come to mind, also crypto looks like one
that could actually do some work.


  reply	other threads:[~2009-12-08 13:36 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-02  3:56 [PATCHSET tip/sched/core] sched: concurrency managed workqueue related sched patches Tejun Heo
2009-12-02  3:56 ` [PATCH 1/7] sched: revert 498657a478c60be092208422fefa9c7b248729c2 Tejun Heo
2009-12-02 10:42   ` [tip:sched/core] sched: Revert 498657a478c60be092208422fefa9c7b248729c2 tip-bot for Tejun Heo
2009-12-02  3:56 ` [PATCH 2/7] sched: rename preempt_notifiers to sched_notifiers and refactor implementation Tejun Heo
2009-12-02  3:56 ` [PATCH 3/7] sched: refactor try_to_wake_up() Tejun Heo
2009-12-02  9:05   ` Mike Galbraith
2009-12-02  9:51     ` Tejun Heo
2009-12-02 10:10       ` Mike Galbraith
2009-12-02 10:14         ` Tejun Heo
2009-12-02 11:01           ` Peter Zijlstra
2009-12-03  6:11   ` [PATCH UDPATED " Tejun Heo
2009-12-02  3:56 ` [PATCH 4/7] sched: implement force_cpus_allowed() Tejun Heo
2009-12-04 10:40   ` Peter Zijlstra
2009-12-04 10:43     ` Peter Zijlstra
2009-12-07  4:34       ` Tejun Heo
2009-12-07  8:35         ` Peter Zijlstra
2009-12-07 10:34           ` Tejun Heo
2009-12-07 10:54             ` Peter Zijlstra
2009-12-07 11:07               ` Tejun Heo
2009-12-08  8:41                 ` Tejun Heo
2009-12-08  9:02                   ` Peter Zijlstra
2009-12-08  9:12                     ` Tejun Heo
2009-12-08 10:34                       ` Peter Zijlstra
2009-12-08 10:38                         ` Peter Zijlstra
2009-12-08 11:26                           ` Tejun Heo
2009-12-08 11:24                         ` Tejun Heo
2009-12-08 11:48                           ` Peter Zijlstra
2009-12-08 11:56                             ` Tejun Heo
2009-12-08 12:10                               ` Peter Zijlstra
2009-12-08 12:23                                 ` Tejun Heo
2009-12-08 13:35                                   ` Peter Zijlstra [this message]
2009-12-09  5:25                                     ` Tejun Heo
2009-12-09  7:41                                       ` Peter Zijlstra
2009-12-09  8:03                                         ` Tejun Heo
2009-12-02  3:56 ` [PATCH 5/7] sched: make sched_notifiers unconditional Tejun Heo
2009-12-02  3:56 ` [PATCH 6/7] sched: add wakeup/sleep sched_notifiers and allow NULL notifier ops Tejun Heo
2009-12-02  3:56 ` [PATCH 7/7] sched: implement try_to_wake_up_local() Tejun Heo
2009-12-03  6:13   ` [PATCH UPDATED " Tejun Heo
2009-12-04 10:47     ` Peter Zijlstra
2009-12-07  3:31       ` Tejun Heo
2009-12-04 10:44   ` [PATCH " Peter Zijlstra
2009-12-07  3:26     ` Tejun Heo
2009-12-07  8:50       ` Peter Zijlstra
2009-12-07  8:56         ` Peter Zijlstra
2009-12-07 10:27           ` Tejun Heo
2009-12-08  8:53             ` Peter Zijlstra
2009-12-08  9:16               ` 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=1260279351.3935.1375.camel@laptop \
    --to=peterz@infradead.org \
    --cc=avi@redhat.com \
    --cc=efault@gmx.de \
    --cc=ego@in.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rusty@rustcorp.com.au \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --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.