From patchwork Tue Aug 16 07:03:11 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hector Martin X-Patchwork-Id: 12944374 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 5E309C2BB41 for ; Tue, 16 Aug 2022 07:05:09 +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: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:In-Reply-To:References: List-Owner; bh=A91HuE/cQUBS6lrSQSZbRhuKUHJwF/yfWp1SI6p5REw=; b=z2n1lSw7bLsWSK jJApcZLpNddgnlQqXIkT+zHsdO4fTDj5r4truGwG7cvOCKe9LvjgnvS5BE1Rj1rmJEB0SGRKHVCm1 8CqOxQM+gOuViWH33zvEQ7ukJwlG8RUOdLALbb2NrKTi1Yfns8+BuChA5ar3SogZ4385B+lYtzwGs zxv7iNkf0th/7CEIatbObIk2sjwMFTOSVSk5erBNoDcIlaVUTNddt23xzOMzH8xw4XTdcWoEgGQF9 PryF5Kbzd3M+Ek+qSjk/5B0xZH5xKkfA9bLUgjiBi7uQDFJrnmFtAAt+MUUXhgRF/6U7upkUN0mb6 ItXAKKOIVk3vbBHYCSFA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNqbw-00EFbh-Ud; Tue, 16 Aug 2022 07:03:37 +0000 Received: from marcansoft.com ([212.63.210.85] helo=mail.marcansoft.com) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNqbt-00EF8a-Aj for linux-arm-kernel@lists.infradead.org; Tue, 16 Aug 2022 07:03:35 +0000 Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: hector@marcansoft.com) by mail.marcansoft.com (Postfix) with ESMTPSA id E22484195A; Tue, 16 Aug 2022 07:03:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=marcan.st; s=default; t=1660633407; bh=kUwKJCbfhFuqlbG5x8uMKMwiqOIdhALURpt/vstQk4s=; h=From:To:Cc:Subject:Date; b=oZugG863BdtFggJGMD5WWyU3O9TQMqUzQI0Q0opxDi9IBIIqfCX8bmf5N2YMyCiRZ 2keRbaZtAzfI4XvjVQnFFtSCOUOYOXWaZyrqqG+ut2RkHO3zGaiMgjnKx2yzNSXjrY bL0tH7UpAmYsOIYiNTgRd5LkWG2+SpzNXCpk5BfDO7izM9Wj4UYSLYoFzMD3cpXTZ8 ItJ56cokotDvC/wfZXdCqNw5B15pSrAFIdJqh7kKil/FvjbCbQlT+5659azbnI/9q9 FIqW/hbHLrCYNw8X0r/KatC+tbn1Qx3TCxeSt4gV/vPUZv3KtMdasKqGbmccXcukE5 vd9SMut5DVlkQ== From: Hector Martin To: Will Deacon , Peter Zijlstra , Arnd Bergmann , Ingo Molnar Cc: Alan Stern , Andrea Parri , Boqun Feng , Nicholas Piggin , David Howells , Jade Alglave , Luc Maranget , "Paul E. McKenney" , Akira Yokosawa , Daniel Lustig , Joel Fernandes , Mark Rutland , Jonathan Corbet , Tejun Heo , jirislaby@kernel.org, Marc Zyngier , Catalin Marinas , Oliver Neukum , Linus Torvalds , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Asahi Linux , Hector Martin , stable@vger.kernel.org Subject: [PATCH] locking/atomic: Make test_and_*_bit() ordered on failure Date: Tue, 16 Aug 2022 16:03:11 +0900 Message-Id: <20220816070311.89186-1-marcan@marcan.st> X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220816_000333_798409_191C5F78 X-CRM114-Status: GOOD ( 16.66 ) 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 These operations are documented as always ordered in include/asm-generic/bitops/instrumented-atomic.h, and producer-consumer type use cases where one side needs to ensure a flag is left pending after some shared data was updated rely on this ordering, even in the failure case. This is the case with the workqueue code, which currently suffers from a reproducible ordering violation on Apple M1 platforms (which are notoriously out-of-order) that ends up causing the TTY layer to fail to deliver data to userspace properly under the right conditions. This change fixes that bug. Change the documentation to restrict the "no order on failure" story to the _lock() variant (for which it makes sense), and remove the early-exit from the generic implementation, which is what causes the missing barrier semantics in that case. Without this, the remaining atomic op is fully ordered (including on ARM64 LSE, as of recent versions of the architecture spec). Suggested-by: Linus Torvalds Cc: stable@vger.kernel.org Fixes: e986a0d6cb36 ("locking/atomics, asm-generic/bitops/atomic.h: Rewrite using atomic_*() APIs") Fixes: 61e02392d3c7 ("locking/atomic/bitops: Document and clarify ordering semantics for failed test_and_{}_bit()") Signed-off-by: Hector Martin Reviewed-by: Arnd Bergmann Acked-by: Will Deacon --- Documentation/atomic_bitops.txt | 2 +- include/asm-generic/bitops/atomic.h | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/Documentation/atomic_bitops.txt b/Documentation/atomic_bitops.txt index 093cdaefdb37..d8b101c97031 100644 --- a/Documentation/atomic_bitops.txt +++ b/Documentation/atomic_bitops.txt @@ -59,7 +59,7 @@ Like with atomic_t, the rule of thumb is: - RMW operations that have a return value are fully ordered. - RMW operations that are conditional are unordered on FAILURE, - otherwise the above rules apply. In the case of test_and_{}_bit() operations, + otherwise the above rules apply. In the case of test_and_set_bit_lock(), if the bit in memory is unchanged by the operation then it is deemed to have failed. diff --git a/include/asm-generic/bitops/atomic.h b/include/asm-generic/bitops/atomic.h index 3096f086b5a3..71ab4ba9c25d 100644 --- a/include/asm-generic/bitops/atomic.h +++ b/include/asm-generic/bitops/atomic.h @@ -39,9 +39,6 @@ arch_test_and_set_bit(unsigned int nr, volatile unsigned long *p) unsigned long mask = BIT_MASK(nr); p += BIT_WORD(nr); - if (READ_ONCE(*p) & mask) - return 1; - old = arch_atomic_long_fetch_or(mask, (atomic_long_t *)p); return !!(old & mask); } @@ -53,9 +50,6 @@ arch_test_and_clear_bit(unsigned int nr, volatile unsigned long *p) unsigned long mask = BIT_MASK(nr); p += BIT_WORD(nr); - if (!(READ_ONCE(*p) & mask)) - return 0; - old = arch_atomic_long_fetch_andnot(mask, (atomic_long_t *)p); return !!(old & mask); }