linux-kernel - Re: [PATCH] mm/memcg: Skip high limit check in root memcg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y/S3GHT1P6awZaPb@dhcp22.suse.cz>
Date:   Tue, 21 Feb 2023 13:20:40 +0100
From:   Michal Hocko <mhocko@...e.com>
To:     Haifeng Xu <haifeng.xu@...pee.com>
Cc:     hannes@...xchg.org, shakeelb@...gle.com, muchun.song@...ux.dev,
        akpm@...ux-foundation.org, cgroups@...r.kernel.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/memcg: Skip high limit check in root memcg

On Tue 21-02-23 18:29:39, Haifeng Xu wrote:
> 
> 
> On 2023/2/14 23:56, Michal Hocko wrote:
> > On Fri 10-02-23 09:45:50, Haifeng Xu wrote:
> >> The high limit checks the memory usage from given memcg to root memcg.
> >> However, there is no limit in root memcg. So this check makes no sense
> >> and we can ignore it.
> > 
> > Is this check actually addining any benefit? Have you measured aby
> > performance gains by this change?
> > 
> >> Signed-off-by: Haifeng Xu <haifeng.xu@...pee.com>
> >> ---
> >>  mm/memcontrol.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> >> index 73afff8062f9..a31a56598f29 100644
> >> --- a/mm/memcontrol.c
> >> +++ b/mm/memcontrol.c
> >> @@ -2780,6 +2780,10 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
> >>  	do {
> >>  		bool mem_high, swap_high;
> >>  
> >> +		/* There is no need for root memcg to check high limit */
> >> +		if (mem_cgroup_is_root(memcg))
> >> +			break;
> >> +
> >>  		mem_high = page_counter_read(&memcg->memory) >
> >>  			READ_ONCE(memcg->memory.high);
> >>  		swap_high = page_counter_read(&memcg->swap) >
> >> -- 
> >> 2.25.1
> > 
> 
> test steps:
> 1. mkdir -p /sys/fs/cgroup/memory/test
> 2. echo $$ > /sys/fs/cgroup/memory/test/cgroup.procs
> 3. ./mmap_test
> 
> The test result show that with or without the patch, the time taken is almost the same.

This is in line with my expectation. So the question is whether the
additional check is really worth it. 
-- 
Michal Hocko
SUSE Labs