[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230724012419.2317649-8-sashal@kernel.org>
Date: Sun, 23 Jul 2023 21:24:05 -0400
From: Sasha Levin <sashal@...nel.org>
To: linux-kernel@...r.kernel.org, stable@...r.kernel.org
Cc: Geert Uytterhoeven <geert+renesas@...der.be>,
Peter Zijlstra <peterz@...radead.org>,
Arnd Bergmann <arnd@...db.de>,
Tony Lindgren <tony@...mide.com>,
Ulf Hansson <ulf.hansson@...aro.org>,
Sasha Levin <sashal@...nel.org>
Subject: [PATCH AUTOSEL 5.10 08/22] iopoll: Call cpu_relax() in busy loops
From: Geert Uytterhoeven <geert+renesas@...der.be>
[ Upstream commit b407460ee99033503993ac7437d593451fcdfe44 ]
It is considered good practice to call cpu_relax() in busy loops, see
Documentation/process/volatile-considered-harmful.rst. This can not
only lower CPU power consumption or yield to a hyperthreaded twin
processor, but also allows an architecture to mitigate hardware issues
(e.g. ARM Erratum 754327 for Cortex-A9 prior to r2p0) in the
architecture-specific cpu_relax() implementation.
In addition, cpu_relax() is also a compiler barrier. It is not
immediately obvious that the @op argument "function" will result in an
actual function call (e.g. in case of inlining).
Where a function call is a C sequence point, this is lost on inlining.
Therefore, with agressive enough optimization it might be possible for
the compiler to hoist the:
(val) = op(args);
"load" out of the loop because it doesn't see the value changing. The
addition of cpu_relax() would inhibit this.
As the iopoll helpers lack calls to cpu_relax(), people are sometimes
reluctant to use them, and may fall back to open-coded polling loops
(including cpu_relax() calls) instead.
Fix this by adding calls to cpu_relax() to the iopoll helpers:
- For the non-atomic case, it is sufficient to call cpu_relax() in
case of a zero sleep-between-reads value, as a call to
usleep_range() is a safe barrier otherwise. However, it doesn't
hurt to add the call regardless, for simplicity, and for similarity
with the atomic case below.
- For the atomic case, cpu_relax() must be called regardless of the
sleep-between-reads value, as there is no guarantee all
architecture-specific implementations of udelay() handle this.
Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
Acked-by: Peter Zijlstra (Intel) <peterz@...radead.org>
Acked-by: Arnd Bergmann <arnd@...db.de>
Reviewed-by: Tony Lindgren <tony@...mide.com>
Reviewed-by: Ulf Hansson <ulf.hansson@...aro.org>
Link: https://lore.kernel.org/r/45c87bec3397fdd704376807f0eec5cc71be440f.1685692810.git.geert+renesas@glider.be
Signed-off-by: Sasha Levin <sashal@...nel.org>
---
include/linux/iopoll.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
index 2c8860e406bd8..0417360a6db9b 100644
--- a/include/linux/iopoll.h
+++ b/include/linux/iopoll.h
@@ -53,6 +53,7 @@
} \
if (__sleep_us) \
usleep_range((__sleep_us >> 2) + 1, __sleep_us); \
+ cpu_relax(); \
} \
(cond) ? 0 : -ETIMEDOUT; \
})
@@ -95,6 +96,7 @@
} \
if (__delay_us) \
udelay(__delay_us); \
+ cpu_relax(); \
} \
(cond) ? 0 : -ETIMEDOUT; \
})
--
2.39.2
Powered by blists - more mailing lists