Date:   Tue, 21 Feb 2023 16:21:17 +0100
From:   Michal Hocko <mhocko@suse.com>
To:     Haifeng Xu <haifeng.xu@shopee.com>
Cc:     hannes@cmpxchg.org, shakeelb@google.com, muchun.song@linux.dev,
        akpm@linux-foundation.org, cgroups@vger.kernel.org,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/memcg: Skip high limit check in root memcg
Message-ID: <Y/ThbQNqrdny4+Pf@dhcp22.suse.cz>
References: <20230210094550.5125-1-haifeng.xu@shopee.com>
 <Y+uvRKo7OQ02yB4K@dhcp22.suse.cz>
 <82918a12-d83e-10c0-0e04-eec26657b699@shopee.com>
 <Y/S3GHT1P6awZaPb@dhcp22.suse.cz>
 <99bdfbec-2de4-b432-9649-09557d3f95d6@shopee.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <99bdfbec-2de4-b432-9649-09557d3f95d6@shopee.com>
Precedence: bulk

On Tue 21-02-23 22:21:45, Haifeng Xu wrote:
[...]
> >> 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. 
> 
> This patch doesn't bring any obvious benifit or harm, but the high
> limit check in root memcg seems a little weird.  Maybe we can add this
> check

Well, I do not see the code to look weird TBH. There is nothing wrong in
doing the check for the root memcg. It is a bit pointless but it is not
incorrect.

> It all depends on your viewpoint.

From my POV, I prefer changes that either fix something (correctness
issue or a performance issue/improvement) or improve readbility. The
check doesn't fix anything and I am not convinced about an improved
readabilit either.

Thanks for the patch anyway!
-- 
Michal Hocko
SUSE Labs