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=-9.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS 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 0DA9CC433E2 for ; Mon, 14 Sep 2020 23:10:54 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A2F0320897 for ; Mon, 14 Sep 2020 23:10:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=intel-com.20150623.gappssmtp.com header.i=@intel-com.20150623.gappssmtp.com header.b="OscoufG8" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A2F0320897 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id BAEC96B0082; Mon, 14 Sep 2020 19:10:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B60066B0083; Mon, 14 Sep 2020 19:10:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A4E536B0085; Mon, 14 Sep 2020 19:10:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0049.hostedemail.com [216.40.44.49]) by kanga.kvack.org (Postfix) with ESMTP id 8B02A6B0082 for ; Mon, 14 Sep 2020 19:10:52 -0400 (EDT) Received: from smtpin02.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 286AD3630 for ; Mon, 14 Sep 2020 23:10:52 +0000 (UTC) X-FDA: 77263213944.02.mom77_5f14b9f2710c Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin02.hostedemail.com (Postfix) with ESMTP id EA24810097AA8 for ; Mon, 14 Sep 2020 23:10:51 +0000 (UTC) X-HE-Tag: mom77_5f14b9f2710c X-Filterd-Recvd-Size: 7900 Received: from mail-ed1-f68.google.com (mail-ed1-f68.google.com [209.85.208.68]) by imf31.hostedemail.com (Postfix) with ESMTP for ; Mon, 14 Sep 2020 23:10:50 +0000 (UTC) Received: by mail-ed1-f68.google.com with SMTP id g4so1243295edk.0 for ; Mon, 14 Sep 2020 16:10:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=R+itrPmdb1uT2Do7NFJXS/q5A1e11LUUi6cUnbB8ULo=; b=OscoufG8IHAPrzgYF4fy7Cn/cK7ubF9tgWt8ZSshsxggI+VMUPvxKjh5r2Y2J5l2uh UMyGvicURId/BnfvWei+UmGlQYfxb5gUlFtgYBcc/sJkxR+BZ2rRPTNZH5n/MxAVMRLN CezpW17Hi9Ria77HBMgvk5dRIHYzNvD4OTOZUVYZXT31uiRdBc4PlG/C/OYuQ4Untxyu kIt724BvF7EyKYooWsH51W5qekRgsO4eUc1/CVX37OQ6wcqYsq+DZM1YYO1oZVsFiCsF nflntEDKVeno16Chu1iM5WP+Uts0fcm5u8ZwXwx+sYYJvRnI59vxmBtjLMtucxbocrlI NYeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=R+itrPmdb1uT2Do7NFJXS/q5A1e11LUUi6cUnbB8ULo=; b=EmB4s8Pa3AmnzjcHAwXDnRXfey7d45OedAVXXpMQ5+GCaOW/48KX/aqD1xjoGWr1An +hGdEHsBwhHoIbbcvouNTS7xO0fotNc0wRNw0lJXEKpXWfrIMOnlbls7fxw8nToMTM2w mmMZqvuw4D6AJS54PBOFOxmi/Zxqp+iAJGXGUCMBqQ9CzKDtnUnNTAFk35Kj1QLJtOgo kEpLRyKZPdHxNRmDxnSmxvw6eWobiPIdAQmJoxL5EWFQ86ji79eHYWKthMXuLa8JmbCy 6AzEoLbHhd87IvSJ4eYiQnapQIjfubjUT2yb2WWYt4jWORBF2d4vXhLwSZcocWY0vyY7 3oQw== X-Gm-Message-State: AOAM533llTqP9XBLj8uIxjaHDnco9jOseAdDrCa35IVSfvf9ei9lR7aF Fc9q1Z3l6BIpG8JcKZIZJ7C4uu2is3wbqlYt2jtsEw== X-Google-Smtp-Source: ABdhPJz25sdc/c10PW3FBm8YfGcEy9UNPAY+TzoW85B7fIMK8zMcw5r6ppC0h9BjQjMtQWUZ3domYRB1253CIsTQm98= X-Received: by 2002:aa7:c511:: with SMTP id o17mr20039691edq.300.1600125049629; Mon, 14 Sep 2020 16:10:49 -0700 (PDT) MIME-Version: 1.0 References: <20200914224509.17699-1-rcampbell@nvidia.com> In-Reply-To: <20200914224509.17699-1-rcampbell@nvidia.com> From: Dan Williams Date: Mon, 14 Sep 2020 16:10:38 -0700 Message-ID: Subject: Re: [PATCH] mm: remove extra ZONE_DEVICE struct page refcount To: Ralph Campbell Cc: Linux MM , kvm-ppc@vger.kernel.org, nouveau@lists.freedesktop.org, Linux Kernel Mailing List , Ira Weiny , Matthew Wilcox , Jerome Glisse , John Hubbard , Alistair Popple , Christoph Hellwig , Jason Gunthorpe , Bharata B Rao , Zi Yan , "Kirill A . Shutemov" , Yang Shi , Paul Mackerras , Ben Skeggs , Andrew Morton Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: EA24810097AA8 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam01 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 Mon, Sep 14, 2020 at 3:45 PM Ralph Campbell wrote: > > ZONE_DEVICE struct pages have an extra reference count that complicates the > code for put_page() and several places in the kernel that need to check the > reference count to see that a page is not being used (gup, compaction, > migration, etc.). Clean up the code so the reference count doesn't need to > be treated specially for ZONE_DEVICE. > > Signed-off-by: Ralph Campbell > --- > > Matthew Wilcox, Ira Weiny, and others have complained that ZONE_DEVICE > struct page reference counting is ugly/broken. This is my attempt to > fix it and it works for the HMM migration self tests. Can you link to a technical description of what's broken? Or better yet, summarize that argument in the changelog? > I'm only sending this out as a RFC since I'm not that familiar with the > DAX, PMEM, XEN, and other uses of ZONE_DEVICE struct pages allocated > with devm_memremap_pages() or memremap_pages() but my best reading of > the code looks like it might be OK. I could use help testing these > configurations. Back in the 4.15 days I could not convince myself that some code paths blindly assumed that pages with refcount==0 were on an lru list. Since then, struct page has been reorganized to not collide the ->pgmap back pointer with the ->lru list and there have been other cleanups for page pinning that might make this incremental cleanup viable. You also need to fix up ext4_break_layouts() and xfs_break_dax_layouts() to expect ->_refcount is 0 instead of 1. This also needs some fstests exposure. > I have a modified THP migration patch series that applies on top of > this one and is cleaner since I don't have to add code to handle the > +1 reference count. The link below is for the earlier v2: > ("mm/hmm/nouveau: add THP migration to migrate_vma_*") > https://lore.kernel.org/linux-mm/20200902165830.5367-1-rcampbell@nvidia.com > > > arch/powerpc/kvm/book3s_hv_uvmem.c | 1 - > drivers/gpu/drm/nouveau/nouveau_dmem.c | 1 - > include/linux/memremap.h | 6 +-- > include/linux/mm.h | 39 --------------- > lib/test_hmm.c | 1 - > mm/gup.c | 44 ----------------- > mm/memremap.c | 20 ++++---- > mm/migrate.c | 5 -- > mm/swap.c | 66 +++++++++++--------------- > 9 files changed, 41 insertions(+), 142 deletions(-) This diffstat is indeed appealing. > > diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c > index 84e5a2dc8be5..00d97050d7ff 100644 > --- a/arch/powerpc/kvm/book3s_hv_uvmem.c > +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c > @@ -711,7 +711,6 @@ static struct page *kvmppc_uvmem_get_page(unsigned long gpa, struct kvm *kvm) > > dpage = pfn_to_page(uvmem_pfn); > dpage->zone_device_data = pvt; > - get_page(dpage); > lock_page(dpage); > return dpage; > out_clear: > diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c > index a13c6215bba8..2a4bbe01a455 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c > +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c > @@ -324,7 +324,6 @@ nouveau_dmem_page_alloc_locked(struct nouveau_drm *drm) > return NULL; > } > > - get_page(page); > lock_page(page); > return page; > } > diff --git a/include/linux/memremap.h b/include/linux/memremap.h > index 4e9c738f4b31..7dd9802d2612 100644 > --- a/include/linux/memremap.h > +++ b/include/linux/memremap.h > @@ -67,9 +67,9 @@ enum memory_type { > > struct dev_pagemap_ops { > /* > - * Called once the page refcount reaches 1. (ZONE_DEVICE pages never > - * reach 0 refcount unless there is a refcount bug. This allows the > - * device driver to implement its own memory management.) > + * Called once the page refcount reaches 0. The reference count is > + * reset to 1 before calling page_free(). This allows the > + * device driver to implement its own memory management. I'd clarify the order events / responsibility of the common core page_free() and the device specific page_free(). At the same time, why not update drivers to expect that the page is already refcount==0 on entry? Seems odd to go through all this trouble to make the reference count appear to be zero to the wider kernel but expect that drivers get a fake reference on entry to their ->page_free() callbacks.