lkml.org 
[lkml]   [2021]   [Dec]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH 2/2] mm/vmscan.c: Prevent allocating shrinker_info on offlined nodes
>>
>> 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

\
 
 \ /
  Last update: 2021-12-07 11:16    [W:0.087 / U:0.524 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site