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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.00.1002161343070.23037@chino.kir.corp.google.com>
Date:	Tue, 16 Feb 2010 13:52:53 -0800 (PST)
From:	David Rientjes <rientjes@...gle.com>
To:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Rik van Riel <riel@...hat.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Nick Piggin <npiggin@...e.de>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Balbir Singh <balbir@...ux.vnet.ibm.com>,
	Lubos Lunak <l.lunak@...e.cz>, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org
Subject: Re: [patch 3/7 -mm] oom: select task from tasklist for mempolicy
 ooms

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/