All of lore.kernel.org
 help / color / mirror / Atom feed
From: "tip-bot2 for Sebastian Andrzej Siewior" <tip-bot2@linutronix.de>
To: linux-tip-commits@vger.kernel.org
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: [tip: locking/urgent] x86/percpu: Remove volatile from arch_raw_cpu_ptr().
Date: Tue, 05 Apr 2022 08:28:56 -0000	[thread overview]
Message-ID: <164914733657.389.13602692959226451396.tip-bot2@tip-bot2> (raw)
In-Reply-To: <20220328145810.86783-2-bigeasy@linutronix.de>

The following commit has been merged into the locking/urgent branch of tip:

Commit-ID:     1c1e7e3c23dd25f938302428eeb22c3dda2c3427
Gitweb:        https://git.kernel.org/tip/1c1e7e3c23dd25f938302428eeb22c3dda2c3427
Author:        Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate:    Mon, 28 Mar 2022 16:58:08 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 05 Apr 2022 09:59:38 +02:00

x86/percpu: Remove volatile from arch_raw_cpu_ptr().

The volatile attribute in the inline assembly of arch_raw_cpu_ptr()
forces the compiler to always generate the code, even if the compiler
can decide upfront that its result is not needed.

For instance invoking __intel_pmu_disable_all(false) (like
intel_pmu_snapshot_arch_branch_stack() does) leads to loading the
address of &cpu_hw_events into the register while compiler knows that it
has no need for it. This ends up with code like:

|	movq	$cpu_hw_events, %rax			#, tcp_ptr__
|	add	%gs:this_cpu_off(%rip), %rax		# this_cpu_off, tcp_ptr__
|	xorl	%eax, %eax				# tmp93

It also creates additional code within local_lock() with !RT &&
!LOCKDEP which is not desired.

By removing the volatile attribute the compiler can place the
function freely and avoid it if it is not needed in the end.
By using the function twice the compiler properly caches only the
variable offset and always loads the CPU-offset.

this_cpu_ptr() also remains properly placed within a preempt_disable()
sections because
- arch_raw_cpu_ptr() assembly has a memory input ("m" (this_cpu_off))
- prempt_{dis,en}able() fundamentally has a 'barrier()' in it

Therefore this_cpu_ptr() is already properly serialized and does not
rely on the 'volatile' attribute.

Remove volatile from arch_raw_cpu_ptr().

