LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Dmitry Vyukov <dvyukov@google.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: syzbot <syzbot+bab151e82a4e973fa325@syzkaller.appspotmail.com>,
	cgroups@vger.kernel.org, Johannes Weiner <hannes@cmpxchg.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>,
	syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
	Vladimir Davydov <vdavydov.dev@gmail.com>,
	Dmitry Torokhov <dtor@google.com>
Subject: Re: WARNING in try_charge
Date: Mon, 6 Aug 2018 12:34:30 +0200
Message-ID: <CACT4Y+ZJsDo1gjzHvbFVqHcrL=tFJXTAAWLs9mAJSv3+LiCdmA@mail.gmail.com> (raw)
In-Reply-To: <20180806094827.GH19540@dhcp22.suse.cz>

On Mon, Aug 6, 2018 at 11:48 AM, Michal Hocko <mhocko@kernel.org> wrote:
> On Mon 06-08-18 11:30:37, Dmitry Vyukov wrote:
>> On Mon, Aug 6, 2018 at 11:15 AM, Michal Hocko <mhocko@kernel.org> wrote:
> [...]
>> > More interesting stuff is higher in the kernel log
>> > : [  366.435015] oom-kill:constraint=CONSTRAINT_MEMCG,nodemask=(null),cpuset=/,mems_allowed=0,oom_memcg=/ile0,task_memcg=/ile0,task=syz-executor3,pid=23766,uid=0
>> > : [  366.449416] memory: usage 112kB, limit 0kB, failcnt 1605
>> >
>> > Are you sure you want to have hard limit set to 0?
>>
>> syzkaller really does not mind to have it.
>
> So what do you use it for? What do you actually test by this setting?

syzkaller is kernel fuzzer, it finds kernel bugs by doing whatever is
doable from user-space. Some of that may not make sense, but it does
not matter because kernel should still stand still.

> [...]
>> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> > index 4603ad75c9a9..852cd3dbdcd9 100644
>> > --- a/mm/memcontrol.c
>> > +++ b/mm/memcontrol.c
>> > @@ -1388,6 +1388,8 @@ static bool mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
>> >         bool ret;
>> >
>> >         mutex_lock(&oom_lock);
>> > +       pr_info("task=%s pid=%d invoked memcg oom killer. oom_victim=%d\n",
>> > +                       current->comm, current->pid, tsk_is_oom_victim(current));
>> >         ret = out_of_memory(&oc);
>> >         mutex_unlock(&oom_lock);
>> >         return ret;
>> >
>> > Anyway your memcg setup is indeed misconfigured. Memcg with 0 hard limit
>> > and basically no memory charged by existing tasks is not going to fly
>> > and the warning is exactly to call that out.
>>
>>
>> Please-please-please do not mix kernel bugs and notices to user into
>> the same bucket:
>
> Well, WARN_ON used to be a standard way to make user aware of a
> misbehavior. In this case it warns about a pottential runaway when memcg
> is misconfigured. I do not insist on using WARN_ON here of course. If
> there is a general agreement that such a condition is better handled by
> pr_err then I am fine with it. Users tend to be more sensitive on
> WARN_ONs though.

The docs change was acked by Greg, and Andrew took it into mm, Linus
was CCed too. It missed the release because I guess it's comments only
change, but otherwise it should reach upstream tree on the next merge
window.

WARN is _not_ a common way to notify users today. syzbot reports _all_
WARN occurrences and you can see there are not many of them now
(probably 1 another now, +dtor for that one):
https://syzkaller.appspot.com#upstream
There is probably some long tail that we need to fix. We really do
want systematic testing capability. You do not want every of 2 billion
linux users to come to you with this kernel splat, just so that you
can explain to them that it's some programs of their machines doing
something wrong, right?

