linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Max Krasnyansky <maxk@qualcomm.com>
To: Oleg Nesterov <oleg@tv-sign.ru>
Cc: Peter Zijlstra <peterz@infradead.org>,
	mingo@elte.hu, Andrew Morton <akpm@linux-foundation.org>,
	David Rientjes <rientjes@google.com>, Paul Jackson <pj@sgi.com>,
	menage@google.com, linux-kernel@vger.kernel.org,
	Mark Hounschell <dmarkh@cfl.rr.com>
Subject: Re: workqueue cpu affinity
Date: Wed, 11 Jun 2008 12:21:39 -0700	[thread overview]
Message-ID: <485025C3.4020409@qualcomm.com> (raw)
In-Reply-To: <20080611160815.GA150@tv-sign.ru>

Oleg Nesterov wrote:
> On 06/10, Max Krasnyansky wrote:
>> Here is some backgound on this. Full cpu isolation requires some tweaks to the
>> workqueue handling. Either the workqueue threads need to be moved (which is my
>> current approach), or work needs to be redirected when it's submitted.
> 
> _IF_ we have to do this, I think it is much better to move cwq->thread.
Ok. btw That's what I'm doing now from user-space.

>> Peter Zijlstra wrote:
>>> The advantage of creating a more flexible or fine-grained flush is that
>>> large machine also profit from it.
>> I agree, our current workqueue flush scheme is expensive because it has to
>> schedule on each online cpu. So yes improving flush makes sense in general.
> 
> Yes, it is easy to implement flush_work(struct work_struct *work) which
> only waits for that work, so it can't hang unless it was enqueued on the
> isolated cpu.
> 
> But in most cases it is enough to just do
> 
> 	if (cancel_work_sync(work))
> 		work->func(work);
Cool. That would.
btw Somehow I thought that you already implemented flush_work(). I do not see
it 2.6.25 but I could've sworn that I saw a patch flying by. Must have been
something else. Do you mind adding that ?

> Or we can add flush_workqueue_cpus(struct workqueue_struct *wq, cpumask_t *cpu_map).
That'd be special casing. I mean something will have to know what cpus cannot
be flushed. I liked your proposal above much better.

> But I don't think we should change the behaviour of flush_workqueue().
> 
>> This will require a bit of surgery across the entire tree. There is a lot of
>> code that calls flush_scheduled_work()
> 
> Almost all of them should be changed to use cancel_work_sync().

That'd be a lot of changes.

git grep flush_scheduled_work | wc
    154     376    8674

Hmm, I guess maybe not that bad. I might actually do that :-).

Max

  reply	other threads:[~2008-06-11 19:21 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-05 19:57 [patch] sched: prevent bound kthreads from changing cpus_allowed David Rientjes
2008-06-05 20:29 ` Paul Jackson
2008-06-05 21:12   ` David Rientjes
2008-06-09 20:59     ` Max Krasnyanskiy
2008-06-09 22:07       ` David Rientjes
2008-06-10  4:23         ` Max Krasnyansky
2008-06-10 17:04           ` David Rientjes
2008-06-10 16:30         ` cpusets and kthreads, inconsistent behaviour Max Krasnyansky
2008-06-10 18:47           ` David Rientjes
2008-06-10 20:44             ` Max Krasnyansky
2008-06-10 20:54               ` David Rientjes
2008-06-10 21:15                 ` Max Krasnyansky
2008-06-10  6:44       ` [patch] sched: prevent bound kthreads from changing cpus_allowed Peter Zijlstra
2008-06-10 15:38         ` Max Krasnyansky
2008-06-10 17:00           ` Oleg Nesterov
2008-06-10 17:19             ` Peter Zijlstra
2008-06-10 20:24               ` workqueue cpu affinity Max Krasnyansky
2008-06-11  6:49                 ` Peter Zijlstra
2008-06-11 19:02                   ` Max Krasnyansky
2008-06-12 18:44                     ` Peter Zijlstra
2008-06-12 19:10                       ` Max Krasnyanskiy
2008-06-11 16:08                 ` Oleg Nesterov
2008-06-11 19:21                   ` Max Krasnyansky [this message]
2008-06-11 19:21                   ` Max Krasnyansky
2008-06-12 16:35                     ` Oleg Nesterov
2008-06-11 20:44                   ` Max Krasnyansky
2008-06-10 18:00             ` [patch] sched: prevent bound kthreads from changing cpus_allowed Max Krasnyansky
2008-06-05 20:52 ` Daniel Walker
2008-06-05 21:47 ` Paul Jackson
2008-06-10 10:28 ` Ingo Molnar
2008-06-10 17:47 ` Oleg Nesterov

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=485025C3.4020409@qualcomm.com \
    --to=maxk@qualcomm.com \
    --cc=akpm@linux-foundation.org \
    --cc=dmarkh@cfl.rr.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=menage@google.com \
    --cc=mingo@elte.hu \
    --cc=oleg@tv-sign.ru \
    --cc=peterz@infradead.org \
    --cc=pj@sgi.com \
    --cc=rientjes@google.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).