Re: [PATCH] Memoryless nodes: use "node_memory_map" for cpuset mems_allowed validation

Nishanth Aravamudan
2007-07-23T21:48:17Z

On 23.07.2007 [16:59:52 -0400], Lee Schermerhorn wrote:
> On Mon, 2007-07-23 at 12:09 -0700, Nishanth Aravamudan wrote:
> > On 20.07.2007 [16:49:24 -0400], Lee Schermerhorn wrote:
> > > This fixes a problem I encountered testing Christoph's memoryless nodes
> > > series. Applies atop that series. Other than this, series holds up
> > > under what testing I've been able to do this week.
> > >
> > > Memoryless Nodes: use "node_memory_map" for cpusets mems_allowed validation
> > >
> > > cpusets try to ensure that any node added to a cpuset's
> > > mems_allowed is on-line and contains memory. The assumption
> > > was that online nodes contained memory. Thus, it is possible
> > > to add memoryless nodes to a cpuset and then add tasks to this
> > > cpuset. This results in continuous series of oom-kill and other
> > > console stack traces and apparent system hang.
> > >
> > > Change cpusets to use node_states[N_MEMORY] [a.k.a.
> > > node_memory_map] in place of node_online_map when vetting
> > > memories. Return error if admin attempts to write a non-empty
> > > mems_allowed node mask containing only memoryless-nodes.
> > >
> > > Signed-off-by: Lee Schermerhorn <lee.schermerhorn@hp.com>
> >
> > Lee, while looking at this change, I think it ends up fixing
> > cpuset_mems_allowed() to return nodemasks that only include nodes in
> > node_states[N_MEMORY]. However, cpuset_current_mems_allowed is a
> > lockless macro which would still be broken. I think it would need to
> > becom a static inline nodes_and() in the CPUSET case and a #define
> > node_states[N_MEMORY] in the non-CPUSET case?
> >
> > Or perhaps we should adjust cpusets to make it so that the mems_allowed
> > member only includes nodes that are set in node_states[N_MEMORY]?
>
>
> I thought that's what my patch to nodelist_parse() did. It ensures that
> current->mems_allowed is correct [contains at least one node with
> memory, and only nodes with memory] at the time it is installed, but
> doesn't consider memory hot plug and node off-lining. Is this
> [offline/hotplug] your point?

And everytime it is updated, right? (current->mems_allowed). My concern
is purely whether I can then directly use cpuset_current_mems_allowed in
the interleave code for hugetlb.c and it will do the right thing. It
will work, if the #define is changed for !CPUSETS and if your change
guarantess current->mems_allowed is always consistent with
node_states[N_MEMORY].

I think I simply was confused about the full impact of your changes, as
I don't know cpusets that well. I'm going to try and test a memoryless
node box I have at work w/ your change, though, and see what happens.

> Seems like that is an issue that exists in the unpatched code as
> well--i.e., unlike cpuset_mems_allowed(), the lockless, "_current_"
> version does not vet current->mems_allowed against the
> nodes_online_mask. So, all valid nodes in current->mems_allowed could
> have been off-lined since the mask was installed. Am I reading this
> right?

True -- I honestly don't know. I doubt much of this code has been fully
audited for full node unplug?

Thanks,
Nish

--
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center