| Message ID | cover.1683722688.git.geert+renesas@glider.be |
|---|---|
| Headers | show |
| Series | iopoll: Busy loop and timeout improvements | expand |
From: Tony Lindgren > Sent: 11 May 2023 07:49 > > * Geert Uytterhoeven <geert+renesas@glider.be> [230510 13:23]: > > 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. Don't you also need to call cond_resched() (at least some times). Otherwise the process can't be pre-empted and a RT process that last ran on that cpu will never be scheduled. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
From: Geert Uytterhoeven > Sent: 23 May 2023 08:30 > > Hi David, > > On Thu, May 11, 2023 at 12:49 PM David Laight <David.Laight@aculab.com> wrote: > > > * Geert Uytterhoeven <geert+renesas@glider.be> [230510 13:23]: > > > > 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. > > > > Don't you also need to call cond_resched() (at least some times). > > Otherwise the process can't be pre-empted and a RT process > > that last ran on that cpu will never be scheduled. > > According to [1], cond_resched() must be called at least once per few > tens of milliseconds. Hmmm.... tens of milliseconds is really much too long for RT threads. A limit nearer 1ms would be barely acceptable. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Hi all, When implementing a polling loop, a common review comment is to use one of the read*_poll_timeout*() helpers. Unfortunately this is not always possible, or might introduce subtle bugs. This patch series aims to improve these helpers, so they can gain wider use. 1. The first patch improves busy-looping behavior of both the atomic and non-atomic read*_poll_timeout*() helpers. The issue addressed by this patch was discussed before[1-2], but I am not aware of any patches moving forward. 2. The second patch fixes timeout handling of the atomic variants. Some of the issues addressed by this patch were mitigated in various places[3-5], and some of these findings may of interest to the authors of [6-8]. The first patch was sent before, and already received some acks, but I hadn't queued it yet as a depedency for more read*_poll_timeout*() use, because I ran into the issue fixed by the second patch. Changes compared to v1[9]: - Add Acked-by, - Add compiler barrier and inlining explanation (thanks, Peter!), - Add patch [2/2]. Thanks for your comments! [1] "Re: [PATCH 6/7] clk: renesas: rcar-gen3: Add custom clock for PLLs" https://lore.kernel.org/all/CAMuHMdWUEhs=nwP+a0vO2jOzkq-7FEOqcJ+SsxAGNXX1PQ2KMA@mail.gmail.com [2] "Re: [PATCH v2] clk: samsung: Prevent potential endless loop in the PLL set_rate ops" https://lore.kernel.org/all/20200811164628.GA7958@kozik-lap [3] b3e9431854e8f305 ("bus: ti-sysc: Fix timekeeping_suspended warning on resume") [4] 44a9e78f9242872c ("clk: samsung: Prevent potential endless loop in the PLL ops") [5] 3d8598fb9c5a7783 ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended") [6] bd40cbb0e3b37a4d ("PM: domains: Move genpd's time-accounting to ktime_get_mono_fast_ns()") [7] c155f6499f9797f2 ("PM-runtime: Switch accounting over to ktime_get_mono_fast_ns()") [8] 15efb47dc560849d ("PM-runtime: Fix deadlock with ktime_get()") [9] https://lore.kernel.org/r/8d492ee4a391bd089a01c218b0b4e05cf8ea593c.1674729407.git.geert+renesas@glider.be/ Geert Uytterhoeven (2): iopoll: Call cpu_relax() in busy loops iopoll: Do not use timekeeping in read_poll_timeout_atomic() include/linux/iopoll.h | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)