Message ID | 20230210094550.5125-1-haifeng.xu@shopee.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp861288wrn; Fri, 10 Feb 2023 01:58:25 -0800 (PST) X-Google-Smtp-Source: AK7set94Vq/i7MGDlCEYNCLWso+n8jDcW3mmKeYq/jRC+pAGhDysAV7GggQX3nr9yvj44cRDsYeQ X-Received: by 2002:a17:90b:3b82:b0:230:ae93:2677 with SMTP id pc2-20020a17090b3b8200b00230ae932677mr15317362pjb.29.1676023105043; Fri, 10 Feb 2023 01:58:25 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1676023105; cv=none; d=google.com; s=arc-20160816; b=y7MRAFYfR202wA78KkGU/weTSvKaiIpH4rYbubVVvx2UYErKzehssYEblJB/XZo4aV j3N70XdAu5DBN+IO0qtV0xwEAzBcZAjCHAw4SKXCqJ+D/9Z8OvhuX8kgyzrILQFQfp22 8ikdq52vdAqe6aPHruf/iNNkpHHOhER755ghC2w6C52G4auQy7LQxSOGmEwYhnnaTYYX I9SSOUvxNSShxFm4AHPiwFAl933UuNBCDoCFsqIr8GWhBQKlfbUfBefGFkfg0aSgxu7e sKVaAstQCRy9e2ocEK+0sX4OXRsJG/kzQErjOLpZWbfbaRfkptj0at4IUYAepA8ubIym F94g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=ppfFWpSAcZ1aESEIi3DI7xxDUI8KrBz5/PkXTDog9ro=; b=w6tgnQCC1SYDy28A55d0sR3262FXp0wYSHEAktTK93vpFA+2PYTHtc82UZcIYOqxKS Bj91r5tt+LeHcR0Ib5B7eW01C8fSPvzzmqUiN8yw+kZqpsyRxtyv7qzMv0kMOIrnHinH 7bg86EMvvubuB9KIhvaV16j0sS5z6WRX+maZfZmUoXt5DIUzlIjkwTts7JCZ2/wMqp6Y SoVWeSga3A12Xjn8dXY0BmHFlKrLjjx8FSljlrR7qnpJDP7tOaLbOiunq1YxYmpBoeW3 5uDo8f96ST+fZrW5Jy1jIBhn2P5fNO8ClGSANbx6ZvReXOcp0jawpddwDAs+r//n8tSH m5dw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@shopee.com header.s=shopee.com header.b=LWCJXAT7; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=shopee.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id c14-20020a17090a674e00b00232cc8a3603si4400465pjm.112.2023.02.10.01.58.12; Fri, 10 Feb 2023 01:58:25 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@shopee.com header.s=shopee.com header.b=LWCJXAT7; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=shopee.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231774AbjBJJq2 (ORCPT <rfc822;ybw1215001957@gmail.com> + 99 others); Fri, 10 Feb 2023 04:46:28 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41704 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231379AbjBJJq0 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 10 Feb 2023 04:46:26 -0500 Received: from mail-pj1-x102d.google.com (mail-pj1-x102d.google.com [IPv6:2607:f8b0:4864:20::102d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8756E64641 for <linux-kernel@vger.kernel.org>; Fri, 10 Feb 2023 01:46:25 -0800 (PST) Received: by mail-pj1-x102d.google.com with SMTP id gj9-20020a17090b108900b0023114156d36so8776409pjb.4 for <linux-kernel@vger.kernel.org>; Fri, 10 Feb 2023 01:46:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=shopee.com; s=shopee.com; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=ppfFWpSAcZ1aESEIi3DI7xxDUI8KrBz5/PkXTDog9ro=; b=LWCJXAT7I+EECVCLtdt5TrDfY1jP64bH1PN4m10e5ID9k8mohRc90Id/D9P5YQ0zo8 tpNUNA/I+Y2qw21VFtzvbQq/iFU1JdeU5uWGqhcnZwqEX8DlWw+irAGC01fg4inhfNUj qfVuPkwbTvxBq/JFjbEKnDjL/vfK4QSLJHUBBrCzAQJAmSckmSmlND9nVCiYZS9FwxM7 +mUz9uyIMg8cibzSLzifH+bR9e0N1BIuhqHHXFjp60clgMKLA9obumlPzfdkCI9SomZS xSKsStJ8KUfMd4aBExGiaG0EOb+1TG/W36cWikejp1fQnLVQm6sVf36/6dWb00RBzrR7 nhUw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ppfFWpSAcZ1aESEIi3DI7xxDUI8KrBz5/PkXTDog9ro=; b=ztHhFiPz6/aC69ON41gV5kRo5CEKUvBp2DZJs3jmKnSOnK7AOxoS5SGgrkmEuHuXPX jqeJQ8tYqHf0COpWVo5qPwFAW/cN5RQFeAJTWiqEelPDlAa2Bl9idClJ3FEKO9JtWLxS N7OzoE8KDdB+XVaDnM2Igz9dIAzb0sD88dHCpyyHLFzx+Mvrefhd8gBxsGy9NyCuJKzJ oj5OnnCy7PdRcMPRhAhEToDZKMH3fhUevN26O/qXHxoQ/IMe3K//v1B7wKM7LBR+6e4Z ZQw8cct7aDRTArFjuN3HrEDTMCVxEnmTrtIsdG2LuHne1UIvdefm5DRgDwzFX2wcyIPa ZDHA== X-Gm-Message-State: AO0yUKWCLwxeVBfBSaFfdGnr+K7hH2bq7ewzlMymKICvwWSp/FOjpNQA jGrWY/RyCkwqChkQ0S+AdjtXag== X-Received: by 2002:a17:902:d48e:b0:199:472b:927f with SMTP id c14-20020a170902d48e00b00199472b927fmr10878196plg.51.1676022384998; Fri, 10 Feb 2023 01:46:24 -0800 (PST) Received: from ubuntu-haifeng.default.svc.cluster.local ([101.127.248.173]) by smtp.gmail.com with ESMTPSA id p11-20020a170902a40b00b0019460ac7c6asm2935186plq.283.2023.02.10.01.46.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Feb 2023 01:46:24 -0800 (PST) From: Haifeng Xu <haifeng.xu@shopee.com> To: hannes@cmpxchg.org Cc: mhocko@kernel.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, Haifeng Xu <haifeng.xu@shopee.com> Subject: [PATCH] mm/memcg: Skip high limit check in root memcg Date: Fri, 10 Feb 2023 09:45:50 +0000 Message-Id: <20230210094550.5125-1-haifeng.xu@shopee.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1757437602930104803?= X-GMAIL-MSGID: =?utf-8?q?1757437602930104803?= |
Series |
mm/memcg: Skip high limit check in root memcg
|
|
Commit Message
Haifeng Xu
Feb. 10, 2023, 9:45 a.m. UTC
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.
Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
---
mm/memcontrol.c | 4 ++++
1 file changed, 4 insertions(+)
Comments
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@shopee.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
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@shopee.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. #include <sys/mman.h> #include <sys/types.h> #include <unistd.h> #include <stdlib.h> #include <stdio.h> #include <fcntl.h> #include <ctype.h> #include <string.h> #include <inttypes.h> #define SIZE (5 * 1024 * 1024 * 1024) int64_t current_time_ms() { struct timeval time; gettimeofday(&time, NULL); int64_t s1 = (int64_t)(time.tv_sec) * 1000; int64_t s2 = (time.tv_usec / 1000); return s1 + s2; } int main(int argc, char* argv[]) { void * buf; size_t size = SIZE; int64_t start, cost; buf = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, 0, 0); if (buf < 0 ) { printf("mmap failed\n"); exit(-1); } start = current_time_ms(); mlock(buf, size); cost = current_time_ms() - start; printf("cost: %" PRId64 " ms\n", cost); munmap(buf, size); return 0; }
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@shopee.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.
On 2023/2/21 20:20, Michal Hocko wrote: > 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@shopee.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. 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?It all depends on your viewpoint. Thanks.
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!
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) >