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 7F153C4332F for ; Thu, 24 Nov 2022 03:33:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 23BF56B0072; Wed, 23 Nov 2022 22:33:35 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1C5DB6B0074; Wed, 23 Nov 2022 22:33:35 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 08CDA6B0075; Wed, 23 Nov 2022 22:33:35 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id E94756B0072 for ; Wed, 23 Nov 2022 22:33:34 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id BAB2AC127E for ; Thu, 24 Nov 2022 03:33:34 +0000 (UTC) X-FDA: 80166915948.10.BBADF61 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf02.hostedemail.com (Postfix) with ESMTP id E68A180010 for ; Thu, 24 Nov 2022 03:33:33 +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=SiYzGWsRarEdt81Og2LTbZsHP1GWYUw0h1JMLzGK6gc=; b=rWgR4hAalUnGhNvEqX826xzxNB K4oSWO0gcBRj+m4eI4DUB8xXzQmbQbVC1L4TCRMe9n+8t8EqVLyXLrMdFcnRb7yUmv4c4gPYKFlhY VoO1DFAyA5IhXAc7Yln76y40O3ok5lfnVACt4nq6k+BJ08meFBD9tZyo9vptBQFqYmDAt6suEdqmj ri8conSsbCyAf9i0HtJhTB5Fx/iC59MDaKsyxbij1y6G5BNhwFv74bjhcFyPffZvAAi5gMbN52XRy ELp3UPyvx4jLBihtwHYg/ci9KOIIoAchx7k1knEUucZS+RTd/Jj3o20u09jIW/+gudAskJVN9jBpz D4h0odvg==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1oy2zT-008LGP-Sl; Thu, 24 Nov 2022 03:33:31 +0000 Date: Thu, 24 Nov 2022 03:33:31 +0000 From: Matthew Wilcox To: Alistair Popple Cc: David Hildenbrand , Hugh Dickins , Gavin Shan , linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, william.kucharski@oracle.com, ziy@nvidia.com, kirill.shutemov@linux.intel.com, zhenyzha@redhat.com, shan.gavin@gmail.com, riel@surriel.com Subject: Re: [PATCH] mm: migrate: Fix THP's mapcount on isolation Message-ID: References: <20221123005752.161003-1-gshan@redhat.com> <871qptrvsw.fsf@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <871qptrvsw.fsf@nvidia.com> ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1669260814; a=rsa-sha256; cv=none; b=53cb3hJZwslPpKtWO/XsaSe7iC5V1ffXmROvMKTo1NbWBpDBDSzhf1ljz932DMeEGP7o46 g97vKh/kHF/8wgBkDhhNZjb2k0oQOw1qy2dKE/+yQ6mB/f+GTRQN8BKZfOQv8bOSYs13dK RXjcwFnVgyStlXkj8wEgxmdQ8bpBfqE= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=rWgR4hAa; dmarc=none; spf=none (imf02.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1669260814; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=SiYzGWsRarEdt81Og2LTbZsHP1GWYUw0h1JMLzGK6gc=; b=YFJEIaicUpibUf71mEiejVpxmB6kxs+60QCko2gs7c7GoF4yVCurDp/cZKpTqP8u3rbwiI qHl8lD3UMJlTjJBpuawlYfZ994d5zS+BTsEH7WMit4BIskcc+IfSzWyfgKF1DUtk/+fr3X jdh6OXh0poVkLFEZSFViYqdy2QXtGcE= X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: E68A180010 X-Rspam-User: Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=rWgR4hAa; dmarc=none; spf=none (imf02.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: 6wbxbpn1m1wdgaf91h1k3izq79ogxziw X-HE-Tag: 1669260813-581792 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 Thu, Nov 24, 2022 at 12:06:56PM +1100, Alistair Popple wrote: > > David Hildenbrand writes: > > > On 23.11.22 06:14, Hugh Dickins wrote: > >> On Wed, 23 Nov 2022, Gavin Shan wrote: > >> > >>> The issue is reported when removing memory through virtio_mem device. > >>> The transparent huge page, experienced copy-on-write fault, is wrongly > >>> regarded as pinned. The transparent huge page is escaped from being > >>> isolated in isolate_migratepages_block(). The transparent huge page > >>> can't be migrated and the corresponding memory block can't be put > >>> into offline state. > >>> > >>> Fix it by replacing page_mapcount() with total_mapcount(). With this, > >>> the transparent huge page can be isolated and migrated, and the memory > >>> block can be put into offline state. > >>> > >>> Fixes: 3917c80280c9 ("thp: change CoW semantics for anon-THP") > >>> Cc: stable@vger.kernel.org # v5.8+ > >>> Reported-by: Zhenyu Zhang > >>> Suggested-by: David Hildenbrand > >>> Signed-off-by: Gavin Shan > >> Interesting, good catch, looked right to me: except for the Fixes > >> line > >> and mention of v5.8. That CoW change may have added a case which easily > >> demonstrates the problem, but it would have been the wrong test on a THP > >> for long before then - but only in v5.7 were compound pages allowed > >> through at all to reach that test, so I think it should be > >> Fixes: 1da2f328fa64 ("mm,thp,compaction,cma: allow THP migration for > >> CMA allocations") > >> Cc: stable@vger.kernel.org # v5.7+ > >> Oh, no, stop: this is not so easy, even in the latest tree. > >> Because at the time of that "admittedly racy check", we have no hold > >> at all on the page in question: and if it's PageLRU or PageCompound > >> at one instant, it may be different the next instant. Which leaves it > >> vulnerable to whatever BUG_ON()s there may be in the total_mapcount() > >> path - needs research. *Perhaps* there are no more BUG_ON()s in the > >> total_mapcount() path than in the existing page_mapcount() path. > >> I suspect that for this to be safe (before your patch and more so > >> after), > >> it will be necessary to shift the "admittedly racy check" down after the > >> get_page_unless_zero() (and check the sequence of operations when a > >> compound page is initialized). > > > > Grabbing a reference first sounds like the right approach to me. > > I think you're right. Without a page reference I don't think it is even > safe to look at struct page, at least not without synchronisation > against memory hot unplug which could remove the struct page. From a > quick glance I didn't see anything here that obviously did that though. Memory hotplug is the offending party here. It has to make sure that everything else is definitely quiescent before removing the struct pages. Otherwise you can't even try_get a refcount.