linux-kernel - Re: [patch -mm v2] mm: introduce oom_adj

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]

Date:	Mon, 3 Aug 2009 10:42:44 +0900
From:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To:	David Rientjes <rientjes@...gle.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Rik van Riel <riel@...hat.com>,
	Paul Menage <menage@...gle.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [patch -mm v2] mm: introduce oom_adj_child

On Sat, 1 Aug 2009 13:26:52 -0700 (PDT)
David Rientjes <rientjes@...gle.com> wrote:

> On Sat, 1 Aug 2009, KAMEZAWA Hiroyuki wrote:
> 
> > Summarizing I think now .....
> >   - rename mm->oom_adj as mm->effective_oom_adj
> >   - re-add per-thread oom_adj
> >   - update mm->effective_oom_adj based on per-thread oom_adj
> >   - if necessary, plz add read-only /proc/pid/effective_oom_adj file.
> >     or show 2 values in /proc/pid/oom_adj
> >   - rewrite documentation about oom_score.
> >    " it's calclulated from  _process's_ memory usage and oom_adj of
> >     all threads which shares a memor  context".
> >    This behavior is not changed from old implemtation, anyway.
> >  - If necessary, rewrite oom_kill itself to scan only thread group
> >    leader. It's a way to go regardless of  vfork problem.
> > 
> 
> Ok, so you've abandoned the signal_struct proposal and now want to add it 
per-signal is also ok, just I didn't write.

> back to task_struct with an effective member in mm_struct by changing the 
> documentation.  Hmm.
> 
> This solves the livelock problem by adding additional tunables, but 
> doesn't match how the documentation describes the use case for 
> /proc/pid/oom_adj.  Your argument is that the behavior of that value can't 
> change: that it must be per-thread.  And that allowance leads to one of 
> two inconsistent scenarios:
> 
>  - /proc/pid/oom_score is inconsistent when tuning /proc/pid/oom_adj if it
>    relies on the per-thread oom_adj; it now really represents nothing but
>    an incorrect value if other threads share that memory and misleads the
>    user on how the oom killer chooses victims, or

What's why I said to show effective_oom_adj if necessary..

> 
>  - /proc/pid/oom_score is inconsistent when the thread that set the
>    effective per-mm oom_adj exits and it is now obsolete since you have
>    no way to determine what the next effective oom_adj value shall be.
> 
plz re-caluculate it. it's not a big job if done in lazy way.


> Determining the next effective per-mm oom_adj isn't possible when the only 
> threads sharing the mm remaining have different per-thread oom_adj values.  
> That's a horribly inconsistent state to be getting into because it allows 
> oom_score to change when a thread exits, which is completely unknown to 
> userspace, OR is allows the effective per-mm oom_adj to be different from 
> all threads sharing the same memory (and, thus, /proc/pid/oom_score not 
> being representative of any thread's /proc/pid/oom_adj).
> 
A _sane_ user will just set oom_adj to thread-group-leader.
Do you think users are too fool to set per-thread oom_adj independently ?
No problems in real world.


> > I think documentation is wrong. It should say "you should think of
> > multi-thread effect to oom_adj/oom_score".
> > 
> 
> It's more likely than not that applications were probably written to the 
> way the documentation described the two files: that is, adjust 
> /proc/pid/oom_score by tuning /proc/pid/oom_adj instead of relying on an 
> undocumented implementation detail concerning the tuning of oom_adj for a 
> vfork'd child prior to exec().  The user is probably unaware of the oom 
> killer's implementation and simply interprets a higher oom_score as a more 
> likely candidate for oom kill.  My patches preserve that in all scenarios 
> without altering the documentation or adding additional files that would 
> be required to leave the oom_adj value itself in an inconsistent state as 
> you propose.
> 
No.  My understanding is this.

 - oom_adj is designed considering vfork(), of course. then. per-thread.
 - oom_score has been incorrect in multi-threaded system. The user will not
   be affected.
 - you fixed livelock but breaks the feature.


Thanks,
-Kame
  




--
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/