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 16:27:24 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     Michal Hocko <mhocko@suse.com>,
        Muchun Song <songmuchun@bytedance.com>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        Andrew Morton <akpm@linux-foundation.org>
CC:     "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>
Subject: Re: [PATCH] mm: mempolicy: fix policy_nodemask() for
 MPOL_PREFERRED_MANY case
Message-ID: <YuuC7MBQ1bX3jA0j@feng-clx>
References: <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>
 <Yurd2iYp4XMIYM7T@feng-clx>
 <YupwjN6K6e6V3y+Q@dhcp22.suse.cz>
 <Yurj0sOXgGf40AJE@feng-clx>
 <Yup2UimZJgbgFb3S@dhcp22.suse.cz>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <Yup2UimZJgbgFb3S@dhcp22.suse.cz>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?D3NKeaeURZjLPQhB3eXVzHKWEVAeaFuTBMCvx8ZHJ6nh8V8YGwJ5goTWvPPV?=
 =?us-ascii?Q?egQoX9wQYG/eJ1TcGDef22QyhX09zs7MQhMh2bIjvnJsJyVpDwQxQ4S06gUg?=
 =?us-ascii?Q?D8UcgZHMKEDFaZGEzaS047oOSOWT6kNuXTdpSF3zEMKc2qyiRYQNzqjjN1co?=
 =?us-ascii?Q?x1u1oV2Diry6SXc996oFkvbg1MPP8oSEim+ThXKj6wkfEyhc2qqZdZMXvDrt?=
 =?us-ascii?Q?e2kq4l4Rs+VgBoWF4R2xAMxrJZpIbtmyg+CAve0FyZm/25FRnTrjDR320KJW?=
 =?us-ascii?Q?fmT3Ltdr/R+vDccPcv0J0fjJHav3Tlqr4/A/8oA3hAe1KFL+lmq1jVfBRXZK?=
 =?us-ascii?Q?BzwLBuxG3i/AfTBT0av7f7ppYltYk4UGv0B/qzWyQ+Uj9EbreXtyGeOjFMLX?=
 =?us-ascii?Q?uzcfMIV3DkVMsmrzxFTkbP9YiFduy2g1c0RkiIwdD1vQT2TaaBIj/dK/clFG?=
 =?us-ascii?Q?rGcH8nX/YICU/fxw+RQ4nNc8G7QgXpzpAVZVOptCwao6M5qRGWyFUCvu45Jr?=
 =?us-ascii?Q?TSF2cZ0EC4DMX8//KoIVYnU6P7kw4QLvk2lSx8L/6UlP4BZvyp7KlQYbF0wQ?=
 =?us-ascii?Q?aR4ZaCdbrUa74u7sw5SlP02c0rtjQ7/hgbSEvTkY9ssRgCbT/7WSGRjywhqh?=
 =?us-ascii?Q?IQ4T92TolMqJvkZp0xxiTam76IQjs5fZdRO3dJMnHaTgPpQqNzwV8HD/Zs6h?=
 =?us-ascii?Q?oAooEPg/5nEtvN/Uj3KWLy1o6heKOpuoyYxY9MjyBaQaXLqX7l84bvkts8yU?=
 =?us-ascii?Q?BGZveeg+rxIBnJu5OAlfwMij72zd6WZXMO6crOzt3xmUkKxnySOl+/SFIcml?=
 =?us-ascii?Q?0kiT2wKd3LVtXxQATIU/qtnSsCZTk992CL081IM7g31FeyHRq0e2qICjUW92?=
 =?us-ascii?Q?AYyIthMenBfsChJ6h5w1J6lEhV/dwkYsPZSIVhv8RVZitKPbDroWTNKGCJtu?=
 =?us-ascii?Q?XSNGAjQsg5GNlXYexOInakAAolIyXNyzw3s4NRnlWiZfg1A7dliVZ7vc+wFI?=
 =?us-ascii?Q?eelw5N1/J8Ccj4lXDfWq4+hF9QDvtag2gLLVVOmJE4eARHUpg3bM/N4oXVOw?=
 =?us-ascii?Q?p7LyWVgGwac4RHQz7sWhVzqjTSSkOH1glVf3fbEzHP/xU6T44ONo4hll4p3z?=
 =?us-ascii?Q?ayoHPfmGyF7z7d+27HHyzQTSwKd7u+3Dr07X3hffJfjfPZhlJIpBov6JHqIY?=
 =?us-ascii?Q?cZvVVLLiK9kR+NG/4QUzzV9n0ASI/nUj4v8Lp8sB6Rey+1OTwkbCVzEedHZE?=
 =?us-ascii?Q?jex7jmaUe2RhWAJiFtonzcKMHxuKvJKVohs3+Y7e4rGLcNzmeZaICNdtZnBw?=
 =?us-ascii?Q?5g57Vv/8eMqtW1Y8StSoJ3eRPbWiOq0jLoyKmXaI8v4a/lMrEbLU5LBmoEfX?=
 =?us-ascii?Q?jrCrwTIfKxOOaADwrdoo1aN67EMGH1M37dLjjAIda4V8KQWAHek6wWWgyTj6?=
 =?us-ascii?Q?WvZwOqLKWZiJ+HQpvxlRr+ZwWblA8I2yv/ww4texsDPYjTHHdiqkREqe3DyO?=
 =?us-ascii?Q?HAU+Oq1J/UYCTlr6zgESWJPyuNGFOm042jmLyKLoqtg5Iq643OkNEJW/XDoP?=
 =?us-ascii?Q?OW98WwTcYv6O2yJt5VHXtDwXoO05ij2hOyG3uKj/?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 922ce0b8-134f-44d6-2a0e-08da75f35208
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Aug 2022 08:28:34.1534
 (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: SmeDKDrdUMn9m8nUYwiIzzBzpadDgQzNCI2BuM+9g+ek2zhCiCbpLGXaZSPt2fGa4dUN3JT8pOxWnvXUtbaYdQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6381
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        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 09:21:22PM +0800, Michal Hocko wrote:
> On Thu 04-08-22 05:08:34, Feng Tang wrote:
> [...]
> > Do we still need the other nodemask API I proposed earlier which has
> > no parameter of gfp_flag, and used for __nr_hugepages_store_common?
> 
> I would touch as little code as possible.

OK.

Please review the following patch, thanks! - Feng

---
From a2db9a57da616bb3ea21e48a4a9ceb5c2cf4f7a2 Mon Sep 17 00:00:00 2001
From: Feng Tang <feng.tang@intel.com>
Date: Thu, 4 Aug 2022 09:39:24 +0800
Subject: [PATCH RFC] mm/hugetlb: add dedicated func to get 'allowed' nodemask for
 current process

Muchun Song found that after MPOL_PREFERRED_MANY policy was introduced
in commit b27abaccf8e8 ("mm/mempolicy: add MPOL_PREFERRED_MANY for multiple preferred nodes")
[1], the policy_nodemask_current()'s semantics for this new policy
has been changed, which returns 'preferred' nodes instead of 'allowed'
nodes, and could hurt the usage of its caller in hugetlb:
allowed_mems_nr().

Michal found the policy_nodemask_current() is only used by hugetlb,
and suggested to move it to hugetlb code with more explicit name to
enforce the 'allowed' semantics for which only MPOL_BIND policy
matters.

One note for the new policy_mbind_nodemask() is, the cross check
from MPOL_BIND, gfp flags and cpuset configuration can lead to
a no available node case, which is considered to be broken
configuration and 'NULL' (equals all nodes) is returned.

[1]. https://lore.kernel.org/lkml/20220801084207.39086-1-songmuchun@bytedance.com/t/
Reported-by: Muchun Song <songmuchun@bytedance.com>
Suggested-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 include/linux/mempolicy.h | 32 ++++++++++++++++++++------------
 mm/hugetlb.c              | 24 ++++++++++++++++++++----
 mm/mempolicy.c            | 20 --------------------
 3 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/include/linux/mempolicy.h b/include/linux/mempolicy.h
index 668389b4b53d..ea0168fffb4c 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;
@@ -189,6 +182,26 @@ static inline bool mpol_is_preferred_many(struct mempolicy *pol)
 	return  (pol->mode == MPOL_PREFERRED_MANY);
 }
 
