Messages in this thread | | | Date | Mon, 21 Jul 2008 22:10:01 -0700 | From | Max Krasnyansky <> | Subject | Re: [PATCH] cpu hotplug, sched:Introduce cpu_active_map and redoscheddomainmanagment (take 2) |
| |
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
| |