Re: [PATCH v3] lib/stackdepot: fix gfp flags manipulation in __stack_depot_save()

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: Alexander Potapenko <glider@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"Huang, Ying" <ying.huang@intel.com>,
	syzbot <syzbot+ece2915262061d6e0ac1@syzkaller.appspotmail.com>,
	syzkaller-bugs@googlegroups.com,
	Mel Gorman <mgorman@techsingularity.net>,
	Vlastimil Babka <vbabka@suse.cz>,
	Andrey Konovalov <andreyknvl@gmail.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Andrey Ryabinin <ryabinin.a.a@gmail.com>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Marco Elver <elver@google.com>,
	kasan-dev <kasan-dev@googlegroups.com>,
	linux-mm <linux-mm@kvack.org>
Subject: Re: [PATCH v3] lib/stackdepot: fix gfp flags manipulation in __stack_depot_save()
Date: Wed, 21 Jun 2023 23:07:07 +0900	[thread overview]
Message-ID: <34aab39f-10c0-bb72-832b-d44a8ef96c2e@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <CAG_fn=XBBVBj9VcFkirMNj9sQOHvx2Q12o9esDkgPB0BP33DKg@mail.gmail.com>

On 2023/06/21 21:56, Alexander Potapenko wrote:
>> 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.
> 
> 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.

Then, isn't "[PATCH] kasan,kmsan: remove __GFP_KSWAPD_RECLAIM usage from
kasan/kmsan" the better fix?

>>   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.
> 
> 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.

Excuse me, but "alloc_flags &= ~__GFP_NOFAIL;" will not discard flags in
GFP_NOFS / 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.

I feel that making such change is killing more than needed; there is
no need to discard __GFP_KSWAPD_RECLAIM when GFP_KERNEL is given.

"[PATCH] kasan,kmsan: remove __GFP_KSWAPD_RECLAIM usage from kasan/kmsan"
looks the better.