linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: hannes@cmpxchg.org, akpm@linux-foundation.org,
	alan@llwyncelyn.cymru, hch@lst.de, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 1/2] Revert "vmalloc: back off when the current task is killed"
Date: Sat, 7 Oct 2017 09:59:36 +0200	[thread overview]
Message-ID: <20171007075936.nldmvdt6nhujufec@dhcp22.suse.cz> (raw)
In-Reply-To: <201710071305.GJF12474.HSOtLFFJVQFOOM@I-love.SAKURA.ne.jp>

On Sat 07-10-17 13:05:24, Tetsuo Handa wrote:
> Johannes Weiner wrote:
> > On Sat, Oct 07, 2017 at 11:21:26AM +0900, Tetsuo Handa wrote:
> > > On 2017/10/05 19:36, Tetsuo Handa wrote:
> > > > I don't want this patch backported. If you want to backport,
> > > > "s/fatal_signal_pending/tsk_is_oom_victim/" is the safer way.
> > > 
> > > If you backport this patch, you will see "complete depletion of memory reserves"
> > > and "extra OOM kills due to depletion of memory reserves" using below reproducer.
> > > 
> > > ----------
> > > #include <linux/module.h>
> > > #include <linux/slab.h>
> > > #include <linux/oom.h>
> > > 
> > > static char *buffer;
> > > 
> > > static int __init test_init(void)
> > > {
> > > 	set_current_oom_origin();
> > > 	buffer = vmalloc((1UL << 32) - 480 * 1048576);
> > 
> > That's not a reproducer, that's a kernel module. It's not hard to
> > crash the kernel from within the kernel.
> > 
> 
> When did we agree that "reproducer" is "userspace program" ?
> A "reproducer" is a program that triggers something intended.

This way of argumentation is just ridiculous. I can construct whatever
code to put kernel on knees and there is no way around it.

The patch in question was supposed to mitigate a theoretical problem
while it caused a real issue seen out there. That is a reason to
revert the patch. Especially when a better mitigation has been put
in place. You are right that replacing fatal_signal_pending by
tsk_is_oom_victim would keep the original mitigation in pre-cd04ae1e2dc8
kernels but I would only agree to do that if the mitigated problem was
real. And this doesn't seem to be the case. If any of the stable kernels
regresses due to the revert I am willing to put a mitigation in place.
 
> Year by year, people are spending efforts for kernel hardening.
> It is silly to say that "It's not hard to crash the kernel from
> within the kernel." when we can easily mitigate.

This is true but we do not spread random hacks around for problems that
are not real and there are better ways to address them. In this
particular case cd04ae1e2dc8 was a better way to address the problem in
general without spreading tsk_is_oom_victim all over the place.
 
> Even with cd04ae1e2dc8, there is no point with triggering extra
> OOM kills by needlessly consuming memory reserves.

Yet again you are making unfounded claims and I am really fed up
arguing discussing that any further.
-- 
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:[~2017-10-07  7:59 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-03 22:55 tty crash due to auto-failing vmalloc Johannes Weiner
2017-10-03 23:51 ` Alan Cox
2017-10-04  8:33 ` Michal Hocko
2017-10-04 18:58 ` Johannes Weiner
2017-10-04 18:59   ` [PATCH 1/2] Revert "vmalloc: back off when the current task is killed" Johannes Weiner
2017-10-04 20:49     ` Tetsuo Handa
2017-10-04 21:00       ` Johannes Weiner
2017-10-04 21:42         ` Tetsuo Handa
2017-10-04 23:21           ` Johannes Weiner
2017-10-04 22:32     ` Andrew Morton
2017-10-04 23:18       ` Johannes Weiner
2017-10-05  7:57         ` Michal Hocko
2017-10-05 10:36           ` Tetsuo Handa
2017-10-05 10:49             ` Michal Hocko
2017-10-07  2:21             ` Tetsuo Handa
2017-10-07  2:51               ` Johannes Weiner
2017-10-07  4:05                 ` Tetsuo Handa
2017-10-07  7:59                   ` Michal Hocko [this message]
2017-10-07  9:57                     ` Tetsuo Handa
2017-10-05  6:49     ` Vlastimil Babka
2017-10-05  7:54     ` Michal Hocko
2017-10-04 18:59   ` [PATCH 2/2] tty: fall back to N_NULL if switching to N_TTY fails during hangup Johannes Weiner

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=20171007075936.nldmvdt6nhujufec@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=alan@llwyncelyn.cymru \
    --cc=hannes@cmpxchg.org \
    --cc=hch@lst.de \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).