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

[Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>]

On 2023/05/22 12:07, Huang, Ying wrote:
> Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:
> 
>> 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.
> 
> Except debug code, where do you find locking issues for waking up kswapd?

I'm not aware of lockdep reports except debug code.

But due to too many locking dependency, lockdep gives up tracking all dependency (e.g.

  https://syzkaller.appspot.com/bug?extid=8a249628ae32ea7de3a2
  https://syzkaller.appspot.com/bug?extid=a70a6358abd2c3f9550f
  https://syzkaller.appspot.com/bug?extid=9bbbacfbf1e04d5221f7
  https://syzkaller.appspot.com/bug?extid=b04c9ffbbd2f303d00d9

). I want to reduce locking patterns where possible. pgdat->{kswapd,kcompactd}_wait.lock
and zonelist_update_seq are candidates which need not to be held from interrupt context.

> 
>> 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...
> 
>>From performance perspective, it's better to wake up kswapd as early as
> possible.  Because it can reduce the possibility of the direct
> reclaiming, which may case very long latency.

My expectation is that a __GFP_DIRECT_RECLAIM allocation request which happened
after a !__GFP_KSWAPD_RECLAIM allocation request wakes kswapd before future
__GFP_DIRECT_RECLAIM allocation requests have to perform the direct reclaiming.

> 
> Best Regards,
> Huang, Ying
>