All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Vladimir Davydov <vdavydov@virtuozzo.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	David Rientjes <rientjes@google.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] exit: clear TIF_MEMDIE after exit_task_work
Date: Mon, 29 Feb 2016 19:21:31 +0100	[thread overview]
Message-ID: <20160229182131.GP16930@dhcp22.suse.cz> (raw)
In-Reply-To: <1456765329-14890-1-git-send-email-vdavydov@virtuozzo.com>

On Mon 29-02-16 20:02:09, Vladimir Davydov wrote:
> An mm_struct may be pinned by a file. An example is vhost-net device
> created by a qemu/kvm (see vhost_net_ioctl -> vhost_net_set_owner ->
> vhost_dev_set_owner). If such process gets OOM-killed, the reference to
> its mm_struct will only be released from exit_task_work -> ____fput ->
> __fput -> vhost_net_release -> vhost_dev_cleanup, which is called after
> exit_mmap, where TIF_MEMDIE is cleared. As a result, we can start
> selecting the next victim before giving the last one a chance to free
> its memory. In practice, this leads to killing several VMs along with
> the fattest one.

I am wondering why our PF_EXITING protection hasn't fired up. This is
not done in the mmotm tree but I guess you have seen the issue with the
linus tree, right? Do you have a log with oom reports available?

To be honest I do not feel very comfortable about moving the
exit_oom_victim even further down in do_exit path behind even less clear
locking or other dependencies.

Let's see if we can do any better for this particular case. 

> Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>
> ---
>  kernel/exit.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/exit.c b/kernel/exit.c
> index fd90195667e1..cc50e12165f7 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -434,8 +434,6 @@ static void exit_mm(struct task_struct *tsk)
>  	task_unlock(tsk);
>  	mm_update_next_owner(mm);
>  	mmput(mm);
> -	if (test_thread_flag(TIF_MEMDIE))
> -		exit_oom_victim(tsk);
>  }
>  
>  static struct task_struct *find_alive_thread(struct task_struct *p)
> @@ -746,6 +744,8 @@ void do_exit(long code)
>  		disassociate_ctty(1);
>  	exit_task_namespaces(tsk);
>  	exit_task_work(tsk);
> +	if (test_thread_flag(TIF_MEMDIE))
> +		exit_oom_victim(tsk);
>  	exit_thread();
>  
>  	/*
> -- 
> 2.1.4

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

WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org>
To: Vladimir Davydov <vdavydov@virtuozzo.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	David Rientjes <rientjes@google.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] exit: clear TIF_MEMDIE after exit_task_work
Date: Mon, 29 Feb 2016 19:21:31 +0100	[thread overview]
Message-ID: <20160229182131.GP16930@dhcp22.suse.cz> (raw)
In-Reply-To: <1456765329-14890-1-git-send-email-vdavydov@virtuozzo.com>

On Mon 29-02-16 20:02:09, Vladimir Davydov wrote:
> An mm_struct may be pinned by a file. An example is vhost-net device
> created by a qemu/kvm (see vhost_net_ioctl -> vhost_net_set_owner ->
> vhost_dev_set_owner). If such process gets OOM-killed, the reference to
> its mm_struct will only be released from exit_task_work -> ____fput ->
> __fput -> vhost_net_release -> vhost_dev_cleanup, which is called after
> exit_mmap, where TIF_MEMDIE is cleared. As a result, we can start
> selecting the next victim before giving the last one a chance to free
> its memory. In practice, this leads to killing several VMs along with
> the fattest one.

I am wondering why our PF_EXITING protection hasn't fired up. This is
not done in the mmotm tree but I guess you have seen the issue with the
linus tree, right? Do you have a log with oom reports available?

To be honest I do not feel very comfortable about moving the
exit_oom_victim even further down in do_exit path behind even less clear
locking or other dependencies.

Let's see if we can do any better for this particular case. 

> Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>
> ---
>  kernel/exit.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/exit.c b/kernel/exit.c
> index fd90195667e1..cc50e12165f7 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -434,8 +434,6 @@ static void exit_mm(struct task_struct *tsk)
>  	task_unlock(tsk);
>  	mm_update_next_owner(mm);
>  	mmput(mm);
> -	if (test_thread_flag(TIF_MEMDIE))
> -		exit_oom_victim(tsk);
>  }
>  
>  static struct task_struct *find_alive_thread(struct task_struct *p)
> @@ -746,6 +744,8 @@ void do_exit(long code)
>  		disassociate_ctty(1);
>  	exit_task_namespaces(tsk);
>  	exit_task_work(tsk);
> +	if (test_thread_flag(TIF_MEMDIE))
> +		exit_oom_victim(tsk);
>  	exit_thread();
>  
>  	/*
> -- 
> 2.1.4

-- 
Michal Hocko
SUSE Labs

  reply	other threads:[~2016-02-29 18:21 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-29 17:02 [PATCH] exit: clear TIF_MEMDIE after exit_task_work Vladimir Davydov
2016-02-29 17:02 ` Vladimir Davydov
2016-02-29 18:21 ` Michal Hocko [this message]
2016-02-29 18:21   ` Michal Hocko
2016-02-29 18:44   ` Michal Hocko
2016-02-29 18:44     ` Michal Hocko
2016-03-01 15:52 ` Michal Hocko
2016-03-01 15:52   ` Michal Hocko
2016-03-01 15:57   ` Michael S. Tsirkin
2016-03-01 15:57     ` Michael S. Tsirkin
2016-03-01 16:08     ` Michal Hocko
2016-03-01 16:08       ` Michal Hocko
2016-03-01 16:14       ` Michael S. Tsirkin
2016-03-01 16:14         ` Michael S. Tsirkin
2016-03-01 16:22       ` Michael S. Tsirkin
2016-03-01 16:22         ` Michael S. Tsirkin
2016-03-01 16:35         ` Michal Hocko
2016-03-01 16:35           ` Michal Hocko
2016-03-01 16:46           ` Michael S. Tsirkin
2016-03-01 16:46             ` Michael S. Tsirkin
2016-03-01 17:17             ` Michal Hocko
2016-03-01 17:17               ` Michal Hocko
2016-03-01 17:20               ` Michael S. Tsirkin
2016-03-01 17:20                 ` Michael S. Tsirkin
2016-03-14 16:39                 ` Michal Hocko
2016-03-14 16:39                   ` Michal Hocko
2016-06-07 12:50                   ` Michal Hocko
2016-06-07 12:50                     ` Michal Hocko
2016-06-13 11:50                     ` Michal Hocko
2016-06-13 11:50                       ` Michal Hocko
2016-06-13 13:52                       ` Tetsuo Handa
2016-06-13 13:52                         ` Tetsuo Handa
2016-06-13 14:00                         ` Michal Hocko
2016-06-13 14:00                           ` Michal Hocko
2016-06-13 18:11                       ` Michael S. Tsirkin
2016-06-13 18:11                         ` Michael S. Tsirkin

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=20160229182131.GP16930@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=rientjes@google.com \
    --cc=vdavydov@virtuozzo.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.