Messages in this thread Patch in this message | | | Date | Fri, 20 Dec 2019 11:00:33 +0100 | From | Peter Zijlstra <> | Subject | Re: [RFC][PATCH 1/4] sched: Force the address order of each sched class descriptor |
| |
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.
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?
> > /* > > * Align to a 32 byte boundary equal to the > > * alignment gcc 4.5 uses for a struct > > @@ -308,6 +326,7 @@ > > #define DATA_DATA \ > > *(.xiptext) \ > > *(DATA_MAIN) \ > > + SCHED_DATA \ > > *(.ref.data) \ > > Doesn't this make the structs end up in .data (writable) rather than > .rodata?
Right! That wants fixing.
| |