Re: oom killer rewrite

[KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>]

Hi

> KOSAKI,
> 
> I've been notified that my entire oom killer rewrite has been dropped from 
> -mm based solely on your feedback.  The problem is that I have absolutely 
> no idea what issues you have with the changes that haven't already been 
> addressed (nobody else does, either, it seems).

That's simple. A regression and an incompatibility are absolutely
unacceptable. They should be removed. Your patches have some funny parts,
but, afaik, nobody said funny requirement itself is wrong. They only said
your requirement don't have to cause any pain to other users.

Zero risk patches are always acceptable.

> 
> The last work I've done on the patches are to ask those involved in the 
> review (including you) and linux-mm whether there were any outstanding 
> issues that anyone has, and I've asked that twice.  I've received no 
> response either time.
> 
> Please respond with a list of your objections to the rewrite (which is 
> available at 
> http://www.kernel.org/pub/linux/kernel/people/rientjes/oom-killer-rewrite
> so we can move forward.

I've reviewed all of your patches. the result is here.

> oom-filter-tasks-not-sharing-the-same-cpuset.patch
	ok, no objection.
	I'm still afraid this patch reinstanciate old bug. but at that time,
	we can drop it solely. this patch is enough bisectable.

> oom-sacrifice-child-with-highest-badness-score-for-parent.patch
	ok, no objection.
	It's good patch.

> oom-select-task-from-tasklist-for-mempolicy-ooms.patch
	ok, no objection.

> oom-remove-special-handling-for-pagefault-ooms.patch
	ok, no objection.

> oom-badness-heuristic-rewrite.patch
	No. All of rewrite is bad idea. Please make separate some
	individual patches.
	All rewrite thing break bisectability. Perhaps it can steal
	a lot of time from MM developers.
	This patch have following parts.
	1) Add oom_score_adj
	2) OOM score normalization
	3) forkbomb detector
	4) oom_forkbomb_thres new knob
 	5) Root user get 3% bonus instead 400%

	all except (2) seems ok. but I'll review them again after separation.
	but you can't insert your copyright. 

> oom-deprecate-oom_adj-tunable.patch
	NAK. you can't change userland use-case at all. This patch
	only makes bug report flood and streal our time.

> oom-replace-sysctls-with-quick-mode.patch
	NAK. To change sysctl makes confusion to userland.
	You have to prove such deprecated sysctl was alread unused.
	But the fact is, there is users. I have hear some times such
	use case and recent bug reporter said that's used.

	https://bugzilla.kernel.org/show_bug.cgi?id=15058

> oom-avoid-oom-killer-for-lowmem-allocations.patch
	I don't like this one. 64bit arch have big (e.g. 2/4G)
	DMA_ZONE/DMA32_ZONE. So, if we create small guest kernel
	on KVM (or Xen), Killing processes may help. IOW, this
	one is conceptually good. but this check way is brutal.

	but even though it's ok. Let's go merge it. this patch is
	enough small.
	If any problem is occur, we can revert this one easily.


> oom-remove-unnecessary-code-and-cleanup.patch
	ok, no objection.

> oom-default-to-killing-current-for-pagefault-ooms.patch
	NAK.
	1) this patch break panic_on_oom
	2) At this merge window, Nick change almost all architecture's
	   page hault handler. now almost all arch use
	   pagefault_out_of_memory. your description has been a bit obsoleted.

> oom-avoid-race-for-oom-killed-tasks-detaching-mm-prior-to-exit.patch
	no objection. but afaik Oleg already pointed out "if (!p->mm)" is bad.
	So, Don't we need push his patch instead?

> oom-hold-tasklist_lock-when-dumping-tasks.patch
	ok, no objection.

> oom-give-current-access-to-memory-reserves-if-it-has-been-killed.patch
	ok, no objection.

> oom-avoid-sending-exiting-tasks-a-sigkill.patch
	ok, no objection

> oom-cleanup-oom_kill_task.patch
	ok, no objection

> oom-cleanup-oom_badness.patch
	ok, no objection

The above "no objection" mean you can feel free to use my reviewed-by tag.


Thanks.



--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>