All of lore.kernel.org
 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:47 -0700	[thread overview]
Message-ID: <485025CB.8050505@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 work.
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

  parent 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
2008-06-11 19:21                   ` Max Krasnyansky [this message]
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=485025CB.8050505@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 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.