[v3,0/7] iopoll: Busy loop and timeout improvements + conversions

Message

Geert Uytterhoeven June 2, 2023, 8:50 a.m. UTC

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 be of interest
     to the authors of [6-8].

The first two patches were sent before, and already received some acks
and reviews.  I plan to queue these in an immutable and tagged branch
after the weekend, for consumption by myself, and by other interested
parties.

The last five patches are new, and convert several Renesas-specific
drivers from open-coded loops to the fixed read*_poll_timeout_atomic()
helpers:
  - I plan to queue the clk and soc patches in renesas-devel resp.
    renesas-clk for v6.5,
  - The IOMMU patch can wait for v6.6, unless the IOMMU maintainers
    already want to merge the immutable branch, too.

Thanks for your comments!

Changes compared to v2[9]:
  - Add Acked-by, Reviewed-by,
  - Add comment about not using timekeeping, and its impact,
  - Add conversion patches 3-7.

Changes compared to v1[10]:
  - 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=[e-mail]
[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/all/[e-mail]

[10] https://lore.kernel.org/r/8d492[e-mail]/

Geert Uytterhoeven (7):
  iopoll: Call cpu_relax() in busy loops
  iopoll: Do not use timekeeping in read_poll_timeout_atomic()
  clk: renesas: cpg-mssr: Convert to readl_poll_timeout_atomic()
  clk: renesas: mstp: Convert to readl_poll_timeout_atomic()
  clk: renesas: rzg2l: Convert to readl_poll_timeout_atomic()
  soc: renesas: rmobile-sysc: Convert to readl_poll_timeout_atomic()
  iommu/ipmmu-vmsa: Convert to read_poll_timeout_atomic()

 drivers/clk/renesas/clk-mstp.c         | 18 ++++++---------
 drivers/clk/renesas/renesas-cpg-mssr.c | 31 +++++++++-----------------
 drivers/clk/renesas/rzg2l-cpg.c        | 16 +++++--------
 drivers/iommu/ipmmu-vmsa.c             | 15 +++++--------
 drivers/soc/renesas/rmobile-sysc.c     | 31 +++++++++-----------------
 include/linux/iopoll.h                 | 24 +++++++++++++++-----
 6 files changed, 58 insertions(+), 77 deletions(-)

Comments

Geert Uytterhoeven June 5, 2023, 3:08 p.m. UTC | #1

On Fri, Jun 2, 2023 at 10:51 AM Geert Uytterhoeven
<[e-mail]> wrote:
> 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 be of interest
>      to the authors of [6-8].
>
> The first two patches were sent before, and already received some acks
> and reviews.  I plan to queue these in an immutable and tagged branch
> after the weekend, for consumption by myself, and by other interested
> parties.

FTR...
The following changes since commit ac9a78681b921877518763ba0e89202254349d1b:

  Linux 6.4-rc1 (2023-05-07 13:34:35 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
tags/iopoll-busy-loop-timeout-tag

for you to fetch changes up to 7349a69cf3125e92d48e442d9f400ba446fa314f:

  iopoll: Do not use timekeeping in read_poll_timeout_atomic()
(2023-06-05 15:35:27 +0200)

----------------------------------------------------------------
iopoll: Busy loop and timeout improvements

----------------------------------------------------------------
Geert Uytterhoeven (2):
      iopoll: Call cpu_relax() in busy loops
      iopoll: Do not use timekeeping in read_poll_timeout_atomic()

 include/linux/iopoll.h | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

Gr{oetje,eeting}s,

                        Geert