From: Jan Kara <jack@suse.cz> To: Dan Williams <dan.j.williams@intel.com> Cc: Jan Kara <jack@suse.cz>, Matthew Wilcox <mawilcox@microsoft.com>, linux-nvdimm@lists.01.org, Dave Chinner <david@fromorbit.com>, linux-kernel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, Christoph Hellwig <hch@lst.de> Subject: Re: [PATCH v7 04/14] xfs, dax: introduce xfs_dax_aops Date: Thu, 29 Mar 2018 17:30:51 +0200 [thread overview] Message-ID: <20180329153051.xostr764yps6bjg2@quack2.suse.cz> (raw) In-Reply-To: <152167305269.5268.2789359915651914497.stgit@dwillia2-desk3.amr.corp.intel.com> On Wed 21-03-18 15:57:32, Dan Williams wrote: > In preparation for the dax implementation to start associating dax pages > to inodes via page->mapping, we need to provide a 'struct > address_space_operations' instance for dax. Otherwise, direct-I/O > triggers incorrect page cache assumptions and warnings like the > following: > > WARNING: CPU: 27 PID: 1783 at fs/xfs/xfs_aops.c:1468 > xfs_vm_set_page_dirty+0xf3/0x1b0 [xfs] > [..] > 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 > > ...where the default set_page_dirty() handler assumes that dirty state > is being tracked in 'struct page' flags. > > Cc: Jeff Moyer <jmoyer@redhat.com> > Cc: Matthew Wilcox <mawilcox@microsoft.com> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Suggested-by: Jan Kara <jack@suse.cz> > Suggested-by: Dave Chinner <david@fromorbit.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> Looks good to me. You can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/xfs/xfs_aops.c | 21 +++++++++++++++++---- > fs/xfs/xfs_aops.h | 1 + > fs/xfs/xfs_iops.c | 5 ++++- > 3 files changed, 22 insertions(+), 5 deletions(-) > > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > index 9c6a830da0ee..5f1f5948ecc2 100644 > --- a/fs/xfs/xfs_aops.c > +++ b/fs/xfs/xfs_aops.c > @@ -1194,16 +1194,22 @@ xfs_vm_writepages( > int ret; > > xfs_iflags_clear(XFS_I(mapping->host), XFS_ITRUNCATED); > - if (dax_mapping(mapping)) > - return dax_writeback_mapping_range(mapping, > - xfs_find_bdev_for_inode(mapping->host), wbc); > - > ret = write_cache_pages(mapping, wbc, xfs_do_writepage, &wpc); > if (wpc.ioend) > ret = xfs_submit_ioend(wbc, wpc.ioend, ret); > return ret; > } > > +STATIC int > +xfs_dax_writepages( > + struct address_space *mapping, > + struct writeback_control *wbc) > +{ > + xfs_iflags_clear(XFS_I(mapping->host), XFS_ITRUNCATED); > + return dax_writeback_mapping_range(mapping, > + xfs_find_bdev_for_inode(mapping->host), wbc); > +} > + > /* > * Called to move a page into cleanable state - and from there > * to be released. The page should already be clean. We always > @@ -1505,3 +1511,10 @@ const struct address_space_operations xfs_address_space_operations = { > .is_partially_uptodate = block_is_partially_uptodate, > .error_remove_page = generic_error_remove_page, > }; > + > +const struct address_space_operations xfs_dax_aops = { > + .direct_IO = xfs_vm_direct_IO, > + .writepages = xfs_dax_writepages, > + .set_page_dirty = noop_set_page_dirty, > + .invalidatepage = noop_invalidatepage, > +}; > diff --git a/fs/xfs/xfs_aops.h b/fs/xfs/xfs_aops.h > index 88c85ea63da0..69346d460dfa 100644 > --- a/fs/xfs/xfs_aops.h > +++ b/fs/xfs/xfs_aops.h > @@ -54,6 +54,7 @@ struct xfs_ioend { > }; > > extern const struct address_space_operations xfs_address_space_operations; > +extern const struct address_space_operations xfs_dax_aops; > > int xfs_setfilesize(struct xfs_inode *ip, xfs_off_t offset, size_t size); > > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c > index 56475fcd76f2..951e84df5576 100644 > --- a/fs/xfs/xfs_iops.c > +++ b/fs/xfs/xfs_iops.c > @@ -1272,7 +1272,10 @@ xfs_setup_iops( > case S_IFREG: > inode->i_op = &xfs_inode_operations; > inode->i_fop = &xfs_file_operations; > - inode->i_mapping->a_ops = &xfs_address_space_operations; > + if (IS_DAX(inode)) > + inode->i_mapping->a_ops = &xfs_dax_aops; > + else > + inode->i_mapping->a_ops = &xfs_address_space_operations; > break; > case S_IFDIR: > if (xfs_sb_version_hasasciici(&XFS_M(inode->i_sb)->m_sb)) > -- Jan Kara <jack@suse.com> SUSE Labs, CR _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz> To: Dan Williams <dan.j.williams@intel.com> Cc: linux-nvdimm@lists.01.org, Jeff Moyer <jmoyer@redhat.com>, Matthew Wilcox <mawilcox@microsoft.com>, Ross Zwisler <ross.zwisler@linux.intel.com>, Christoph Hellwig <hch@lst.de>, Jan Kara <jack@suse.cz>, Dave Chinner <david@fromorbit.com>, linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v7 04/14] xfs, dax: introduce xfs_dax_aops Date: Thu, 29 Mar 2018 17:30:51 +0200 [thread overview] Message-ID: <20180329153051.xostr764yps6bjg2@quack2.suse.cz> (raw) In-Reply-To: <152167305269.5268.2789359915651914497.stgit@dwillia2-desk3.amr.corp.intel.com> On Wed 21-03-18 15:57:32, Dan Williams wrote: > In preparation for the dax implementation to start associating dax pages > to inodes via page->mapping, we need to provide a 'struct > address_space_operations' instance for dax. Otherwise, direct-I/O > triggers incorrect page cache assumptions and warnings like the > following: > > WARNING: CPU: 27 PID: 1783 at fs/xfs/xfs_aops.c:1468 > xfs_vm_set_page_dirty+0xf3/0x1b0 [xfs] > [..] > 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 > > ...where the default set_page_dirty() handler assumes that dirty state > is being tracked in 'struct page' flags. > > Cc: Jeff Moyer <jmoyer@redhat.com> > Cc: Matthew Wilcox <mawilcox@microsoft.com> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Suggested-by: Jan Kara <jack@suse.cz> > Suggested-by: Dave Chinner <david@fromorbit.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> Looks good to me. You can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/xfs/xfs_aops.c | 21 +++++++++++++++++---- > fs/xfs/xfs_aops.h | 1 + > fs/xfs/xfs_iops.c | 5 ++++- > 3 files changed, 22 insertions(+), 5 deletions(-) > > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c > index 9c6a830da0ee..5f1f5948ecc2 100644 > --- a/fs/xfs/xfs_aops.c > +++ b/fs/xfs/xfs_aops.c > @@ -1194,16 +1194,22 @@ xfs_vm_writepages( > int ret; > > xfs_iflags_clear(XFS_I(mapping->host), XFS_ITRUNCATED); > - if (dax_mapping(mapping)) > - return dax_writeback_mapping_range(mapping, > - xfs_find_bdev_for_inode(mapping->host), wbc); > - > ret = write_cache_pages(mapping, wbc, xfs_do_writepage, &wpc); > if (wpc.ioend) > ret = xfs_submit_ioend(wbc, wpc.ioend, ret); > return ret; > } > > +STATIC int > +xfs_dax_writepages( > + struct address_space *mapping, > + struct writeback_control *wbc) > +{ > + xfs_iflags_clear(XFS_I(mapping->host), XFS_ITRUNCATED); > + return dax_writeback_mapping_range(mapping, > + xfs_find_bdev_for_inode(mapping->host), wbc); > +} > + > /* > * Called to move a page into cleanable state - and from there > * to be released. The page should already be clean. We always > @@ -1505,3 +1511,10 @@ const struct address_space_operations xfs_address_space_operations = { > .is_partially_uptodate = block_is_partially_uptodate, > .error_remove_page = generic_error_remove_page, > }; > + > +const struct address_space_operations xfs_dax_aops = { > + .direct_IO = xfs_vm_direct_IO, > + .writepages = xfs_dax_writepages, > + .set_page_dirty = noop_set_page_dirty, > + .invalidatepage = noop_invalidatepage, > +}; > diff --git a/fs/xfs/xfs_aops.h b/fs/xfs/xfs_aops.h > index 88c85ea63da0..69346d460dfa 100644 > --- a/fs/xfs/xfs_aops.h > +++ b/fs/xfs/xfs_aops.h > @@ -54,6 +54,7 @@ struct xfs_ioend { > }; > > extern const struct address_space_operations xfs_address_space_operations; > +extern const struct address_space_operations xfs_dax_aops; > > int xfs_setfilesize(struct xfs_inode *ip, xfs_off_t offset, size_t size); > > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c > index 56475fcd76f2..951e84df5576 100644 > --- a/fs/xfs/xfs_iops.c > +++ b/fs/xfs/xfs_iops.c > @@ -1272,7 +1272,10 @@ xfs_setup_iops( > case S_IFREG: > inode->i_op = &xfs_inode_operations; > inode->i_fop = &xfs_file_operations; > - inode->i_mapping->a_ops = &xfs_address_space_operations; > + if (IS_DAX(inode)) > + inode->i_mapping->a_ops = &xfs_dax_aops; > + else > + inode->i_mapping->a_ops = &xfs_address_space_operations; > break; > case S_IFDIR: > if (xfs_sb_version_hasasciici(&XFS_M(inode->i_sb)->m_sb)) > -- Jan Kara <jack@suse.com> SUSE Labs, CR
next prev parent reply other threads:[~2018-03-29 15:24 UTC|newest] Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-03-21 22:57 [PATCH v7 00/14] dax: fix dma vs truncate/hole-punch Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-21 22:57 ` [PATCH v7 01/14] dax: store pfns in the radix Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-21 22:57 ` [PATCH v7 02/14] fs, dax: prepare for dax-specific address_space_operations Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-29 15:28 ` Jan Kara 2018-03-29 15:28 ` Jan Kara 2018-03-21 22:57 ` [PATCH v7 03/14] block, dax: remove dead code in blkdev_writepages() Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-29 15:29 ` Jan Kara 2018-03-29 15:29 ` Jan Kara 2018-03-21 22:57 ` [PATCH v7 04/14] xfs, dax: introduce xfs_dax_aops Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-29 15:30 ` Jan Kara [this message] 2018-03-29 15:30 ` Jan Kara 2018-03-21 22:57 ` [PATCH v7 05/14] ext4, dax: introduce ext4_dax_aops Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-29 15:40 ` Jan Kara 2018-03-29 15:40 ` Jan Kara 2018-03-29 15:40 ` Jan Kara 2018-03-29 18:09 ` Christoph Hellwig 2018-03-29 18:09 ` Christoph Hellwig 2018-03-29 18:09 ` Christoph Hellwig 2018-03-29 22:47 ` Dan Williams 2018-03-29 22:47 ` Dan Williams 2018-03-29 22:47 ` Dan Williams 2018-03-21 22:57 ` [PATCH v7 06/14] ext2, dax: introduce ext2_dax_aops Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-29 15:44 ` Jan Kara 2018-03-29 15:44 ` Jan Kara 2018-03-21 22:57 ` [PATCH v7 07/14] fs, dax: use page->mapping to warn if truncate collides with a busy page Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-29 16:02 ` Jan Kara 2018-03-29 16:02 ` Jan Kara 2018-03-29 19:02 ` Dan Williams 2018-03-29 19:02 ` Dan Williams 2018-03-29 23:02 ` Dan Williams 2018-03-29 23:02 ` Dan Williams 2018-03-30 8:22 ` Jan Kara 2018-03-30 8:22 ` Jan Kara 2018-03-21 22:57 ` [PATCH v7 08/14] mm, dax: enable filesystems to trigger dev_pagemap ->page_free callbacks Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-29 16:36 ` Jan Kara 2018-03-29 16:36 ` Jan Kara 2018-03-29 16:36 ` Jan Kara 2018-03-21 22:57 ` [PATCH v7 09/14] mm, dev_pagemap: introduce CONFIG_DEV_PAGEMAP_OPS Dan Williams 2018-03-21 22:57 ` Dan Williams 2018-03-22 7:49 ` Christoph Hellwig 2018-03-22 7:49 ` Christoph Hellwig 2018-03-30 10:37 ` Jan Kara 2018-03-30 10:37 ` Jan Kara 2018-03-30 10:37 ` Jan Kara 2018-03-21 22:58 ` [PATCH v7 10/14] memremap: mark devm_memremap_pages() EXPORT_SYMBOL_GPL Dan Williams 2018-03-21 22:58 ` Dan Williams 2018-03-22 7:51 ` Christoph Hellwig 2018-03-22 7:51 ` Christoph Hellwig 2018-03-21 22:58 ` [PATCH v7 11/14] mm, fs, dax: handle layout changes to pinned dax mappings Dan Williams 2018-03-21 22:58 ` Dan Williams 2018-03-21 22:58 ` Dan Williams 2018-03-21 22:58 ` [PATCH v7 12/14] xfs: prepare xfs_break_layouts() to be called with XFS_MMAPLOCK_EXCL Dan Williams 2018-03-21 22:58 ` Dan Williams 2018-03-22 7:25 ` Christoph Hellwig 2018-03-22 7:25 ` Christoph Hellwig 2018-03-30 15:16 ` Darrick J. Wong 2018-03-30 15:16 ` Darrick J. Wong 2018-03-21 22:58 ` [PATCH v7 13/14] xfs: prepare xfs_break_layouts() for another layout type Dan Williams 2018-03-21 22:58 ` Dan Williams 2018-03-22 7:27 ` Christoph Hellwig 2018-03-22 7:27 ` Christoph Hellwig 2018-03-22 15:50 ` Dan Williams 2018-03-22 15:50 ` Dan Williams 2018-03-21 22:58 ` [PATCH v7 14/14] xfs, dax: introduce xfs_break_dax_layouts() Dan Williams 2018-03-21 22:58 ` Dan Williams 2018-03-21 22:58 ` Dan Williams 2018-03-22 7:43 ` Christoph Hellwig 2018-03-22 7:43 ` Christoph Hellwig 2018-03-22 16:28 ` Dan Williams 2018-03-22 16:28 ` Dan Williams 2018-03-22 17:02 ` Christoph Hellwig 2018-03-22 17:02 ` Christoph Hellwig
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20180329153051.xostr764yps6bjg2@quack2.suse.cz \ --to=jack@suse.cz \ --cc=dan.j.williams@intel.com \ --cc=david@fromorbit.com \ --cc=hch@lst.de \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nvdimm@lists.01.org \ --cc=linux-xfs@vger.kernel.org \ --cc=mawilcox@microsoft.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.