LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Max Krasnyanskiy <maxk@qualcomm.com>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>, Paul Jackson <pj@sgi.com>,
	LKML <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@elte.hu>,
	Gregory Haskins <ghaskins@novell.com>
Subject: Re: [CPUISOL] CPU isolation extensions
Date: Mon, 28 Jan 2008 13:42:54 -0800	[thread overview]
Message-ID: <479E4C5E.3010804@qualcomm.com> (raw)
In-Reply-To: <1201551730.28547.54.camel@lappy>

Peter Zijlstra wrote:
> On Mon, 2008-01-28 at 14:00 -0500, Steven Rostedt wrote:
>> On Mon, 28 Jan 2008, Max Krasnyanskiy wrote:
>>>>>   [PATCH] [CPUISOL] Support for workqueue isolation
>>>> The thing about workqueues is that they should only be woken on a CPU if
>>>> something on that CPU accessed them. IOW, the workqueue on a CPU handles
>>>> work that was called by something on that CPU. Which means that
>>>> something that high prio task did triggered a workqueue to do some work.
>>>> But this can also be triggered by interrupts, so by keeping interrupts
>>>> off the CPU no workqueue should be activated.
>>> No no no. That's what I though too ;-). The problem is that things like NFS and friends
>>> expect _all_ their workqueue threads to report back when they do certain things like
>>> flushing buffers and stuff. The reason I added this is because my machines were getting
>>> stuck because CPU0 was waiting for CPU1 to run NFS work queue threads even though no IRQs
>>> or other things are running on it.
>> This sounds more like we should fix NFS than add this for all workqueues.
>> Again, we want workqueues to run on the behalf of whatever is running on
>> that CPU, including those tasks that are running on an isolcpu.
> 
> agreed, by looking at my top output (and not the nfs code) it looks like
> it just spawns a configurable number of active kernel threads which are
> not cpu bound by in any way. I think just removing the isolated cpus
> from their runnable mask should take care of them.

Actually NFS was just one example. I cannot remember of a top of my head what else was there
but there are definitely other users of work queues that expect all the threads to run at
some point in time.
Also if you think about it. The patch does _exactly_ what you propose. It removes workqueue 
threads from isolated CPUs. But instead of doing just for NFS and/or other subsystems 
separately it just does it in a generic way by simply not starting those threads in first 
place.
 
>>>>>   [PATCH] [CPUISOL] Isolated CPUs should be ignored by the "stop machine"
>>>> This I find very dangerous. We are making an assumption that tasks on an
>>>> isolated CPU wont be doing things that stopmachine requires. What stops
>>>> a task on an isolated CPU from calling something into the kernel that
>>>> stop_machine requires to halt?
>>> I agree in general. The thing is though that stop machine just kills any kind of latency
>>> guaranties. Without the patch the machine just hangs waiting for the stop-machine to run
>>> when module is inserted/removed. And running without dynamic module loading is not very
>>> practical on general purpose machines. So I'd rather have an option with a big red warning
>>> than no option at all :).
>> Well, that's something one of the greater powers (Linus, Andrew, Ingo)
>> must decide. ;-)
> 
> I'm in favour of better engineered method, that is, we really should try
> to solve these problems in a proper way. Hacks like this might be fine
> for custom kernels, but I think we should have a higher standard when it
> comes to upstream - we all have to live many years with whatever we put
> in there, we'd better think well about it.

100% agree. That's why I said mentioned that this patches is controversial in the first place. 
Right now those short from rewriting module loading to not use stop machine there is no other 
option. I'll think some more about it. If you guys have other ideas please drop me a note.

Thanx
Max

  reply	other threads:[~2008-01-28 21:43 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-28  4:09 maxk
2008-01-28  4:09 ` [PATCH] [CPUISOL] Add config options for CPU isolation maxk
2008-01-28  4:09   ` [PATCH] [CPUISOL] Export CPU isolation bits maxk
2008-01-28  4:09     ` [PATCH] [CPUISOL] Do not route IRQs to the CPUs isolated at boot maxk
2008-01-28  4:09       ` [PATCH] [CPUISOL] Support for workqueue isolation maxk
2008-01-28  4:09         ` [PATCH] [CPUISOL] Isolated CPUs should be ignored by the "stop machine" maxk
2008-01-28  9:08 ` [CPUISOL] CPU isolation extensions Peter Zijlstra
2008-01-28 14:59   ` Paul Jackson
2008-01-28 16:34     ` Steven Rostedt
2008-01-28 16:44       ` Peter Zijlstra
2008-01-28 18:54         ` Max Krasnyanskiy
2008-01-28 18:46       ` Max Krasnyanskiy
2008-01-28 19:00         ` Steven Rostedt
2008-01-28 20:22           ` Peter Zijlstra
2008-01-28 21:42             ` Max Krasnyanskiy [this message]
2008-02-05  0:32             ` CPU isolation and workqueues [was Re: [CPUISOL] CPU isolation extensions] Max Krasnyanskiy
2008-01-28 18:37     ` [CPUISOL] CPU isolation extensions Max Krasnyanskiy
2008-01-28 19:06       ` Paul Jackson
2008-01-28 21:47         ` Max Krasnyanskiy
2008-01-31 19:06         ` Integrating cpusets and cpu isolation [was Re: [CPUISOL] CPU isolation extensions] Max Krasnyanskiy
2008-02-02  6:16           ` Paul Jackson
2008-02-03  5:57             ` Max Krasnyansky
2008-02-03  7:53               ` Paul Jackson
2008-02-04  6:03                 ` Max Krasnyansky
2008-02-04 10:54                   ` Paul Jackson
2008-02-04 23:19                     ` Max Krasnyanskiy
2008-02-05  2:46                       ` Paul Jackson
2008-02-05  4:08                         ` Max Krasnyansky
2008-01-28 18:32   ` [CPUISOL] CPU isolation extensions Max Krasnyanskiy
2008-01-28 19:10     ` Paul Jackson
2008-01-28 23:41     ` Daniel Walker
2008-01-29  0:12       ` Max Krasnyanskiy
2008-01-29  1:33         ` Daniel Walker
2008-02-04  6:53           ` Max Krasnyansky
2008-01-31 12:16 ` Mark Hounschell
2008-01-31 19:13   ` Max Krasnyanskiy

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=479E4C5E.3010804@qualcomm.com \
    --to=maxk@qualcomm.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=ghaskins@novell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=pj@sgi.com \
    --cc=rostedt@goodmis.org \
    --subject='Re: [CPUISOL] CPU isolation extensions' \
    /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

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).