Messages in this thread | | | From | Linus Torvalds <> | Date | Tue, 7 Mar 2023 12:54:07 -0800 | Subject | Re: [PATCH] x86/resctrl: avoid compiler optimization in __resctrl_sched_in |
| |
On Tue, Mar 7, 2023 at 12:43 PM Jakub Jelinek <jakub@redhat.com> wrote: > > Are we actually talking here about "p" constraint or about p/P (x86) modifiers > (asm ("%p0" : : "i" (42));)?
In this case it's actually the "p" constraint.
And the "percpu_stable_op()" thing uses it exactly because it thinks it wants the "I don't care about data dependencies in memory, because this memory location doesn't change".
Of course, that "this memory location doesn't change" is then always technically a lie. It's exactly things like "current" that obviously *does* change in the big picture, but from the context of a particular _thread_, "current" is a fixed value.
Which then causes problems when you use that "percpu_stable_op()" thing from within the scheduler (generally without being *aware* of this issue at all, since the use is hidden a few macro expansions down).
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 midesigned to begin with
basically, the actual scheduling functions should *never* look at "current" at all. They are - byu definition - changing it, and they already *know* what both the "incoming current" (aka "prev_p") and "new current" (aka "next_p") are.
So any scheduling function that uses "current" is hot garbage.
In this case, that hot garbage is resctrl_sched_in().
I do not believe this is a compiler issue. This is literally just a kernel bug, and that resctrl code being very very wrong.
Linus
| |