Re: [PATCH 2/2] mm: thp: fix transparent_hugepage/defrag = madvise || always

[Michal Hocko <mhocko@suse.com>]

On Wed 29-08-18 16:28:16, Michal Hocko wrote:
> On Wed 29-08-18 09:28:21, Zi Yan wrote:
> [...]
> > This patch triggers WARN_ON_ONCE() in policy_node() when MPOL_BIND is used and THP is on.
> > Should this WARN_ON_ONCE be removed?
> > 
> > 
> > /*
> > * __GFP_THISNODE shouldn't even be used with the bind policy
> > * because we might easily break the expectation to stay on the
> > * requested node and not break the policy.
> > */
> > WARN_ON_ONCE(policy->mode == MPOL_BIND && (gfp & __GFP_THISNODE));
> 
> This is really interesting. It seems to be me who added this warning but
> I cannot simply make any sense of it. Let me try to dig some more.

OK, I get it now. The warning seems to be incomplete. It is right to
complain when __GFP_THISNODE disagrees with MPOL_BIND policy but that is
not what we check here. Does this heal the warning?
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index da858f794eb6..7bb9354b1e4c 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1728,7 +1728,10 @@ static int policy_node(gfp_t gfp, struct mempolicy *policy,
 		 * because we might easily break the expectation to stay on the
 		 * requested node and not break the policy.
 		 */
-		WARN_ON_ONCE(policy->mode == MPOL_BIND && (gfp & __GFP_THISNODE));
+		if (policy->mode == MPOL_BIND && (gfp & __GFP_THISNODE)) {
+			nodemask_t *nmask = policy_nodemask(gfp, policy);
+			WARN_ON_ONCE(!node_isset(nd, *nmask));
+		}
 	}
 
 	return nd;
-- 
Michal Hocko
SUSE Labs