Re: [syzbot] [mm?] kernel BUG in vma_replace_policy

From: Matthew Wilcox <willy@infradead.org>
To: Hugh Dickins <hughd@google.com>
Cc: Suren Baghdasaryan <surenb@google.com>,
	Yang Shi <shy828301@gmail.com>, Michal Hocko <mhocko@suse.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	syzbot <syzbot+b591856e0f0139f83023@syzkaller.appspotmail.com>,
	akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, syzkaller-bugs@googlegroups.com
Subject: Re: [syzbot] [mm?] kernel BUG in vma_replace_policy
Date: Fri, 15 Sep 2023 19:26:11 +0100	[thread overview]
Message-ID: <ZQShw8lESIBle7GF@casper.infradead.org> (raw)
In-Reply-To: <da13f5a-b87f-d776-e58a-d492fa2773f9@google.com>

On Thu, Sep 14, 2023 at 09:26:15PM -0700, Hugh Dickins wrote:
> On Thu, 14 Sep 2023, Suren Baghdasaryan wrote:
> > Yes, I just finished running the reproducer on both upstream and
> > linux-next builds listed in
> > https://syzkaller.appspot.com/bug?extid=b591856e0f0139f83023 and the
> > problem does not happen anymore.
> > I'm fine with your suggestion too, just wanted to point out it would
> > introduce change in the behavior. Let me know how you want to proceed.
> 
> Well done, identifying the mysterious cause of this problem:
> I'm glad to hear that you've now verified that hypothesis.
> 
> You're right, it would be a regression to follow Matthew's suggestion.
> 
> Traditionally, modulo bugs and inconsistencies, the queue_pages_range()
> phase of do_mbind() has done the best it can, gathering all the pages it
> can that need migration, even if some were missed; and proceeds to do the
> mbind_range() phase if there was nothing "seriously" wrong (a gap causing
> -EFAULT).  Then at the end, if MPOL_MF_STRICT was set, and not all the
> pages could be migrated (or MOVE was not specified and not all pages
> were well placed), it returns -EIO rather than 0 to inform the caller
> that not all could be done.
> 
> There have been numerous tweaks, but I think most importantly
> 5.3's d883544515aa ("mm: mempolicy: make the behavior consistent when
> MPOL_MF_MOVE* and MPOL_MF_STRICT were specified") added those "return 1"s
> which stop the pagewalk early.  In my opinion, not an improvement - makes
> it harder to get mbind() to do the best job it can (or is it justified as
> what you're asking for if you say STRICT?).

I suspect you agree that it's inconsistent to stop early.  Userspace
doesn't know at which point we found an unmovable page, so it can't behave
rationally.  Perhaps we should remove the 'early stop' and attempt to
migrate every page in the range, whether it's before or after the first
unmovable page?