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 C6A90C021BE for ; Wed, 26 Feb 2025 02:09:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 679FC280002; Tue, 25 Feb 2025 21:09:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 62A69280001; Tue, 25 Feb 2025 21:09:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4F24C280002; Tue, 25 Feb 2025 21:09:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 30C65280001 for ; Tue, 25 Feb 2025 21:09:57 -0500 (EST) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id B53541407B5 for ; Wed, 26 Feb 2025 02:09:56 +0000 (UTC) X-FDA: 83160465192.30.D3C874C Received: from mail-pj1-f47.google.com (mail-pj1-f47.google.com [209.85.216.47]) by imf13.hostedemail.com (Postfix) with ESMTP id E61CF20004 for ; Wed, 26 Feb 2025 02:09:54 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=iZjb9ptx; spf=pass (imf13.hostedemail.com: domain of jingxiangzeng.cas@gmail.com designates 209.85.216.47 as permitted sender) smtp.mailfrom=jingxiangzeng.cas@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740535795; 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=nno2UsIqQ7zkFtwHIMnDwlNIxWJyTTkEGIom3dOEG7o=; b=KRq+pGjUBQ9dbMsBvDsrJN+E/tRRi6v5UZwhIMcOO5zoyv/EICQqRKgcuSNeBVwCB67XTH tSdmkaDeMNMWTPYIoy1XjlkYIqWkKpd0tv/UtnEdtVqcjGIf2eZ0et57CJfbYSszK8ZJw1 peK/CB0HBut1aUHlqsiC5HjGaZ4NHJk= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=iZjb9ptx; spf=pass (imf13.hostedemail.com: domain of jingxiangzeng.cas@gmail.com designates 209.85.216.47 as permitted sender) smtp.mailfrom=jingxiangzeng.cas@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740535795; a=rsa-sha256; cv=none; b=0/8im6g0xX3x31tdebns/0w5gmD8QXLLc0MVXxf4CbLpruzEYeCjj9qsCTNhGMsq9qBcA0 mJpY1kwDdh8wOKGotqaq0d9r30iwpjCVWM8uMhiKBObGaj5qJZ3hU2fUo19sKxlZw4C/L3 Oj70RyXNYQbmPRiBGYfd1mohtys6m9g= Received: by mail-pj1-f47.google.com with SMTP id 98e67ed59e1d1-2fc3db58932so9300914a91.2 for ; Tue, 25 Feb 2025 18:09:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740535794; x=1741140594; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=nno2UsIqQ7zkFtwHIMnDwlNIxWJyTTkEGIom3dOEG7o=; b=iZjb9ptx7Gt7Ysl1nc+ooVd1XuElmBJDAOphnSgiSIDJuDXZy46+c4Rdheckfr7zFN sUd3G67Hvkm+EhY0fVzFWlg/6ivJUCQ2pjjOO96Zt1YIxg5z7cuBihO4lXlF75IuZ0iI iSSYFYYWWsB0xkRVF/iTT2nMNqrUQZNkn/7GQDDSWM7NavSqEe3h7EcVgkb/tODJX3N3 S3LLS+XYkWoLysiadNddzdpSyo1wS0cxNRj+Jlt+/JXTnKjHoZYAm9d4IOw1VNXhUXgV vOQn8vrRzNHbi4POntWP9ugGauptKKGZy+FubfMAn6vupMO9ZbA0tctWC1Rxi/bwUZTD uGQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740535794; x=1741140594; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=nno2UsIqQ7zkFtwHIMnDwlNIxWJyTTkEGIom3dOEG7o=; b=EoZe0vVQSwCPtseKh7NgAI+Rkd/I1cF/eJWHuL5W2yYC8jBnPvT+YLpvGP4TgHzo/I kvdRDDL1GR/NSpnWl2U/snjd/gPfOnU+Y1ijRynOE0pUH71tQk7Hvg3Pr0w9FH1vs9O8 mqVtxauy8UxlWfD7osdZlLCA8oko/lPqjqJyOlxGnBP0d3P1w58ztAVN+6xeanxva+uH Fw4b7t2RQeJO2IVpw626igGZdRyXnYuFwZBvPyqn4fhClzDnV8EqFmSDb/XQSlEZvin5 YC26Dx2LhcaUEn9BLqQOwOgQVtjPyOAQDpJfo4b7kCkT28J6Hxbh9zdapNluhRSdcYU/ 6JQQ== X-Forwarded-Encrypted: i=1; AJvYcCW9SCcvLwGDEmwFIyUVE1nL+rCXC6gGhr4FCCCpO6/n9M58rjWLq9UWM7GMSvCsQJMHixpqDYdvbQ==@kvack.org X-Gm-Message-State: AOJu0YzJeZZV9R9TI8vYIs5ao/PIZf3uUmzngHvTw5L2pWABvHfj0Beo iXrc3TNq4YGESyHnSNKkGLq9R+2ilMtThQO5Hz5q93TU94lvLfcYlAAgd4eCsLkCja0IZyKpSlf jJlycGKEnXVJo+W7HdDxQX1r8Cdc= X-Gm-Gg: ASbGnctkfVOJwMqo7zhg3rkeOwrrCiI7gyLWKJSgR997MPxqQofXv3FwX0hYY1RJeA8 21xHGiw0jCmf5QZcKSe48oWmyEMYTMTF0f54VlvZr45u3wAHgNHcTNeRvvgNC9YINa4f3nLZgOi +H7eAlIos= X-Google-Smtp-Source: AGHT+IGpgluIvluDzAXi5w7lMH9Qg8dV62hL0M+5mAySku6cd/ryPORiVALf2OcI1hVcp/avqXfZeh/GBucJakQCCTU= X-Received: by 2002:a17:90b:3e83:b0:2ee:d824:b559 with SMTP id 98e67ed59e1d1-2fce7b04f5fmr31495626a91.28.1740535793699; Tue, 25 Feb 2025 18:09:53 -0800 (PST) MIME-Version: 1.0 References: <20250225153020.2514685-1-jingxiangzeng.cas@gmail.com> <20250226011147.GB1500140@cmpxchg.org> In-Reply-To: <20250226011147.GB1500140@cmpxchg.org> From: jingxiang zeng Date: Wed, 26 Feb 2025 10:09:42 +0800 X-Gm-Features: AWEUYZnG5u8y2buc6xshtTIqweYRtOTonnIgq1ZKt8xBNIMTi7L1GyWqU2Kwj14 Message-ID: Subject: Re: [PATCH] mm/list_lru: make the case where mlru is NULL as unlikely To: Johannes Weiner Cc: Shakeel Butt , 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 Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: E61CF20004 X-Stat-Signature: zai3necypydqpz7r3xwkkqp3io6t99jq X-HE-Tag: 1740535794-101425 X-HE-Meta: U2FsdGVkX1/DtI6RUuMpYi5cw3qONmur/7KaRnuBjn8ggv3Iv3tZLZpL75wm8tFcA7/iS7zZXGltQNX0RPwt1J0cluI+TWxi8LZk66qzCTKuxWJrY0aYOhRsMhmxZceG3tVvtcqogQqTvGBgTe0K6VF4cBG8Bata6OTtQkezTmB+cOSCK484CFipC7szv57JMaqhp4FP+6/FzxU+GskTYCyu/vQaeOyCty8MUMk/pk5B5Or4QuKrBNyf66b80v4o4oLvYiQeEK0GdfCI0rv4XhAYX4lTsNdMNAm1YL4/uRffCAs0XMdozyHtOeJNQF1Ryeg+8Jfm/2lbpXzOpiAbd4xp+fAVw2UytHMcG1zLNo8nyYCwBWoLL9J8uRRD1oGD2KduvFJSdQm4XWqxFVCPHA0EqKr7CyK1YX5J936RhGa/HEQh24Kb3eNPX/rDcHR+X7zW25XK3anGnqR3PYo1J87JrxpJ7I0xK3cMjT6mLYo36Frym0OP/L5wYMTjHDmYU+DoCSUImztSaalBrZvfSPMFkSUBQNqYZ5TR30ZeZdZ3YALtSn00u/3RZVZRhUIHwEzGBUqqvhrBz5/oASzkJE78+x2E3mYJdrE8Ho2wgw+h/7ac35NiplKNEl2TmlsgpO8AszTVCCrMScldAZhS8FxWnhVEaGRl2roLLCpQWoN5bMUWz0FAgfqQ6Ea5yamVcoj2usnAFxkDtep55HEOfEdOuBFhZRvV07oS4VhGQZg4AJvDAYpfbHIutCj7aHq4bgJ6OazCSTGylWU3KoUgdOV25fFBiksclRjQiD+L4OcF0os+nrtvBD9FyIqUbEsHcCMXmNhUynSp6IXiF0c970NaY48K/4+VWI4629gMoNVjyyxgxCr7+W9JjKIXEiQ0D+LbY8unfGtphxwWOxB9cmka+w1O8oh6tctyWf789Qr9YH/fh3RwW02ifOj24ChNXDH/1WtRQ+LicmcGmgL GKe8mgKr IaklYExUr7/eCXA15UoDEXRfH8em9fjG1sz+c9bPfOZ8VNN4klBeBmuV+LTS6W9yAcMMQBy84Q99Mz0hP39EsLplVDPUrS3Sl3p+WBeJN2UaLL1pCgYX8tUoUOfJPNxoAzoxcFYliaZP/1M39h/B55CKhvpE1/EAn9W7juBn2NgLwJGTRoUuCk0A3MjSurfyCKujZZOShDdiDTPHeKSMeRTw82GnISMqR2WC+ssgbWFb/rDiuoV2DNF4aDSagnXI4kV/xlMPEZTumd78PDKCSMW66LL7dCpSAEF+rIoza5Bkn4AfyCGZRbQHw+Gb73Z15PwnVfd1RVptDsG44xmA18Bz8hjZ9KCwqrvQNXYHT8B1q4xaENgFUwmCl6XyQhFivuyvj2/ya8epJjuihpbR7uodskLADY6CAE7UZsFxFqc9HX3JKwB443hZI2siFIHRUNXow2SBKO9pObxSO15ZLvrQ1meth5QkYcwtHrU4SpB4Tupx16hmzWv1b3LoSctSu+9wWIMVgkaX8FUX3wSPsmyHthQ== 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 Wed, 26 Feb 2025 at 09:12, 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? I agree with this implementation; reusing the mlru for the next attempt when the tree insertion fails is more appropriate. > > 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); > + > return xas_error(&xas); > } > #else >