Received-SPF: softfail (google.com: domain of transitioning linux-ext4-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Tue, 15 Feb 2022 22:01:00 -0400
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Felix Kuehling <felix.kuehling@amd.com>
Cc:     Christoph Hellwig <hch@lst.de>,
        David Hildenbrand <david@redhat.com>,
        Alex Sierra <alex.sierra@amd.com>, akpm@linux-foundation.org,
        linux-mm@kvack.org, rcampbell@nvidia.com,
        linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
        amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        jglisse@redhat.com, apopple@nvidia.com, willy@infradead.org
Subject: Re: [PATCH v6 01/10] mm: add zone device coherent type memory support
Message-ID: <20220216020100.GC4160@nvidia.com>
References: <beb38138-2266-1ff8-cc82-8fe914bed862@redhat.com>
 <f2af73c1-396b-168f-7f86-eb10b3b68a26@redhat.com>
 <a24d82d9-daf9-fa1a-8b9d-5db7fe10655e@amd.com>
 <078dd84e-ebbc-5c89-0407-f5ecc2ca3ebf@redhat.com>
 <20220215144524.GR4160@nvidia.com>
 <20220215183209.GA24409@lst.de>
 <20220215194107.GZ4160@nvidia.com>
 <ac3d5157-9251-f9fb-a973-f268ce58b4e0@amd.com>
 <20220215214749.GA4160@nvidia.com>
 <002ad572-4d32-7133-06f3-aa680c297be2@amd.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <002ad572-4d32-7133-06f3-aa680c297be2@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?MqxmYfStJQ+alTmZjERLOXnCmiduoC/rHPkbJb56fTkUNFYw7ZMVmw8+6A9H?=
 =?us-ascii?Q?AcQt5mX9HLiww/lZe8RDYBQOp3OAzDD7L4+miXlkDpZ3yMjXcK7unEZol/4A?=
 =?us-ascii?Q?hdJRLkzdC1y/Zrq9kWyuXHBY7e+SoUJUAVvWSu5PQiUYuI3pBMBdK8gKJcTy?=
 =?us-ascii?Q?LiTeteIVWOvE+sR3+T7PSUa4rnU8/EUJNfVe8nNb/RheIGB4TkTHF0CWNTMX?=
 =?us-ascii?Q?fBFD7XdbP5aJAUuOl5qZeAdWeIKzrqJD63+zQmioCTVo1cbaoPoi+EuQUnai?=
 =?us-ascii?Q?hYNZpK/Q8bvYF1zTcJyoLeAjKMjBArcd0nU1dGFOA5jIVV4hwT9kLbP04Imx?=
 =?us-ascii?Q?m3Zi5QQvLZqB9BfGRdNJzhVTW6G9gN3RwJI2j+J872ViKTiB3xppDjCXU1j7?=
 =?us-ascii?Q?N8iJfyB+b/HpAfjN0VavUtQ9mdOBN/sPSiQ9Ha9t0xchQhxBo4KinHVcIy52?=
 =?us-ascii?Q?hPGQG2LybzwJoxU/xzN6Xyueo2wwcKK0f/lSwi6hLg38dPrSCZIdVFU7DF5N?=
 =?us-ascii?Q?NDjyQtgJhU0Tqtn75N9Vu9+oR0HrSwrKeEJFJ9UJ+a6VK0Hx9uixXd87T7Av?=
 =?us-ascii?Q?v0ce9RKP9nkMYwt+SguF4L5E7e8TUJSQp709NC4OtN2hsq2RSruysLMUK+j3?=
 =?us-ascii?Q?CofU8NJKqMlwKqIzF9qo/UZOmVh3JsoNq2+7PEjzWLOH2VOONEY5UOnYQ9FN?=
 =?us-ascii?Q?pPB30+asBsdjpXLiaG4LkQp8gp1a35Nv7rm8fKrHcjVv0K/yrnBLbMH4pK0u?=
 =?us-ascii?Q?1z2CcooC/Z+DiEgqTA01EH5TFTHrl7cISm80s2V5GzGkcBi+Rd/beaOKBBrP?=
 =?us-ascii?Q?mOlsA2aAXZ6TIeeR3/SHLlJFH5AAdCiAnD2YSRvEPDMW11sWnn9QsFSnGRzt?=
 =?us-ascii?Q?J+ZIY31bGF99a1l7G4LHXVB1gB3tZLJi3buC3TXx1MsYhrYP3LOrKoOHAWEv?=
 =?us-ascii?Q?W6c/TpQo6qIR/483vB3fh8GON9i+hmn1zqVPuh5X7XJ0HBUftqK8vT8mKnvL?=
 =?us-ascii?Q?IvLx+9LIp28g2hVbf8uXrM6q6ATNp48/1DYqmN5xxWpL9BoZBBdXfOV5xWNE?=
 =?us-ascii?Q?sozSPOsM7Pk3SlFxTQ/qdxmnLKp/V+jOmcFk0Q0N5MQW+/YfHMl1gzg5YWcw?=
 =?us-ascii?Q?rO0UN61YwycY4vgLtqAKPRvPddLB+eL9p3jLaHFKD6WzPckLTcNf1NMWieS1?=
 =?us-ascii?Q?+FclWZUHEQDQfwmcUpiJuIDH4YuspbC1dLdUObfgDm3rQ3Xay9XzZj9b/5+Y?=
 =?us-ascii?Q?4KwCwjj2ZZji5+YDhhjnI4427aycNXKYSlSo+D0s5ND3nsw0TE2g/BVZMwTp?=
 =?us-ascii?Q?uRYQTtD+3EJxH/eOshE43IZGIpNQ/DaOLu88H8uUVMm4bNCMrTE451B5Z09c?=
 =?us-ascii?Q?c1IJ/ogLRiDfpzssvVaTCIADZye3I+LXGGAi4CED36r8zGHnsqyYTryvbb8D?=
 =?us-ascii?Q?xo1yds0LdPJfKD23BQg3NI12irg1mDr+9EqyIDoA3O6BE7ut19kIdxfLsqpN?=
 =?us-ascii?Q?vo1Hu/IjSbo5wPFKIoxrKijF/ZkDRB7NhOFUFFNH7j+5qrP/IAO3wkXpkqx6?=
 =?us-ascii?Q?4WDdd4MtcAiYN1cEMbM=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 432d30b6-42fe-4e78-e68e-08d9f0f02e60
X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB4192.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Feb 2022 02:01:01.2898
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: lmnYE9/kIc6qgO44NMdQQR1Os9oHk1iuCRyel9njb12gDRTQLvuoCCkQRrKFtP2P
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4090
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE
	autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org

On Tue, Feb 15, 2022 at 05:49:07PM -0500, Felix Kuehling wrote:

> > Userspace does
> >   1) mmap(MAP_PRIVATE) to allocate anon memory
> >   2) something to trigger migration to install a ZONE_DEVICE page
> >   3) munmap()
> > 
> > Who decrements the refcout on the munmap?
> > 
> > When a ZONE_DEVICE page is installed in the PTE is supposed to be
> > marked as pte_devmap and that disables all the normal page refcounting
> > during munmap().
> > 
> > fsdax makes this work by working the refcounts backwards, the page is
> > refcounted while it exists in the driver, when the driver decides to
> > remove it then unmap_mapping_range() is called to purge it from all
> > PTEs and then refcount is decrd. munmap/fork/etc don't change the
> > refcount.
>
> Hmm, that just means, whether or not there are PTEs doesn't really
> matter.

