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 8DEAFEB64D7 for ; Wed, 21 Jun 2023 12:57:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E3AEE8D0002; Wed, 21 Jun 2023 08:57:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DEAF48D0001; Wed, 21 Jun 2023 08:57:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CB2718D0002; Wed, 21 Jun 2023 08:57:04 -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 BD8638D0001 for ; Wed, 21 Jun 2023 08:57:04 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 8DC461C8525 for ; Wed, 21 Jun 2023 12:57:04 +0000 (UTC) X-FDA: 80926755168.25.F7B3575 Received: from mail-io1-f48.google.com (mail-io1-f48.google.com [209.85.166.48]) by imf07.hostedemail.com (Postfix) with ESMTP id AF05A4001F for ; Wed, 21 Jun 2023 12:57:02 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=HguHzfhH; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf07.hostedemail.com: domain of glider@google.com designates 209.85.166.48 as permitted sender) smtp.mailfrom=glider@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1687352222; 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:dkim-signature; bh=evIj23C5RuYH7l01ysd2yt+gQqbHOQ1C3V6W2tREipI=; b=AQnq8pliq2AyaHANPgQpLk5tsGACPOaCGg7qrH+zMcgV5RUqgXobe9BH3Hj4zzhK5yxCpm h02zTeUir8WZX082MExAfXPLyqL2bbndAyLg4SnhJE0pAP85WOrJnbL1rpDpMi6Ajg4jxO SOYIQRywbie+F/fIGCd7FDTrfhjFzeA= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=HguHzfhH; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf07.hostedemail.com: domain of glider@google.com designates 209.85.166.48 as permitted sender) smtp.mailfrom=glider@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687352222; a=rsa-sha256; cv=none; b=64IECi2o2hoJ8t0f+roO2Jayov+QorGa6EnvBwQOtuvrtCxVA6HcKSfrf/YVu6vPZWpXEc qBa3mfq/lgio95CrHYv746fEwAFQ0/JQHk2lrq33uXs4Risb97/L5f6OmU2zGR0PzCyX6/ X2VM7/4goRTvy/FDh+KXyHw2ddFK2q8= Received: by mail-io1-f48.google.com with SMTP id ca18e2360f4ac-77de8cc13b4so281937139f.2 for ; Wed, 21 Jun 2023 05:57:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1687352222; x=1689944222; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=evIj23C5RuYH7l01ysd2yt+gQqbHOQ1C3V6W2tREipI=; b=HguHzfhHxJQECs7bVsgTPrCQwxCtKprfwrmTGI0qAOvvOfPZVt1zsws4N/YYs4+xFp xc/zoGEYK2lM0dm6Kw7iaBW44bZdoQvDNkFKo9bApU8YBJZt3BAPMMrfkQKVPGYdYKwR XTYJcuVZ3n3cQ1cWbmyL8SrC9VX7mPmRI/AoMoUgMFUCH4cfdB2gA3dK+VHlBcwMWX7N eO9Enn08wgdyIRFVeNiRLWwF98c2UhTnJl6dcSKXkv9kRbu4Fk0Kk7e7NSxq1b0T7m+G s39aV5x2lRYIhwK7C8zcX6X1zvIOfHGRU9DzUTVBMdwB6SB6Cqt25bIt9oLMEqmM5kfx RbWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687352222; x=1689944222; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=evIj23C5RuYH7l01ysd2yt+gQqbHOQ1C3V6W2tREipI=; b=KZrpYCvj5PmvSq+bO7a+HmbKa7VdvtZSb+xH4t3GFvao04/ZNiOcTwRXjG5LiLusfr 67vZfD9Uolw/b6Uq6lPrxwK+BrlYFW85vMgMCtfRkqfov1zIleW0F9TJ0MV0wKXCEL1B tvl6pjUDIRXwKjqshYK7ZJwzH14a2cEH4tkZ+TYrUUq5yYdyShhoucjmUWIE0xPfmivc pzBmDCjUUA1CkCZmvtyaEX4d4lSlYaThhbcUGZnKuXG/f6Aj9/XzM6tkwmXGvygU09hz qYoNG2QX6Zm06xQPkI5TH8sDjuf1JoxUIZBx55yzWlst1xc6HFTglglADhHATpIg42WL itJw== X-Gm-Message-State: AC+VfDwqc8WfHxhvZnAJzfUolRiRV9x+vIkn9y+Y4q4n4E1hKQA/MuED bRvKveHfnKkRkHNN0sBZ8cZLnYVb8hpLnrl47nKu0g== X-Google-Smtp-Source: ACHHUZ7bSR6TjY9RT9XWVtJCLNzd2DYujtpvMAfjLHrskgWB+O7rwD46hMU5jtTP9zYufHJnRNpp1WWyeKmCtKJUxSQ= X-Received: by 2002:a05:6602:399:b0:77e:3d2f:d1f8 with SMTP id f25-20020a056602039900b0077e3d2fd1f8mr8694965iov.10.1687352221671; Wed, 21 Jun 2023 05:57:01 -0700 (PDT) MIME-Version: 1.0 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> In-Reply-To: <19d6c965-a9cf-16a5-6537-a02823d67c0a@I-love.SAKURA.ne.jp> From: Alexander Potapenko Date: Wed, 21 Jun 2023 14:56:25 +0200 Message-ID: Subject: Re: [PATCH v3] lib/stackdepot: fix gfp flags manipulation in __stack_depot_save() To: Tetsuo Handa Cc: Andrew Morton , "Huang, Ying" , syzbot , syzkaller-bugs@googlegroups.com, Mel Gorman , Vlastimil Babka , Andrey Konovalov , Dmitry Vyukov , Andrey Ryabinin , Vincenzo Frascino , Marco Elver , kasan-dev , linux-mm Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: ck7onnfm8rikhmstdh9ox9h6j1nk5mgz X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: AF05A4001F X-HE-Tag: 1687352222-499971 X-HE-Meta: U2FsdGVkX1/Oq6cY//6mxzqPOGreypbLfG26E8kedPCKgx2ZxA0mZ3ecLzlFKszKyGjDcwri1ag5rXc0b8Y1HVMMWUj+8GgwHzRTgfRc9Vnbt+tDsnGbmQuETkRl3/3gjfhAcVZgtQ/37uoyEaKNzetUglN+aAVu2izi6OkcxMjLmZQkbAl2IgZgRWbOVTsrDEUhavamMnfucEI1HazttU+3bVyndYWFyrRhyQu3MR4SNMw3esBJ1TQnAESFL60nJ9ywA/wMOq3+tDcnA0cpX07NtE7ZlYSCWgDrdVU7BALo7G0/GpxMP+/sC4/7aVywMEqyK0GjWNXBdCIMHX/XH9ChY4pzdJVVjL+hC7TyoI5vfnH+94cwyc5RMtFVwE+NTN/3ZIFkSUq66oTGfQ48sWOTWDlQi0KKEySoCmd3HntZbqvZXhhxFqrdynTt6czgUWegFUdwHMo0N3cwWdcF8/jEj2XJ6Q5bNIIiFtUzRT0mJAbVxRneMgsPqCTzuXn1Ybjn2EEOMCIaG5bJQOkLtQphl6vTWIFZGgYBYt8C2GhfEipNC0mZA6dQSzgZ4gPcTIZC6uGEkbfpuXrC0Zjm5Nz4KUXE9S38tC6jDkEbDefe3AZ3yqCKRf/VD5nAjYKswgW02w2Gp6uAsni9irTVFNgoUjUR3vpDjtx0tjJ+x69ZHcGnz8fy6WgYQ2FD51FJbFUgzPhaOkGdbVUs7d7ixnkWajHbPq/kE5VgNTucfKjHhaOuaGOweQ4n8FkzF7IsMz/nqPF+4SKCuxdceq7t9XtcF8AscpXFA3cZ3xdYxqhejpG5yc3tJQQi68MYf7CjCT2uynP925lxB0YKy9J95jhyozVXc0NJfHu7Q37t/ytAw1yDetkAlqpUjhLTfuSjxu5ezY9PLb6TG4aH7Ng0/raFdAnjp1osEsqwVSuLgIx+tSX4LFAlAIfZQGptqYl6pkUI/jvI2LCnCyVvlqH JqtNWyeH XZ6HA/5Omo1RAslMizG0sVWj971WM1zx0k9Mr+cV/IxquuposliYM30ll7y9RYwIHBMSKrs8BAsn92DWxSgfJ3G/SBUIF+7qxghpGi7X8gd/+AYKAItjCi03o2fgCTq0IYcPirZ4AhrvWuh1+qxQGxMyouAwwIIPGyYS3PxXQPmwmSyRso8hyYd8/yCNmCfpTCyiK9QaxOlvXcfjWk2SfcZ1wCOPh3tENqpwfT50ZQOeNUiQVx5XeE8xUV9qC1ZjAZp9wz9SZIfRrf1dfKXpKvptMUtgv+cQDCk5VJfOT8QeSDTowSTPsBvDWx4NU+f02bTBUy6kqDs9efcSZc98Lno1vVG0BHq4fiFZ0Mp5uibhqyFp1EH0j/m26jFwjv/h0fAVvK0UCfdtK2V0CEWGHRsycqPdWdvDWYO3WfznnHnGqJuZnkCeiecJvmuf/cQyEIlOgpA77eDOwISMy3p93bcW4p7yqpvHtyORl1IgZa18CEtfsUQF7ccDcB1dmnd/z9buyoTQDUbWtiRzAM9yi5bDAhLCO52e2VOOPFGG06BicHbbd7F/wrrVroU25Vi6q/kjSGJ4m01Tt0Evq2x6dyYX9hW7DWyAsAu9b65w3+uHMWOijfQ1EMFgwuQ== 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 Sat, Jun 10, 2023 at 1:40=E2=80=AFPM Tetsuo Handa wrote: > > 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 &=3D ~GFP_ZONEMASK;" l= ine > which is pointless because "alloc_flags &=3D (GFP_ATOMIC | GFP_KERNEL);= " > line will also zero out zone modifiers. Good catch, we indeed do not need the GFP_ZONEMASK line now. But looks like you'll need it at least in the __GFP_NOFAIL branch? > 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. Bu= t > __stack_depot_save() is a debugging function where all callers have to > be able to survive allocation failures. This, but also the allocation should not deadlock. E.g. KMSAN can call __stack_depot_save() from almost any function in the kernel, so we'd better avoid heavyweight memory reclaiming, because that in turn may call __stack_depot_save() again. > > 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 happe= n > somewhere else by the moment __stack_depot_save() is called for the nex= t > time. > > Therefore, not waking kswapd/kcompactd when doing allocation for > __stack_depot_save() will be acceptable from the memory reclaim latency > perspective. Ack. > While we are at it, let's make __stack_depot_save() accept __GFP_NORETRY > and __GFP_RETRY_MAYFAIL flags, based on the following reason. Looks like you're accepting a whole bunch of flags in addition to __GFP_NORETRY and __GFP_RETRY_MAYFAIL - maybe list the two explicitly? > 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. As you noted above, stackdepot is a debug feature anyway, so invoking OOM killer because there is no memory for an order-2 allocation might be an acceptable behavior? > 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_NOI= O > for doing order-0 allocation. What if the caller passed GFP_NOFS to avoid calling back into FS, and discarding that flag would result in a recursion? Same for GFP_NOIO. > 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... Right now stackdepot allows the following list of flags: __GFP_HIGH, __GFP_KSWAPD_RECLAIM, __GFP_DIRECT_RECLAIM, __GFP_IO, __GFP_FS. We could restrict it further to __GFP_HIGH | __GFP_DIRECT_RECLAIM to be on the safe side - plus allow __GFP_NORETRY and __GFP_RETRY_MAYFAIL. > Reported-by: syzbot > Closes: https://syzkaller.appspot.com/bug?extid=3Dece2915262061d6e0ac1 > 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 caller= s > 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=3DUTTbkGeOX0teGcNOeobtgV=3DmfGOefZpV= -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 lon= g *entries, > * contexts and I/O. > */ > alloc_flags &=3D ~GFP_ZONEMASK; > - alloc_flags &=3D (GFP_ATOMIC | GFP_KERNEL); > + if (!(alloc_flags & __GFP_DIRECT_RECLAIM)) > + alloc_flags &=3D __GFP_HIGH; > + else > + alloc_flags &=3D ~__GFP_NOFAIL; > alloc_flags |=3D __GFP_NOWARN; > page =3D alloc_pages(alloc_flags, DEPOT_POOL_ORDER); > if (page) > -- > 2.18.4 > >