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, 4 Aug 2022 04:43:06 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     Michal Hocko <mhocko@suse.com>
CC:     Muchun Song <songmuchun@bytedance.com>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
        "bwidawsk@kernel.org" <bwidawsk@kernel.org>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Mike Kravetz <mike.kravetz@oracle.com>
Subject: Re: [PATCH] mm: mempolicy: fix policy_nodemask() for
 MPOL_PREFERRED_MANY case
Message-ID: <Yurd2iYp4XMIYM7T@feng-clx>
References: <YuecP/RKXWz7QAs0@feng-skl>
 <YuidPA9knCOoaT0c@FVFYT0MHHV2J>
 <Yui7hWZYMX31ktOr@feng-skl>
 <YujGy8EIeZc1Avc7@FVFYT0MHHV2J>
 <YujUyCIBjFj+FzX5@feng-skl>
 <YujoLQt09Js/sSQL@dhcp22.suse.cz>
 <YuoYkPk+YzdPNvmN@feng-snb>
 <YuolieBmdaIzoS3t@dhcp22.suse.cz>
 <Yuqs+BTpfh9/PjtP@feng-clx>
 <Yupb+1mmn9sQ/G8K@dhcp22.suse.cz>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <Yupb+1mmn9sQ/G8K@dhcp22.suse.cz>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?oOug88UKotP+UPWo3LHkxMwVmz8YhBaoNyCsTqfg3y0DEhfkb2ds0BZhj5Rm?=
 =?us-ascii?Q?75I8YzbL9k5OI23fG5hXY0er/pvuxFiq6F88UxM8DTkiuXXUEUg/L5HutABr?=
 =?us-ascii?Q?m+jFkewkI1eTWwtFIaQ6Di1Bl2E0al46HtZTKswimDUoEKcYDdAGSBtQeAip?=
 =?us-ascii?Q?49PnvIak+EAW5hkF7EcOlaLGPELNKSwFTB9YBXTv4qNYawdWzaVnEE3c+/qu?=
 =?us-ascii?Q?FTwQbV1d+92z+yTQ53PjlNyZBiiY9NNLKscirtJpW6nl0RaBjeUzGpNzHqR/?=
 =?us-ascii?Q?W1JA18Q/t79k/fMUSmTGAigdLL9OLv57BYTglZocJTPpWwdyvtuHGfSL4kMR?=
 =?us-ascii?Q?L/1ZDQa5b6BzW82UrvLiwI1GhTCpQVnZhGwcBOEjDOVkxAje/CR7uG1dFriD?=
 =?us-ascii?Q?Q5Fmygkg67eilYutFWZ3ngpzz2ofO1j/eUt9Q1Zwwx69/gWNq2BWe+QShbcj?=
 =?us-ascii?Q?AEk8jshfHatMH8C4QysHl52/Fub0WDZeJbuNxv3viJA2NiC1An6U6LhHMHFl?=
 =?us-ascii?Q?+LJO7uT+/QThHdOBOAQ4I8bQf2dzwMSXGvxEk1grP+cUdLEJ194RCg0Ny7B8?=
 =?us-ascii?Q?nBvNb7xW+zXVmdMyg3K54yhcKKOOpkDshILnqREkZgMn0J1Kzl4GuGKlDzic?=
 =?us-ascii?Q?z1DsSl9fTF3P7pnY70ywlk9AyIM8tMZbFErvEeryUudxCwogt4LGjkbfHdf7?=
 =?us-ascii?Q?c1xZgJXaE9JtPZsnB9Jz7q7tp0dvZSl/VS7p+Sst6xJDC8R3cSQ0FZ6zpOdH?=
 =?us-ascii?Q?1Sgtwupv7Ti2DjjTvTIOFWhZxSptwJifIjot/baLw4DhNXxU6r+Idr+mNQsa?=
 =?us-ascii?Q?Y1M8N47h5hJ/ODJL+pJbUPjAfJoqFPQkqbDgnImHkegLDk5SthItkrpMlsEF?=
 =?us-ascii?Q?v3yx+zyVHQ6wNhQY8T+FfrWo8aa6hEjyk0f2lAWJHOuXP7PbDhu+V0yWaPZ5?=
 =?us-ascii?Q?VFG2U5bVToHdChDKXsVxYUeO/FVnG3Rw5gmECyCzfz2g22Arw/CRSLwUIbT3?=
 =?us-ascii?Q?rRmZQOLkvlhZNvOm7BFFqge6SHvq+uYVPWcnq+uXAHsAU6IOxxjubyp8RSMf?=
 =?us-ascii?Q?iUZfO0VOerRnmrYkj6J4N1+DKLCT46B1fd1aHb7oF+n6e6qPZlHsSzjjAOcL?=
 =?us-ascii?Q?p8MNFVx41ZuaIPFty441gzS2A7R/AQyrvl0MDBHqieVsOpMQyiHgVS1X8Tu8?=
 =?us-ascii?Q?CLm2LiO/Rwvtv1pk8ImIWitsTHE/W7mlgURN2m3kBbkFrZMzO2gLMlFtGAzk?=
 =?us-ascii?Q?GZvEca4YlS6Z6IA7C7FidLaiynn6dmiST+hmAoJzMl1XyXygrmIObdjHa3St?=
 =?us-ascii?Q?IeW0xiPsPElme9nEvRHzC/43WHL0jwBxEHSEC4jkIYHDI9jMuZJ5zXxdDrN5?=
 =?us-ascii?Q?XdnDgu0T5i8JKopPCcNBVJR/FlrGfZZns9iDUFh2gwl891MNSsudnz9vF3Kc?=
 =?us-ascii?Q?IAki9Jny2DYZmwCdWMAEvysX9A9wTzf0o5V50Q1ZVYhbkkZEMiXZIU9dGw/z?=
 =?us-ascii?Q?3nivkc2OAY258jV0sHNcJ3bs/nX4V8xSo22L2LH3Solst5ri//eoPr4yqDGo?=
 =?us-ascii?Q?z2bAv+Xy9K09fW2E9h+Mbur4EC+giW2XB1PEB96h?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0bc16bd0-5efc-4873-ad4f-08da754e09d8
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Aug 2022 12:45:26.1484
 (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: vpXFhJ2b+QJtXgGygMd1l2CB8ADu9sqB89TQOPbz47vb2FFZsfM6ROQvrtg9DoWMXdS05qJhXNEY0liQPxxaiA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR11MB1638
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00,DATE_IN_FUTURE_06_12,
        DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,
        RCVD_IN_DNSWL_HI,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, Aug 03, 2022 at 07:28:59PM +0800, Michal Hocko wrote:
> On Thu 04-08-22 01:14:32, Feng Tang wrote:
> [...]
> > Ok, I change it as below:
> 
> Wouldn't it be better to make this allowed_mems_nr specific to be
> explicit about the intention?
 
Yes, it is.

> Not that I feel strongly about that.
> 
> > ---
> >  mm/hugetlb.c | 28 +++++++++++++++++++++++-----
> >  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> Not even compile tested
>  include/linux/mempolicy.h | 12 ------------
>  mm/hugetlb.c              | 24 ++++++++++++++++++++----
>  2 files changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/mempolicy.h b/include/linux/mempolicy.h
> index 668389b4b53d..e38b0ef20b8b 100644
> --- a/include/linux/mempolicy.h
> +++ b/include/linux/mempolicy.h
> @@ -151,13 +151,6 @@ extern bool mempolicy_in_oom_domain(struct task_struct *tsk,
>  				const nodemask_t *mask);
>  extern nodemask_t *policy_nodemask(gfp_t gfp, struct mempolicy *policy);
>  
> -static inline nodemask_t *policy_nodemask_current(gfp_t gfp)
> -{
> -	struct mempolicy *mpol = get_task_policy(current);
> -
> -	return policy_nodemask(gfp, mpol);
> -}
> -
>  extern unsigned int mempolicy_slab_node(void);
>  
>  extern enum zone_type policy_zone;
> @@ -294,11 +287,6 @@ static inline void mpol_put_task_policy(struct task_struct *task)
>  {
>  }
>  
> -static inline nodemask_t *policy_nodemask_current(gfp_t gfp)
> -{
> -	return NULL;
> -}
> -
>  static inline bool mpol_is_preferred_many(struct mempolicy *pol)
>  {
>  	return  false;
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index a18c071c294e..6cacbc9b15a1 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4330,18 +4330,34 @@ static int __init default_hugepagesz_setup(char *s)
>  }
>  __setup("default_hugepagesz=", default_hugepagesz_setup);
>  
> +struct mempolicy *policy_mbind_nodemask(gfp_t gfp)
> +{
> +#ifdef CONFIG_MEMPOLICY
> +	struct mempolicy *mpol = get_task_policy(current);
> +
> +	/*
> +	 * only enforce MBIND which overlaps with cpuset policy (from policy_nodemask)
> +	 * specifically for hugetlb case
> +	 */
> +	if (mpol->mode == MPOL_BIND &&
> +		(apply_policy_zone(mpol, gfp_zone(gfp)) &&
> +		 cpuset_nodemask_valid_mems_allowed(&policy->nodes))
> +		return &mpol->nodes;
> +#endif
> +	return NULL;

I saw the logic is not changed, and it confused me that if there is
no qualified node, it will still return NULL which effectively equals
node_states[N_MEMORY], while I think it should return a all zero
nodemasks.

Thanks,
Feng

> +}
> +
>  static unsigned int allowed_mems_nr(struct hstate *h)
>  {
>  	int node;
>  	unsigned int nr = 0;
> -	nodemask_t *mpol_allowed;
> +	nodemask_t *mbind_nodemask;
>  	unsigned int *array = h->free_huge_pages_node;
>  	gfp_t gfp_mask = htlb_alloc_mask(h);
>  
> -	mpol_allowed = policy_nodemask_current(gfp_mask);
> -
> +	mbind_nodemask = policy_mbind_nodemask(gfp_mask);
>  	for_each_node_mask(node, cpuset_current_mems_allowed) {
> -		if (!mpol_allowed || node_isset(node, *mpol_allowed))
> +		if (!mbind_nodemask || node_isset(node, *mbind_nodemask))
>  			nr += array[node];
>  	}
>  
> -- 
> Michal Hocko
> SUSE Labs