Yes, that is the FSDAX model

> It should work the same as it does for DEVICE_PRIVATE pages. I'm not sure
> where DEVICE_PRIVATE page's refcounts are decremented on unmap, TBH. But I
> can't find it in our driver, or in the test_hmm driver for that matter.

It is not the same as DEVICE_PRIVATE because DEVICE_PRIVATE uses swap
entries. The put_page for that case is here:

static unsigned long zap_pte_range(struct mmu_gather *tlb,
				struct vm_area_struct *vma, pmd_t *pmd,
				unsigned long addr, unsigned long end,
				struct zap_details *details)
{
[..]
		if (is_device_private_entry(entry) ||
		    is_device_exclusive_entry(entry)) {
			struct page *page = pfn_swap_entry_to_page(entry);

			if (unlikely(zap_skip_check_mapping(details, page)))
				continue;
			pte_clear_not_present_full(mm, addr, pte, tlb->fullmm);
			rss[mm_counter(page)]--;

			if (is_device_private_entry(entry))
				page_remove_rmap(page, false);

			put_page(page);

However the devmap case will return NULL from vm_normal_page() and won't
do the put_page() embedded inside the __tlb_remove_page() in the
pte_present() block in the same function.

After reflecting for awhile, I think Christoph's idea is quite
good. Just make it so you don't set pte_devmap() on your pages and
then lets avoid pte_devmap for all refcount correct ZONE_DEVICE pages.

Jason