Received-SPF: pass (google.com: domain of linux-ext4+bounces-2027-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 11 Apr 2024 09:29:27 -0300
From: Jason Gunthorpe <jgg@nvidia.com>
To: Alistair Popple <apopple@nvidia.com>
Cc: linux-mm@kvack.org, david@fromorbit.com, dan.j.williams@intel.com,
	jhubbard@nvidia.com, rcampbell@nvidia.com, willy@infradead.org,
	linux-fsdevel@vger.kernel.org, jack@suse.cz, djwong@kernel.org,
	hch@lst.de, david@redhat.com, ruansy.fnst@fujitsu.com,
	nvdimm@lists.linux.dev, linux-xfs@vger.kernel.org,
	linux-ext4@vger.kernel.org, jglisse@redhat.com
Subject: Re: [RFC 03/10] pci/p2pdma: Don't initialise page refcount to one
Message-ID: <20240411122927.GR5383@nvidia.com>
References: <cover.fe275e9819458a4bbb9451b888cafb88af8867d4.1712796818.git-series.apopple@nvidia.com>
 <a443974e64917824e078485d4e755ef04c89d73f.1712796818.git-series.apopple@nvidia.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a443974e64917824e078485d4e755ef04c89d73f.1712796818.git-series.apopple@nvidia.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?ngsN4LmrmMB7JCMGJWV7UoYXqhTZm8ZbP78IeTs30Vrq+o70QvdW8IHISWuA?=
 =?us-ascii?Q?q75++o187vy5fGu3fgRLe0eiLbJGWQMsWsLBUO+noo2mBUXnKJSo4U7Ksupc?=
 =?us-ascii?Q?6nro58Vk+B5w+LSKiEhWWYvnfI/b5fjrUpu8AesncAaouvBGIPpSroh57Rpy?=
 =?us-ascii?Q?PCA9NLKwA5m6TZ4Rtv1LCaX84BBDcAhdMMU1iip+YTZU3FCFWMu0zqlQOhEG?=
 =?us-ascii?Q?kpVfyMTnMpQFRVyw5oUmGKL1jx+w/86ll9AJoc9bKOOjYZvZmusC1436y59C?=
 =?us-ascii?Q?orDyd8tIAcuKnnotgrA6jAAN/PQlWW7SqDHczdrE3zpDmghYKLHJ6f6HJ0NI?=
 =?us-ascii?Q?06KjS3pWe66Bxo3eJNXHZ07AHg34Ld5d9y3aF5P9ZWL1lVDsFgteiqztg0+8?=
 =?us-ascii?Q?oqaod1si7VqEguQQMHFe8/6ls9hKXqVY0iJ9zy/w81OXZH4QSM8ZeayNGutu?=
 =?us-ascii?Q?W35vK3YaWBqv99p8J/o7BQ+DB806/7HVjjBEAFxEH5SrpYhWZKFOR14xQ8hU?=
 =?us-ascii?Q?f966vMRHf2kNFH8AY5iXyamW5hj55wHCyN3hjj2DuSOOBNjNQx3xrgLWnzG5?=
 =?us-ascii?Q?kcoyYloK/7dT0pNibSCLJisIvfZ0YqbZfDMV3mhoen1MAjv54QctzufIcwCV?=
 =?us-ascii?Q?RHMU+CuuAPRMv+hWhmyHiEtX9lY+6BGrX2MN6oYb8RMTMseNj3MEHUHXg0zS?=
 =?us-ascii?Q?fsJflme7l+S+M9ZhZ1J1o/TmqLlR9qpeT7HXsL/77eDOM9UXYp1ZqjK0JQGv?=
 =?us-ascii?Q?yojrCcvTPXemoo9AH2g0NI31pLDR6peurvroJ9v2LvQq/aDVLZlTsc64dNoO?=
 =?us-ascii?Q?+7Kpc/yIGjqjPHjTe5Sl6VxXCdNCuhUqEIlApXeMlkSn1te6m5a5hDxNkcY/?=
 =?us-ascii?Q?Irs5T8eX0dulto8KeRDzjnp2fMR/EceuQdi1DfUmhqfpCeYMnDV2py4gT7wb?=
 =?us-ascii?Q?rYQ/ymhdzABsBKcsBZX59daHA1Me2/mbQO1j0t3Qi9klGTdVHnohvsJUZqSd?=
 =?us-ascii?Q?4VNQxu0GN8rYm06SzZ/Ktmr3j63iUAaMV7w36jMfqTTldGYd0LrgpnO0KR6R?=
 =?us-ascii?Q?Hwijr93Kfmf/ETewKTorPdb/yuVBki/M7gsVz4wfUSIScQwzXBkVFfDFZDaC?=
 =?us-ascii?Q?i0+GGNFg6eAILPy8ChIJQYViDZTXQoNb6QqVZyl1eBSeVfpSlyqApTPHTH0r?=
 =?us-ascii?Q?WkwqBQ5QvWIwoeG/CP+MUg/m6ZwfqosNveC+nXAsl731Jmsr08L+7pluOj5q?=
 =?us-ascii?Q?n6GVRoDv1uqhpO2GgPGIaZVOPsLoix0f6Z3qFLP51gM5u/DVvFBJa2HmTaQ4?=
 =?us-ascii?Q?K6w3RXEKL/FEXXBXCtn9EXoJ59fkL0CilXw/3rMeDVWMulstrrb7EiTP28HY?=
 =?us-ascii?Q?hggghhKI8hitis7ITZMxENaErvUQaqMhyPWthpC1KONjgF9KVGFFk2L1QoX5?=
 =?us-ascii?Q?xy52smduGleznFcmB6GveZu574QYOrv55BCujhuTcKAXKW0cWFDl2Bk0Dhde?=
 =?us-ascii?Q?LsqMprTa44HXQCznFUbVi46K3gd1qgpHD4LACcpuh/Q0Af27D3C2N7VL+aX2?=
 =?us-ascii?Q?OUvX//0q8FCGs0Z+INOzDNCsstMIrzfelV6h0Tfn?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 44b01c31-d922-44fa-6aee-08dc5a2307d7
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3849.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Apr 2024 12:29:28.2740
 (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: zltAxAxARGcmFQQEFIc0HPTF1bMjhuHnsgnkaJCbF0y2Sysl9oKqM2KSJqpqYPCR
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB8773

On Thu, Apr 11, 2024 at 10:57:24AM +1000, Alistair Popple wrote:
> The reference counts for ZONE_DEVICE private pages should be
> initialised by the driver when the page is actually allocated by the
> driver allocator, not when they are first created. This is currently
> the case for MEMORY_DEVICE_PRIVATE and MEMORY_DEVICE_COHERENT pages
> but not MEMORY_DEVICE_PCI_P2PDMA pages so fix that up.
> 
> Signed-off-by: Alistair Popple <apopple@nvidia.com>
> ---
>  drivers/pci/p2pdma.c | 2 ++
>  mm/memremap.c        | 8 ++++----
>  mm/mm_init.c         | 4 +++-
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index fa7370f..ab7ef18 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -128,6 +128,8 @@ static int p2pmem_alloc_mmap(struct file *filp, struct kobject *kobj,
>  		goto out;
>  	}
>  
> +	get_page(virt_to_page(kaddr));
> +

Should this be 

 set_page_count(page, 1)

If the refcount is already known to be 0 ?

> @@ -508,15 +508,15 @@ void free_zone_device_page(struct page *page)
>  	page->mapping = NULL;
>  	page->pgmap->ops->page_free(page);
>  
> -	if (page->pgmap->type != MEMORY_DEVICE_PRIVATE &&
> -	    page->pgmap->type != MEMORY_DEVICE_COHERENT)
> +	if (page->pgmap->type == MEMORY_DEVICE_PRIVATE ||
> +	    page->pgmap->type == MEMORY_DEVICE_COHERENT)
> +		put_dev_pagemap(page->pgmap);

Not related, but we should really be getting rid of this devmap
refcount traffic too, IMHO..

If an implementation wants this then it should hook the page
free/alloc callbacks and do this, not put it in the core code.

Jason