linux-ext4 - Re: [RFC 04/10] fs/dax: Don't track page mapping/index

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]

Date: Fri, 12 Apr 2024 10:31:52 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: Jan Kara <jack@...e.cz>, Alistair Popple <apopple@...dia.com>
CC: <linux-mm@...ck.org>, <david@...morbit.com>, <dan.j.williams@...el.com>,
	<jhubbard@...dia.com>, <rcampbell@...dia.com>, <willy@...radead.org>,
	<jgg@...dia.com>, <linux-fsdevel@...r.kernel.org>, <jack@...e.cz>,
	<djwong@...nel.org>, <hch@....de>, <david@...hat.com>,
	<ruansy.fnst@...itsu.com>, <nvdimm@...ts.linux.dev>,
	<linux-xfs@...r.kernel.org>, <linux-ext4@...r.kernel.org>,
	<jglisse@...hat.com>
Subject: Re: [RFC 04/10] fs/dax: Don't track page mapping/index

Jan Kara wrote:
> On Thu 11-04-24 10:57:25, Alistair Popple wrote:
> > The page->mapping and page->index fields are normally used by the
> > pagecache and rmap for looking up virtual mappings of pages. FS DAX
> > implements it's own kind of page cache and rmap look ups so these
> > fields are unnecessary. They are currently only used to detect
> > error/warning conditions which should never occur.
> > 
> > A future change will change the way shared mappings are detected by
> > doing normal page reference counting instead, so remove the
> > unnecessary checks.
> > 
> > Signed-off-by: Alistair Popple <apopple@...dia.com>
> ...
> > -/*
> > - * When it is called in dax_insert_entry(), the shared flag will indicate that
> > - * whether this entry is shared by multiple files.  If so, set the page->mapping
> > - * PAGE_MAPPING_DAX_SHARED, and use page->share as refcount.
> > - */
> > -static void dax_associate_entry(void *entry, struct address_space *mapping,
> > -		struct vm_area_struct *vma, unsigned long address, bool shared)
> > -{
> > -	unsigned long size = dax_entry_size(entry), pfn, index;
> > -	int i = 0;
> > -
> > -	if (IS_ENABLED(CONFIG_FS_DAX_LIMITED))
> > -		return;
> > -
> > -	index = linear_page_index(vma, address & ~(size - 1));
> > -	for_each_mapped_pfn(entry, pfn) {
> > -		struct page *page = pfn_to_page(pfn);
> > -
> > -		if (shared) {
> > -			dax_page_share_get(page);
> > -		} else {
> > -			WARN_ON_ONCE(page->mapping);
> > -			page->mapping = mapping;
> > -			page->index = index + i++;
> > -		}
> > -	}
> > -}
> 
> Hum, but what about existing uses of folio->mapping and folio->index in
> fs/dax.c? AFAICT this patch breaks them. What am I missing? How can this
> ever work?

Right, as far as I can see every fsdax filesystem would need to be
converted to use dax_holder_operations() so that the fs can backfill
->mapping and ->index.