From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B7C3EC021B8 for ; Wed, 26 Feb 2025 21:08:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 329DB280002; Wed, 26 Feb 2025 16:08:47 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2B199280001; Wed, 26 Feb 2025 16:08:47 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 152DA280002; Wed, 26 Feb 2025 16:08:47 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id EA42B280001 for ; Wed, 26 Feb 2025 16:08:46 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 9DD32A27C9 for ; Wed, 26 Feb 2025 21:08:46 +0000 (UTC) X-FDA: 83163335052.03.71B289B Received: from out-174.mta1.migadu.com (out-174.mta1.migadu.com [95.215.58.174]) by imf10.hostedemail.com (Postfix) with ESMTP id 348AFC0006 for ; Wed, 26 Feb 2025 21:08:43 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=o4e7Wk7P; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf10.hostedemail.com: domain of shakeel.butt@linux.dev designates 95.215.58.174 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740604125; a=rsa-sha256; cv=none; b=lBqKKu05LhyoWzd0eVagKYZ03SaxLujIKoLA9oqFsnOEmFwbGeUwNWMZpYxSECxbr1KB3N HNnQYjAex5/ArAeJ1+JdRWuvd8MuB2CXkvC6a+Zbdm2Dh8OFKAmoJaFSDTeIr1yEbRgEHh ATT6INGnWOjF2EQD/ASXbYOdUxLCYsY= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740604125; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=IsdnsmAcbyFXxei9s2oiPI34yZ5sQZVcfT5A12uwKAQ=; b=Iesy9daiBMbtgIjeIz2TNWPN5z9RjgtiU729IHeKL1OFxiuaZ1gh2OaojNNkUhL/gV5aXp Ipewtib8zkEe8yG9DR0hCMhiS8V/Q4K1/DMaQvvy1XvEA48d9QKj8gS0k1veghwIuHdtlW KcYLj8yO+Oj6JcTrMsOmNP7O6pc4d5E= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=o4e7Wk7P; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf10.hostedemail.com: domain of shakeel.butt@linux.dev designates 95.215.58.174 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev Date: Wed, 26 Feb 2025 13:08:32 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1740604122; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=IsdnsmAcbyFXxei9s2oiPI34yZ5sQZVcfT5A12uwKAQ=; b=o4e7Wk7P6q7R729gB/jqEXPu45kJi0awVLyeaO8sJVIrB2CmyySveCszF5Hp7um0b1cAsM 2lSfmvVZUeT49zS0iYy8WNhH0wsRu4qUJxWSkHgK2YiMeOOgCIGfQDqNub9pWMJ4bTIa/d AuFJGSHKqrc6dwumVYtRPIO1WlAs20k= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Shakeel Butt To: Johannes Weiner Cc: Jingxiang Zeng , linux-mm@kvack.org, akpm@linux-foundation.org, mhocko@kernel.org, roman.gushchin@linux.dev, muchun.song@linux.dev, chengming.zhou@linux.dev, kasong@tencent.com, kernel test robot Subject: Re: [PATCH] mm/list_lru: make the case where mlru is NULL as unlikely Message-ID: References: <20250225153020.2514685-1-jingxiangzeng.cas@gmail.com> <20250226011147.GB1500140@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250226011147.GB1500140@cmpxchg.org> X-Migadu-Flow: FLOW_OUT X-Rspamd-Queue-Id: 348AFC0006 X-Rspamd-Server: rspam08 X-Rspam-User: X-Stat-Signature: c1a9ouyaboj6fgsj4qrmkpbhw7jcfaph X-HE-Tag: 1740604123-156998 X-HE-Meta: U2FsdGVkX1/+MrU6GgbA2UloW6LoTFILVEU0UVSE0eVP5wfrMPZ+TKdQPLgonemCiwmVuXCt8Co7jH0osnSR2ARFWcaCNc49eY9DPaPVUrMwW/x8XrUpdiYpvVOxMvX42Xg+ZaU0TbZQsh6Etqu7fxE5+3uaSM1bw3fXzDytoaXnXBHBWEF7sUv0xMHgPZzlEIkK63TQVaJtvv8P4eJg+hahdAhK+vQdOCzNZ8aIPcxnFFHajpUi319QMfhqIUgKAC2YPC0UgAkYayJYRRHK4YYa5gHVTgDFVp3LD0MUR7wFhgFxMxLEE+il/i19LWTAwklWQPFq7ZS/ytC62jRpGzvoDgjrTlS3it9dqfIY3JTdqEacs3kD46Xa0oAZW6yPBTwMudLHrDQs9FOINtFod1SusJnxXBkVG3v9oqo4nkixIfM3E+a3Hzl3nqTaQQiwyV34vhdBq1nwynU3XWrA1bVg62/1xIrFcFXS1YY/QmgKtoBGAj+bOGjYg/k/4komGbedh+n4qSSv02m2PqSvhkrT64ZWa9OYzv4s7l0oEnu5zccXlHHFQ5DUTWzNHcvV9wd2JHgdw0LNGd8floDfRG1JBZXB5Er3Z5/ti+6EI4sZjkrosgUxuSLXABHesuJ10TM+z94dbkCK6LBllzbwTdXoC41y6XKpRaPqtN6ASUvDNlTD0FqsUEhDd1fxzRs2oxASFmrV7RShybJKYHLT7OR/2R950s8OnHEDr9KwRTIbIdLrBpbSVYf2TD+VlN64vRqwSwGoyfK1qL+CfTPdCoo2y5Yan0qif1/wnVNe2dpRHbrrWCILyMNbizPZ8V2l7a+fbdJv7Z/ovojLTIWEMHqb8soHdCnWdFZYEOXlxxRK7gLFc/+VMupDWL35ZF2imHUFQonVRa+JKgl3vQxsKEfgk4AU4fmdUzTS2Te1RrhwktmuFVv1QjeFi+TgL+3d1PNUqIVxnXF7ao3xlr6 LQ2vljrM QSkDsm/hXVPfJOBBd7/c2uZ6sjWB0RWlCpy+7jMaYUiwUZnhOVcvZDp4JTXEwLb6qFdQtQ5+ckmbyN/rsq5Cz2YSVZRVMgERiOANsEta8EzD74yAFsG2lGSqkABd85zU+libl5a8D33+HoQNP4b6RF1s3qMRMczbzUbCKE1w5PkFdm0sJ8z7KrAA6TZuir5lNYK3JiEfy89QVg4aH9iaWlOWaKlu7YTWPwVxR2QYGbd5x5DTm0cX5/ahMK5h4L3vHVq1msVgw+C8wZMRcxvu5l0hQR/vdIV+WUP61e0Ol7tTU4Lo= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Feb 25, 2025 at 08:11:47PM -0500, Johannes Weiner wrote: > On Tue, Feb 25, 2025 at 08:23:12AM -0800, Shakeel Butt wrote: > > On Tue, Feb 25, 2025 at 11:30:20PM +0800, Jingxiang Zeng wrote: > > > From: Zeng Jingxiang > > > > > > In the following memcg_list_lru_alloc() function, mlru here is almost > > > always NULL, so in most cases this should save a function call, mark > > > mlru as unlikely to optimize the code. > > > do { > > > xas_lock_irqsave(&xas, flags); > > > if (!xas_load(&xas) && !css_is_dying(&pos->css)) { > > > xas_store(&xas, mlru); > > > if (!xas_error(&xas)) > > > mlru = NULL; > > > } > > > xas_unlock_irqrestore(&xas, flags); > > > } while (xas_nomem(&xas, GFP_KERNEL)); > > > > if (mlru) > > > kfree(mlru); > > > > > > Reported-by: kernel test robot > > > Closes: https://lore.kernel.org/oe-kbuild-all/202412290924.UTP7GH2Z-lkp@intel.com/ > > > Signed-off-by: Zeng Jingxiang > > > --- > > > 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. > > I actually like it in this case. It's an "active comment" that this > only happens in the failure case and we don't routinely free here. > > That said, does it have to free the mlru inside the loop at all? If > the tree insertion fails, why not reuse it for the next attempt? > > diff --git a/mm/list_lru.c b/mm/list_lru.c > index 7d69434c70e0..490473af3122 100644 > --- a/mm/list_lru.c > +++ b/mm/list_lru.c > @@ -510,7 +510,7 @@ int memcg_list_lru_alloc(struct mem_cgroup *memcg, struct list_lru *lru, > gfp_t gfp) > { > unsigned long flags; > - struct list_lru_memcg *mlru; > + struct list_lru_memcg *mlru = NULL; > struct mem_cgroup *pos, *parent; > XA_STATE(xas, &lru->xa, 0); > > @@ -535,9 +535,11 @@ int memcg_list_lru_alloc(struct mem_cgroup *memcg, struct list_lru *lru, > parent = parent_mem_cgroup(pos); > } > > - mlru = memcg_init_list_lru_one(lru, gfp); > - if (!mlru) > - return -ENOMEM; > + if (!mlru) { > + mlru = memcg_init_list_lru_one(lru, gfp); > + if (!mlru) > + return -ENOMEM; > + } > xas_set(&xas, pos->kmemcg_id); > do { > xas_lock_irqsave(&xas, flags); > @@ -548,10 +550,11 @@ int memcg_list_lru_alloc(struct mem_cgroup *memcg, struct list_lru *lru, > } > xas_unlock_irqrestore(&xas, flags); > } while (xas_nomem(&xas, gfp)); > - if (mlru) > - kfree(mlru); > } while (pos != memcg && !css_is_dying(&pos->css)); > > + if (unlikely(mlru)) > + kfree(mlru); Yup this looks good. Will unlikely() shutup the warning from bot?