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 3EBC2C021B2 for ; Wed, 26 Feb 2025 01:12:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7BA286B0082; Tue, 25 Feb 2025 20:11:59 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 769016B0083; Tue, 25 Feb 2025 20:11:59 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 609B16B0085; Tue, 25 Feb 2025 20:11:59 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 454E66B0082 for ; Tue, 25 Feb 2025 20:11:59 -0500 (EST) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id D097F52281 for ; Wed, 26 Feb 2025 01:11:58 +0000 (UTC) X-FDA: 83160319116.22.8AB5BD5 Received: from mail-qk1-f171.google.com (mail-qk1-f171.google.com [209.85.222.171]) by imf08.hostedemail.com (Postfix) with ESMTP id 7B70C160018 for ; Wed, 26 Feb 2025 01:11:56 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=wDTHQUTw; spf=pass (imf08.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.171 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740532316; 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=N4294A9lkUS95G+GiSGhyB+kZNF5Qkx6Rc1mkwgR/eY=; b=5WJH63p0jS6A5jIxFP83z9ADHIKOMM/H6JYJudLlN0tsQcWdu0JV5faQl5XEpbM/LO838I y+NNbyBdoo93sHfiO/zsrYb+TbHgHAJ8zGjnzxd30hxWj83Q7Lj15bsGOY1SAPmwrNrK2N LNh5Vhsa/2pjK7Xldw1uwZLkfMLJGKY= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=wDTHQUTw; spf=pass (imf08.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.171 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740532316; a=rsa-sha256; cv=none; b=hF4I77q5u6BeUA+/ujjTxBJ7taQPRcLTuPb15ZgBztqWUMke+ERl5EwHuCP6jieKL59utt TrV9f6iZyXOnYb6nW+FAjdeDlQ0wQdO6f+6GlG+u6XmYxwZAn9L0+5uSGbkKtAK4P8lvs2 XEvsJ/lNaxNXBzs+FYDs/RDkyjhiTBI= Received: by mail-qk1-f171.google.com with SMTP id af79cd13be357-7c0a1677aebso572209385a.0 for ; Tue, 25 Feb 2025 17:11:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1740532315; x=1741137115; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=N4294A9lkUS95G+GiSGhyB+kZNF5Qkx6Rc1mkwgR/eY=; b=wDTHQUTw2WYvIl1hq/ffN28BXSJO44pyHJiO/sii3t1FU0XnbQoFC4Avu0qDDLq5LB wMpGnwO+qBaFLZdyb/y+BI9qmbEhvLCdjK7Fmcwy017Z6s48e632opVQ+QwIwhZoUKT+ esjDzFnU6oW068wDUdi0Dr/dh9F5e0uXqC9ov2oLtlhaIz9U+9GpkBCmBdVcf8TSi9M6 CfAY37M6v1Sd+1W/L6jFoTQgHGCMmgbBO5B/8JHHOBe46T36y+9mNafEwa2u2sdI8CVg LW5bUGAETcZ2MRx3h02GRwDyJVHlo0bwaZqMZUjmrMeEcmz+WNeM5+Gr8LiDzD/ULlFs LG0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740532315; x=1741137115; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=N4294A9lkUS95G+GiSGhyB+kZNF5Qkx6Rc1mkwgR/eY=; b=cpK534Tl/J20o9pIjPMK9+6UxOWVWvcn7Za8rrOLGjQagKFEN1mn3yQlLn4fvmYMp9 8wbuX+SKN/fUEJUgvx3d/Gmp28Tax/DVpJ4vnBXOVz1tPapxPj5VPN+YSYaydZjOxjfa reiTziDRoj1It2cI1RaAj/Yqu6s4Mq55UN0cbUAONv7Xx46ufINmG+sqlECBaZaM6Z7/ wK2OrmqJnHckOk4Nbeq7WPR51xTZH4N9BQcpvPHtg8HfTVrVHRzqsf+pP1Spso5ju6jg DyN/n20IJSKRYqUj4uIZ7o0FwdfYjIpftIskZsarb7k1jpOg4gX2WCV78mqH7otmwSHb 52uw== X-Forwarded-Encrypted: i=1; AJvYcCUD29biMF8sWsBGanG6gxqzNuhbuhVA/edgHYz+WTd0iQH0RzfcNOFFsczeq4LIgAUGEV8AcA4kWg==@kvack.org X-Gm-Message-State: AOJu0YyddBPNi1/CkxD3LPyauiujatNvHz786YIhltCCPAkuJf88bRRl O/LB2mZslBDibM7eZq64lnPA/YvaIqGxs8CmZsp6KsMCZ2RCOk0IdcI1L3KQG+Y= X-Gm-Gg: ASbGncsbGczV8gy2stymmXGyyk2vHus41s8Q8ohPuduc8h0GGVQC26rnCOMcDyQIeSr ARWzLxVqvuAZ98xhScuNyv7R9UPnd4STItqPJfhsQqGKj1qMDD7HCUoVNp4huLV30IkHGAJpjXi iQ8q2PAwoZs6j4GnVqzp1Zzmbn92aFl7ZGq4KAvcGE2R+5PGi8w6hYkYDMfD7zjjpVFwbJzdLKa kdLuTql/Cf/TKQoP/lXyttTiKy6Gz5gumKOoT36sKeWAQ2zhs5WTVYC8nQxrocxLa7DAN6GlLGH DPhaKmkl96ewIFm8IhJzGyjT X-Google-Smtp-Source: AGHT+IHCTAK5XD3qgMp2AKnEBbidhjlTj4lmm67TP+Mi0O1W4Hx/apcG2TWs1qyRUcmeuJpvyVF9Pw== X-Received: by 2002:a05:620a:4393:b0:7c0:add8:175d with SMTP id af79cd13be357-7c0cf8ce29emr2908846185a.18.1740532315454; Tue, 25 Feb 2025 17:11:55 -0800 (PST) Received: from localhost ([2603:7000:c01:2716:da5e:d3ff:fee7:26e7]) by smtp.gmail.com with UTF8SMTPSA id af79cd13be357-7c23c33f84asm178291185a.111.2025.02.25.17.11.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Feb 2025 17:11:52 -0800 (PST) Date: Tue, 25 Feb 2025 20:11:47 -0500 From: Johannes Weiner To: Shakeel Butt 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: <20250226011147.GB1500140@cmpxchg.org> References: <20250225153020.2514685-1-jingxiangzeng.cas@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Queue-Id: 7B70C160018 X-Stat-Signature: 56o9168h77xbmxw9mwxwmhcao5oezm7d X-Rspamd-Server: rspam03 X-HE-Tag: 1740532316-939377 X-HE-Meta: U2FsdGVkX19PBunvtncIOJwd+phJKrUC3msdExXTaBhNQxNPJVsFIPsGE/nnPKzYnvJ+jTYfOHhny+zDwoQgWAs2uk6jtCWtXLQDMIXCeeAfHixqK6sWHkGy6sKbixSu6kEP2oNrQgXUdMyGX2LtSJMILVIP+c2KCec/x5KhjzJBjranX/AyGSkX5kyob6r2hfMTwAPz7lub+6ZWl7ICLUKtFiFozUmDTEl8BpU/djy1huUrAfglLN3fZDrlcButjyWEeLw8CflWigkvuSgyhqED5RG8hfuxxxr6ULvI9S5LfL+O57Uwf60AkuM1spS1/pcEoUntzXrPZQhGjrCYGjPTYftV4Jr58La73jZ3kfQlUiotFMJr6FvWzg2/Pz2pCY75C4cqnGY+KUHCZWjQhsmdjVtZFAfQJBleISdgm4U3/Rx1sg2xPEW4PlqkGohzGUaun1M/zEQMEgWjEpivfOwd6yX2KhGvnHnntvMxPg/FTdEVICWwENoX7G9Kfq5ebryfLYga8Ly/XwKoMsMfjlNtsP5+Ehqmw2rIIvUQn77PviDxx5XwyMKCUAynbGhkOpHX4BEIv6onSsLCH4GynbZUSJjjsPnweCetmu7TsJ6bG9Fx//zXFLBv3784WN0ycvym4ZAF4OVCoJuoJlTJtP1WvEX8O4qNQohgzowAQxV1m0MY7QMOhNjWks3K377bli1TZAxsYLGj31Lnw5J983rGEF9zrOnFrt4XTBnqZYKoj9qLGmRdQWlU56EMfMEkmb9yEISKjGte5q9O7VueeduS24BthmDeki+6hIiwc58W/eKAHv91V9j24Z3TifHMtzH3jLAxfbjNeZWKcLq74Lv++WRylWTlfGGGfQUBhTh10qrRKwXOGesJ210MiUdg+fyCjSQGYba2+63Mk0Tp3P0p0dB99K5Hj/JviszwfjHdtJyyj7q6sN1f9K35WM2rMVY0d5HbYYUIvQ57Xts 9sURxyjv z6qc9Y/2Fwt9FrUFWPBSt7HC7y1DClkHYpF8whibdLwBe8TfBHYZakf0NKjktFP9kezkDKWG/zvy5uzPvuFnsfWzT1yF7qXhcbPYJkaFMUMNGzC61aPtwdpkYkqI/SOTiLiF2aSh/M9p1hGeK27RBjo1iDF1mAFZ2uQj1quLUTgp9xAYztmMVTX5Yu6Ae2jZiFOYbAXJ7MgQ3hSZ7i/qOTnzFX5eBVGF0np4hs1VW3Uut9HyQ1cAr++TfOmLPAGGELhtQNpEozx7Yt9w4pDs8NInU6L7v2hx8arZDZbUgzig2wa5hM9ZgjHgORj3EkVd0MoBFc3w4m8AKvliZ2CnyBGOnWK2HDXVFfGTrkpp6z9Jhp06xmf49ZC6pSm9YOqVqger+b0sNk8JkAOQC26d2iazdQGTR9RKJ21/C/DubT869GdJH5a8cW7xNIS/9Fm4/HzN8UbTAg2HPvvhO0nvtCHSrUyx01xkFayziMn9leeLc7w+aTHFXE9bWz7r0qpDlQ2mogF0dA8wAJk8LUrn5mXCfTJW4EBfgaHxbjJWKtc9AMX2hxoYTCDzVO/QiHmDC5lafpR9ZSX3E8ps= 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: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); + return xas_error(&xas); } #else