Messages in this thread | | | Date | Thu, 5 Feb 2009 18:44:36 +0100 | Subject | Re: [PATCH 2/3] work_on_cpu: Use our own workqueue. | From | Dmitry Adamushko <> |
| |
2009/2/4 Andrew Morton <akpm@linux-foundation.org>: > On Wed, 4 Feb 2009 22:35:19 +0100 > Ingo Molnar <mingo@elte.hu> wrote: > >> >> * Andrew Morton <akpm@linux-foundation.org> wrote: >> >> > mm/pdflush.c: >> > >> > wtf what the heck is all that stuff and who added it? weird. >> > >> > Leave it alone I guess. Can admins manually move kernel threads to >> > other CPUs? >> >> they can - and there's even tools that do that (there's some -rt tools where >> you can put kernel thread priorities into a config file). >> > > Oh well, DontDoThatThen. > > I expect that the same argument applies to most of the set_cpus_allowed() > callsites - they're run by root-only code. Sure, root can (with > careful timing) move root's own thread onto the wrong CPU in the middle > of microcode loading. In which case root gets to own both pieces.
Another issue is that those set_cpus_allowed() callsites may effectivelly cancel the effect of sched_setaffinity() being run by an administrator in parallel with a target process calling e.g. cpufreq_get(0)
[ there are a couple of callsites in drivers/{video,pcmcia}, possibly running in a process context for which sched_setaffinity() with mask != 'all_cpus_set' may be legitimate from admin's POV :-) ]
iow, not all use-cases are so obvious (like microcode) to fall into the category of "didn't you know that this action could do some unsynchronized cpu-mask-fiddling work behind your back". Not to say that the existence of sych category is wrong, imho.
(regarding microcode)
> This code is just nuts. What's the point in pinning itself to > a CPU for the act of loading the microcode into main > memory? It's only > the loading of the microcode which should care about > which CPU > executes the code. ie: apply_microcode().
Well, basically I tried to preserve the existing mechanisms/schemes as much as possible when reworking this code and yes, I'm the person to be blamed for this 'nuts' code.
> The code needs some laundering, switch to >schedule_work_on().
I had a patch doing exactly this but then there were other concerns with the 'schedule_work_on()' approach (e.g. http://linux.derkeiler.com/Mailing-Lists/Kernel/2008-08/msg02827.html)
then I had another idea (run it from start_secondary() or something like this), but I never managed to look at this issue again (and noone else seemed to care about really running "it as early as possible" ;-)
In any case, it should be fixable one way or another.
> Ensure that the callback functions don't take >microcode_mutex.
yes, these actually look redundant in cpu-hotplug paths (other callers call get/put_online_cpus() so this part should be ok).
Will fix. Thanks for your comments!
-- Best regards, Dmitry Adamushko
| |