From patchwork Fri Jun 2 08:50:37 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Geert Uytterhoeven X-Patchwork-Id: 13264897 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B0186C7EE24 for ; Fri, 2 Jun 2023 08:51:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=wCHjW1Mse0XeY5xJd3CNcv2VjZzA10suEUMnGRU5/yU=; b=tEtro6CjnOwCoG Vxa9OMmO10KV0FvPu9eIrx6SCZe/lh25SrhzU4L28m/Y3RC0UvjgLtMBIqq4xngkf7YxZ6bgg/5au qq1LM0FBaEXHJC5g9M5Apfwb1e3SPWVID5hIoR+0KbR7iFM+5vwIteGgJGjv51oddFCmz5Byn2WQJ QGAQdoGCiZYsFo0r5xa8W4pmyb4yP63ZOe097M6WuVz27vS1RtZlkGRt1AS1gq8h7hesT957pwF47 nUBb4Zf5H4MiEG8OWGegbOQ6WbaqWeOM1jcovM4eS+Ghbi0Fb/xKzw0hIjrUS8VV4tL0MI34QO5oW 1UOMNeo+67gRM8ggeR/Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q50VJ-006EKO-2A; Fri, 02 Jun 2023 08:51:25 +0000 Received: from xavier.telenet-ops.be ([2a02:1800:120:4::f00:14]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q50VC-006ECl-32 for linux-arm-kernel@lists.infradead.org; Fri, 02 Jun 2023 08:51:22 +0000 Received: from ramsan.of.borg ([IPv6:2a02:1810:ac12:ed30:158c:2ccf:1f70:e136]) by xavier.telenet-ops.be with bizsmtp id 48qo2A0041tRZS8018qoDF; Fri, 02 Jun 2023 10:51:12 +0200 Received: from rox.of.borg ([192.168.97.57]) by ramsan.of.borg with esmtp (Exim 4.95) (envelope-from ) id 1q50UO-00BhYB-SP; Fri, 02 Jun 2023 10:50:48 +0200 Received: from geert by rox.of.borg with local (Exim 4.95) (envelope-from ) id 1q50Uh-00APxV-Vr; Fri, 02 Jun 2023 10:50:47 +0200 From: Geert Uytterhoeven To: Michael Turquette , Stephen Boyd , Yoshihiro Shimoda , Magnus Damm , Joerg Roedel , Robin Murphy Cc: Tomasz Figa , Sylwester Nawrocki , Will Deacon , Arnd Bergmann , Wolfram Sang , Dejin Zheng , Kai-Heng Feng , Nicholas Piggin , Heiko Carstens , Peter Zijlstra , Russell King , John Stultz , Thomas Gleixner , Tony Lindgren , Krzysztof Kozlowski , Tero Kristo , Ulf Hansson , "Rafael J . Wysocki" , Vincent Guittot , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-renesas-soc@vger.kernel.org, linux-pm@vger.kernel.org, iommu@lists.linux.dev, linux-kernel@vger.kernel.org, Geert Uytterhoeven Subject: [PATCH v3 2/7] iopoll: Do not use timekeeping in read_poll_timeout_atomic() Date: Fri, 2 Jun 2023 10:50:37 +0200 Message-Id: <3d2a2f4e553489392d871108797c3be08f88300b.1685692810.git.geert+renesas@glider.be> X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230602_015119_125508_E1DCC629 X-CRM114-Status: GOOD ( 20.90 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org read_poll_timeout_atomic() uses ktime_get() to implement the timeout feature, just like its non-atomic counterpart. However, there are several issues with this, due to its use in atomic contexts: 1. When called in the s2ram path (as typically done by clock or PM domain drivers), timekeeping may be suspended, triggering the WARN_ON(timekeeping_suspended) in ktime_get(): WARNING: CPU: 0 PID: 654 at kernel/time/timekeeping.c:843 ktime_get+0x28/0x78 Calling ktime_get_mono_fast_ns() instead of ktime_get() would get rid of that warning. However, that would break timeout handling, as (at least on systems with an ARM architectured timer), the time returned by ktime_get_mono_fast_ns() does not advance while timekeeping is suspended. Interestingly, (on the same ARM systems) the time returned by ktime_get() does advance while timekeeping is suspended, despite the warning. 2. Depending on the actual clock source, and especially before a high-resolution clocksource (e.g. the ARM architectured timer) becomes available, time may not advance in atomic contexts, thus breaking timeout handling. Fix this by abandoning the idea that one can rely on timekeeping to implement timeout handling in all atomic contexts, and switch from a global time-based to a locally-estimated timeout handling. In most (all?) cases the timeout condition is exceptional and an error condition, hence any additional delays due to underestimating wall clock time are irrelevant. Signed-off-by: Geert Uytterhoeven Acked-by: Arnd Bergmann Reviewed-by: Tony Lindgren Reviewed-by: Ulf Hansson --- The first issue was seen with the rcar-sysc driver in the BSP, as the BSP contains modifications to the resume sequence of various PM Domains. v3: - Add Acked-by, Reviewed-by, - Add comment about not using timekeeping, and its impact, v2: - New. --- include/linux/iopoll.h | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h index 0417360a6db9b0d6..19a7b00baff43595 100644 --- a/include/linux/iopoll.h +++ b/include/linux/iopoll.h @@ -74,6 +74,10 @@ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either * case, the last read value at @args is stored in @val. * + * This macro does not rely on timekeeping. Hence it is safe to call even when + * timekeeping is suspended, at the expense of an underestimation of wall clock + * time, which is rather minimal with a non-zero delay_us. + * * When available, you'll probably want to use one of the specialized * macros defined below rather than this macro directly. */ @@ -81,22 +85,30 @@ delay_before_read, args...) \ ({ \ u64 __timeout_us = (timeout_us); \ + s64 __left_ns = __timeout_us * NSEC_PER_USEC; \ unsigned long __delay_us = (delay_us); \ - ktime_t __timeout = ktime_add_us(ktime_get(), __timeout_us); \ - if (delay_before_read && __delay_us) \ + u64 __delay_ns = __delay_us * NSEC_PER_USEC; \ + if (delay_before_read && __delay_us) { \ udelay(__delay_us); \ + if (__timeout_us) \ + __left_ns -= __delay_ns; \ + } \ for (;;) { \ (val) = op(args); \ if (cond) \ break; \ - if (__timeout_us && \ - ktime_compare(ktime_get(), __timeout) > 0) { \ + if (__timeout_us && __left_ns < 0) { \ (val) = op(args); \ break; \ } \ - if (__delay_us) \ + if (__delay_us) { \ udelay(__delay_us); \ + if (__timeout_us) \ + __left_ns -= __delay_ns; \ + } \ cpu_relax(); \ + if (__timeout_us) \ + __left_ns--; \ } \ (cond) ? 0 : -ETIMEDOUT; \ })