From: "Nysal Jan K.A." <nysal@linux.ibm.com>
To: Andrew Morton <akpm@linux-foundation.org>,
Michael Ellerman <mpe@ellerman.id.au>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Segher Boessenkool <segher@kernel.crashing.org>,
Stephen Rothwell <sfr@canb.auug.org.au>,
Peter Zijlstra <peterz@infradead.org>,
linuxppc-dev@lists.ozlabs.org,
"Nysal Jan K.A." <nysal@linux.ibm.com>,
Nathan Chancellor <nathan@kernel.org>,
Nick Desaulniers <ndesaulniers@google.com>,
Bill Wendling <morbo@google.com>,
Justin Stitt <justinstitt@google.com>,
Vlastimil Babka <vbabka@suse.cz>,
"Liam R. Howlett" <Liam.Howlett@Oracle.com>,
Mark Brown <broonie@kernel.org>, Michal Hocko <mhocko@suse.com>,
Kent Overstreet <kent.overstreet@linux.dev>,
Rick Edgecombe <rick.p.edgecombe@intel.com>,
linux-kernel@vger.kernel.org, llvm@lists.linux.dev
Subject: [PATCH v2] sched/membarrier: Fix redundant load of membarrier_state
Date: Tue, 29 Oct 2024 11:21:28 +0530 [thread overview]
Message-ID: <20241029055133.121418-1-nysal@linux.ibm.com> (raw)
In-Reply-To: <20241007053936.833392-1-nysal@linux.ibm.com>
On architectures where ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
is not selected, sync_core_before_usermode() is a no-op.
In membarrier_mm_sync_core_before_usermode() the compiler does not
eliminate redundant branches and load of mm->membarrier_state
for this case as the atomic_read() cannot be optimized away.
Here's a snippet of the code generated for finish_task_switch() on powerpc
prior to this change:
1b786c: ld r26,2624(r30) # mm = rq->prev_mm;
.......
1b78c8: cmpdi cr7,r26,0
1b78cc: beq cr7,1b78e4 <finish_task_switch+0xd0>
1b78d0: ld r9,2312(r13) # current
1b78d4: ld r9,1888(r9) # current->mm
1b78d8: cmpd cr7,r26,r9
1b78dc: beq cr7,1b7a70 <finish_task_switch+0x25c>
1b78e0: hwsync
1b78e4: cmplwi cr7,r27,128
.......
1b7a70: lwz r9,176(r26) # atomic_read(&mm->membarrier_state)
1b7a74: b 1b78e0 <finish_task_switch+0xcc>
This was found while analyzing "perf c2c" reports on kernels prior
to commit c1753fd02a00 ("mm: move mm_count into its own cache line")
where mm_count was false sharing with membarrier_state.
There is a minor improvement in the size of finish_task_switch().
The following are results from bloat-o-meter for ppc64le:
GCC 7.5.0
---------
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-32 (-32)
Function old new delta
finish_task_switch 884 852 -32
GCC 12.2.1
----------
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-32 (-32)
Function old new delta
finish_task_switch.isra 852 820 -32
LLVM 17.0.6
-----------
add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-36 (-36)
Function old new delta
rt_mutex_schedule 120 104 -16
finish_task_switch 792 772 -20
Results on aarch64:
GCC 14.1.1
----------
add/remove: 0/2 grow/shrink: 1/1 up/down: 4/-60 (-56)
Function old new delta
get_nohz_timer_target 352 356 +4
e843419@0b02_0000d7e7_408 8 - -8
e843419@01bb_000021d2_868 8 - -8
finish_task_switch.isra 592 548 -44
Signed-off-by: Nysal Jan K.A. <nysal@linux.ibm.com>
---
V1 -> V2:
- Add results for aarch64
- Add a comment describing the changes
---
include/linux/sched/mm.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index 928a626725e6..b13474825130 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -531,6 +531,13 @@ enum {
static inline void membarrier_mm_sync_core_before_usermode(struct mm_struct *mm)
{
+ /*
+ * The atomic_read() below prevents CSE. The following should
+ * help the compiler generate more efficient code on architectures
+ * where sync_core_before_usermode() is a no-op.
+ */
+ if (!IS_ENABLED(CONFIG_ARCH_HAS_SYNC_CORE_BEFORE_USERMODE))
+ return;
if (current->mm != mm)
return;
if (likely(!(atomic_read(&mm->membarrier_state) &
--
2.47.0
next prev parent reply other threads:[~2024-10-29 5:52 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-07 5:39 [PATCH] sched/membarrier: Fix redundant load of membarrier_state Nysal Jan K.A.
2024-10-25 0:29 ` Michael Ellerman
2024-10-25 2:40 ` Stephen Rothwell
2024-10-25 3:22 ` Segher Boessenkool
2024-10-25 12:40 ` Mathieu Desnoyers
2024-10-25 18:30 ` Nysal Jan K.A.
2024-10-29 5:51 ` Nysal Jan K.A. [this message]
2024-10-29 17:51 ` [PATCH v2] " Mathieu Desnoyers
2024-10-29 23:29 ` Michael Ellerman
2024-10-30 13:33 ` Segher Boessenkool
2024-11-18 9:04 ` Michal Hocko
2024-11-18 9:25 ` Peter Zijlstra
2024-11-18 9:51 ` Michal Hocko
2025-01-09 8:46 ` Michal Hocko
2025-03-03 6:04 ` [PATCH v2 RESEND] " Nysal Jan K.A.
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=20241029055133.121418-1-nysal@linux.ibm.com \
--to=nysal@linux.ibm.com \
--cc=Liam.Howlett@Oracle.com \
--cc=akpm@linux-foundation.org \
--cc=broonie@kernel.org \
--cc=justinstitt@google.com \
--cc=kent.overstreet@linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=llvm@lists.linux.dev \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhocko@suse.com \
--cc=morbo@google.com \
--cc=mpe@ellerman.id.au \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=peterz@infradead.org \
--cc=rick.p.edgecombe@intel.com \
--cc=segher@kernel.crashing.org \
--cc=sfr@canb.auug.org.au \
--cc=vbabka@suse.cz \
/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.