+static inline int apply_policy_zone(struct mempolicy *policy, enum zone_type zone)
+{
+	enum zone_type dynamic_policy_zone = policy_zone;
+
+	BUG_ON(dynamic_policy_zone == ZONE_MOVABLE);
+
+	/*
+	 * if policy->nodes has movable memory only,
+	 * we apply policy when gfp_zone(gfp) = ZONE_MOVABLE only.
+	 *
+	 * policy->nodes is intersect with node_states[N_MEMORY].
+	 * so if the following test fails, it implies
+	 * policy->nodes has movable memory only.
+	 */
+	if (!nodes_intersects(policy->nodes, node_states[N_HIGH_MEMORY]))
+		dynamic_policy_zone = ZONE_MOVABLE;
+
+	return zone >= dynamic_policy_zone;
+}
+
 
 #else
 
@@ -294,11 +307,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..ad84bb85b6de 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);
 
+static nodemask_t *policy_mbind_nodemask(gfp_t gfp)
+{
+#ifdef CONFIG_NUMA
+	struct mempolicy *mpol = get_task_policy(current);
+
+	/*
+	 * Only enforce MPOL_BIND policy 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(&mpol->nodes)))
+		return &mpol->nodes;
+#endif
+	return NULL;
+}
+
 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];
 	}
 
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index d39b01fd52fe..5553bd53927f 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1805,26 +1805,6 @@ bool vma_policy_mof(struct vm_area_struct *vma)
 	return pol->flags & MPOL_F_MOF;
 }
 
-static int apply_policy_zone(struct mempolicy *policy, enum zone_type zone)
-{
-	enum zone_type dynamic_policy_zone = policy_zone;
-
-	BUG_ON(dynamic_policy_zone == ZONE_MOVABLE);
-
-	/*
-	 * if policy->nodes has movable memory only,
-	 * we apply policy when gfp_zone(gfp) = ZONE_MOVABLE only.
-	 *
-	 * policy->nodes is intersect with node_states[N_MEMORY].
-	 * so if the following test fails, it implies
-	 * policy->nodes has movable memory only.
-	 */
-	if (!nodes_intersects(policy->nodes, node_states[N_HIGH_MEMORY]))
-		dynamic_policy_zone = ZONE_MOVABLE;
-
-	return zone >= dynamic_policy_zone;
-}
-
 /*
  * Return a nodemask representing a mempolicy for filtering nodes for
  * page allocation
-- 
2.27.0