From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755354AbdLTOi1 (ORCPT ); Wed, 20 Dec 2017 09:38:27 -0500 Received: from mx2.suse.de ([195.135.220.15]:58067 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754664AbdLTOiY (ORCPT ); Wed, 20 Dec 2017 09:38:24 -0500 Date: Wed, 20 Dec 2017 15:38:22 +0100 From: Jan Kara To: Dan Williams Cc: Christoph Hellwig , "linux-nvdimm@lists.01.org" , Jan Kara , Matthew Wilcox , "linux-kernel@vger.kernel.org" , linux-xfs , Linux MM , Jeff Moyer , Ross Zwisler , linux-fsdevel , Andrew Morton Subject: Re: [PATCH 14/15] dax: associate mappings with inodes, and warn if dma collides with truncate Message-ID: <20171220143822.GB31584@quack2.suse.cz> References: <150949209290.24061.6283157778959640151.stgit@dwillia2-desk3.amr.corp.intel.com> <150949217152.24061.9869502311102659784.stgit@dwillia2-desk3.amr.corp.intel.com> <20171110090818.GE4895@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue 19-12-17 17:11:38, Dan Williams wrote: > On Fri, Nov 10, 2017 at 1:08 AM, Christoph Hellwig 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) > [..] > CPU: 27 PID: 1783 Comm: dma-collision Tainted: G O > 4.15.0-rc2+ #984 > [..] > Call Trace: > set_page_dirty_lock+0x40/0x60 > bio_set_pages_dirty+0x37/0x50 > iomap_dio_actor+0x2b7/0x3b0 > ? iomap_dio_zero+0x110/0x110 > iomap_apply+0xa4/0x110 > iomap_dio_rw+0x29e/0x3b0 > ? iomap_dio_zero+0x110/0x110 > ? xfs_file_dio_aio_read+0x7c/0x1a0 [xfs] > xfs_file_dio_aio_read+0x7c/0x1a0 [xfs] > xfs_file_read_iter+0xa0/0xc0 [xfs] > __vfs_read+0xf9/0x170 > vfs_read+0xa6/0x150 > SyS_pread64+0x93/0xb0 > entry_SYSCALL_64_fastpath+0x1f/0x96 Honza -- Jan Kara SUSE Labs, CR