Re: [PATCH] cpu hotplug, sched:Introduce cpu_active_map and redoscheddomainmanagment (take 2)

[Max Krasnyansky <maxk@qualcomm.com>]

Gregory Haskins wrote:
> Peter Zijlstra <a.p.zijlstra@chello.nl> wrote: 
>> I'm thinking doing it explicitly with the new cpu mask is clearer and
>> easier to understand than 'hiding' the variable in the root domain and
>> having to understand all the domain/root-domain stuff.
>>
>> I think this was Linus' main point. It should be easier to understand
>> this code.
> 
> While I can appreciate this sentiment, note that we conceptually require
> IMO the notion that the root-domain masks present.  E.g. we really dont
> want to migrate to just cpus_active_map, but rather
> rq->rd->span & cpus_active_map (otherwise we could route outside
> of a disjoint cpuset).  And this is precisely what rq->rd->online is (a
> cached version of cpus_active_map & rq->rd->span).
> 
> So while I can understand the motivation to keep things simple, note that
> I tried to design the root-domain stuff to be as simple as possible while
> still meeting the requirements for working within disjoint sets.  I am
> open to other suggestions, but I see nothing particularly complex or
> wrong with whats going on there currently.  Perhaps this very
> conversation is evidence that I needed to comment better ;)
> 
>>
>> So, if there is functional overlap with the root domain stuff, it might
>> be good to remove that bit and use the cpu_active_map stuff for that
>> instead.
> 
> I think we would be doing the code that does use it a disservice, per above.

Sorry for the delay. I finally had a chance to read through this thread again
and to look at the rq->rd->online logic.

One thing that came up during original discussion with Linus and Dmitry is
that cpuset can call partition_sched_domains() at random (user writes into
/dev/cpuset/...) but the scheduler used to rely on a certain sequence of the
domain creation/deletion during cpu hotplug. That's exactly what caused the
problem in the first place. My patch that fixed domain destruction by the
hotplug events changed the sequence the scheduler relied on and things broke.

Greg, correct me if I'm wrong but we seem to have exact same issue with the
rq->rq->online map. Lets take "cpu going down" for example. We're clearing
rq->rd->online bit on DYING event, but nothing AFAICS prevents another cpu
calling rebuild_sched_domains()->partition_sched_domains() in the middle of
the hotplug sequence.
partition_sched_domains() will happily reset rd->rq->online mask and things
will fail. I'm talking about this path

__build_sched_domains() -> cpu_attach_domain() -> rq_attach_root()
	...
	cpu_set(rq->cpu, rd->span);
	if (cpu_isset(rq->cpu, cpu_online_map))
		set_rq_online(rq);
	...

--

btw Why didn't we convert sched*.c to use rq->rd->online when it was
introduced ? ie Instead of using cpu_online_map directly.

Max