All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
To: Michal Hocko <mhocko@kernel.org>, David Rientjes <rientjes@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [patch -mm] mm, oom: remove oom_lock from exit_mmap
Date: Sat, 14 Jul 2018 06:18:58 +0900	[thread overview]
Message-ID: <44d26c25-6e09-49de-5e90-3c16115eb337@i-love.sakura.ne.jp> (raw)
In-Reply-To: <20180713142612.GD19960@dhcp22.suse.cz>

On 2018/07/13 23:26, Michal Hocko wrote:
> On Thu 12-07-18 14:34:00, David Rientjes wrote:
> [...]
>> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
>> index 0fe4087d5151..e6328cef090f 100644
>> --- a/mm/oom_kill.c
>> +++ b/mm/oom_kill.c
>> @@ -488,9 +488,11 @@ void __oom_reap_task_mm(struct mm_struct *mm)
>>  	 * Tell all users of get_user/copy_from_user etc... that the content
>>  	 * is no longer stable. No barriers really needed because unmapping
>>  	 * should imply barriers already and the reader would hit a page fault
>> -	 * if it stumbled over a reaped memory.
>> +	 * if it stumbled over a reaped memory. If MMF_UNSTABLE is already set,
>> +	 * reaping as already occurred so nothing left to do.
>>  	 */
>> -	set_bit(MMF_UNSTABLE, &mm->flags);
>> +	if (test_and_set_bit(MMF_UNSTABLE, &mm->flags))
>> +		return;
> 
> This could lead to pre mature oom victim selection
> oom_reaper			exiting victim
> oom_reap_task			exit_mmap
>   __oom_reap_task_mm		  __oom_reap_task_mm
> 				    test_and_set_bit(MMF_UNSTABLE) # wins the race
>   test_and_set_bit(MMF_UNSTABLE)
> set_bit(MMF_OOM_SKIP) # new victim can be selected now.
> 
> Besides that, why should we back off in the first place. We can
> race the two without any problems AFAICS. We already do have proper
> synchronization between the two due to mmap_sem and MMF_OOM_SKIP.
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index fc41c0543d7f..4642964f7741 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -3073,9 +3073,7 @@ void exit_mmap(struct mm_struct *mm)
>  		 * which clears VM_LOCKED, otherwise the oom reaper cannot
>  		 * reliably test it.
>  		 */
> -		mutex_lock(&oom_lock);
>  		__oom_reap_task_mm(mm);
> -		mutex_unlock(&oom_lock);
>  
>  		set_bit(MMF_OOM_SKIP, &mm->flags);

David and Michal are using different version as a baseline here.
David is making changes using timeout based back off (in linux-next.git)
which is inappropriately trying to use MMF_UNSTABLE for two purposes.

Michal is making changes using current code (in linux.git) which does not
address David's concern.

My version ( https://marc.info/?l=linux-mm&m=153119509215026 ) is
making changes using current code which also provides oom-badness
based back off in order to address David's concern.

>  		down_write(&mm->mmap_sem);

Anyway, I suggest doing

  mutex_lock(&oom_lock);
  set_bit(MMF_OOM_SKIP, &mm->flags);
  mutex_unlock(&oom_lock);

like I mentioned at
http://lkml.kernel.org/r/201807130620.w6D6KiAJ093010@www262.sakura.ne.jp
even if we make changes on top of linux-next's timeout based back off.


  reply	other threads:[~2018-07-13 21:27 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-12 21:34 [patch -mm] mm, oom: remove oom_lock from exit_mmap David Rientjes
2018-07-13  6:20 ` Tetsuo Handa
2018-07-13 14:26 ` Michal Hocko
2018-07-13 21:18   ` Tetsuo Handa [this message]
2018-07-16  6:13     ` Michal Hocko
2018-07-16  7:04       ` Tetsuo Handa
2018-07-16  7:44         ` Michal Hocko
2018-07-16 10:38           ` Tetsuo Handa
2018-07-16 11:15             ` Michal Hocko
2018-07-17  4:22     ` David Rientjes
2018-07-17  4:14   ` David Rientjes

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=44d26c25-6e09-49de-5e90-3c16115eb337@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=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.