Re: [patch 3/7 -mm] oom: select task from tasklist for mempolicy ooms

David Rientjes
2010-02-16T21:53:01Z

On Tue, 16 Feb 2010, KOSAKI Motohiro wrote:

> > We need to get a refcount on the mempolicy to ensure it doesn't get freed
> > from under us, tsk is not necessarily current.
>
> Hm.
> if you explanation is correct, I think your patch have following race.
>
>
> CPU0 CPU1
> ----------------------------------------------
> mempolicy_nodemask_intersects()
> mempolicy = tsk->mempolicy;
> do_exit()
> mpol_put(tsk_mempolicy)
> mpol_get(mempolicy);
>

True, good point. It looks like we'll need to include mempolicy
detachment in exit_mm() while under task_lock() and then synchronize with
that. It's a legitimate place to do it since no memory allocation will be
done after its mm is detached, anyway.

> > For MPOL_F_LOCAL, we need to check whether the task's cpu is on a node
> > that is allowed by the zonelist passed to the page allocator. In the
> > second revision of this patchset, this was changed to
> >
> > node_isset(cpu_to_node(task_cpu(tsk)), *mask)
> >
> > to check. It would be possible for no memory to have been allocated on
> > that node and it just happens that the tsk is running on it momentarily,
> > but it's the best indication we have given the mempolicy of whether
> > killing a task may lead to future memory freeing.
>
> This calculation is still broken. In general, running cpu and allocation node
> is not bound.

Not sure what you mean, MPOL_F_LOCAL means that allocations will happen on
the node of the cpu on which it is running. The cpu-to-node mapping
doesn't change, only the cpu on which it is running may change. That may
be restricted by sched_setaffinity() or cpusets, however, so this task may
never allocate on any other node (i.e. it may run on another cpu, but
always one local to a specific node). That's enough of an indication that
it should be a candidate for kill: we're trying to eliminate tasks that
may never allocate on current's nodemask from consideration. In other
words, it would be unfair for two tasks that are isolated to their own
cpus on different physical nodes using MPOL_F_LOCAL for NUMA optimizations
to have the other needlessly killed when current can't allocate there
anyway.