At 2025-08-27 16:40:21, "Vlastimil Babka" wrote: >On 8/27/25 09:45, yangshiguang wrote: >> >> >> >> >> At 2025-08-27 13:17:31, "Harry Yoo" wrote: >>>On Mon, Aug 25, 2025 at 05:42:52PM +0200, Vlastimil Babka wrote: >>>> On 8/25/25 14:40, Matthew Wilcox wrote: >>>> > On Mon, Aug 25, 2025 at 08:17:37PM +0800, yangshiguang1011@163.com wrote: >>>> >> Avoid deadlock caused by implicitly waking up kswapd by >>>> >> passing in allocation flags. >>>> > [...] >>>> >> + /* Preemption is disabled in ___slab_alloc() */ >>>> >> + gfp_flags &= ~(__GFP_DIRECT_RECLAIM); >>>> > >>>> > If you don't mean __GFP_KSWAPD_RECLAIM here, the explanation needs to >>>> > be better. >>>> >>>> It was suggested by Harry here: >>>> https://lore.kernel.org/all/aKKhUoUkRNDkFYYb@harry >>>> >>>> I think the comment is enough? Disabling preemption means we can't direct >>>> reclaim, but we can wake up kswapd. If the slab caller context is such that >>>> we can't, __GFP_KSWAPD_RECLAIM already won't be in the gfp_flags. >>> >>>To make it a little bit more verbose, this ^^ explanation can be added to the >> >>>changelog? >> >> >> ok, will be easier to understand. >> >>> >>>> But I think we should mask our also __GFP_NOFAIL and add __GFP_NOWARN? >>> >> >>>That sounds good.> >>>> (we should get some common helpers for these kinds of gfp flag manipulations >>>> already) >>> >>>Any ideas for its name? >>> >>>gfp_dont_try_too_hard(), >>>gfp_adjust_lightweight(), >>>gfp_adjust_mayfail(), >>>... >>> >>>I'm not good at naming :/ >> >>> >> >> How about this? >> >> /* Preemption is disabled in ___slab_alloc() */ >> - gfp_flags &= ~(__GFP_DIRECT_RECLAIM); >> + gfp_flags = (gfp_flags & ~(__GFP_DIRECT_RECLAIM | __GFP_NOFAIL)) | >> + __GFP_NOWARN; > >I'd suggest using gfp_nested_flags() and & ~(__GFP_DIRECT_RECLAIM); > However, gfp has been processed by gfp_nested_mask() in stack_depot_save_flags(). Still need to call here? set_track_prepare() ->stack_depot_save_flags() >> >-- >>>Cheers, >>>Harry / Hyeonggon