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 D842EC77B7A for ; Mon, 12 Jun 2023 01:31:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2F16C8E0002; Sun, 11 Jun 2023 21:31:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 27A7A6B0074; Sun, 11 Jun 2023 21:31:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 11C3D8E0002; Sun, 11 Jun 2023 21:31:40 -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 EDCB66B0072 for ; Sun, 11 Jun 2023 21:31:39 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 9C2791A024A for ; Mon, 12 Jun 2023 01:31:39 +0000 (UTC) X-FDA: 80892368718.10.2B5761E Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by imf29.hostedemail.com (Postfix) with ESMTP id 110C1120006 for ; Mon, 12 Jun 2023 01:31:35 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=BczpxA7a; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf29.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.115 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686533497; 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=zb+VyWArCJzEWCyQ0Yq1Nh1fslyQBkR3uFLjC1f4htU=; b=3J9t3+9KRE2iIw44sg0yoOtqjQXr8LxD4RKVLRvaC/zCB9BAEBmY1XMFtlQ27c6IhhPs33 N3JwpswX8DMzL8HUWaczxMIRHEZoNqA5bub6Sh7FZuOhlKWigWcCaU0Lxyml7tcFcNZRbw /Jv7hAahgdaEtbJXXnanj+ry69Ar85U= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=BczpxA7a; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf29.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.115 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686533497; a=rsa-sha256; cv=none; b=Xg80sQ8zQmeqQNFfHlb/xmsF+cdLFtd5s9AMMh260hM8Mu54Qh3Fq7ggeLMdDQAkfl8uV9 6QDDsaK1IZIUCUMjaRsax+p+AqrIrJQWl5WbCalOLOFgnM6NBe0Mnpk8SA8ZLZ/YO7UslE S2asHMF+82izNnM8FhhHRlQmKmSqZvk= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1686533496; x=1718069496; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=mgdQYi5e9PJhLCbcjiaqMKR0cLvCJawrWMZlmhYH/eA=; b=BczpxA7aTodavJnGBEWr5b8NeUqGZ8jdLyUFVCmycfkuVRsu2dDyMkLN AybdLtC5eeiO2dCMpwSe7GxI5K+1GsNFOXETmMEzZ47ZrPtZNMQ8ayOgr DGnUAMK59S4T6hRJouvtDhZfW04ZT0xlI/1cZHNFDAAYGMAYzDAZjW+Mc j861eMT2ypjkkDeg4L++IeOS4a2pWASXMbwd6NiZzmQ4wgZ8kvHxkIakt CkcNBzNdLcMA9DjJggH+R/P6crZY0wzDQFlt7IEWmmraJgNFG4Pg5wang T1CQmQ/1FSrboaswAQ2MTVBL+N7JTqa7ZdjjTlzLXYw/kQ7uP2MOWHMZX w==; X-IronPort-AV: E=McAfee;i="6600,9927,10738"; a="357916465" X-IronPort-AV: E=Sophos;i="6.00,235,1681196400"; d="scan'208";a="357916465" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Jun 2023 18:31:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10738"; a="711024244" X-IronPort-AV: E=Sophos;i="6.00,235,1681196400"; d="scan'208";a="711024244" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Jun 2023 18:31:30 -0700 From: "Huang, Ying" To: Tetsuo Handa Cc: Andrew Morton , Alexander Potapenko , syzbot , , Mel Gorman , Vlastimil Babka , Andrey Konovalov , Dmitry Vyukov , Andrey Ryabinin , Vincenzo Frascino , Marco Elver , kasan-dev , linux-mm Subject: Re: [PATCH v3] lib/stackdepot: fix gfp flags manipulation in __stack_depot_save() References: <000000000000cef3a005fc1bcc80@google.com> <656cb4f5-998b-c8d7-3c61-c2d37aa90f9a@I-love.SAKURA.ne.jp> <87353gx7wd.fsf@yhuang6-desk2.ccr.corp.intel.com> <20230609153124.11905393c03660369f4f5997@linux-foundation.org> <19d6c965-a9cf-16a5-6537-a02823d67c0a@I-love.SAKURA.ne.jp> Date: Mon, 12 Jun 2023 09:30:27 +0800 In-Reply-To: <19d6c965-a9cf-16a5-6537-a02823d67c0a@I-love.SAKURA.ne.jp> (Tetsuo Handa's message of "Sat, 10 Jun 2023 20:40:33 +0900") Message-ID: <871qiha2mk.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspamd-Queue-Id: 110C1120006 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: fgwu4dws9j9s3ekmghqreh6mhtzd9je4 X-HE-Tag: 1686533495-839617 X-HE-Meta: U2FsdGVkX18agMcAZCEXeIHuIl60tJf0TYPAD2Touq5m2RRgLis6WJpJte1TjoeQQ7D1bTnAcll5xAKB3VbpCfLeGcAfd/f3g9qODDr1F89ZsKnuW1SozGjLy/gMrniddxxqRK7qeNccT1BkwSk9KAX1+b00B2QcI+e2d0P10k6/JFmzXo5DL1j5hqtZOKYMOgpPFXNAKTHY1VVoAgS1GkTOppjmNBnT5v9lvOyNq4fNVUsJKbw1KcJ3b/giHtW2+EN7xTqEc0LYxfBYntb5KRoQcy+Toy73+vyvUfUpKFv0BVwphbGId+GJCPvv0iP95tTd+fNiihyGjQhZOHR6t01tvy8B8dhjMVp4FvNdRaI3++jAhL/IYVlHE9O7REqgMjet3qiyl2ve9XRObtZEypUWBsJaEMNmpz+Sk8OSGq8yt3GCJ0+7aPOrApqcFvr7+DDxCzy+H2u+9O3iaH6Ti+VanSZvVcRnA4nGyAXIORwTpd0gCHoWFxtqeqa2eTONFL7jEM379ZTlHGcZvJmYq2Kxjgzz4+8HEZY5UlLDy71i426DydXXHRqd9rFPYVLW3NzYgmK5KWKp2NBAfGSsengm6ZCS93BRp24q+b4nOhpCLbGSpaDgkvpzNNcSadD+ZkS68pFuiCLTADCYH3iLWoVlRTfD8JQutaGVVmEIIVP4FUaIOLgcir8WQ46Kk5mAC+rdFWy5P1yIu3lDZyCU7VkX86Iqj9fz621GsVsNdaFaa3DiseA7PzNXr2OYlEtB3o9JkYT39u30hpCfFbiWSuW/XP1zfPbDyk60erq52YbKqLS70p9PugSD5GuVryYNE4MRDtEGHB9kT7Zne9Q1SdamSZt9dKj+mRB9OIC+83kz32mnc4vAfqsHcTD/m6OJE1maPwcAkow+Lv7eTIoZJLzk4uAO1UG4NTZSkBH/ulSZrJB9yeN+I66Y/Dy6oSEFULH0oDptOWzR3EdXMp5 nRPFkKt+ udNytCnwMddbZFk7mMV11dxTwF0JMRgoZ4rx4tfTSnjPExu1lUjbNkvKUahq/FbGhuL9SMYwzJX4nJ++sxatlfyQUTlCLg3GaHyaXKXQEoplR/9sqkm0xpyAUpFR5TftQSDdlHpROLJaEkU6kkWLQlMAUkHw8r6xX9gZGo/k52bhG9Sp3uwLPEJpH6Jx6F1yM2DPGGm3d3RtNrBIHXkpS2wWGo0TpF+bJ5NFkOKTZ2zHGyXJ3MLBKzq0rSWm3luUemDCo1xPHO3/JgJ3QFDiMWiUn5vuWL3qVqNpeUQXtS/kMBhPPHWMerJ3B2QqOCW7/RJNuF1o6Gn/kRvHlYQQ4xzC+elhAOVt9FQ/KluDdAs4hdeDas7Gy22xW5EotpD8smqKGyFGdf7B51gcmUHI3nDpqwrEEvpqyFm44NYJ6y6Y4gqbvhMrvFqTXTa8k2eOS/zx5hLo9ADMV2BOk6iNlLqnqmzQK+QBDqBItjBBnnCzTw1UEQW8DZVZXnal9xGaipIFpbDKYhpT3J6WmNKzE1f9tZcCobvADY881 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: Tetsuo Handa writes: > syzbot is reporting lockdep warning in __stack_depot_save(), for > __kasan_record_aux_stack() is passing GFP_NOWAIT which will result in > calling wakeup_kcompactd() from wakeup_kswapd() from wake_all_kswapds() > from __alloc_pages_slowpath(). > > Strictly speaking, __kasan_record_aux_stack() is responsible for removing > __GFP_KSWAPD_RECLAIM flag in order not to wake kswapd which in turn wakes > kcompactd. But since KASAN and KMSAN functions might be called with > arbitrary locks held, we should consider removing __GFP_KSWAPD_RECLAIM > flag from KASAN and KMSAN. And this patch goes one step further; let's > remove __GFP_KSWAPD_RECLAIM flag in the __stack_depot_save() side, based > on the following reasons. > > Reason 1: > > Currently, __stack_depot_save() has "alloc_flags &= ~GFP_ZONEMASK;" line > which is pointless because "alloc_flags &= (GFP_ATOMIC | GFP_KERNEL);" > line will also zero out zone modifiers. But why is __stack_depot_save() > trying to mask gfp flags supplied by the caller? > > I guess that __stack_depot_save() tried to be as robust as possible. But > __stack_depot_save() is a debugging function where all callers have to > be able to survive allocation failures. Scattering low-level gfp flags > like 0 or __GFP_HIGH should be avoided in order to replace GFP_NOWAIT or > GFP_ATOMIC. > > Reason 2: > > __stack_depot_save() from stack_depot_save() is also called by > ref_tracker_alloc() from __netns_tracker_alloc() from > netns_tracker_alloc() from get_net_track(), and some of get_net_track() > users are passing GFP_ATOMIC because waking kswapd/kcompactd is safe. > But even if we mask __GFP_KSWAPD_RECLAIM flag at __stack_depot_save(), > it is very likely that allocations with __GFP_KSWAPD_RECLAIM flag happen > somewhere else by the moment __stack_depot_save() is called for the next > time. > > Therefore, not waking kswapd/kcompactd when doing allocation for > __stack_depot_save() will be acceptable from the memory reclaim latency > perspective. TBH, I don't like to remove __GFP_KSWAPD_RECLAIM flag unnecessarily. But this is only my personal opinion. > While we are at it, let's make __stack_depot_save() accept __GFP_NORETRY > and __GFP_RETRY_MAYFAIL flags, based on the following reason. > > Reason 3: > > Since DEPOT_POOL_ORDER is defined as 2, we must mask __GFP_NOFAIL flag > in order not to complain rmqueue(). But masking __GFP_NORETRY flag and > __GFP_RETRY_MAYFAIL flag might be overkill. > > The OOM killer might be needlessly invoked due to order-2 allocation if > GFP_KERNEL is supplied by the caller, despite the caller might have > passed GFP_KERNEL for doing order-0 allocation. > > Allocation for order-2 might stall if GFP_NOFS or GFP_NOIO is supplied > by the caller, despite the caller might have passed GFP_NOFS or GFP_NOIO > for doing order-0 allocation. > > Generally speaking, I feel that doing order-2 allocation from > __stack_depot_save() with gfp flags supplied by the caller is an > unexpected behavior for the callers. We might want to use only order-0 > allocation, and/or stop using gfp flags supplied by the caller... Per my understanding, this isn't locking issue reported by syzbot? If so, I suggest to put this in a separate patch. > Reported-by: syzbot > Closes: https://syzkaller.appspot.com/bug?extid=ece2915262061d6e0ac1 > Suggested-by: Alexander Potapenko > Cc: Huang, Ying > Signed-off-by: Tetsuo Handa > --- > Changes in v3: > Huang, Ying thinks that masking __GFP_KSWAPD_RECLAIM flag in the callers > side is preferable > ( https://lkml.kernel.org/r/87fs7nyhs3.fsf@yhuang6-desk2.ccr.corp.intel.com ). > But Alexander Potapenko thinks that masking __GFP_KSWAPD_RECLAIM flag > in the callee side would be the better > ( https://lkml.kernel.org/r/CAG_fn=UTTbkGeOX0teGcNOeobtgV=mfGOefZpV-NTN4Ouus7xA@mail.gmail.com ). > I took Alexander's suggestion, and added reasoning for masking > __GFP_KSWAPD_RECLAIM flag in the callee side. > > Changes in v2: > Mask __GFP_KSWAPD_RECLAIM flag in the callers, suggested by Huang, Ying > ( https://lkml.kernel.org/r/87edn92jvz.fsf@yhuang6-desk2.ccr.corp.intel.com ). > > lib/stackdepot.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 2f5aa851834e..33ebefaa7074 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -405,7 +405,10 @@ depot_stack_handle_t __stack_depot_save(unsigned long *entries, > * contexts and I/O. > */ > alloc_flags &= ~GFP_ZONEMASK; > - alloc_flags &= (GFP_ATOMIC | GFP_KERNEL); > + if (!(alloc_flags & __GFP_DIRECT_RECLAIM)) > + alloc_flags &= __GFP_HIGH; Why not just alloc_flags &= ~__GFP_KSWAPD_RECLAIM; ? > + else > + alloc_flags &= ~__GFP_NOFAIL; > alloc_flags |= __GFP_NOWARN; > page = alloc_pages(alloc_flags, DEPOT_POOL_ORDER); > if (page) Best Regards, Huang, Ying