Message ID | 20230623171232.892937-1-bigeasy@linutronix.de |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp5936462vqr; Fri, 23 Jun 2023 10:35:23 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4QxdM0kb7FmbBmBgWpRpZVAfMIgwd1y7hMByuTFlty971Hw2L0XJku37rnWHele5dHHl08 X-Received: by 2002:a05:6359:67a3:b0:132:e3de:cbe3 with SMTP id sq35-20020a05635967a300b00132e3decbe3mr1867811rwb.31.1687541722899; Fri, 23 Jun 2023 10:35:22 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687541722; cv=none; d=google.com; s=arc-20160816; b=O9eeFuvbn/z2OFRPbirk2pXAEPan7Mgvr1a5Xi6PXNnEl+4vH8vgW6KMWxkAIXJ1aX +uRxWtpOOr7VXXzx/kzGLNTdoc1sCnY4CHn7gl4/TQnpXgdf/P2G2yc4MGemUt9FaMvZ m2M93o8asZk6gYGspYjVsskpqFlxPcLYyQRlrBfTdmW6BZ27BM7F4XdVriFXtA8e8VGs AQG1wscUxAEzoiHkQhYnbqN8aMdLxbh+3Pl34i6wu9fX8He0pZRIFKRD2zDxdAJPHRwL ck392svKZqmn4iXM16zeRBra+mCg10v+WiLhu+CqVD1HbgYVmqR9jJErppec7XrKlAzX I39A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:dkim-signature:dkim-signature:from; bh=x7mPbDQnnkVRw/X9cygYC/TR6vXzcXX4SBLDkMXnm/E=; b=B2k9ExRijz3cfbFGbAEYmo8r2nWq1MXhP9na+C3mUfN/mMJH4M4P2uUhKlXUwLCYTi ZQ1N4bxUNWTupB/I2g/a8XE9FHJhnVIAxNVp8VQbJJ6aHAfkH12xopzkhyBFL+MkbLv3 V1w7IxPqPGLPYkmRuHv2j+oPe3trv/E6u6t08PE9vge4dtZ/ZbL5WzxtDxo+ZLV4XKGo KPiARh+eSavuOJs1GDfGrn9tlLVkLSd++357qG7dna8yiSen1u++wmdSErVW4ZNRlAvC mvnX8UdAPWBUmiT8E9t+4W4LFFmjGV38/zMc1LBUxbvh6rtdE+bqGrhYVuH8jrC/g9o7 QVDQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=so0dYC6Z; dkim=neutral (no key) header.i=@linutronix.de header.s=2020e header.b=0IrnBgyy; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id m24-20020a637d58000000b00553b5116cefsi1333070pgn.16.2023.06.23.10.35.09; Fri, 23 Jun 2023 10:35:22 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=so0dYC6Z; dkim=neutral (no key) header.i=@linutronix.de header.s=2020e header.b=0IrnBgyy; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231547AbjFWRMu (ORCPT <rfc822;duw91626@gmail.com> + 99 others); Fri, 23 Jun 2023 13:12:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42626 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231228AbjFWRMq (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 23 Jun 2023 13:12:46 -0400 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F13411981 for <linux-kernel@vger.kernel.org>; Fri, 23 Jun 2023 10:12:43 -0700 (PDT) From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1687540361; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=x7mPbDQnnkVRw/X9cygYC/TR6vXzcXX4SBLDkMXnm/E=; b=so0dYC6ZPkrddiaQFe/ZvLI9mhhRRMEG7vBD1rAvbekbtvx5yuMcBOpxS6E+JZLFmVAUhq gBqThCmPJjiu8n0biHp+SNkyULF2cICRdg0GIZPPcUAZO1nuGQjYdB+vF3K0VleMfiVc/t 6RCZhRgagZf8dQC7Dw/aOXrqBnPhG9h16MYwLrXq2OL+tc5A805LmHnb+zADkFD4tpyXVl 49lMKBuUpfLyce3126ZklbYFyfAOTs0OTcBMC/bXJ3R3ZKUpA/Z8jF3goVWGjMSnWx+jr3 VU5d6Tc51FKiUOwk62t4wHxwXp82ixQF+jxM2NYCu9H38E7ApMZJ47YvXoc5DA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1687540361; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=x7mPbDQnnkVRw/X9cygYC/TR6vXzcXX4SBLDkMXnm/E=; b=0IrnBgyyvuh8AQrCOb9IMtBRgkE4iusIj1B0LImfMddij1r9MqAIJZ5f7KjdAfQYyg/3Vj vUnfwaZhxmLFF/CA== To: linux-mm@kvack.org, linux-kernel@vger.kernel.org Cc: "Luis Claudio R. Goncalves" <lgoncalv@redhat.com>, Andrew Morton <akpm@linux-foundation.org>, Boqun Feng <boqun.feng@gmail.com>, Ingo Molnar <mingo@redhat.com>, John Ogness <john.ogness@linutronix.de>, Mel Gorman <mgorman@techsingularity.net>, Michal Hocko <mhocko@suse.com>, Peter Zijlstra <peterz@infradead.org>, Petr Mladek <pmladek@suse.com>, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>, Thomas Gleixner <tglx@linutronix.de>, Waiman Long <longman@redhat.co>, Will Deacon <will@kernel.org> Subject: [PATCH v2 0/2] seqlock,mm: lockdep annotation + write_seqlock_irqsave() Date: Fri, 23 Jun 2023 19:12:30 +0200 Message-Id: <20230623171232.892937-1-bigeasy@linutronix.de> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1769515749697916464?= X-GMAIL-MSGID: =?utf-8?q?1769515749697916464?= |
Series |
seqlock,mm: lockdep annotation + write_seqlock_irqsave()
|
|
Message
Sebastian Andrzej Siewior
June 23, 2023, 5:12 p.m. UTC
Hi, this has been a single patch (2/2) but then it was pointed out that the lockdep annotation in seqlock needs to be adjusted to fully close the printk window so that there is no printing after the seq-lock has been acquired and before printk_deferred_enter() takes effect. I'm sending both patches in this series so both sides (locking and mm) are aware of the situation. I hope that both patches can be applied independently via their subsystem tree (the lockdep splat + deadlock is low risk). The original thread starts at https://lore.kernel.org/20230621104034.HT6QnNkQ@linutronix.de Sebastian
Comments
On 2023/06/24 2:12, Sebastian Andrzej Siewior wrote: > Hi, > > this has been a single patch (2/2) but then it was pointed out that the > lockdep annotation in seqlock needs to be adjusted to fully close the > printk window so that there is no printing after the seq-lock has been > acquired and before printk_deferred_enter() takes effect. > > I'm sending both patches in this series so both sides (locking and mm) > are aware of the situation. > I hope that both patches can be applied independently via their subsystem > tree (the lockdep splat + deadlock is low risk). > > The original thread starts at > https://lore.kernel.org/20230621104034.HT6QnNkQ@linutronix.de > > Sebastian The original thread is too long to read. Below is a full summary for locking maintainers to accept [PATCH 1/2]. When commit 1ca7d67cf5d5 ("seqcount: Add lockdep functionality to seqcount/seqlock structures") added seqcount_acquire(&s->dep_map) check to write_seqcount_begin_nested() and seqcount_release(&s->dep_map) check to write_seqcount_end(), the ordering of updating s->sequence and doing lockdep annotation was not important. But since commit 3d36424b3b58 ("mm/page_alloc: fix race condition between build_all_zonelists and page allocation") started calling read_seqbegin(&zonelist_update_seq)/read_seqretry(&zonelist_update_seq) from kmalloc(GFP_ATOMIC) path, commit 1007843a9190 ("mm/page_alloc: fix potential deadlock on zonelist_update_seq seqlock") tried to close the race window using __build_all_zonelists() { + local_irq_save(flags); + printk_deferred_enter(); write_seqlock(&zonelist_update_seq); (...snipped...) write_sequnlock(&zonelist_update_seq); + printk_deferred_exit(); + local_irq_restore(flags); } pattern. The reason behind this ordering was to satisfy "printk_deferred_enter() depends on local IRQs being disabled" and make sure that "no synchronous printk() (for whatever reasons, not only printk() from build_zonelists() from __build_all_zonelists(), but also including printk() from lockdep, soft-lockup, KCSAN etc.) happens between write_seqlock() and write_sequnlock() . However, Sebastian Andrzej Siewior mentioned that this ordering is problematic if CONFIG_PREEMPT_RT=y, for disabling local IRQs conflicts with "spin_lock(&zonelist_update_seq.lock) from write_seqlock(&zonelist_update_seq) needs to be able to sleep", and Sebastian is proposing __build_all_zonelists() { - local_irq_save(flags); - printk_deferred_enter(); - write_seqlock(&zonelist_update_seq); + write_seqlock_irqsave(&zonelist_update_seq, flags); + printk_deferred_enter(); (...snipped...) + printk_deferred_exit(); + write_sequnlock_irqrestore(&zonelist_update_seq, flags); - write_sequnlock(&zonelist_update_seq); - printk_deferred_exit(); - local_irq_restore(flags); } change as [PATCH 2/2]. Since write_seqlock_irqsave() becomes write_seqlock() if CONFIG_PREEMPT_RT=y, this change can solve the conflict. In order to accept this proposal, we need to make sure that no synchronous printk() happens between write_seqlock_irqsave(&zonelist_update_seq, flags) made zonelist_update_seq.seqcount odd and printk_deferred_enter() takes effect and no synchronous printk() happens between printk_deferred_exit() took effect and write_sequnlock_irqrestore(&zonelist_update_seq, flags) makes zonelist_update_seq.seqcount even , and Sebastian is proposing static inline void do_write_seqcount_begin_nested(seqcount_t *s, int subclass) { - do_raw_write_seqcount_begin(s); seqcount_acquire(&s->dep_map, subclass, 0, _RET_IP_); + do_raw_write_seqcount_begin(s); } static inline void do_write_seqcount_end(seqcount_t *s) { - seqcount_release(&s->dep_map, _RET_IP_); do_raw_write_seqcount_end(s); + seqcount_release(&s->dep_map, _RET_IP_); } as [PATCH 1/2]. With [PATCH 1/2] and [PATCH 2/2], possibility of synchronous printk() changes like below. __build_all_zonelists() { write_seqlock_irqsave(&zonelist_update_seq, flags) { __write_seqlock_irqsave(&zonelist_update_seq) { spin_lock_irqsave(&zonelist_update_seq.lock, flags); // local IRQs disabled = synchronous printk() from IRQs is disabled here do_write_seqcount_begin(&zonelist_update_seq.seqcount.seqcount) { do_write_seqcount_begin_nested(&zonelist_update_seq.seqcount.seqcount, 0) { seqcount_acquire(&zonelist_update_seq.seqcount.seqcount.dep_map, 0, 0, _RET_IP_); // synchronous printk() from lockdep might happen here do_raw_write_seqcount_begin(&zonelist_update_seq.seqcount.seqcount) { seqcount_acquire(&zonelist_update_seq.seqcount.seqcount.dep_map, 0, 0, _RET_IP_); // zonelist_update_seq.seqcount.seqcount.sequence is guaranteed to be even = kmalloc(GFP_ATOMIC) with port lock held is safe = synchronous printk() is safe kcsan_nestable_atomic_begin(); // KCSAN is diabled = synchronous printk() from KCSAN is disabled here zonelist_update_seq.seqcount.seqcount.sequence++; // zonelist_update_seq.seqcount.seqcount.sequence is now odd = kmalloc(GFP_ATOMIC) with port lock held is not safe = synchronous printk() is not safe } } } } } printk_deferred_enter(); // synchronous printk() from whatever reason is disabled here (...snipped...) printk_deferred_exit(); // synchronous printk() from whatever reason is enabled here write_sequnlock_irqrestore(&zonelist_update_seq, flags) { do_write_seqcount_end(&zonelist_update_seq.seqcount.seqcount) { do_raw_write_seqcount_end(&zonelist_update_seq.seqcount.seqcount) { zonelist_update_seq.seqcount.seqcount.sequence++; // zonelist_update_seq.seqcount.seqcount.sequence is now even = kmalloc(GFP_ATOMIC) with port lock held is safe = synchronous printk() is safe kcsan_nestable_atomic_end(); // KCSAN is enabled = synchronous printk() from KCSAN is enabled here } seqcount_release(&zonelist_update_seq.seqcount.seqcount.dep_map, _RET_IP_); // synchronous printk() from lockdep might happen here } spin_unlock_irqrestore(&zonelist_update_seq.lock, flags); // local IRQs enabled = synchronous printk() from IRQs is enabled here } }