Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 1 Mar 2022 04:18:37 +0000
From:   Matthew Wilcox <willy@infradead.org>
To:     Hugh Dickins <hughd@google.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH next] mm/thp: fix collapse_file()'s try_to_unmap(folio,)
Message-ID: <Yh2enUcd2AMP4gq4@casper.infradead.org>
References: <3f187b6c-e5e8-e66d-e0c0-7455ca6abb4c@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <3f187b6c-e5e8-e66d-e0c0-7455ca6abb4c@google.com>
Precedence: bulk

On Sat, Feb 26, 2022 at 06:22:47PM -0800, Hugh Dickins wrote:
> The foliation of THP collapse_file()'s call to try_to_unmap() is
> currently wrong, crashing on a test in rmap_walk() when xas_next()
> delivered a value (after which page has been loaded independently).

Argh.  I have a fear of this exact bug, and I must have missed checking
for it this time.  I hate trying to keep two variables in sync, so my
preferred fix for this is to remove it for this merge window:

+++ b/mm/khugepaged.c
@@ -1699,8 +1699,7 @@ static void collapse_file(struct mm_struct *mm,
 
        xas_set(&xas, start);
        for (index = start; index < end; index++) {
-               struct folio *folio = xas_next(&xas);
-               struct page *page = &folio->page;
+               struct page *page = xas_next(&xas);
 
                VM_BUG_ON(index != xas.xa_index);
                if (is_shmem) {
@@ -1835,7 +1834,8 @@ static void collapse_file(struct mm_struct *mm,
                }
 
                if (page_mapped(page))
-                       try_to_unmap(folio, TTU_IGNORE_MLOCK | TTU_BATCH_FLUSH);
+                       try_to_unmap(page_folio(page),
+                                       TTU_IGNORE_MLOCK | TTU_BATCH_FLUSH);
 
                xas_lock_irq(&xas);
                xas_set(&xas, index);

(ie revert the first hunk).  I'll come back to khugepaged in the next
merge window and convert this function properly.  It's going to take
some surgery to shmem in order to use folios there first ...