From mboxrd@z Thu Jan 1 00:00:00 1970 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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 48F27EB64DA for ; Sun, 25 Jun 2023 02:27:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B47478D0002; Sat, 24 Jun 2023 22:27:41 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AF7808D0001; Sat, 24 Jun 2023 22:27:41 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9BF2A8D0002; Sat, 24 Jun 2023 22:27:41 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 8CE5D8D0001 for ; Sat, 24 Jun 2023 22:27:41 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 574D84072D for ; Sun, 25 Jun 2023 02:27:41 +0000 (UTC) X-FDA: 80939684322.18.69EA0DB Received: from www262.sakura.ne.jp (www262.sakura.ne.jp [202.181.97.72]) by imf12.hostedemail.com (Postfix) with ESMTP id 7AFBE40006 for ; Sun, 25 Jun 2023 02:27:38 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=none; dmarc=none; spf=none (imf12.hostedemail.com: domain of penguin-kernel@I-love.SAKURA.ne.jp has no SPF policy when checking 202.181.97.72) smtp.mailfrom=penguin-kernel@I-love.SAKURA.ne.jp ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1687660059; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=45xarGm8Dv2oXS+P0eRCwqM+2XR7E7V4iBKNMvRtr7s=; b=5sNGkWb6YqraAsghU4yht7KDVYReRMoskoXw3acDI5oQZd4Vx5aTd8jBZQrw54PVfQGUsM DlUzMpSrfddHqiaEN55HtN9uUR6lq+2iQxmY+juEbWasTb+XzPYBgnbTV8PdlSRZdKf8ce 4BnNyxpGOE8iYEnwvu8T3nLR59z1Djk= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=none; dmarc=none; spf=none (imf12.hostedemail.com: domain of penguin-kernel@I-love.SAKURA.ne.jp has no SPF policy when checking 202.181.97.72) smtp.mailfrom=penguin-kernel@I-love.SAKURA.ne.jp ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687660059; a=rsa-sha256; cv=none; b=wmHNxnHK8QAZoZ9JZLJn66pVjuuCZWMuvfkw5h2YcE3skpbQ4uYPuEsJW7vKLS6T9nuTMG AnFArto6N+TtF/IFlLKEUncAyuJLAJyJQIrw1sKphy/P06O0YHJ7YIzsuGL69aWZgCnP1Y isuJZ7oPug/3WI9R0Fe4qrfmWsYHDss= Received: from fsav411.sakura.ne.jp (fsav411.sakura.ne.jp [133.242.250.110]) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTP id 35P2RIF6015937; Sun, 25 Jun 2023 11:27:18 +0900 (JST) (envelope-from penguin-kernel@I-love.SAKURA.ne.jp) Received: from www262.sakura.ne.jp (202.181.97.72) by fsav411.sakura.ne.jp (F-Secure/fsigk_smtp/550/fsav411.sakura.ne.jp); Sun, 25 Jun 2023 11:27:17 +0900 (JST) X-Virus-Status: clean(F-Secure/fsigk_smtp/550/fsav411.sakura.ne.jp) Received: from [192.168.1.6] (M106072142033.v4.enabler.ne.jp [106.72.142.33]) (authenticated bits=0) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTPSA id 35P2RHIb015934 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NO); Sun, 25 Jun 2023 11:27:17 +0900 (JST) (envelope-from penguin-kernel@I-love.SAKURA.ne.jp) Message-ID: <3120e8b4-a3e9-3612-f232-b89afe56532f@I-love.SAKURA.ne.jp> Date: Sun, 25 Jun 2023 11:27:18 +0900 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH v2 0/2] seqlock,mm: lockdep annotation + write_seqlock_irqsave() Content-Language: en-US To: Sebastian Andrzej Siewior , linux-mm@kvack.org, linux-kernel@vger.kernel.org Cc: "Luis Claudio R. Goncalves" , Andrew Morton , Boqun Feng , Ingo Molnar , John Ogness , Mel Gorman , Michal Hocko , Peter Zijlstra , Petr Mladek , Thomas Gleixner , Waiman Long , Will Deacon References: <20230623171232.892937-1-bigeasy@linutronix.de> From: Tetsuo Handa In-Reply-To: <20230623171232.892937-1-bigeasy@linutronix.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 7AFBE40006 X-Stat-Signature: sixpjb3q66kymmghuq14ptx6p81h4xgi X-Rspam-User: X-HE-Tag: 1687660058-290098 X-HE-Meta: U2FsdGVkX1/uqR/hC11M1EI3ib3/5ruqZuFkK+g/gGdSnesSUvMMIK+ol9ElBwAz0BS7wQjiZ6bR/z3Yjzls3BPLHPbSNtAWcM9luxAa0+nD41tyd5oy6yux7fyvnv6W+ZEOG5p6C4POMerLP74PGnYC59tsjlZU5nvHCUwpaHK+mmCFDLwiTK7FNrqp9AeWpZQt9rdqh5OW/T1rItw7o0a0I/SdgKmecVC1NBfweS5KXSXo01vfT3Y6ifnvH16c2bw3f6XN8n7q5Vm/uN1CEv3DMQrcZkxq+1iRIH2zLN/r6Ml9lIdd33YApviQXoWVd0/26g4+g5ML44w18vrPowkFTM0ekwLctfh+WtY1QUAeI/hm99U7ADsgj17MZ/SqyyaGBC2dJzFFOfn8f3sLQHAi6DwpIP4Gv2jgkAZwhbkOzBxbXcclAB3b297eroCCzYbSUgjFOlb5TPhhGAqSdH0fPHFl+X3Sayqbep+qcly8k9SbmIpb3pDy+cRcirNrADK6nnf4CyO3KGJZLrfdrBk5gA002B1OmtJG8KRmb48PJy20NWjJJ/okL2WOhYPpuQgdRQFr8Q9fddyk+npIOMss8Arr4CZUcFu6VBA8YxF89qfkrygtDWKRnrAM/e06CsSMI/2XR/mkK1Yq5wPpTuAzb806qoLu76F8sIvspdzatmFjUoBC+earaoJJF3DiSLwiepKkI4vwby6izLVRK5FyTB27VN2akE9gF8+QBeDyBMM8PxGBOq+L1wDVfP1oOsVmdBFWWjedk1lw2oAZyXU0v0a2Ir4Y5jMjKlL8T9Lp+aRzdbxu1jTgshG2KbC1ImXaw1eDie156A/OGmIcf9RPX+OPuiZi8V769T9u7SKh7LbRE9k7Tlvu0BiPHSX8oWKn/q34WP2uFb5PNza2mKyVBVfnA4C/3UEDswT0DmriXlsCizA3Rc3NjSsV3o2cYVzejUppClZvBj0TVY/ jh21tf/h f/WnUTFazpWvcUsyYabN5csN3fdiWu5GD3MagUWegVe+n+Ip9Vl7esjVzMZsqRmhQk7vfXxf/s1/QyknjuYwEyAgP5zf3hODOYgnxfQ01jQv7rncgq2o/1dAV+HYnxlWSt5ZD705YqvIJqlCahLy9V56+2tYOjhWyxLae2bctezmQ/WAvXf9cHQTbCByKtVy6lZyD31yy8aDKwtNU4Y1xyBlxGIR69fOiflIXNBwpluEQ35ZegoVuQwcb48O2QlsikRujLh/QWxCBXyxQS/UsM7pMZMsVn1MkpufzKCQZ0cIFx3a90afufBw6pKZRkXbNGid3vOvoXg+QW9GQVDXs/+usIMaLSGLFBUj0cPbPfSLV8M4EcNXjJ5kX88+rglXj7QXX2UxRwmqItZs= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: 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 } }