From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail137.messagelabs.com (mail137.messagelabs.com [216.82.249.19]) by kanga.kvack.org (Postfix) with ESMTP id 271376B01DD for ; Tue, 1 Jun 2010 14:44:36 -0400 (EDT) Received: from hpaq12.eem.corp.google.com (hpaq12.eem.corp.google.com [172.25.149.12]) by smtp-out.google.com with ESMTP id o51IiURp023949 for ; Tue, 1 Jun 2010 11:44:30 -0700 Received: from pzk31 (pzk31.prod.google.com [10.243.19.159]) by hpaq12.eem.corp.google.com with ESMTP id o51IiMQU011036 for ; Tue, 1 Jun 2010 11:44:29 -0700 Received: by pzk31 with SMTP id 31so2601624pzk.16 for ; Tue, 01 Jun 2010 11:44:29 -0700 (PDT) Date: Tue, 1 Jun 2010 11:44:23 -0700 (PDT) From: David Rientjes Subject: Re: [patch -mm 08/18] oom: badness heuristic rewrite In-Reply-To: <20100601163627.245D.A69D9226@jp.fujitsu.com> Message-ID: References: <20100601163627.245D.A69D9226@jp.fujitsu.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org To: KOSAKI Motohiro Cc: Andrew Morton , Rik van Riel , Nick Piggin , Oleg Nesterov , KAMEZAWA Hiroyuki , Balbir Singh , linux-mm@kvack.org List-ID: On Tue, 1 Jun 2010, KOSAKI Motohiro wrote: > > This a complete rewrite of the oom killer's badness() heuristic which is > > used to determine which task to kill in oom conditions. The goal is to > > make it as simple and predictable as possible so the results are better > > understood and we end up killing the task which will lead to the most > > memory freeing while still respecting the fine-tuning from userspace. > > > > The baseline for the heuristic is a proportion of memory that each task is > > currently using in memory plus swap compared to the amount of "allowable" > > memory. "Allowable," in this sense, means the system-wide resources for > > unconstrained oom conditions, the set of mempolicy nodes, the mems > > attached to current's cpuset, or a memory controller's limit. The > > proportion is given on a scale of 0 (never kill) to 1000 (always kill), > > roughly meaning that if a task has a badness() score of 500 that the task > > consumes approximately 50% of allowable memory resident in RAM or in swap > > space. > > > > The proportion is always relative to the amount of "allowable" memory and > > not the total amount of RAM systemwide so that mempolicies and cpusets may > > operate in isolation; they shall not need to know the true size of the > > machine on which they are running if they are bound to a specific set of > > nodes or mems, respectively. > > > > Root tasks are given 3% extra memory just like __vm_enough_memory() > > provides in LSMs. In the event of two tasks consuming similar amounts of > > memory, it is generally better to save root's task. > > > > Because of the change in the badness() heuristic's baseline, it is also > > necessary to introduce a new user interface to tune it. It's not possible > > to redefine the meaning of /proc/pid/oom_adj with a new scale since the > > ABI cannot be changed for backward compatability. Instead, a new tunable, > > /proc/pid/oom_score_adj, is added that ranges from -1000 to +1000. It may > > be used to polarize the heuristic such that certain tasks are never > > considered for oom kill while others may always be considered. The value > > is added directly into the badness() score so a value of -500, for > > example, means to discount 50% of its memory consumption in comparison to > > other tasks either on the system, bound to the mempolicy, in the cpuset, > > or sharing the same memory controller. > > > > /proc/pid/oom_adj is changed so that its meaning is rescaled into the > > units used by /proc/pid/oom_score_adj, and vice versa. Changing one of > > these per-task tunables will rescale the value of the other to an > > equivalent meaning. Although /proc/pid/oom_adj was originally defined as > > a bitshift on the badness score, it now shares the same linear growth as > > /proc/pid/oom_score_adj but with different granularity. This is required > > so the ABI is not broken with userspace applications and allows oom_adj to > > be deprecated for future removal. > > > > Signed-off-by: David Rientjes > > nack > Why? If it's because the patch is too big, I've explained a few times that functionally you can't break it apart into anything meaningful. I do not believe it is better to break functional changes into smaller patches that simply change function signatures to pass additional arguments that are unused in the first patch, for example. If it's because it adds /proc/pid/oom_score_adj in the same patch, that's allowed since otherwise it would be useless with the old heuristic. In other words, you cannot apply oom_score_adj's meaning to the bitshift in any sane way. I'll suggest what I have multiple times: the easiest way to review the functional change here is to merge the patch into your own tree and then review oom_badness(). I agree that the way the diff comes out it is a little difficult to read just from the patch form, so merging it and reviewing the actual heuristic function is the easiest way. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org