All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: mhocko@kernel.org
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 22:27:52 +0900	[thread overview]
Message-ID: <201604132227.BDI51567.VMOFOHFOLQtSFJ@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <20160413130858.GI14351@dhcp22.suse.cz>

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

WARNING: multiple messages have this Message-ID (diff)
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: mhocko@kernel.org
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 22:27:52 +0900	[thread overview]
Message-ID: <201604132227.BDI51567.VMOFOHFOLQtSFJ@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <20160413130858.GI14351@dhcp22.suse.cz>

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

--
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:28 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 [this message]
2016-04-13 13:27       ` Tetsuo Handa
2016-04-13 13:45       ` Michal Hocko
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=201604132227.BDI51567.VMOFOHFOLQtSFJ@I-love.SAKURA.ne.jp \
    --to=penguin-kernel@i-love.sakura.ne.jp \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=oleg@redhat.com \
    --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.