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;
Date:   Thu, 27 Oct 2022 13:57:20 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     Waiman Long <longman@redhat.com>
CC:     Andrew Morton <akpm@linux-foundation.org>,
        Johannes Weiner <hannes@cmpxchg.org>,
        "Hocko, Michal" <mhocko@suse.com>, Tejun Heo <tj@kernel.org>,
        Zefan Li <lizefan.x@bytedance.com>,
        "Huang, Ying" <ying.huang@intel.com>,
        "aneesh.kumar@linux.ibm.com" <aneesh.kumar@linux.ibm.com>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "cgroups@vger.kernel.org" <cgroups@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Hansen, Dave" <dave.hansen@intel.com>,
        "Chen, Tim C" <tim.c.chen@intel.com>,
        "Yin, Fengwei" <fengwei.yin@intel.com>
Subject: Re: [PATCH] mm/vmscan: respect cpuset policy during page demotion
Message-ID: <Y1odwO1/5fxqvQNn@feng-clx>
References: <20221026074343.6517-1-feng.tang@intel.com>
 <da533477-da08-88cb-1e76-46364f1a6151@redhat.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <da533477-da08-88cb-1e76-46364f1a6151@redhat.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?VCkyx9V7HQoZVZkTxM6VQWPyyRTNaLHXvaerJbH2gAA65jDKMd96fOGQqj5L?=
 =?us-ascii?Q?6Bh7oGJVHsLA/pLZOmusCa9xcV1uX4mLsnpuBcukkMwXF8CjssERLwW2iBSj?=
 =?us-ascii?Q?FnG0lgiQbbXA5+x0P7cIboVBdOTx5fafWEuoMW/SyiAuBb+u/ffROilLIFaQ?=
 =?us-ascii?Q?/49P4ZiV4Qzy4FYPcGyv0Ds1iJXONZ28AtTyUip13htsLVKuusNqvgyIo5w2?=
 =?us-ascii?Q?7cutlu1Oz8ja4b7/VgiznPfijJvuegRBAvCHKmgz3XgIm4Vx8ICqCfZ/fdW6?=
 =?us-ascii?Q?w5hZqwjksNWDEQz/xeesQ7kyWkoptPg0eaQIF5bRmnDGzjDgnw88EWFtmHVl?=
 =?us-ascii?Q?hO4W3SXG/Sx7pqIfGdkWFaskJdg5N1xXZTHqZY5vI/dwb9cMv/S9mGKatDQ3?=
 =?us-ascii?Q?mYjzD7wo4f0I3fx8hBXjfE1wJGjHhGyr7z20RSnqrTpXjpQMEuM7z4hODuZ0?=
 =?us-ascii?Q?bX9/A9gfb/lpkJGjg0WLFQGnemSZeViEBv2n/p1yQnGf7nlQ7e3RmoyEo4uS?=
 =?us-ascii?Q?omg/C/Zj7hLC2V8XsipUadwMEOORouZOK44AjatvJ/yedwLfKUfQySDal9zg?=
 =?us-ascii?Q?5/t6IVcjGvGKJ3cEFRubwC0wNqvhjF4t2MGqHKvNc/BY/U7NwcwWjEsGdbGb?=
 =?us-ascii?Q?O/kKP4pWZySSu/CCbzYUv6jfT/OykKiZUbPsPXIm8vf78MqbcHQFMe0ZHx1o?=
 =?us-ascii?Q?91KbHMYVj3lSdHrdkIJfVITdKW0AxOOPKIck+HWmW1J2su5KBKFBdIK1GmDC?=
 =?us-ascii?Q?xQz5yGOP4dfJUHHPbSKk03DaFMa0xqMFPdlCZPuPAVKBGqc4/zFgiFE6FIgF?=
 =?us-ascii?Q?nA3r+J11itOqqua8l8nYzXJL8vkvKHGQCyUN5zzrM97Eu4EkrOo5STAO1nLv?=
 =?us-ascii?Q?bYGn5OFZSTvjBw3CiTBCtB8JD664jfQ20WANv0BD+p/TmvIXJiqc25Inuvb4?=
 =?us-ascii?Q?l6WuEq3sc3cRVFVC1WrfNhea3xlmOeYOt19L/AxT/iuuTyDs7ueGbL9buLiI?=
 =?us-ascii?Q?KDsN2zUWqfvyNjE0Fd58Goyge8JOWzjdzZWKrlQN/VzutmAYkDzFzo+t4Z4G?=
 =?us-ascii?Q?Rr3u3VCvlQJ5ELd4DZEdL91QWJ781vrkPff81+5L8dtneY0aI9nrmr1LGxo1?=
 =?us-ascii?Q?DAosEo2vKWR/h7gI3Wm8PCPHpypJZ9LiD9Ipk9g4diSQlR8xCz6r1cJWGVAx?=
 =?us-ascii?Q?OQogL9HgDwZMRAA4Eqz0mMPDZa2evoB4cNaw/nXB4jgzSdvcgWkgibP/YM2N?=
 =?us-ascii?Q?IuhUXKSQ5j4+M4rtDf4rsK5GKbPciQMUkw40BHnokCbJDG+9ro7F6kzGN8sk?=
 =?us-ascii?Q?PyuAgVr/inbEvhVhgNUQ2yeSjhEqga4wULzM4+RthRrhi6w4BqYozcW+DMXO?=
 =?us-ascii?Q?TX+LSK18WMha+WkeSBg7JJhw4lnm/7/6siUtFh3jMJ8naDnNs1N1zAxk7qy5?=
 =?us-ascii?Q?qACXBs18UPoYgmGUbi8ToY9fnnjUFhV43tb6j/kV+JUOGFkWeI8og/SpaTK5?=
 =?us-ascii?Q?trYYvvM6bpCH15sUAaYwj/emb17jtDhiC2rtYltSe74HAMnzGYec79ovyhE+?=
 =?us-ascii?Q?+Q5gV5Sw2dFRxzne4t7XShfT0987KIsAfxFVd71T?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e97d5156-9376-412b-f856-08dab7e02263
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Oct 2022 05:57:30.5435
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: isivTBKyBBlFPjsZlZ+HvE2QNRaVnKpaOI8ElXbbKRTTnNtn913EX9xxWSaxI996NNuMlB28B7EVQFfZE5bFZQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5435
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        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

