archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <>
To: Jakub Jelinek <>
Cc: Segher Boessenkool <>,
	Peter Zijlstra <>,
	Nick Desaulniers <>,
	Stephane Eranian <>,,,,,,,,,,
	Nathan Chancellor <>,
	clang-built-linux <>,
	Borislav Petkov <>, "H. Peter Anvin" <>,
Subject: Re: [PATCH] x86/resctrl: avoid compiler optimization in __resctrl_sched_in
Date: Tue, 7 Mar 2023 12:54:07 -0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <ZAeh8g0nr3IFRSVI@tucnak>

On Tue, Mar 7, 2023 at 12:43 PM Jakub Jelinek <> 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

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.


  reply	other threads:[~2023-03-07 21:00 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <>
     [not found] ` <>
     [not found]   ` <>
2023-03-07 11:35     ` [PATCH] x86/resctrl: avoid compiler optimization in __resctrl_sched_in Peter Zijlstra
2023-03-07 17:48       ` Linus Torvalds
2023-03-07 18:43       ` Segher Boessenkool
2023-03-07 20:43         ` Jakub Jelinek
2023-03-07 20:54           ` Linus Torvalds [this message]
2023-03-07 21:06             ` Linus Torvalds
2023-03-07 21:35               ` Luck, Tony
2023-03-07 21:58                 ` Nick Desaulniers
2023-03-08  6:13               ` Stephane Eranian
2023-03-08 23:25                 ` Linus Torvalds
2023-03-08 16:02               ` Moger, Babu
2023-03-07 21:11             ` Luck, Tony
2023-03-07 21:14               ` Linus Torvalds
2023-03-07 21:23                 ` Luck, Tony
2023-03-08  0:36                   ` Luck, Tony
2023-03-07 21:16               ` Nick Desaulniers
2023-03-07 21:19                 ` Linus Torvalds
2023-03-07 21:22                   ` Nick Desaulniers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='' \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).