From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7DF7FC433F5 for ; Thu, 24 Mar 2022 03:24:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 700636B0072; Wed, 23 Mar 2022 23:24:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6AFF36B0073; Wed, 23 Mar 2022 23:24:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 577E46B0074; Wed, 23 Mar 2022 23:24:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0112.hostedemail.com [216.40.44.112]) by kanga.kvack.org (Postfix) with ESMTP id 443B26B0072 for ; Wed, 23 Mar 2022 23:24:17 -0400 (EDT) Received: from smtpin28.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id BD735A5A92 for ; Thu, 24 Mar 2022 03:24:16 +0000 (UTC) X-FDA: 79277836512.28.F16B259 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf11.hostedemail.com (Postfix) with ESMTP id BD24140005 for ; Thu, 24 Mar 2022 03:24:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=WemKHo27BHrxX0DNCu2DtrZxFOSz3ezuwtVf3oaw5Kw=; b=rWkFMa9GprKcqbUUgEnsA48cai +dlLlIoL62t1O2DNgUbgwmAi8SH46EH38CumZGc5jsYxSy52UuZO99gQKETSLY2wsPL9CMQHFmoWu yRbNGgQC9GN2uKohuLaLTZoOWgmbPwP5gbdCrZkf9o8scscO5M7XJ6aYFuB4gPQyyQJYJyR4n/+5x AEFkMIZfl+WxWlsHGUg64qFs62p1VaK4HdjNTBy4a1xxdprgOJ8tLzOEXVp3BNp2Ummvh9vAnO5tu keonZhML/Cq/0YPhKguJZTNaQn7LLzEPD1fkkRvZaRXl+6AoZ3746C0uk2dFCEJfs0/dsNSje9EBc FzAfUfUw==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1nXE52-00D50X-QR; Thu, 24 Mar 2022 03:24:08 +0000 Date: Thu, 24 Mar 2022 03:24:08 +0000 From: Matthew Wilcox To: Alistair Popple Cc: Sebastian Andrzej Siewior , linux-mm@kvack.org, Andrew Morton , Thomas Gleixner Subject: Re: BUG_ON() in pfn_swap_entry_to_page() Message-ID: References: <871qyt4g4a.fsf@nvdebian.thelocal> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <871qyt4g4a.fsf@nvdebian.thelocal> X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: BD24140005 X-Rspam-User: Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=rWkFMa9G; dmarc=none; spf=none (imf11.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org X-Stat-Signature: 57p374ba58fk71381ziek881tbxat7si X-HE-Tag: 1648092255-92654 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Mar 23, 2022 at 11:29:12AM +1100, Alistair Popple wrote: > Matthew Wilcox writes: > > It's not been reported before, but it seems obvious why it triggers. > > I think it's ffa65753c431 "mm/migrate.c: rework migration_entry_wait() > > to not take a pageref". > > It's obvious the BUG_ON() is because the page isn't locked, but it's less > obvious to me at least why we have a migration entry pointing to an unlocked > page. > > > There's a lot of inlining going on, so let's write this out: > > > > __handle_mm_fault > > handle_pte_fault > > do_swap_page(): > > > > entry = pte_to_swp_entry(vmf->orig_pte); > > if (unlikely(non_swap_entry(entry))) { > > if (is_migration_entry(entry)) { > > migration_entry_wait(vma->vm_mm, vmf->pmd, > > vmf->address); > > > > We don't (and can't) have the underlying page locked here. We're just > > waiting for it to finish migration. > > Why can't the page be locked here? Waiting for migration to finish is equivalent > to waiting for the page to be unlocked. __unmap_and_move() follows this rough > sequence: > > __unmap_and_move() > if (!trylock_page(page)) { > if (!force || mode == MIGRATE_ASYNC) > goto out; > > /* > * It's not safe for direct compaction to call lock_page. > * For example, during page readahead pages are added locked > * to the LRU. Later, when the IO completes the pages are > * marked uptodate and unlocked. However, the queueing > * could be merging multiple pages for one bio (e.g. > * mpage_readahead). If an allocation happens for the > * second or third page, the process can end up locking > * the same page twice and deadlocking. Rather than > * trying to be clever about what pages can be locked, > * avoid the use of lock_page for direct compaction > * altogether. > */ > if (current->flags & PF_MEMALLOC) > goto out; > > lock_page(page); > } > > [...] > > if (unlikely(!trylock_page(newpage))) > goto out_unlock; > > [...] > > } else if (page_mapped(page)) { > /* Establish migration ptes */ > VM_BUG_ON_PAGE(PageAnon(page) && !PageKsm(page) && !anon_vma, > page); > try_to_migrate(page, 0); > page_was_mapped = true; > } > > if (!page_mapped(page)) > rc = move_to_new_page(newpage, page, mode); > > if (page_was_mapped) > remove_migration_ptes(page, > rc == MIGRATEPAGE_SUCCESS ? newpage : page, false); > > Both the source and destination pages (page and newpage respectively) are locked > prior to installing migration entries in try_to_migrate() and unlocked after > migration entries are removed. In order to remove a migration entry the PTL is > required. Therefore while holding the PTL for a migration entry it should be > impossible to observe it pointing to an unlocked page. Hm, right. We check the PTE initially, then take the PTL in __migration_entry_wait() and then re-check that it's still a migration entry. I can't think of how we're seeing this ...