Messages in this thread | ![/](/images/icornerl.gif) | | From | Stephane Eranian <> | Date | Tue, 7 Mar 2023 22:13:27 -0800 | Subject | Re: [PATCH] x86/resctrl: avoid compiler optimization in __resctrl_sched_in |
| |
On Tue, Mar 7, 2023 at 1:06 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Tue, Mar 7, 2023 at 12:54 PM Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > I think the problem is that the <asm/resctrl.h> code is disgusting and > > horrible in multiple ways: > > > > (a) it shouldn't define and declare a static function in a header file > > > > (b) the resctrl_sched_in() inline function is misdesigned to begin with > > Ok, so here's a *ttoally* untested and mindless patch to maybe fix > what I dislike about that resctl code. > > Does it fix the code generation issue? I have no idea. But this is > what I would suggest is the right answer, without actually knowing the > code any better, and just going on a mindless rampage. > > It seems to compile for me, fwiw. >
On Tue, Mar 7, 2023 at 3:01 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Tue, Mar 7, 2023 at 2:56 PM Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > On Tue, Mar 7, 2023 at 2:03 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > > > Sounds like Stephane is going to re-run the internal tests he used to > > > discover the issue with your diff applied, if you don't mind holding > > > out for another Tested-by tag. EOM > > > > Ack. I am in no hurry. > > > > In fact, I'd prefer to just get the patch sent back to me with a > > commit message too, if somebody has the energy. I don't need the > > credit for a trivial thing like that. > > Sure, then maybe Stephane you can supply a v2 with updated commit message and a > > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> > I verified Linus' patch on my test case on AMD Zen3 and it works as expected, i.e., the limit is enforced. I had tried a similar approach myself as well and it worked.
I think passing the task pointer is the proper approach because we are in a sched_in routine and I would expect the task scheduled in to be passed as argument instead of having the function retrieve it from the current pointer.
Thanks.
Tested-by: Stephane Eranian <eranian@google.com>
> Linus
| ![\](/images/icornerr.gif) |