[ bigeasy: Added Linus' explanation why this_cpu_ptr() is not moved out
  of a preempt_disable() section without the 'volatile' attribute. ]

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20220328145810.86783-2-bigeasy@linutronix.de
---
 arch/x86/include/asm/percpu.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index a3c33b7..13c0d63 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -38,9 +38,9 @@
 #define arch_raw_cpu_ptr(ptr)				\
 ({							\
 	unsigned long tcp_ptr__;			\
-	asm volatile("add " __percpu_arg(1) ", %0"	\
-		     : "=r" (tcp_ptr__)			\
-		     : "m" (this_cpu_off), "0" (ptr));	\
+	asm ("add " __percpu_arg(1) ", %0"		\
+	     : "=r" (tcp_ptr__)				\
+	     : "m" (this_cpu_off), "0" (ptr));		\
 	(typeof(*(ptr)) __kernel __force *)tcp_ptr__;	\
 })
 #else

  reply	other threads:[~2022-04-05 10:26 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-21 11:11 [GIT PULL] locking changes for v5.18 Ingo Molnar
2022-03-22 22:05 ` Linus Torvalds
2022-03-22 22:19   ` Borislav Petkov
2022-03-22 22:58     ` Linus Torvalds
2022-03-23  7:11       ` Sebastian Andrzej Siewior
2022-03-23 11:09         ` [PATCH] locking/local_lock: Pretend to use the per-CPU variable if not needed Sebastian Andrzej Siewior
2022-03-23 17:17           ` Linus Torvalds
2022-03-24 17:39             ` [PATCH 0/3] Remove volatile from arch_raw_cpu_ptr() and revert the hacks Sebastian Andrzej Siewior
2022-03-24 17:39               ` [PATCH 1/3] x86/percpu: Remove volatile from arch_raw_cpu_ptr() Sebastian Andrzej Siewior
2022-03-24 17:39               ` [PATCH 2/3] Revert "locking/local_lock: Make the empty local_lock_*() function a macro." Sebastian Andrzej Siewior
2022-03-24 17:39               ` [PATCH 3/3] Revert "mm/page_alloc: mark pagesets as __maybe_unused" Sebastian Andrzej Siewior
2022-03-24 18:28               ` [PATCH 0/3] Remove volatile from arch_raw_cpu_ptr() and revert the hacks Linus Torvalds
2022-03-28 13:55                 ` Peter Zijlstra
2022-03-28 14:59                   ` Sebastian Andrzej Siewior
2022-03-28 14:58                 ` [PATCH v2 " Sebastian Andrzej Siewior
2022-03-28 14:58                   ` [PATCH v2 1/3] x86/percpu: Remove volatile from arch_raw_cpu_ptr() Sebastian Andrzej Siewior
2022-04-05  8:28                     ` tip-bot2 for Sebastian Andrzej Siewior [this message]
2022-03-28 14:58                   ` [PATCH v2 2/3] Revert "locking/local_lock: Make the empty local_lock_*() function a macro." Sebastian Andrzej Siewior
2022-04-05  8:28                     ` [tip: locking/urgent] " tip-bot2 for Sebastian Andrzej Siewior
2022-03-28 14:58                   ` [PATCH v2 3/3] Revert "mm/page_alloc: mark pagesets as __maybe_unused" Sebastian Andrzej Siewior
2022-04-05  8:28                     ` [tip: locking/urgent] " tip-bot2 for Sebastian Andrzej Siewior
2022-03-23 11:21       ` [PATCH] x86/defconfig: Enable WERROR Borislav Petkov
2022-03-23 17:19         ` Linus Torvalds
2022-03-23 17:33           ` Borislav Petkov
2022-03-24  8:31           ` [PATCH] x86/config: Make the x86 defconfigs a bit more usable Ingo Molnar
2022-03-24  9:12             ` David Laight
2022-03-24 15:47             ` Nathan Chancellor
2022-03-25 11:52               ` Andy Shevchenko
2022-03-27 19:04                 ` Ingo Molnar
2022-03-27 19:03               ` Ingo Molnar
2022-03-28 15:41                 ` Nathan Chancellor
2022-09-02  8:50                   ` Ingo Molnar
2022-09-02  9:18                     ` Masahiro Yamada
2022-09-04  9:48                       ` Ingo Molnar
2022-09-05  2:16                         ` Masahiro Yamada
2022-09-05  9:54                           ` Ingo Molnar
     [not found]                             ` <CAK7LNAQyiNpbLuVjjQ8-GOQECtfQZqsNS8xH0E2ZkLAHYtXt7A@mail.gmail.com>
2022-09-10 17:28                               ` Linus Torvalds
2022-03-24  8:16         ` [tip: x86/urgent] x86/defconfig: Enable WERROR tip-bot2 for Borislav Petkov
2022-03-25 11:41       ` [GIT PULL] locking changes for v5.18 Andy Shevchenko
2022-03-25 12:23         ` Peter Zijlstra
2022-03-25 13:06           ` Andy Shevchenko
2022-03-25 17:29         ` Linus Torvalds
2022-03-25 17:53           ` Andy Shevchenko
2022-03-22 22:38   ` Peter Zijlstra
2022-03-22 23:26   ` Linus Torvalds
2022-03-24  8:40     ` Ingo Molnar
2022-03-24 10:19       ` Borislav Petkov
2022-03-24 23:19         ` Nick Desaulniers
2022-03-22 23:27 ` pr-tracker-bot

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=164914733657.389.13602692959226451396.tip-bot2@tip-bot2 \
    --to=tip-bot2@linutronix.de \
    --cc=bigeasy@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.