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 4B432EB64D7 for ; Mon, 26 Jun 2023 13:16:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DB62B8D0003; Mon, 26 Jun 2023 09:16:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D66268D0001; Mon, 26 Jun 2023 09:16:14 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C57058D0003; Mon, 26 Jun 2023 09:16:14 -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 B6CBC8D0001 for ; Mon, 26 Jun 2023 09:16:14 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 77A3814070F for ; Mon, 26 Jun 2023 13:16:14 +0000 (UTC) X-FDA: 80944947468.12.C403426 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf28.hostedemail.com (Postfix) with ESMTP id 59E6FC0014 for ; Mon, 26 Jun 2023 13:16:10 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=GNr7J8UB; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf28.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.29 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687785371; a=rsa-sha256; cv=none; b=563gWGOkI+2Q5kXaZEv8J7Fwze734yVNK590SUN0Zs+IJbB5uNfsnmog0Ogucf9nWhb2Pj S/xXQloq/FqKPX87PNKZNzjvF4F7D7gjp0UWCxj1HURUrhwXXC9EOQTN8GBQ1prUbBEVyU /ZnYcDZwFo1RSF+1VZNZCRHPOYI0FfI= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=GNr7J8UB; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf28.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.29 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1687785371; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Sxr7NE9BPbbo/jsxfHlnNeotXkyELOXzCC549+pDCjE=; b=O4FyADtNSY37U+pPmeLS57U1Wbjai131Yw0tkcRFOyyICTDCdKn2NYxQr9xckPvAo8bEcG mSx5BOwFAeDw3+8rQ78B8Q23leqBOm89iUD/PIyoQ0PqP5DfIC82eQEB2glu+tGhLi83NC XpC8VbQ0mymscx9xe/GNJH5p1f2CFM0= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id A34C51F896; Mon, 26 Jun 2023 13:16:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1687785369; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Sxr7NE9BPbbo/jsxfHlnNeotXkyELOXzCC549+pDCjE=; b=GNr7J8UB8CfYSClNe8OFHRnp2aV1oCBLQ3iCKYNOlVWam5EN4a56PnewlemRsTLEmDLMTU eKXGGJr1Nr/sKIXvGteqfkQdD7MeqCBGwVaqH85lA/mzcPGH8lCa688rGmtNHSGYYmT9Y7 0XFeTdKNDaKLLQXr1809WB35vlLoOMw= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 7CA0013483; Mon, 26 Jun 2023 13:16:09 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id gTcLHJmPmWR/YQAAMHmgww (envelope-from ); Mon, 26 Jun 2023 13:16:09 +0000 Date: Mon, 26 Jun 2023 15:16:08 +0200 From: Michal Hocko To: Tetsuo Handa Cc: Sebastian Andrzej Siewior , Peter Zijlstra , linux-mm@kvack.org, linux-kernel@vger.kernel.org, "Luis Claudio R. Goncalves" , Andrew Morton , Boqun Feng , Ingo Molnar , John Ogness , Mel Gorman , Petr Mladek , Thomas Gleixner , Waiman Long , Will Deacon Subject: Re: [PATCH v2 1/2] seqlock: Do the lockdep annotation before locking in do_write_seqcount_begin_nested() Message-ID: References: <20230623171232.892937-1-bigeasy@linutronix.de> <20230623171232.892937-2-bigeasy@linutronix.de> <20230626081254.XmorFrhs@linutronix.de> <0a0c768c-227d-c0cd-1b91-5a884d161c1b@I-love.SAKURA.ne.jp> <20230626104831.GT4253@hirez.programming.kicks-ass.net> <3a4ad958-a9a5-c367-a16d-bd89a173a628@I-love.SAKURA.ne.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 59E6FC0014 X-Stat-Signature: 7ngbhqreepps7wh9hdaqz5cw3a4e8q6y X-HE-Tag: 1687785370-462892 X-HE-Meta: U2FsdGVkX1+8Rw01G7+PLQwPhrozFhGfOtZFeE+mBfAum8kkgNv9SSWWe7J5fwDoipt54W0GKE0VQv5w99YeD3q20hVSi4UFK09Homr45U0va6V5+NExHde3ZchtOSAGT7o4u/Nk1toWzS++oxdjMqOSz1wPEQBazEnJeJSUbGvE7Zk0VD0yLqJ/7DzJEEBdM/SPNmVLs3qiLy1qKmQB4uSjyLvhCCafUD/hygAwYhat4kR1irg3y+E1J4QHHB2rLmpRDuRJ5qIOUHZ8ndcB3SrtOFEZRDqI7q1USiZiEWeAyR+9V0LT1F39iDVgAu80ORnjfbxy2CdUijWSyr49wV86P+gfFTCVsLmgTJnaiJfKSg+dLySDh48eSXVUTGmDiU1OIKLooon7HJ+k3gdK3lXxWvdQHq5Xfyds2EG+7J1Td6a7I+8bXzyG2ZPAjFmA/txQi2zLonm729a2DD+kAuWv3CDr/Fc+qx8DEqO6FNd3bT3YQzLFK7npuGIpzTkO1TFaooj6rb+bC6CLn8P6sG+1306V1K8PO8Zk7RK+XA9cvAy+TVuuQKApueCr883F9KOVZW0JV2jDT0KV9AODmv0buDfA/fTq8ZEZIDMTI+8FE++pXriIhfigIp5KWuEY+nwgdJ5v5f5PV11M649rYuDPAVQ+xlZX7PWyFzUDMDlgX5SJF/vsy4T4L16prXQaEYkSKdSL2/yZa8oBZCWebdIhlGJKqM54xiVrz6XV1wf2acATIt+/Uh+/2+T6DaK8On+y6PkdE061ZPsPPXpYDjcXw12eoFPlZ6RqhPgJInKrRGNp0rmKMN0Eoid10DAp2upRmDUYaC08GaPJ+/Zb299nLT8ZRnHJC3JnwafQCiXKGihswUnrps56xwx7cRx9T3me+SJTqrKl4eAcwPt2ay7jYQpLescchu1h5Y6PM9iir2InTZGvN2t7/4ScszT1Yw7+07U7fzrDVe7/N3R /iO00EgC 3Zyb3YSgc0K2PO9JUA9XHhQbCHlq/3hwL6Ll+/y1kWNFppP1IY/hLIyRAnXZ1GWKz9dfj9CteJtBqzva4SokCWX3X3YvovbgsnObr7rdKBr2pCGLgq/Wdz4FnTbyt7OTlZZwwiG7YnQgBjW3t5JI8FizjQuU10sf8zhRYCX9Q7B5Mj16d+TrZPn8cBZ7wlmHCYIaVQ1olrzMar8/yiDe+DCYh2K8OGuhmRhKfd0xQJOw1XHwtFpTSGP/eg7qFj+Hd+nNL75tdFnaGBIF6ExySUNkbgP9Tb5a8/87WzKHnlpyTIIxZ91BpzSWGwjYgvTmLLCq8pJJ6UkWP6YU= 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 Mon 26-06-23 21:27:05, Tetsuo Handa wrote: > On 2023/06/26 20:35, Michal Hocko wrote: > > On Mon 26-06-23 20:26:02, Tetsuo Handa wrote: > >> On 2023/06/26 19:48, Peter Zijlstra wrote: > >>> On Mon, Jun 26, 2023 at 06:25:56PM +0900, Tetsuo Handa wrote: > >>>> On 2023/06/26 17:12, Sebastian Andrzej Siewior wrote: > >>>>> On 2023-06-24 15:54:12 [+0900], Tetsuo Handa wrote: > >>>>>> Why not to do the same on the end side? > >>>>>> > >>>>>> 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_); > >>>>>> } > >>>>> > >>>>> I don't have a compelling argument for doing it. It is probably better > >>>>> to release the lock from lockdep's point of view and then really release > >>>>> it (so it can't be acquired before it is released). > >>>> > >>>> We must do it because this is a source of possible printk() deadlock. > >>>> Otherwise, I will nack on PATCH 2/2. > >>> > >>> Don't be like that... just hate on prink like the rest of us. In fact, > >>> i've been patching out the actual printk code for years because its > >>> unusable garbage. > >>> > >>> Will this actually still be a problem once all the fancy printk stuff > >>> lands? That shouldn't do synchronous prints except to 'atomic' consoles > >>> by default IIRC. > >> > >> Commit 1007843a9190 ("mm/page_alloc: fix potential deadlock on zonelist_update_seq > >> seqlock") was applied to 4.14-stable trees, and CONFIG_PREEMPT_RT is available > >> since 5.3. Thus, we want a fix which can be applied to 5.4-stable and later. > >> This means that we can't count on all the fancy printk stuff being available. > > > > Is there any reason to backport RT specific fixup to stable trees? I > > mean seriously, is there any actual memory hotplug user using > > PREEMPT_RT? I would be more than curious to hear the usecase. > > Even if we don't backport RT specific fixup to stable trees, [PATCH 2/2] requires > that [PATCH 1/2] guarantees that synchronous printk() never happens (for whatever > reasons) between write_seqlock_irqsave(&zonelist_update_seq, flags) and > write_sequnlock_irqrestore(&zonelist_update_seq, flags). I suspect you are overcomplicating this. I do understand that you want to have this 100% airtight but I would argue that this is actually not really necessary. I would be perfectly fine living in the world where this particular path could trigger an unintended printk. IIUC we are mostly talking about lockup detector only, right? AFAIK there is no such na issue _now_ so we are talking about a potential _risk_ only. > If [PATCH 1/2] cannot guarantee it, [PATCH 2/2] will be automatically rejected. > > If [PATCH 2/2] cannot be applied, we have several alternatives. > > Alternative 1: > > Revert both commit 3d36424b3b58 ("mm/page_alloc: fix race condition between build_all_zonelists and page allocation") > and commit 1007843a9190 ("mm/page_alloc: fix potential deadlock on zonelist_update_seq seqlock"). > I don't think this will happen, for nobody will be happy. > > Alternative 2: > > Revert commit 1007843a9190 ("mm/page_alloc: fix potential deadlock on zonelist_update_seq seqlock") > and apply "mm/page_alloc: don't check zonelist_update_seq from atomic allocations" at > https://lkml.kernel.org/r/dfdb9da6-ca8f-7a81-bfdd-d74b4c401f11@I-love.SAKURA.ne.jp . > I think this is reasonable, for this reduces locking dependency. But Michal Hocko did not like it. > > Alternative 3: > > Somehow preserve printk_deferred_enter() => write_seqlock(&zonelist_update_seq) and > write_sequnlock(&zonelist_update_seq) => printk_deferred_exit() pattern. Something like below? > Alternative 4: stop chasing shadows and deal with the fact that this code won't be perfect. Seriously you are trying to address a non-existing problem and blocking a working RT solution which doesn't clutter the code with RT specific baggage. -- Michal Hocko SUSE Labs