From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-130.freemail.mail.aliyun.com (out30-130.freemail.mail.aliyun.com [115.124.30.130]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8136D2572 for ; Fri, 27 Dec 2024 02:03:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.130 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735265035; cv=none; b=qf+OJuxnLTU7TL+EDNQN+miNN7cYospUDw26EOIzLmwfp9wB+O8ivxlz01OuLWDri0OnYYc6ZC//JCpZc7/C+choxD3JZZDRVbJSLY0F6643ezOw+IWl1FwFtck+rGoFLzLxtfYxD8gCP2HIHiOJIacu8m8YV5iMecbNXmGkfCs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735265035; c=relaxed/simple; bh=ymrB2mcSsB0b72l5klWjc4tFpng1Prx9mE2W+VepG3U=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=dUheW92dS+IZ6ELA64C/AVZgQ6CB09HonnxfYVn4ZgCF9epNSV+AFWZlKMmx53nPnOuc8z77/Bmrwv7EvW15Fv4g35/M4lFS22Fi7Wff94hFmE6pq6CHHpHGxXbjQqRqhANK2/XReu9lZSzps8lilAZjp0uDGWBBtYLkGCV1BgQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=bKf+qzj7; arc=none smtp.client-ip=115.124.30.130 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="bKf+qzj7" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1735265031; h=From:To:Subject:Date:Message-ID:MIME-Version:Content-Type; bh=o7eiyI8du7vlvyw9sk7wyWLJDvC8p8O7KuzjSF30GjU=; b=bKf+qzj7OK8r20Wh4NcMSYn5SdzE+WuZu4rZZVuXbtCLpldZSHPUMEPM5haX5NKs39/Xm6WAiBdnv0Tc2KfI2uIVyI4ovMAiqoJbLHzURn002/tbPHkBufmjfbY1HsTMukAEBCQYqoPM8ugMM5xyLZCZD4MHi1JTDYyc2pvnHVE= Received: from DESKTOP-5N7EMDA(mailfrom:ying.huang@linux.alibaba.com fp:SMTPD_---0WMJXw0r_1735265029 cluster:ay36) by smtp.aliyun-inc.com; Fri, 27 Dec 2024 10:03:50 +0800 From: "Huang, Ying" To: Kairui Song Cc: linux-mm@kvack.org, Andrew Morton , Chris Li , Hugh Dickins , Yosry Ahmed , Roman Gushchin , Shakeel Butt , Johannes Weiner , Barry Song , Michal Hocko , linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 1/4] mm, memcontrol: avoid duplicated memcg enable check In-Reply-To: (Kairui Song's message of "Sun, 22 Dec 2024 22:51:56 +0800") References: <20241218114633.85196-1-ryncsn@gmail.com> <20241218114633.85196-2-ryncsn@gmail.com> <87ed1zj10t.fsf@DESKTOP-5N7EMDA> Date: Fri, 27 Dec 2024 10:03:49 +0800 Message-ID: <875xn5hofu.fsf@DESKTOP-5N7EMDA> User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Kairui Song writes: > On Sun, Dec 22, 2024 at 9:33=E2=80=AFPM Huang, Ying > wrote: >> >> Hi, Kairui, > > Hi Ying, > >> >> Sorry for jumping in so late. >> >> Kairui Song writes: >> >> > From: Kairui Song >> > >> > mem_cgroup_uncharge_swap() includes a mem_cgroup_disabled() check, >> > so the caller doesn't need to check that. >> > >> > Signed-off-by: Kairui Song >> > Reviewed-by: Yosry Ahmed >> > Reviewed-by: Roman Gushchin >> > Acked-by: Shakeel Butt >> > Acked-by: Chris Li >> > --- >> > mm/memcontrol.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> > index 7b3503d12aaf..79900a486ed1 100644 >> > --- a/mm/memcontrol.c >> > +++ b/mm/memcontrol.c >> > @@ -4609,7 +4609,7 @@ void mem_cgroup_swapin_uncharge_swap(swp_entry_t= entry, unsigned int nr_pages) >> > * correspond 1:1 to page and swap slot lifetimes: we charge the >> > * page to memory here, and uncharge swap when the slot is freed. >> > */ >> > - if (!mem_cgroup_disabled() && do_memsw_account()) { >> > + if (do_memsw_account()) { >> > /* >> > * The swap entry might not get freed for a long time, >> > * let's not wait for it. The page already received a >> >> I take a look at memcontrol.c, it appears that almost all extern >> functions check mem_cgroup_disabled() as the first step. > > Hmm, just checked memcontrol.c and I saw quite a few extern functions > not doing that, so I think that's not a convention. I still think that it's a good idea to check whether memcg is disabled in the outermost interfaces instead of being buried in some internal functions. >> that this is a convention of memcontrol.c? And the benefit of the >> change is minimal. In contrast, if someone makes more changes to >> mem_cgroup_swapin_uncharge_swap() in the future, he may forget to add >> this back. So, it may be unnecessary to make the change? > > This change is minimal indeed, it only helps to remove a few unneeded > nop, still a gain though. The benefit is minimal too. > I think mem_cgroup_swapin_uncharge_swap should fade away in the future, Good. Then, we don't need to optimize it too. Just let it fade away. > it's only for Cgroup V1, and it's a really simple function, just a > wrapper for mem_cgroup_uncharge_swap, so I think this is not a > problem? > > If you are concerned about this, this patch can be dropped from this > series, rest of the patches still work the same. Just my 2 cents. --- Best Regards, Huang, Ying