Re: [PATCH] cpuset: fix allocating page cache/slab object on the unallowed node when memory spread is set

Paul Menage
2009-02-10T11:37:42Z

On Sun, Feb 8, 2009 at 8:02 PM, Nick Piggin <nickpiggin@yahoo.com.au> wrote:
>
> Is it a problem if mems_allowed can get sampled in an unsafe way?

It could cause an OOM to be incorrectly triggered if the task didn't
see all the mems that it was meant to have access to. (In the extreme
case, it could see no mems at all).

> It will happen only quite rarely. This code seems to be far simpler
> and more robust than the current fragile scheme, so it would be
> nice to use it.

Agreed, the existing task->mems_allowed / cpuset_update_memory_state()
code is a bit unwieldy. It's pretty much all inherited from the
original cpusets code.

A nicer way to do it might be:

- get rid of task->mems_allowed entirely

- have cpuset->mems_allowed be a pointer to an immutable RCU-protected
nodemask (so updating the nodemask for a cpuset would always replace
it with a fresh one and RCU-free the old one)

- make cpuset_zone_allowed_*() use rcu_read_lock() and just check
*(task_cs(current)->mems_allowed) rather than current->mems_allowed

- ensure that get_page_from_freelist() is also RCU-safe (right now I
think it's not since cpuset_zone_allowed_softwall() can sleep, but I
think cgroups/cpusets is sufficiently RCU-safe now that we could quite
likely remove the mutex lock from cpuset_zone_allowed_*()

- add an rcu_read_lock() in hugetlb.c:cpuset_mems_nr()

- figure out where the mempolicy updates currently done in
cpuset_update_task_memory_state() need to occur - this is part of the
code that I'm pretty fuzzy on. (Maybe we can just copy the bits from
Miao's patch for this?)

Anything else?

Paul