From: Johannes Weiner <hannes@cmpxchg.org>
To: Hugh Dickins <hughd@google.com>
Cc: Qian Cai <cai@lca.pw>, Matthew Wilcox <willy@infradead.org>,
LKML <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
aarcange@redhat.com, Alex Shi <alex.shi@linux.alibaba.com>,
daniel.m.jordan@oracle.com, khlebnikov@yandex-team.ru,
kirill@shutemov.name, kravetz@us.ibm.com, mhocko@kernel.org,
mm-commits@vger.kernel.org, tj@kernel.org,
vdavydov.dev@gmail.com, yang.shi@linux.alibaba.com,
linux-mm@kvack.org
Subject: Re: [failures] mm-vmscan-remove-unnecessary-lruvec-adding.patch removed from -mm tree
Date: Fri, 6 Mar 2020 09:54:48 -0500 [thread overview]
Message-ID: <20200306145448.GC2508@cmpxchg.org> (raw)
In-Reply-To: <alpine.LSU.2.11.2003052008510.3016@eggly.anvils>
On Thu, Mar 05, 2020 at 08:17:46PM -0800, Hugh Dickins wrote:
> On Tue, 3 Mar 2020, Alex Shi wrote:
> > 在 2020/3/3 上午6:12, Andrew Morton 写道:
> > >> Thanks for Testing support from Intel 0day and Rong Chen, Fengguang Wu,
> > >> and Yun Wang.
> > > I'm not seeing a lot of evidence of review and test activity yet. But
> > > I think I'll grab patches 01-06 as they look like fairly
> > > straightforward improvements.
> >
> > cc Fengguang and Rong Chen
> >
> > I did some local functional testing and kselftest, they all look fine.
> > 0day only warn me if some case failed. Is it no news is good news? :)
>
> And now the bad news.
>
> Andrew, please revert those six (or seven as they ended up in mmotm).
> 5.6-rc4-mm1 without them runs my tmpfs+loop+swapping+memcg+ksm kernel
> build loads fine (did four hours just now), but 5.6-rc4-mm1 itself
> crashed just after starting - seconds or minutes I didn't see,
> but it did not complete an iteration.
>
> I thought maybe those six would be harmless (though I've not looked
> at them at all); but knew already that the full series is not good yet:
> I gave it a try over 5.6-rc4 on Monday, and crashed very soon on simpler
> testing, in different ways from what hits mmotm.
>
> The first thing wrong with the full set was when I tried tmpfs+loop+
> swapping kernel builds in "mem=700M cgroup_disabled=memory", of course
> with CONFIG_DEBUG_LIST=y. That soon collapsed in a splurge of OOM kills
> and list_del corruption messages: __list_del_entry_valid < list_del <
> __page_cache_release < __put_page < put_page < __try_to_reclaim_swap <
> free_swap_and_cache < shmem_free_swap < shmem_undo_range.
>
> When I next tried with "mem=1G" and memcg enabled (but not being used),
> that managed some iterations, no OOM kills, no list_del warnings (was
> it swapping? perhaps, perhaps not, I was trying to go easy on it just
> to see if "cgroup_disabled=memory" had been the problem); but when
> rebooting after that, again list_del corruption messages and crash
> (I didn't note them down).
>
> So I didn't take much notice of what the mmotm crash backtrace showed
> (but IIRC shmem and swap were in it).
>
> Alex, I'm afraid you're focusing too much on performance results,
> without doing the basic testing needed - I thought we had given you
> some hints on the challenging areas (swapping, move_charge_at_immigrate,
> page migration) when we attached a *correctly working* 5.3 version back
> on 23rd August:
>
> https://lore.kernel.org/linux-mm/alpine.LSU.2.11.1908231736001.16920@eggly.anvils/
>
> (Correctly working, except missing two patches I'd mistakenly dropped
> as unnecessary in earlier rebases: but our discussions with Johannes
> later showed to be very necessary, though their races rarely seen.)
>
> I have not had the time (and do not expect to have the time) to review
> your series: maybe it's one or two small fixes away from being complete,
> or maybe it's still fundamentally flawed, I do not know. I had naively
> hoped that you would help with a patchset that worked, rather than
> cutting it down into something which does not.
I'm a bit confused by this. I, and I believe Alex, kept going down a
different path because it didn't sound like there was a solution to
the compaction race. As I remember, the conversation ended on this:
: Your race here (again, lruvec lock taken then PageLRU observed, but
: page->mem_cgroup changed in between) really questions my whole scheme:
: I am not going to propose a solution now, I'll have to go back and
: recheck my assumptions all over. Certainly isolate_migratepage_block()
: has a harder job than any other, but I need to re-review it all.
https://lore.kernel.org/lkml/alpine.LSU.2.11.1911221616580.1144@eggly.anvils/
That's certainly why I kept looking and eventually proposed using
PageLRU clearing as a lock. Maybe there is a better way to do it, but
I didn't see it.
An LRU list corruption in page_cache_release() suggests a bug in the
way this new locking scheme works or is applied - rather than a
gratuitous divergence from your series that could have been avoided.
> Submitting your series to routine testing is much easier for me than
> reviewing it: but then, yes, it's a pity that I don't find the time
> to report the results on intervening versions, which also crashed.
>
> What I have to do now, is set aside time today and tomorrow, to package
> up the old scripts I use, describe them and their environment, and send
> them to you (cc akpm in case I fall under a bus): so that you can
> reproduce the crashes for yourself, and get to work on them.
I think that would be very useful. tmpfs+loop+swapping+memcg+ksm
kernel builds aren't exactly a go-to test case for most mm developers
(although maybe they should be!)
next prev parent reply other threads:[~2020-03-06 14:54 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200306025041.rERhvnYmB%akpm@linux-foundation.org>
2020-03-06 3:32 ` [failures] mm-vmscan-remove-unnecessary-lruvec-adding.patch removed from -mm tree Qian Cai
2020-03-06 3:38 ` Matthew Wilcox
2020-03-06 3:50 ` Qian Cai
2020-03-06 4:17 ` Hugh Dickins
2020-03-06 4:42 ` Alex Shi
2020-03-06 4:46 ` Qian Cai
2020-03-06 13:30 ` Alex Shi
2020-03-06 14:54 ` Johannes Weiner [this message]
2020-03-06 9:04 ` Alex Shi
2020-03-06 11:58 ` Alex Shi
2020-03-07 2:27 ` Qian Cai
2020-03-07 3:26 ` Alex Shi
2020-03-07 3:31 ` Qian Cai
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=20200306145448.GC2508@cmpxchg.org \
--to=hannes@cmpxchg.org \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=alex.shi@linux.alibaba.com \
--cc=cai@lca.pw \
--cc=daniel.m.jordan@oracle.com \
--cc=hughd@google.com \
--cc=khlebnikov@yandex-team.ru \
--cc=kirill@shutemov.name \
--cc=kravetz@us.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=mm-commits@vger.kernel.org \
--cc=tj@kernel.org \
--cc=vdavydov.dev@gmail.com \
--cc=willy@infradead.org \
--cc=yang.shi@linux.alibaba.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 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).