lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240627054455.GF14837@lst.de>
Date: Thu, 27 Jun 2024 07:44:55 +0200
From: Christoph Hellwig <hch@....de>
To: Alistair Popple <apopple@...dia.com>
Cc: dan.j.williams@...el.com, vishal.l.verma@...el.com,
	dave.jiang@...el.com, logang@...tatee.com, bhelgaas@...gle.com,
	jack@...e.cz, jgg@...pe.ca, catalin.marinas@....com,
	will@...nel.org, mpe@...erman.id.au, npiggin@...il.com,
	dave.hansen@...ux.intel.com, ira.weiny@...el.com,
	willy@...radead.org, djwong@...nel.org, tytso@....edu,
	linmiaohe@...wei.com, david@...hat.com, peterx@...hat.com,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linuxppc-dev@...ts.ozlabs.org,
	nvdimm@...ts.linux.dev, linux-cxl@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
	linux-ext4@...r.kernel.org, linux-xfs@...r.kernel.org,
	jhubbard@...dia.com, hch@....de, david@...morbit.com
Subject: Re: [PATCH 10/13] fs/dax: Properly refcount fs dax pages

> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> index eb61598..b7a31ae 100644
> --- a/drivers/dax/device.c
> +++ b/drivers/dax/device.c
> @@ -126,11 +126,11 @@ static vm_fault_t __dev_dax_pte_fault(struct dev_dax *dev_dax,
>  		return VM_FAULT_SIGBUS;
>  	}
>  
> -	pfn = phys_to_pfn_t(phys, PFN_DEV|PFN_MAP);
> +	pfn = phys_to_pfn_t(phys, 0);
>  
>  	dax_set_mapping(vmf, pfn, fault_size);
>  
> -	return vmf_insert_mixed(vmf->vma, vmf->address, pfn);
> +	return dax_insert_pfn(vmf->vma, vmf->address, pfn, vmf->flags & FAULT_FLAG_WRITE);

Plenty overly long lines here and later.

Q: hould dax_insert_pfn take a vm_fault structure instead of the vma?
Or are the potential use cases that aren't from the fault path?
similar instead of the bool write passing the fault flags might actually
make things more readable than the bool.

Also at least currently it seems like there are no modular users despite
the export, or am I missing something?

> +		blk_queue_flag_set(QUEUE_FLAG_DAX, q);

Just as a heads up, setting of these flags has changed a lot in
linux-next.

>  {
> +	/*
> +	 * Make sure we flush any cached data to the page now that it's free.
> +	 */
> +	if (PageDirty(page))
> +		dax_flush(NULL, page_address(page), page_size(page));
> +

Adding the magic dax_dev == NULL case to dax_flush and going through it
vs just calling arch_wb_cache_pmem directly here seems odd.

But I also don't quite understand how it is related to the rest
of the patch anyway.

> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -373,6 +373,8 @@ static int mlock_pte_range(pmd_t *pmd, unsigned long addr,
>  	unsigned long start = addr;
>  
>  	ptl = pmd_trans_huge_lock(pmd, vma);
> +	if (vma_is_dax(vma))
> +		ptl = NULL;
>  	if (ptl) {

This feels sufficiently magic to warrant a comment.

>  		if (!pmd_present(*pmd))
>  			goto out;
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index b7e1599..f11ee0d 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -1016,7 +1016,8 @@ static void __ref __init_zone_device_page(struct page *page, unsigned long pfn,
>  	 */
>  	if (pgmap->type == MEMORY_DEVICE_PRIVATE ||
>  	    pgmap->type == MEMORY_DEVICE_COHERENT ||
> -	    pgmap->type == MEMORY_DEVICE_PCI_P2PDMA)
> +	    pgmap->type == MEMORY_DEVICE_PCI_P2PDMA ||
> +	    pgmap->type == MEMORY_DEVICE_FS_DAX)
>  		set_page_count(page, 0);
>  }

So we'll skip this for MEMORY_DEVICE_GENERIC only.  Does anyone remember
if that's actively harmful or just not needed?  If the latter it might
be simpler to just set the page count unconditionally here.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