lkml.org 
[lkml]   [2008]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: current linux-2.6.git: cpusets completely broken
Ingo Molnar wrote:
> * Max Krasnyansky <maxk@qualcomm.com> wrote:
>
>> Ingo Molnar wrote:
>>> * Linus Torvalds <torvalds@linux-foundation.org> 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


\
 
 \ /
  Last update: 2008-07-15 11:15    [W:0.093 / U:0.280 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site