All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	David Rientjes <rientjes@google.com>,
	linux-mm@kvack.org, Yong-Taek Lee <ytk.lee@samsung.com>,
	Paul McKenney <paulmck@linux.vnet.ibm.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] mm, oom: Tolerate processes sharing mm with different view of oom_score_adj.
Date: Mon, 11 Feb 2019 16:07:34 +0100	[thread overview]
Message-ID: <20190211150734.GF15609@dhcp22.suse.cz> (raw)
In-Reply-To: <643b94c2-d720-fa95-d6ee-4f0ea6e2686a@i-love.sakura.ne.jp>

On Sat 02-02-19 20:06:07, Tetsuo Handa wrote:
> int main(int argc, char *argv[])
> {
> 	printf("PID=%d\n", getpid());
> 	if (vfork() == 0) {
> 		clone(thread1, malloc(8192) + 8192,
> 		      CLONE_VM | CLONE_FS | CLONE_FILES, NULL);
> 		sleep(1);
> 		_exit(0);
> 	}
> 	return 0;
> }

This program is not correct AFAIU:
   Standard description
       (From POSIX.1) The vfork() function has the same effect as
       fork(2), except that the behavior is undefined if the process
       created by vfork() either modifies any data other than a variable
       of type pid_t used to store the return value from vfork(), or
       returns from the function in which vfork() was called, or calls
       any other function before successfully calling _exit(2) or one of
       the exec(3) family of functions.
> 
>   PID=8802
>   [ 1138.425255] updating oom_score_adj for 8802 (a.out) from 0 to 1000 because it shares mm with 8804 (a.out). Report if this is unexpected.
> 
> Current loop to enforce same oom_score_adj is 99%+ ending in vain.
> And even your "eventually" will remove this loop.

But it keeps the semantic of the mm shared processes share the same
oomd_score_adj so that we do not have to add kludges to the OOM code to
handle with potential corner cases.

Really, this nagging is both unproductive and annoying. You are right
that the printk is overzealous and it can be dropped. The printk is
more than two years old and we haven't heard anybody to care. So the
first and the most obvious thing to do is to remove it. The patch is
trivial and if I was not buried in the backlog I would have posted it
already.  Regarding a potentially expensive for_each_process. This is
unfortunate but the thing we have to pay for in other paths as well
(e.g. exit path) so closing this only here just doesn't really help
much if you are concerned about security and potential stalls will
explode the machine scenarios.. So even though this sucks it is not
earth shattering. CLONE_VM withtout CLONE_SIGHAND simply sucks and
nobody should be using this threading model.

So, please calm down, try to be more productive and try to understand
what people try to tell you rather than shout around "i want my pony".
-- 
Michal Hocko
SUSE Labs

      reply	other threads:[~2019-02-11 15:07 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-16 10:55 [PATCH] " Tetsuo Handa
2019-01-16 11:09 ` Michal Hocko
2019-01-16 11:30   ` Tetsuo Handa
2019-01-16 12:19     ` Michal Hocko
2019-01-16 13:32       ` Tetsuo Handa
2019-01-16 13:41         ` Michal Hocko
2019-01-17 10:40           ` Tetsuo Handa
2019-01-17 15:51           ` Michal Hocko
2019-01-30 22:49             ` [PATCH v2] " Tetsuo Handa
2019-01-31  7:11               ` Michal Hocko
2019-01-31 20:59                 ` Tetsuo Handa
2019-02-01  9:14                   ` Michal Hocko
2019-02-02 11:06                     ` Tetsuo Handa
2019-02-11 15:07                       ` Michal Hocko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190211150734.GF15609@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=rientjes@google.com \
    --cc=torvalds@linux-foundation.org \
    --cc=ytk.lee@samsung.com \
    --subject='Re: [PATCH v2] mm, oom: Tolerate processes sharing mm with different view of oom_score_adj.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.