[PATCH V2 fix 5/6] mm: hugetlb: add a new function to allocate a new gigantic page

On Tue, Nov 29, 2016 at 11:50:37AM +0100, Vlastimil Babka wrote:

quoted

quoted

quoted

+	if (!vma) {
+		if (nid == NUMA_NO_NODE) {
+			if (!init_nodemask_of_mempolicy(nodes_allowed)) {
+				NODEMASK_FREE(nodes_allowed);
+				nodes_allowed = &node_states[N_MEMORY];
+			}
+		} else if (nodes_allowed) {

The check is here.

It's below a possible usage of nodes_allowed as an argument of
init_nodemask_of_mempolicy(mask). Which does

Sorry, I missed that.

        if (!(mask && current->mempolicy))
                return false;

which itself looks like an error at first sight :)

Yes. I agree.

quoted

Do we really need to re-arrange the code here for the explicit check? :)

We don't need it *now* to be correct, but I still find it fragile. Also it
mixes up the semantic of NULL as a conscious "default" value, and NULL as
a side-effect of memory allocation failure. Nothing good can come from that
in the long term :)

Okay, I think we do have the need to do the NULL check for
@nodes_allowed. :)

Thanks
Huang Shijie