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 F38EAC4332F for ; Fri, 2 Dec 2022 04:33:06 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 49FDE6B0071; Thu, 1 Dec 2022 23:33:06 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 428D26B0073; Thu, 1 Dec 2022 23:33:06 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2A1E36B0074; Thu, 1 Dec 2022 23:33:06 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 12B776B0071 for ; Thu, 1 Dec 2022 23:33:06 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id DC2E714031A for ; Fri, 2 Dec 2022 04:33:05 +0000 (UTC) X-FDA: 80196096330.26.CF82815 Received: from mail-vs1-f50.google.com (mail-vs1-f50.google.com [209.85.217.50]) by imf15.hostedemail.com (Postfix) with ESMTP id 7A09CA000F for ; Fri, 2 Dec 2022 04:33:05 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=WOBSqpfG; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf15.hostedemail.com: domain of almasrymina@google.com designates 209.85.217.50 as permitted sender) smtp.mailfrom=almasrymina@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1669955585; a=rsa-sha256; cv=none; b=K/PLizNGtaCCL38DSfRWt7KrcgfVdxzLWIYxtCB0DVEfwXOi6Dg5FDgmQjWsW10S3ZF/Le rUEGtoOgtNYxO3Y+w6ZTWbxs6gDBIN5ane6eJVJyWRrRBo4VZBhXM0nB501pi3iM26FIod eFoNAP7s2FEBvCuDxZ/9oLQz9VzhrGo= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=WOBSqpfG; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf15.hostedemail.com: domain of almasrymina@google.com designates 209.85.217.50 as permitted sender) smtp.mailfrom=almasrymina@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1669955585; 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=BsPgWVv1RWT5qYX/bABkuIpT4gW3nAMII43+pjAqwPI=; b=nkqklbPLiCe5x4aZ1EwMks8L1MCsr3a0/lZzVQz1dr9Vcce0UpeJ1Om49G68lceDUfLcWa 0z7qOeFXp25fOmH52kWNb5j2Lnz8XOE4PLFAYkfzWsvA3L5o1bi0FkJD1MHFPlhjmeUVUp 84oFjHPHF1Cgseu6+KPiPSGwRt6yILM= Received: by mail-vs1-f50.google.com with SMTP id k17so3612440vsr.10 for ; Thu, 01 Dec 2022 20:33:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=BsPgWVv1RWT5qYX/bABkuIpT4gW3nAMII43+pjAqwPI=; b=WOBSqpfGr1BpEGTu+3IgFmv1HYfDcMH2yThHtySw/i+6o/W3cgtYc+oPSeq2g+6SLp FYjkAznLrzEWuYDau3m+pLHUUyaic8mXryvdKzOX2Pj3qTsuzhD5PH9koCYHrkDd8HOh sAmPuFF/fKZl/rczWrK3C5T3YjZvKk2wOyTpLa3L+ULbTnfLf8+b/KWWfV8lAhUwFmgG 9NJfqF3IqZdnSX3cfg6f1vbDOX2s9G+rk40gqXh7DDXlYlbdnj/+rCjeWN9TH4PPumNI kIpHwMdydVz6BYWHRdl4bjT56stk7pQPsTruFT7CLKtJIq9iM28j8UH9pOs9G3Vi1Bcu D62Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=BsPgWVv1RWT5qYX/bABkuIpT4gW3nAMII43+pjAqwPI=; b=uZoG51/a0KO2d5kFu79Xtj27qtvCA66iuEJH0F+pUDE1fTZ1gtvTSlKQ4NAivboHiQ oJ+4MT3HyArXlBx4UMHm+RFqqbuOQcG25LZWvpoiwl3VgoI8bUaS1v6ljoUoX7pB0OR+ nxxkJxXtiwfh4JjDDaZNy35lvjsuzrm/+Bli/qiUoNc7P3llNO/pkryrIY7xVr3UU6cY hvfhQHEuImfzZUQRqs2dOS0GOkW5ikgTVocJU9sTucON61cOR5qZiGD1YwtM5nQBiHMx lMsffF3l2lFeYvrviR64SbqJFjZTNCaQFkyltIuj1lBlub/4anMehPgqX/2wv5stEDoG 8Fow== X-Gm-Message-State: ANoB5pkak40o1mxauHdoSBs3jWsovMwSrQihUdaJkEO1ebN4M5UDb0bz 0G7T/Wr2t3l2EjtG8MGQltku65o4nZAyYjz85gzRoQ== X-Google-Smtp-Source: AA0mqf5eWzRe7uM+6YKE7NiqUC1sL1QlbxS6UZ0K2bEaOV0qa419bYuY7f61mRIMLnZLkbnyomyMVT0wBiKLF/0FVpY= X-Received: by 2002:a05:6102:1c5:b0:3b0:4f9b:238d with SMTP id s5-20020a05610201c500b003b04f9b238dmr28938221vsq.59.1669955584420; Thu, 01 Dec 2022 20:33:04 -0800 (PST) MIME-Version: 1.0 References: <20221130020328.1009347-1-almasrymina@google.com> <878rjq32rr.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <878rjq32rr.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Mina Almasry Date: Thu, 1 Dec 2022 20:32:53 -0800 Message-ID: Subject: Re: [RFC PATCH v2] mm: Add nodes= arg to memory.reclaim To: "Huang, Ying" Cc: Yang Shi , Yosry Ahmed , Tim Chen , weixugc@google.com, shakeelb@google.com, gthelen@google.com, fvdl@google.com, Tejun Heo , Zefan Li , Johannes Weiner , Jonathan Corbet , Michal Hocko , Roman Gushchin , Muchun Song , Andrew Morton , cgroups@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 7A09CA000F X-Stat-Signature: p83tgubizj97zcmp4ocdbo8tcwkmfq85 X-Spamd-Result: default: False [0.10 / 9.00]; SORBS_IRL_BL(3.00)[209.85.217.50:from]; BAYES_HAM(-3.00)[100.00%]; BAD_REP_POLICIES(0.10)[]; RCVD_NO_TLS_LAST(0.10)[]; MIME_GOOD(-0.10)[text/plain]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; FROM_EQ_ENVFROM(0.00)[]; DMARC_POLICY_ALLOW(0.00)[google.com,reject]; RCPT_COUNT_TWELVE(0.00)[20]; DKIM_TRACE(0.00)[google.com:+]; TO_MATCH_ENVRCPT_SOME(0.00)[]; PREVIOUSLY_DELIVERED(0.00)[linux-mm@kvack.org]; R_DKIM_ALLOW(0.00)[google.com:s=20210112]; ARC_SIGNED(0.00)[hostedemail.com:s=arc-20220608:i=1]; FROM_HAS_DN(0.00)[]; R_SPF_ALLOW(0.00)[+ip4:209.85.128.0/17]; TO_DN_SOME(0.00)[]; ARC_NA(0.00)[] X-Rspam-User: X-HE-Tag: 1669955585-799241 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: On Thu, Dec 1, 2022 at 7:26 PM Huang, Ying wrote: > > Mina Almasry writes: > > > The nodes= arg instructs the kernel to only scan the given nodes for > > proactive reclaim. For example use cases, consider a 2 tier memory system: > > > > nodes 0,1 -> top tier > > nodes 2,3 -> second tier > > > > $ echo "1m nodes=0" > memory.reclaim > > > > This instructs the kernel to attempt to reclaim 1m memory from node 0. > > Since node 0 is a top tier node, demotion will be attempted first. This > > is useful to direct proactive reclaim to specific nodes that are under > > pressure. > > > > $ echo "1m nodes=2,3" > memory.reclaim > > > > This instructs the kernel to attempt to reclaim 1m memory in the second tier, > > since this tier of memory has no demotion targets the memory will be > > reclaimed. > > > > $ echo "1m nodes=0,1" > memory.reclaim > > > > Instructs the kernel to reclaim memory from the top tier nodes, which can > > be desirable according to the userspace policy if there is pressure on > > the top tiers. Since these nodes have demotion targets, the kernel will > > attempt demotion first. > > > > Since commit 3f1509c57b1b ("Revert "mm/vmscan: never demote for memcg > > reclaim""), the proactive reclaim interface memory.reclaim does both > > reclaim and demotion. Reclaim and demotion incur different latency costs > > to the jobs in the cgroup. Demoted memory would still be addressable > > by the userspace at a higher latency, but reclaimed memory would need to > > incur a pagefault. > > > > The 'nodes' arg is useful to allow the userspace to control demotion > > and reclaim independently according to its policy: if the memory.reclaim > > is called on a node with demotion targets, it will attempt demotion first; > > if it is called on a node without demotion targets, it will only attempt > > reclaim. > > > > Signed-off-by: Mina Almasry > > --- > > Documentation/admin-guide/cgroup-v2.rst | 15 +++--- > > include/linux/swap.h | 3 +- > > mm/memcontrol.c | 67 ++++++++++++++++++++----- > > mm/vmscan.c | 4 +- > > 4 files changed, 68 insertions(+), 21 deletions(-) > > > > diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst > > index 74cec76be9f2..ac5fcbcd5ae6 100644 > > --- a/Documentation/admin-guide/cgroup-v2.rst > > +++ b/Documentation/admin-guide/cgroup-v2.rst > > @@ -1245,17 +1245,13 @@ PAGE_SIZE multiple when read back. > > This is a simple interface to trigger memory reclaim in the > > target cgroup. > > > > - This file accepts a single key, the number of bytes to reclaim. > > - No nested keys are currently supported. > > + This file accepts a string which contains the number of bytes to > > + reclaim. > > > > Example:: > > > > echo "1G" > memory.reclaim > > > > - The interface can be later extended with nested keys to > > - configure the reclaim behavior. For example, specify the > > - type of memory to reclaim from (anon, file, ..). > > - > > Please note that the kernel can over or under reclaim from > > the target cgroup. If less bytes are reclaimed than the > > specified amount, -EAGAIN is returned. > > @@ -1267,6 +1263,13 @@ PAGE_SIZE multiple when read back. > > This means that the networking layer will not adapt based on > > reclaim induced by memory.reclaim. > > > > + This file also allows the user to specify the nodes to reclaim from, > > + via the 'nodes=' key, example:: > > + > > + echo "1G nodes=0,1" > memory.reclaim > > + > > + The above instructs the kernel to reclaim memory from nodes 0,1. > > + > > memory.peak > > A read-only single value file which exists on non-root > > cgroups. > > diff --git a/include/linux/swap.h b/include/linux/swap.h > > index b61e2007d156..f542c114dffd 100644 > > --- a/include/linux/swap.h > > +++ b/include/linux/swap.h > > @@ -419,7 +419,8 @@ extern unsigned long try_to_free_pages(struct zonelist *zonelist, int order, > > extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg, > > unsigned long nr_pages, > > gfp_t gfp_mask, > > - unsigned int reclaim_options); > > + unsigned int reclaim_options, > > + nodemask_t nodemask); > > extern unsigned long mem_cgroup_shrink_node(struct mem_cgroup *mem, > > gfp_t gfp_mask, bool noswap, > > pg_data_t *pgdat, > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 23750cec0036..a0d7850173a9 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -63,6 +63,7 @@ > > #include > > #include > > #include > > +#include > > #include "internal.h" > > #include > > #include > > @@ -2392,7 +2393,8 @@ static unsigned long reclaim_high(struct mem_cgroup *memcg, > > psi_memstall_enter(&pflags); > > nr_reclaimed += try_to_free_mem_cgroup_pages(memcg, nr_pages, > > gfp_mask, > > - MEMCG_RECLAIM_MAY_SWAP); > > + MEMCG_RECLAIM_MAY_SWAP, > > + NODE_MASK_ALL); > > psi_memstall_leave(&pflags); > > } while ((memcg = parent_mem_cgroup(memcg)) && > > !mem_cgroup_is_root(memcg)); > > @@ -2683,7 +2685,8 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask, > > > > psi_memstall_enter(&pflags); > > nr_reclaimed = try_to_free_mem_cgroup_pages(mem_over_limit, nr_pages, > > - gfp_mask, reclaim_options); > > + gfp_mask, reclaim_options, > > + NODE_MASK_ALL); > > psi_memstall_leave(&pflags); > > > > if (mem_cgroup_margin(mem_over_limit) >= nr_pages) > > @@ -3503,7 +3506,8 @@ static int mem_cgroup_resize_max(struct mem_cgroup *memcg, > > } > > > > if (!try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, > > - memsw ? 0 : MEMCG_RECLAIM_MAY_SWAP)) { > > + memsw ? 0 : MEMCG_RECLAIM_MAY_SWAP, > > + NODE_MASK_ALL)) { > > ret = -EBUSY; > > break; > > } > > @@ -3614,7 +3618,8 @@ static int mem_cgroup_force_empty(struct mem_cgroup *memcg) > > return -EINTR; > > > > if (!try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, > > - MEMCG_RECLAIM_MAY_SWAP)) > > + MEMCG_RECLAIM_MAY_SWAP, > > + NODE_MASK_ALL)) > > nr_retries--; > > } > > > > @@ -6407,7 +6412,8 @@ static ssize_t memory_high_write(struct kernfs_open_file *of, > > } > > > > reclaimed = try_to_free_mem_cgroup_pages(memcg, nr_pages - high, > > - GFP_KERNEL, MEMCG_RECLAIM_MAY_SWAP); > > + GFP_KERNEL, MEMCG_RECLAIM_MAY_SWAP, > > + NODE_MASK_ALL); > > > > if (!reclaimed && !nr_retries--) > > break; > > @@ -6456,7 +6462,8 @@ static ssize_t memory_max_write(struct kernfs_open_file *of, > > > > if (nr_reclaims) { > > if (!try_to_free_mem_cgroup_pages(memcg, nr_pages - max, > > - GFP_KERNEL, MEMCG_RECLAIM_MAY_SWAP)) > > + GFP_KERNEL, MEMCG_RECLAIM_MAY_SWAP, > > + NODE_MASK_ALL)) > > nr_reclaims--; > > continue; > > } > > @@ -6579,21 +6586,54 @@ static ssize_t memory_oom_group_write(struct kernfs_open_file *of, > > return nbytes; > > } > > > > +enum { > > + MEMORY_RECLAIM_NODES = 0, > > + MEMORY_RECLAIM_NULL, > > +}; > > + > > +static const match_table_t if_tokens = { > > + { MEMORY_RECLAIM_NODES, "nodes=%s" }, > > + { MEMORY_RECLAIM_NULL, NULL }, > > +}; > > + > > static ssize_t memory_reclaim(struct kernfs_open_file *of, char *buf, > > size_t nbytes, loff_t off) > > { > > struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of)); > > unsigned int nr_retries = MAX_RECLAIM_RETRIES; > > unsigned long nr_to_reclaim, nr_reclaimed = 0; > > - unsigned int reclaim_options; > > - int err; > > + unsigned int reclaim_options = MEMCG_RECLAIM_MAY_SWAP | > > + MEMCG_RECLAIM_PROACTIVE; > > + char *old_buf, *start; > > + substring_t args[MAX_OPT_ARGS]; > > + int token; > > + char value[256]; > > + nodemask_t nodemask = NODE_MASK_ALL; > > > > buf = strstrip(buf); > > - err = page_counter_memparse(buf, "", &nr_to_reclaim); > > - if (err) > > - return err; > > > > - reclaim_options = MEMCG_RECLAIM_MAY_SWAP | MEMCG_RECLAIM_PROACTIVE; > > + old_buf = buf; > > + nr_to_reclaim = memparse(buf, &buf) / PAGE_SIZE; > > + if (buf == old_buf) > > + return -EINVAL; > > + > > + buf = strstrip(buf); > > + > > + while ((start = strsep(&buf, " ")) != NULL) { > > + if (!strlen(start)) > > + continue; > > + token = match_token(start, if_tokens, args); > > + match_strlcpy(value, args, sizeof(value)); > > Per my understanding, we don't need to copy the string, because strsep() > has replaced " " with "\0". Right? > Unless I'm missing something I don't think this has anything to do with strsep(). `args` is not a null terminated string that can be passed to nodelist_parse(). Instead it is a struct substring_t that has args->to and args->from. To convert substring_t args to a null terminated string, I call match_strlcpy(). I think this is a common pattern doen in a few places. I think args->to may point to '\0' because of how strsep() and match_token() work internally, but I'm somewhat uncomfortable making assumptions about the implementation of these functions here (it may change in the future and break the assumption). > Best Regards, > Huang, Ying > > > + switch (token) { > > + case MEMORY_RECLAIM_NODES: > > + if (nodelist_parse(value, nodemask) < 0) > > + return -EINVAL; > > + break; > > + default: > > + return -EINVAL; > > + } > > + } > > + > > while (nr_reclaimed < nr_to_reclaim) { > > unsigned long reclaimed; > > > > @@ -6610,7 +6650,8 @@ static ssize_t memory_reclaim(struct kernfs_open_file *of, char *buf, > > > > reclaimed = try_to_free_mem_cgroup_pages(memcg, > > nr_to_reclaim - nr_reclaimed, > > - GFP_KERNEL, reclaim_options); > > + GFP_KERNEL, reclaim_options, > > + nodemask); > > > > if (!reclaimed && !nr_retries--) > > return -EAGAIN; > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index 7b8e8e43806b..23fc5b523764 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -6735,7 +6735,8 @@ unsigned long mem_cgroup_shrink_node(struct mem_cgroup *memcg, > > unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg, > > unsigned long nr_pages, > > gfp_t gfp_mask, > > - unsigned int reclaim_options) > > + unsigned int reclaim_options, > > + nodemask_t nodemask) > > { > > unsigned long nr_reclaimed; > > unsigned int noreclaim_flag; > > @@ -6750,6 +6751,7 @@ unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg, > > .may_unmap = 1, > > .may_swap = !!(reclaim_options & MEMCG_RECLAIM_MAY_SWAP), > > .proactive = !!(reclaim_options & MEMCG_RECLAIM_PROACTIVE), > > + .nodemask = &nodemask, > > }; > > /* > > * Traverse the ZONELIST_FALLBACK zonelist of the current node to put > > -- > > 2.38.1.584.g0f3c55d4c2-goog >