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: akpm@linux-foundation.org, oleg@redhat.com, rientjes@google.com,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] oom: consider multi-threaded tasks in task_will_free_mem
Date: Wed, 13 Apr 2016 15:45:20 +0200	[thread overview]
Message-ID: <20160413134520.GK14351@dhcp22.suse.cz> (raw)
In-Reply-To: <201604132227.BDI51567.VMOFOHFOLQtSFJ@I-love.SAKURA.ne.jp>

On Wed 13-04-16 22:27:52, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > > The whole thread group is going down does not mean we make sure that
> > > we will send SIGKILL to other thread groups sharing the same memory which
> > > is possibly holding mmap_sem for write, does it?
> > 
> > And the patch description doesn't say anything about processes sharing
> > mm. This is supposed to be a minor fix of an obviously suboptimal
> > behavior of task_will_free_mem. Can we stick to the proposed patch,
> > please?
> > 
> > If we really do care about processes sharing mm _that_much_ then it
> > should be handled in the separate patch.
> 
> I do care.

then feel free to post a patch. I believe such a change should be
handled in a separate patch. I have intentionally layed out the code
in a way to allow further checks easily.

Separate processes sharing the same mm have lower priority for me
because I do not know of any recent userspace which would use this
strange threading model or do you have anything specific in mind which
would make it more real-life? We will get to this eventually.

> The OOM reaper cannot work unless SIGKILL is sent to a thread
> which is holding mmap_sem for write. Thus, sending SIGKILL to all thread
> groups sharing the mm is needed by your down_write_killable(&mm->mmap_sem)
> changes. Like I wrote at
> http://lkml.kernel.org/r/201604092300.BDI39040.FFSQLJOMHOOVtF@I-love.SAKURA.ne.jp ,
> we cannot fix that problem unless you accept the slowpath.
> 
> I don't like you don't explain your approach for handling the slowpath.
> If you explain your approach for handling the slowpath and I agree on
> your approach, I will also agree on the proposed patches.

I would much appreciate if you _stopped_ conflating different things
together. This is just generating a lot of fuzz and slows the overal
progress.
-- 
Michal Hocko
SUSE Labs

WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: akpm@linux-foundation.org, oleg@redhat.com, rientjes@google.com,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] oom: consider multi-threaded tasks in task_will_free_mem
Date: Wed, 13 Apr 2016 15:45:20 +0200	[thread overview]
Message-ID: <20160413134520.GK14351@dhcp22.suse.cz> (raw)
In-Reply-To: <201604132227.BDI51567.VMOFOHFOLQtSFJ@I-love.SAKURA.ne.jp>

On Wed 13-04-16 22:27:52, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > > The whole thread group is going down does not mean we make sure that
> > > we will send SIGKILL to other thread groups sharing the same memory which
> > > is possibly holding mmap_sem for write, does it?
> > 
> > And the patch description doesn't say anything about processes sharing
> > mm. This is supposed to be a minor fix of an obviously suboptimal
> > behavior of task_will_free_mem. Can we stick to the proposed patch,
> > please?
> > 
> > If we really do care about processes sharing mm _that_much_ then it
> > should be handled in the separate patch.
> 
> I do care.

then feel free to post a patch. I believe such a change should be
handled in a separate patch. I have intentionally layed out the code
in a way to allow further checks easily.

Separate processes sharing the same mm have lower priority for me
because I do not know of any recent userspace which would use this
strange threading model or do you have anything specific in mind which
would make it more real-life? We will get to this eventually.

> The OOM reaper cannot work unless SIGKILL is sent to a thread
> which is holding mmap_sem for write. Thus, sending SIGKILL to all thread
> groups sharing the mm is needed by your down_write_killable(&mm->mmap_sem)
> changes. Like I wrote at
> http://lkml.kernel.org/r/201604092300.BDI39040.FFSQLJOMHOOVtF@I-love.SAKURA.ne.jp ,
> we cannot fix that problem unless you accept the slowpath.
> 
> I don't like you don't explain your approach for handling the slowpath.
> If you explain your approach for handling the slowpath and I agree on
> your approach, I will also agree on the proposed patches.

I would much appreciate if you _stopped_ conflating different things
together. This is just generating a lot of fuzz and slows the overal
progress.
-- 
Michal Hocko
SUSE Labs

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

  reply	other threads:[~2016-04-13 13:45 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-12  9:19 [PATCH] oom: consider multi-threaded tasks in task_will_free_mem Michal Hocko
2016-04-12  9:19 ` Michal Hocko
2016-04-13 11:04 ` Tetsuo Handa
2016-04-13 11:04   ` Tetsuo Handa
2016-04-13 13:08   ` Michal Hocko
2016-04-13 13:08     ` Michal Hocko
2016-04-13 13:27     ` Tetsuo Handa
2016-04-13 13:27       ` Tetsuo Handa
2016-04-13 13:45       ` Michal Hocko [this message]
2016-04-13 13:45         ` Michal Hocko
2016-05-17 18:06     ` Oleg Nesterov
2016-05-17 18:06       ` Oleg Nesterov
2016-04-26 13:57 ` Michal Hocko
2016-04-26 13:57   ` Michal Hocko
2016-05-17 20:28   ` Michal Hocko
2016-05-17 20:28     ` Michal Hocko
2016-05-17 22:21     ` Andrew Morton
2016-05-17 22:21       ` Andrew Morton
2016-05-18  7:16       ` Michal Hocko
2016-05-18  7:16         ` Michal Hocko
2016-05-17 18:42 ` Oleg Nesterov
2016-05-17 18:42   ` Oleg Nesterov
2016-05-17 20:25   ` Michal Hocko
2016-05-17 20:25     ` Michal Hocko

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=20160413134520.GK14351@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=oleg@redhat.com \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=rientjes@google.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.