WARN is really a bad way to inform a user about something. Consider a
non-kernel developer, perhaps even non-programmer. What they see is
"WARNING: CPU: 1 PID: 23767 at mm/memcontrol.c:1710
try_charge+0x734/0x1680" followed by some obscure things and hex
numbers. File:line reference is pointless, they don't what what/where
it is. This one is slightly better because it prints "Memory cgroup
charge failed because of no reclaimable memory! This looks like a
misconfiguration or a kernel bug." before the warning. But still it
says "or a kernel bug", which means that they will come to you. A much
friendlier for user way to say this would be print a message at the
point of misconfiguration saying what exactly is wrong, e.g. "pid $PID
misconfigures cgroup /cgroup/path with mem.limit=0" without a stack
trace (does not give any useful info for user). And return EINVAL if
it can't fly at all? And then leave the "or a kernel bug" part for the
WARNING each occurrence of which we do want to be reported to kernel
developers.

  reply index

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-04 13:33 syzbot
2018-08-04 13:45 ` Tetsuo Handa
2018-08-05 11:33   ` Tetsuo Handa
2018-08-05  8:14 ` syzbot
2018-08-06  9:15 ` Michal Hocko
2018-08-06  9:30   ` Dmitry Vyukov
2018-08-06  9:48     ` Michal Hocko
2018-08-06 10:34       ` Dmitry Vyukov [this message]
2018-08-06 11:02         ` Michal Hocko
2018-08-06 11:57           ` Dmitry Vyukov
2018-08-06 14:21             ` Michal Hocko
2018-08-06 14:58               ` Dmitry Vyukov
2018-08-06 17:30                 ` Michal Hocko
2018-08-06 17:53                   ` Dmitry Vyukov
2018-08-06 15:07               ` Dmitry Vyukov
2018-08-06 15:31               ` Johannes Weiner
2018-08-06 10:39       ` Dmitry Vyukov
2018-08-06 10:47         ` Tetsuo Handa
2018-08-06 11:09           ` Michal Hocko
2018-08-06 11:27           ` syzbot
2018-08-06 11:32             ` Michal Hocko
2018-08-06 11:58               ` Dmitry Vyukov
2018-08-06 14:41               ` Tetsuo Handa
2018-08-06 14:58                 ` Michal Hocko
2018-08-06 15:12                   ` Tetsuo Handa
2018-08-06 14:54               ` David Howells
2018-08-06 15:04                 ` Tetsuo Handa
2018-08-06 11:00         ` syzbot
2018-08-06 15:32         ` Tetsuo Handa
2018-08-06 15:42           ` syzbot
2018-08-06 16:02             ` Tetsuo Handa
2018-08-06 17:44             ` Michal Hocko
2018-08-06 17:49               ` Dmitry Vyukov
2018-08-06 17:56               ` Michal Hocko
2018-08-06 18:13                 ` Michal Hocko
2018-08-06 18:23                   ` syzbot
2018-08-06 18:55                     ` Michal Hocko
2018-08-06 19:12                       ` syzbot
2018-08-06 19:45                         ` Michal Hocko
2018-08-06 19:46                           ` Michal Hocko
2018-08-07 11:18                       ` Dmitry Vyukov
2018-08-07 11:25                         ` Michal Hocko
2018-08-06 18:39                   ` Michal Hocko
2018-08-06 20:26                 ` Tetsuo Handa
2018-08-06 20:34                   ` Michal Hocko
2018-08-06 20:46                     ` Tetsuo Handa
2018-08-06 20:55                       ` Michal Hocko
2018-08-06 21:50                         ` Tetsuo Handa
2018-08-07 10:19                           ` Tetsuo Handa
2018-08-09 13:57 ` Tetsuo Handa
2018-08-09 15:07   ` Michal Hocko
2018-08-09 21:05     ` Tetsuo Handa
2018-08-09 15:34   ` Johannes Weiner

Reply instructions:

You may reply publically 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='CACT4Y+ZJsDo1gjzHvbFVqHcrL=tFJXTAAWLs9mAJSv3+LiCdmA@mail.gmail.com' \
    --to=dvyukov@google.com \
    --cc=cgroups@vger.kernel.org \
    --cc=dtor@google.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=syzbot+bab151e82a4e973fa325@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=vdavydov.dev@gmail.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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox