>>
>> Short term I tend to like [2], because it avoids having to mess with all
>> such instances to eventually get it right and the temporary overhead
>> until we have the code reworked should be really negligible ...
>
> Thanks, David. Basically either option looks fine to me. But I'm a
> little bit concerned about [2]. It silently changes the node requested
> by the callers. It actually papers over potential bugs? And what if
Hi,
It's just a preferred node, so we do have a node fallback already via
the zonelist to other nodes for proper online nodes -- and would have
the proper node fallback when preallcoating all pgdat.
*Not* doing the fallback with a preferred node that is not online could
be considered a BUG instead.
Note that [2] was just a quick draft. We might have to do some minor
adjustments to handle __GFP_THISNODE properly.
But after all, we have:
VM_WARN_ON((gfp_mask & __GFP_THISNODE) && !node_online(nid));
in __alloc_pages_node() and __folio_alloc_node(). So it might not be
worth adjusting at all.
> the callers specify __GFP_THISNODE (I didn't search if such callers
> really exist in the current code)?
>
> How's about a helper function, for example, called
> kvmalloc_best_node()? It does:
>
> void * kvmalloc_best_node(unsigned long size, int flag, int nid)
> {
> bool onlined = node_online(nid);
>
> WARN_ON_ONCE((flag & __GFP_THISNODE) && !onlined);
>
> if (!onlined)
> nid = -1;
>
> return kvmalloc_node(size, GFP_xxx, nid);
> }
We still have to "fix" each and every affected for_each_node() ... code
until we have preallcoation of pgdat for all possible nodes.
--
Thanks,
David / dhildenb
