LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
To: Michal Hocko <mhocko@kernel.org>
Cc: syzbot <syzbot+bab151e82a4e973fa325@syzkaller.appspotmail.com>,
	cgroups@vger.kernel.org, dvyukov@google.com, hannes@cmpxchg.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	syzkaller-bugs@googlegroups.com, vdavydov.dev@gmail.com
Subject: Re: WARNING in try_charge
Date: Tue, 7 Aug 2018 06:50:09 +0900
Message-ID: <9c03213f-c099-378b-e9fd-ed6f2a2afdc3@i-love.sakura.ne.jp> (raw)
In-Reply-To: <20180806205519.GO10003@dhcp22.suse.cz>

On 2018/08/07 5:55, Michal Hocko wrote:
> On Tue 07-08-18 05:46:04, Tetsuo Handa wrote:
>> On 2018/08/07 5:34, Michal Hocko wrote:
>>> On Tue 07-08-18 05:26:23, Tetsuo Handa wrote:
>>>> On 2018/08/07 2:56, Michal Hocko wrote:
>>>>> So the oom victim indeed passed the above force path after the oom
>>>>> invocation. But later on hit the page fault path and that behaved
>>>>> differently and for some reason the force path hasn't triggered. I am
>>>>> wondering how could we hit the page fault path in the first place. The
>>>>> task is already killed! So what the hell is going on here.
>>>>>
>>>>> I must be missing something obvious here.
>>>>>
>>>> YOU ARE OBVIOUSLY MISSING MY MAIL!
>>>>
>>>> I already said this is "mm, oom: task_will_free_mem(current) should ignore MMF_OOM_SKIP for once."
>>>> problem which you are refusing at https://www.spinics.net/lists/linux-mm/msg133774.html .
>>>> And you again ignored my mail. Very sad...
>>>
>>> Your suggestion simply didn't make much sense. There is nothing like
>>> first check is different from the rest.
>>>
>>
>> I don't think your patch is appropriate. It avoids hitting WARN(1) but does not avoid
>> unnecessary killing of OOM victims.
>>
>> If you look at https://syzkaller.appspot.com/text?tag=CrashLog&x=15a1c770400000 , you will
>> notice that both 23766 and 23767 are killed due to task_will_free_mem(current) == false.
>> This is "unnecessary killing of additional processes".
> 
> Have you noticed the mere detail that the memcg has to kill any task
> attempting the charge because the hard limit is 0? There is simply no
> other way around. You cannot charge. There is no unnecessary killing.
> Full stop. We do allow temporary breach of the hard limit just to let
> the task die and uncharge on the way out.
> 

select_bad_process() is called just because
task_will_free_mem("already killed current thread which has not completed __mmput()") == false
is a bug. I'm saying that the OOM killer should not give up as soon as MMF_OOM_SKIP is set.

 static bool oom_has_pending_victims(struct oom_control *oc)
 {
 	struct task_struct *p, *tmp;
 	bool ret = false;
 	bool gaveup = false;
 
 	if (is_sysrq_oom(oc))
 		return false;
 	/*
 	 * Wait for pending victims until __mmput() completes or stalled
 	 * too long.
 	 */
 	list_for_each_entry_safe(p, tmp, &oom_victim_list, oom_victim_list) {
 		struct mm_struct *mm = p->signal->oom_mm;
 
 		if (oom_unkillable_task(p, oc->memcg, oc->nodemask))
 			continue;
 		ret = true;
+		/*
+		 * Since memcg OOM allows forced charge, we can safely wait
+		 * until __mmput() completes.
+		 */
+		if (is_memcg_oom(oc))
+			return true;
 #ifdef CONFIG_MMU
 		/*
 		 * Since the OOM reaper exists, we can safely wait until
 		 * MMF_OOM_SKIP is set.
 		 */
 		if (!test_bit(MMF_OOM_SKIP, &mm->flags)) {
 			if (!oom_reap_target) {
 				get_task_struct(p);
 				oom_reap_target = p;
 				trace_wake_reaper(p->pid);
 				wake_up(&oom_reaper_wait);
 			}
 #endif
 			continue;
 		}
 #endif
 		/* We can wait as long as OOM score is decreasing over time. */
 		if (!victim_mm_stalling(p, mm))
 			continue;
 		gaveup = true;
 		list_del(&p->oom_victim_list);
 		/* Drop a reference taken by mark_oom_victim(). */
 		put_task_struct(p);
 	}
 	if (gaveup)
 		debug_show_all_locks();
 
 	return ret;
 }


  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
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 [this message]
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=9c03213f-c099-378b-e9fd-ed6f2a2afdc3@i-love.sakura.ne.jp \
    --to=penguin-kernel@i-love.sakura.ne.jp \
    --cc=cgroups@vger.kernel.org \
    --cc=dvyukov@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
	public-inbox-index lkml

Example config snippet for mirrors

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