On Wed, Oct 26, 2022 at 10:36:32PM +0800, Waiman Long wrote:
> On 10/26/22 03:43, Feng Tang wrote:
> > In page reclaim path, memory could be demoted from faster memory tier
> > to slower memory tier. Currently, there is no check about cpuset's
> > memory policy, that even if the target demotion node is not allowd
> > by cpuset, the demotion will still happen, which breaks the cpuset
> > semantics.
> >
> > So add cpuset policy check in the demotion path and skip demotion
> > if the demotion targets are not allowed by cpuset.
> >
> > Signed-off-by: Feng Tang <feng.tang@intel.com>
> > ---
> > Hi reviewers,
> >
> > For easy bisectable, I combined the cpuset change and mm change
> > in one patch, if you prefer to separate them, I can turn it into
> > 2 patches.
> >
> > Thanks,
> > Feng
> >
> >   include/linux/cpuset.h |  6 ++++++
> >   kernel/cgroup/cpuset.c | 29 +++++++++++++++++++++++++++++
> >   mm/vmscan.c            | 35 ++++++++++++++++++++++++++++++++---
> >   3 files changed, 67 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
> > index d58e0476ee8e..6fcce2bd2631 100644
> > --- a/include/linux/cpuset.h
> > +++ b/include/linux/cpuset.h
> > @@ -178,6 +178,8 @@ static inline void set_mems_allowed(nodemask_t nodemask)
> >   	task_unlock(current);
> >   }
> >   
> > +extern void cpuset_get_allowed_mem_nodes(struct cgroup *cgroup,
> > +						nodemask_t *nmask);
> >   #else /* !CONFIG_CPUSETS */
> >   
> >   static inline bool cpusets_enabled(void) { return false; }
> > @@ -299,6 +301,10 @@ static inline bool read_mems_allowed_retry(unsigned int seq)
> >   	return false;
> >   }
> >   
> > +static inline void cpuset_get_allowed_mem_nodes(struct cgroup *cgroup,
> > +						nodemask_t *nmask)
> > +{
> > +}
> >   #endif /* !CONFIG_CPUSETS */
> >   
> >   #endif /* _LINUX_CPUSET_H */
> > diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> > index 3ea2e836e93e..cbb118c0502f 100644
> > --- a/kernel/cgroup/cpuset.c
> > +++ b/kernel/cgroup/cpuset.c
> > @@ -3750,6 +3750,35 @@ nodemask_t cpuset_mems_allowed(struct task_struct *tsk)
> >   	return mask;
> >   }
> >   
> > +/*
> > + * Retrieve the allowed memory nodemask for a cgroup.
> > + *
> > + * Set *nmask to cpuset's effective allowed nodemask for cgroup v2,
> > + * and NODE_MASK_ALL (means no constraint) for cgroup v1 where there
> > + * is no guaranteed association from a cgroup to a cpuset.
> > + */
> > +void cpuset_get_allowed_mem_nodes(struct cgroup *cgroup, nodemask_t *nmask)
> > +{
> > +	struct cgroup_subsys_state *css;
> > +	struct cpuset *cs;
> > +
> > +	if (!is_in_v2_mode()) {
> > +		*nmask = NODE_MASK_ALL;
> > +		return;
> > +	}
> 
> You are allowing all nodes to be used for cgroup v1. Is there a reason 
> why you ignore v1?
 
The use case for the API is, for a memory control group, user want to
get its associated cpuset controller's memory policy, so it tries
the memcg --> cgroup --> cpuset chain. IIUC, there is no a reliable
chain for cgroup v1, plus cgroup v2 is the default option for many
distros, the cgroup v1 is bypassed here.

> > +
> > +	rcu_read_lock();
> > +	css = cgroup_e_css(cgroup, &cpuset_cgrp_subsys);
> > +	if (css) {
> > +		css_get(css);
> > +		cs = css_cs(css);
> > +		*nmask = cs->effective_mems;
> > +		css_put(css);
> > +	}
> Since you are holding an RCU read lock and copying out the whole 
> nodemask, you probably don't need to do a css_get/css_put pair.

Thanks for the note!

Thanks,
Feng

> > +
> > +	rcu_read_unlock();
> > +}
> > +
> Cheers,
> 
> Longman
>