Re: current linux-2.6.git: cpusets completely broken

[Max Krasnyansky]

Ingo Molnar wrote:
> * Max Krasnyansky <maxk@xxxxxxxxxxxx> wrote:
>
> > Ingo Molnar wrote:
> > > * Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > > On Mon, 14 Jul 2008, Max Krasnyansky wrote:
> > > > > Did you guys an updated patch ? Dmitry pointed out several things that Linus
> > > > > missed in his original version. I guess I can go through the thread and
> > > > > reconstruct that but if you have a patch I can try let me know.
> > > > I didn't update it, and right now I'm just merging too much (and  
> > > > discussing the merges) to have time.
> > > >
> > > > The patch really needs to have some scheduler person look at the use  
> > > > fo cpu_active_map - I was kind of hoping that Ingo would.
> > > yeah - it's very high on our TODO list :-) Peter, Dmitry and me are  
> > > looking into it.
> > >
> > > I didnt touch most of -tip in the past few days to get a rock solid QA  
> > > track record for all items we have.
> > I just sent you guys a patch. Please take a look. I've probably missed 
> > something but it should close (I think). Also we'd probably at least 
> > want the bits that streamline the domain reinitialization because it 
> > helps with cpusets (ie uses same exact path for all cases).
>
> thanks Max. Since upstream already has Dmitry's it conflicted with your 
> patch - i fixed the interactions up, see it below. (completely untested)
Hmm, I did it on top of 2.6.26 final which has Dmitry's patch. It must have
been something in the -tip.
Anyway, I'll apply it here and retest.

> It's not ready for inclusion yet though - these new
> #ifdefs are quite ugly:
>
>  +#if !defined(CONFIG_CPUSETS)
>  +       partition_sched_domains(0, NULL, NULL);
>  +#else
>  +       rebuild_sched_domains();
>  +#endif
>
> we should just have a single method for refreshing sched domains 
> hierarchy, and in the !CONFIG_CPUSETS case that should simply fall back 
> to partition_sched_domains().
>
> We can do that by making rebuild_sched_domains() the primary method that 
> is called - and in the !CPUSETS case it's an inline that calls 
> partition_sched_domains().

Actually that's exactly what I have in the cpuset part of the patch. But as
I mentioned there is some circular locking issues with rebuild_sched_domains().
(cgroup_lock and get_online_cpus()) and I wanted to fix that fix. Now that I
think about it's kind of unrelated. So in other words I totally agree. I'll
go ahead fix it and resend.

btw While we're at it. Does arch_init_sched_domains() and arch_reinit_sched_domains()
still make sense. I'm talking about naming here. I suppose arch_ part means that it
can be replaced by the arch code. But it's not the case with those two guys.
What do you think ?

> also, small nits:
>
> use #ifdef CONFIG_CPUSETS instead of "#if !defined(CONFIG_CPUSETS)".
Will do.

> and while at it:
>
>  +#if !defined(CONFIG_CPUSETS)
>          /* XXX: Theoretical race here - CPU may be hotplugged now */
>          hotcpu_notifier(update_sched_domains, 0);
>  +#endif
>
> that race should be closed now, hm?
Yeah, I stared at that comment for a second and decided to leave it.
I guess it's still there. I mean in theory cpu can still be hotplugged just
before sched or cpuset register their notifier callbacks.

Max
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/