Re: [PATCH 14/15] dax: associate mappings with inodes, and warn if dma collides with truncate

On Wed, Dec 20, 2017 at 6:38 AM, Jan Kara <jack(a)suse.cz&gt; wrote: > On Tue 19-12-17 17:11:38, Dan Williams wrote: >> On Fri, Nov 10, 2017 at 1:08 AM, Christoph Hellwig <hch(a)lst.de&gt; wrote: >> >> + struct { >> >> + /* >> >> + * ZONE_DEVICE pages are never on an lru or handled by >> >> + * a slab allocator, this points to the hosting device >> >> + * page map. >> >> + */ >> >> + struct dev_pagemap *pgmap; >> >> + /* >> >> + * inode association for MEMORY_DEVICE_FS_DAX page-idle >> >> + * callbacks. Note that we don't use ->mapping since >> >> + * that has hard coded page-cache assumptions in >> >> + * several paths. >> >> + */ >> > >> > What assumptions? I'd much rather fix those up than having two fields >> > that have the same functionality. >> >> [ Reviving this old thread where you asked why I introduce page->inode >> instead of reusing page->mapping ] >> >> For example, xfs_vm_set_page_dirty() assumes that page->mapping being >> non-NULL indicates a typical page cache page, this is a false >> assumption for DAX. My guess at a fix for this is to add >> pagecache_page() checks to locations like this, but I worry about how >> to find them all. Where pagecache_page() is: >> >> bool pagecache_page(struct page *page) >> { >> if (!page->mapping) >> return false; >> if (!IS_DAX(page->mapping->host)) >> return false; >> return true; >> } >> >> Otherwise we go off the rails: >> >> WARNING: CPU: 27 PID: 1783 at fs/xfs/xfs_aops.c:1468 >> xfs_vm_set_page_dirty+0xf3/0x1b0 [xfs] > > But this just shows that mapping->a_ops are wrong for this mapping, doesn't > it? ->set_page_dirty handler for DAX mapping should just properly handle > DAX pages... (and only those) Ah, yes. Now that I change ->mapping to be non-NULL for DAX pages I enable all the address_space_operations to start firing. However, instead of adding DAX specific address_space_operations it appears ->mapping should never be set for DAX pages, because DAX pages are disconnected from the page-writeback machinery.

In other words never setting ->mapping bypasses all the possible broken assumptions and code paths that take page-cache specific actions before calling an address_space_operation.