Messages in this thread | | | Subject | Re: [PATCH sched-devel 0/7] CPU isolation extensions | From | Peter Zijlstra <> | Date | Fri, 22 Feb 2008 12:43:20 +0100 |
| |
On Thu, 2008-02-21 at 18:38 -0800, Max Krasnyanskiy wrote:
> As you suggested I'm sending CPU isolation patches for review/inclusion into > sched-devel tree. They are against 2.6.25-rc2. > You can also pull them from my GIT tree at > git://git.kernel.org/pub/scm/linux/kernel/git/maxk/cpuisol-2.6.git master
Post patches! I can't review a git tree..
> Diffstat: > b/Documentation/ABI/testing/sysfs-devices-system-cpu | 41 ++++++ > b/Documentation/cpu-isolation.txt | 114 ++++++++++++++++++- > b/arch/x86/Kconfig | 1 > b/arch/x86/kernel/genapic_flat_64.c | 5 > b/drivers/base/cpu.c | 48 ++++++++ > b/include/linux/cpumask.h | 3 > b/kernel/Kconfig.cpuisol | 15 ++ > b/kernel/Makefile | 4 > b/kernel/cpu.c | 49 ++++++++ > b/kernel/sched.c | 37 ------ > b/kernel/stop_machine.c | 9 + > b/kernel/workqueue.c | 31 +++-- > kernel/Kconfig.cpuisol | 56 ++++++--- > kernel/cpu.c | 16 +- > 14 files changed, 356 insertions(+), 73 deletions(-) > > List of commits > cpuisol: Make cpu isolation configrable and export isolated map
cpu_isolated_map was a bad hack when it was introduced, I feel we should deprecate it and fully integrate the functionality into cpusets. That would give a much more flexible end-result.
CPU-sets can already isolate cpus by either creating a cpu outside of any set, or a set with a single cpu not shared by any other sets.
This also allows for isolated groups, there are good reasons to isolate groups, esp. now that we have a stronger RT balancer. SMP and hard RT are not exclusive. A design that does not take that into account is too rigid.
> cpuisol: Do not route IRQs to the CPUs isolated at boot
>From the diffstat you're not touching the genirq stuff, but instead hack a single architecture to support this feature. Sounds like an ill designed hack.
A better approach would be to add a flag to the cpuset infrastructure that says whether its a system set or not. A system set would be one that services the general purpose OS and would include things like the IRQ affinity and unbound kernel threads (including unbound workqueues - or single workqueues). This flag would default to on, and by switching it off for the root set, and a select subset you would push the System away from those cpus, thereby isolating them.
> cpuisol: Do not schedule workqueues on the isolated CPUs
(per-cpu workqueues, the single ones are treated in the previous section)
I still strongly disagree with this approach. Workqueues are passive, they don't do anything unless work is provided to them. By blindly not starting them you handicap the system and services that rely on them.
(you even acknowledged this problem, by saying it breaks oprofile for instance - still trying to push a change that knowingly breaks a lot of stuff is bad manners on lkml and not acceptable for mainline)
The way to do this is to avoid the generation of work, not the execution of it.
> cpuisol: Move on-stack array used for boot cmd parsing into __initdata > cpuisol: Documentation updates > cpuisol: Minor updates to the Kconfig options
No idea about these patches,...
> cpuisol: Do not halt isolated CPUs with Stop Machine
Very strong NACK on this one, it breaks a lot of functionality in non-obvious ways, as has been pointed out to you numerous times. Such patches are just not acceptable for mainline - full stop.
Please, the ideas are not bad and have been suggested in the context of -rt a number of times, its just the execution. Design features so that they are flexible and can be used in ways you never imagined them. But more importantly so that they don't knowingly break tons of stuff.
| |