Re: [PATCH] lib/stackdepot: stackdepot: don't use __GFP_KSWAPD_RECLAIM from __stack_depot_save() if atomic context

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: "Huang, Ying" <ying.huang@intel.com>
Cc: syzbot <syzbot+ece2915262061d6e0ac1@syzkaller.appspotmail.com>,
	syzkaller-bugs@googlegroups.com,
	Mel Gorman <mgorman@techsingularity.net>,
	Vlastimil Babka <vbabka@suse.cz>,
	Andrew Morton <akpm@linux-foundation.org>,
	Alexander Potapenko <glider@google.com>,
	Andrey Konovalov <andreyknvl@gmail.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Andrey Ryabinin <ryabinin.a.a@gmail.com>,
	kasan-dev <kasan-dev@googlegroups.com>,
	linux-mm <linux-mm@kvack.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@kernel.org>
Subject: Re: [PATCH] lib/stackdepot: stackdepot: don't use __GFP_KSWAPD_RECLAIM from __stack_depot_save() if atomic context
Date: Mon, 22 May 2023 11:47:25 +0900	[thread overview]
Message-ID: <0471c62b-7047-050a-14f5-f47dfaffaba7@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <87edn92jvz.fsf@yhuang6-desk2.ccr.corp.intel.com>

On 2023/05/22 11:13, Huang, Ying wrote:
>> Any atomic allocation used by KASAN needs to drop __GFP_KSWAPD_RECLAIM bit.
>> Where do we want to drop this bit (in the caller side, or in the callee side)?
> 
> Yes.  I think we should fix the KASAN.  Maybe define a new GFP_XXX
> (instead of GFP_ATOMIC) for debug code?  The debug code may be called at
> almost arbitrary places, and wakeup_kswap() isn't safe to be called in
> some situations.

What do you think about removing __GFP_KSWAPD_RECLAIM from GFP_ATOMIC and GFP_NOWAIT?
Recent reports indicate that atomic allocations (GFP_ATOMIC and GFP_NOWAIT) are not safe
enough to think "atomic". They just don't do direct reclaim, but they do take spinlocks.
Removing __GFP_KSWAPD_RECLAIM from GFP_ATOMIC and GFP_NOWAIT simplifies locking dependency and
reduces latency of atomic allocations (which is important when called from "atomic" context).
I consider that memory allocations which do not do direct reclaim should be geared towards
less locking dependency.

In general, GFP_ATOMIC or GFP_NOWAIT users will not allocate many pages.
It is likely that somebody else tries to allocate memory using __GFP_DIRECT_RECLAIM
right after GFP_ATOMIC or GFP_NOWAIT allocations. We unlikely need to wake kswapd
upon GFP_ATOMIC or GFP_NOWAIT allocations.

If some GFP_ATOMIC or GFP_NOWAIT users need to allocate many pages, they can add
__GFP_KSWAPD_RECLAIM explicitly; though allocating many pages using GFP_ATOMIC or
GFP_NOWAIT is not recommended from the beginning...