On Wed, Jun 21, 2023 at 4:07 PM Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote: > > 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? Perhaps you are right and I shouldn't have insisted on pushing this flag down to stackdepot. If other users (e.g. page_owner) can afford invoking kswapd, then we are good to go, and new compiler-based tools can use the same flags KASAN and KMSAN do. > > >> 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 ? But not for the other if-clause? Anyway, I actually confused GFP_NOIO (which is technically __GFP_RECLAIM) and GFP_NOFS with __GFP_IO/__GFP_FS, thinking that there's a separate pair of GFP flags opposite to __GFP_IO and __GFP_FS. Please disregard. > > > >> 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. > I agree, let's go for it. Sorry for the trouble. -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Liana Sebastian Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
