Messages in this thread | | | Subject | Re: [RFC][PATCH 1/4] sched: Force the address order of each sched class descriptor | From | Rasmus Villemoes <> | Date | Fri, 20 Dec 2019 11:12:37 +0100 |
| |
On 20/12/2019 11.00, Peter Zijlstra wrote: > On Fri, Dec 20, 2019 at 09:52:37AM +0100, Rasmus Villemoes wrote: >> On 19/12/2019 22.44, Steven Rostedt wrote: >>> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org> >>> >>> In order to make a micro optimization in pick_next_task(), the order of the >>> sched class descriptor address must be in the same order as their priority >>> to each other. That is: >>> >>> &idle_sched_class < &fair_sched_class < &rt_sched_class < >>> &dl_sched_class < &stop_sched_class >>> >>> In order to guarantee this order of the sched class descriptors, add each >>> one into their own data section and force the order in the linker script. >> >> I think it would make the code simpler if one reverses these, see other >> reply. > > I started out agreeing, because of that mess around STOP_SCHED_CLASS and > that horrid BEFORE_CRUD thing. > > Then, when I fixed it all up, I saw what it did to Kyrill's patch (#4) > and that ends up looking like: > > - if (likely((prev->sched_class == &idle_sched_class || > - prev->sched_class == &fair_sched_class) && > + if (likely(prev->sched_class >= &fair_sched_class && > > And that's just weird.
I kind of agree, but if one can come up with good enough macro names, I think all that comparison logic should be in helpers either way the array is laid out. Something like
#define sched_class_lower_eq [something] /* perhaps comment on the array order */ sched_class_lower_eq(prev->sched_class, &fair_sched_class)
> Then I had a better look and now... > >>> +/* >>> + * The order of the sched class addresses are important, as they are >>> + * used to determine the order of the priority of each sched class in >>> + * relation to each other. >>> + */ >>> +#define SCHED_DATA \ >>> + *(__idle_sched_class) \ >>> + *(__fair_sched_class) \ >>> + *(__rt_sched_class) \ >>> + *(__dl_sched_class) \ >>> + STOP_SCHED_CLASS > > I'm confused, why does that STOP_SCHED_CLASS need magic here at all? > Doesn't the linker deal with empty sections already by making them 0 > sized?
Yes, but dropping the STOP_SCHED_CLASS define doesn't prevent one from needing some ifdeffery to define highest_sched_class if they are laid out in (higher sched class <-> higher address) order.
Rasmus
| |