On Fri, Feb 28, 2025 at 05:33:28PM +0100, Vlastimil Babka wrote: > On 2/28/25 17:30, Vlastimil Babka wrote: > > On 2/25/25 17:23, Shakeel Butt wrote: > >> On Tue, Feb 25, 2025 at 11:30:20PM +0800, Jingxiang Zeng wrote: > >>> mm/list_lru.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/mm/list_lru.c b/mm/list_lru.c > >>> index 064d2018e265..e7e13513ff8e 100644 > >>> --- a/mm/list_lru.c > >>> +++ b/mm/list_lru.c > >>> @@ -552,7 +552,7 @@ static int memcg_list_lru_alloc(struct mem_cgroup *memcg, struct list_lru *lru) > >>> } > >>> xas_unlock_irqrestore(&xas, flags); > >>> } while (xas_nomem(&xas, GFP_KERNEL)); > >>> - if (mlru) > >>> + if (unlikely(mlru)) > >>> kfree(mlru); > >> > >> The report is saying not to check at all. So, just remove the check and > >> simply call kfree(mlru) as it handles the NULL check efficiently. > > > > BTW, if it's unlikely to be !NULL then it's not really efficient to call > > kfree() unconditionally as it's a function call and those are more expensive > > than inline check (especially with spectre mitigations). Thanks, that warning gave me the impression that unconditional kfree() is always better. > > OTOH making kfree() do the check inline would bloat all callers, and most of > them are likely !NULL. > We could perhaps have a kfree_unlikely() variant doing an inline > "if unlikely(obj) kfree()" if enough places do it. > This makes sense.
