lkml.org 
[lkml]   [2021]   [Dec]   [6]   [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
On Mon 06-12-21 13:43:27, David Hildenbrand wrote:
[...]
> > Now practically speaking !node_online should not apear node_online (note
> > I am attentionally avoiding to say offline and online as that has a
> > completely different semantic) shouldn't really happen for some
> > architectures. x86 should allocate pgdat for each possible node. I do
> > not know what was the architecture in this case but we already have
> > another report for x86 that remains unexplained.
>
> So we'd allocate the pgdat although all we want is just a zonelist. The
> obvious alternative is to implement the fallback where reasonable -- for
> example, in the page allocator. It knows the fallback order:
> build_zonelists(). That's pretty much all we need the preferred_nid for.
>
> So just making prepare_alloc_pages()/node_zonelist() deal with a missing
> pgdat could make sense as well. Something like:
>
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index b976c4177299..2d2649e78766 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -508,9 +508,14 @@ static inline int gfp_zonelist(gfp_t flags)
> *
> * For the case of non-NUMA systems the NODE_DATA() gets optimized to
> * &contig_page_data at compile-time.
> + *
> + * If the node does not have a pgdat yet, returns the zonelist of the
> + * first online node.
> */
> static inline struct zonelist *node_zonelist(int nid, gfp_t flags)
> {
> + if (unlikely(!NODE_DATA(nid)))
> + nid = first_online_node;
> return NODE_DATA(nid)->node_zonelists + gfp_zonelist(flags);
> }

This is certainly possible. But it a) adds a branch to the hotpath and
b) it doesn't solve any other potential dereference of garbage node.

> But of course, there might be value in a proper node-aware fallback list
> as we have in build_zonelists() -- but it remains questionable if the
> difference for these corner cases would be relevant in practice.

Only the platform knows the proper node topology and that includes
memory less nodes. So they should be setting up a node properly and we
shouldn't be dealing with this at the allocator nor any other code.

> Further, if we could have thousands of nodes, we'd have to update each
> and every one when building zone lists ...

Why would that be a practical problem?

> Removing the hotadd_new_pgdat() stuff does sound appealing, though.

Yes our hotplug code could be simplified as well. All we really need is
an arch code to initialize all the possible nodes.
--
Michal Hocko
SUSE Labs

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