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 X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 96ACFC76196 for ; Fri, 19 Jul 2019 21:45:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6BBBB2184E for ; Fri, 19 Jul 2019 21:45:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=nvidia.com header.i=@nvidia.com header.b="M3SU/d2c" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388661AbfGSVpN (ORCPT ); Fri, 19 Jul 2019 17:45:13 -0400 Received: from hqemgate14.nvidia.com ([216.228.121.143]:17656 "EHLO hqemgate14.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727528AbfGSVpM (ORCPT ); Fri, 19 Jul 2019 17:45:12 -0400 Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqemgate14.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA) id ; Fri, 19 Jul 2019 14:45:10 -0700 Received: from hqmail.nvidia.com ([172.20.161.6]) by hqpgpgate101.nvidia.com (PGP Universal service); Fri, 19 Jul 2019 14:45:10 -0700 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Fri, 19 Jul 2019 14:45:10 -0700 Received: from [10.110.48.28] (172.20.13.39) by HQMAIL107.nvidia.com (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Fri, 19 Jul 2019 21:45:08 +0000 Subject: Re: [PATCH v2 2/3] mm/hmm: fix ZONE_DEVICE anon page mapping reuse To: Ralph Campbell , CC: , , "Christoph Hellwig" , Dan Williams , Andrew Morton , Jason Gunthorpe , "Logan Gunthorpe" , Ira Weiny , "Matthew Wilcox" , Mel Gorman , "Jan Kara" , "Kirill A. Shutemov" , Michal Hocko , Andrea Arcangeli , "Mike Kravetz" , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= References: <20190719192955.30462-1-rcampbell@nvidia.com> <20190719192955.30462-3-rcampbell@nvidia.com> X-Nvconfidentiality: public From: John Hubbard Message-ID: <5089cfa6-ba0c-efa8-6beb-6fb1881a57d4@nvidia.com> Date: Fri, 19 Jul 2019 14:45:08 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190719192955.30462-3-rcampbell@nvidia.com> X-Originating-IP: [172.20.13.39] X-ClientProxiedBy: HQMAIL107.nvidia.com (172.20.187.13) To HQMAIL107.nvidia.com (172.20.187.13) Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1563572710; bh=EWMnpOanoTl9uOzSae5VUG9sJSSMfpgHu1hrHwdel1M=; h=X-PGP-Universal:Subject:To:CC:References:X-Nvconfidentiality:From: Message-ID:Date:User-Agent:MIME-Version:In-Reply-To: X-Originating-IP:X-ClientProxiedBy:Content-Type:Content-Language: Content-Transfer-Encoding; b=M3SU/d2cFHqeccbTX1WD+vjc0xuq2ALlX2QJVf5/Rs78JLBNyYft0uB6XGsFEMELg EC3AIY3dhOAsmz/SlrI6BIkahQcfFSb32/kPwBUOhNxYwstDwhPHekaV7PLWs54x2P A4zlnYYgc8fEJrnm+pUHwrAkBEzJqdltcWOYeQKu2rVRgFYx8GrmPh+ZQ9LNyQlgf8 y9iyaJJ5IzDwrSOsjdNyoKNKHpmEmGMHCiBeLc5lxhZt9X7xLmy3O69AVKYNLviqBN 9oWB2Tg3BKBF54n+TNpPp9luglQQm34rmXs9R6dt+zuj7Sf41rPQpKIIK+hwWnPj2K 2JhUTREHJ8Vyw== Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 7/19/19 12:29 PM, Ralph Campbell wrote: > When a ZONE_DEVICE private page is freed, the page->mapping field can be > set. If this page is reused as an anonymous page, the previous value can > prevent the page from being inserted into the CPU's anon rmap table. > For example, when migrating a pte_none() page to device memory: > migrate_vma(ops, vma, start, end, src, dst, private) > migrate_vma_collect() > src[] =3D MIGRATE_PFN_MIGRATE > migrate_vma_prepare() > /* no page to lock or isolate so OK */ > migrate_vma_unmap() > /* no page to unmap so OK */ > ops->alloc_and_copy() > /* driver allocates ZONE_DEVICE page for dst[] */ > migrate_vma_pages() > migrate_vma_insert_page() > page_add_new_anon_rmap() > __page_set_anon_rmap() > /* This check sees the page's stale mapping field */ > if (PageAnon(page)) > return > /* page->mapping is not updated */ >=20 > The result is that the migration appears to succeed but a subsequent CPU > fault will be unable to migrate the page back to system memory or worse. >=20 > Clear the page->mapping field when freeing the ZONE_DEVICE page so stale > pointer data doesn't affect future page use. Reviewed-by: John Hubbard thanks, --=20 John Hubbard NVIDIA >=20 > Fixes: b7a523109fb5c9d2d6dd ("mm: don't clear ->mapping in hmm_devmem_fre= e") > Cc: stable@vger.kernel.org > Signed-off-by: Ralph Campbell > Cc: Christoph Hellwig > Cc: Dan Williams > Cc: Andrew Morton > Cc: Jason Gunthorpe > Cc: Logan Gunthorpe > Cc: Ira Weiny > Cc: Matthew Wilcox > Cc: Mel Gorman > Cc: Jan Kara > Cc: "Kirill A. Shutemov" > Cc: Michal Hocko > Cc: Andrea Arcangeli > Cc: Mike Kravetz > Cc: "J=C3=A9r=C3=B4me Glisse" > --- > kernel/memremap.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) >=20 > diff --git a/kernel/memremap.c b/kernel/memremap.c > index bea6f887adad..98d04466dcde 100644 > --- a/kernel/memremap.c > +++ b/kernel/memremap.c > @@ -408,6 +408,30 @@ void __put_devmap_managed_page(struct page *page) > =20 > mem_cgroup_uncharge(page); > =20 > + /* > + * When a device_private page is freed, the page->mapping field > + * may still contain a (stale) mapping value. For example, the > + * lower bits of page->mapping may still identify the page as > + * an anonymous page. Ultimately, this entire field is just > + * stale and wrong, and it will cause errors if not cleared. > + * One example is: > + * > + * migrate_vma_pages() > + * migrate_vma_insert_page() > + * page_add_new_anon_rmap() > + * __page_set_anon_rmap() > + * ...checks page->mapping, via PageAnon(page) call, > + * and incorrectly concludes that the page is an > + * anonymous page. Therefore, it incorrectly, > + * silently fails to set up the new anon rmap. > + * > + * For other types of ZONE_DEVICE pages, migration is either > + * handled differently or not done at all, so there is no need > + * to clear page->mapping. > + */ > + if (is_device_private_page(page)) > + page->mapping =3D NULL; > + > page->pgmap->ops->page_free(page); > } else if (!count) > __put_page(page); >=20