On Thu 11-02-16 07:22:00, Dan Williams wrote:

On Thu, Feb 11, 2016 at 4:50 AM, Jan Kara [off-list ref] wrote:

quoted

On Wed 10-02-16 15:43:40, Ross Zwisler wrote:

quoted

On Thu, Feb 11, 2016 at 09:03:12AM +1100, Dave Chinner wrote:

quoted

On Wed, Feb 10, 2016 at 01:48:56PM -0700, Ross Zwisler wrote:

quoted

Previously calls to dax_writeback_mapping_range() for all DAX filesystems
(ext2, ext4 & xfs) were centralized in filemap_write_and_wait_range().
dax_writeback_mapping_range() needs a struct block_device, and it used to
get that from inode->i_sb->s_bdev.  This is correct for normal inodes
mounted on ext2, ext4 and XFS filesystems, but is incorrect for DAX raw
block devices and for XFS real-time files.

Instead, call dax_writeback_mapping_range() directly from the filesystem
->writepages function so that it can supply us with a valid block
device. This also fixes DAX code to properly flush caches in response to
sync(2).

Signed-off-by: Ross Zwisler <redacted>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/block_dev.c      | 16 +++++++++++++++-
 fs/dax.c            | 13 ++++++++-----
 fs/ext2/inode.c     | 11 +++++++++++
 fs/ext4/inode.c     |  7 +++++++
 fs/xfs/xfs_aops.c   |  9 +++++++++
 include/linux/dax.h |  6 ++++--
 mm/filemap.c        | 12 ++++--------
 7 files changed, 58 insertions(+), 16 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 39b3a17..fc01e43 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c

@@ -1693,13 +1693,27 @@ static int blkdev_releasepage(struct page *page, gfp_t wait)
  return try_to_free_buffers(page);
 }

+static int blkdev_writepages(struct address_space *mapping,
+                      struct writeback_control *wbc)
+{
+ if (dax_mapping(mapping)) {
+         struct block_device *bdev = I_BDEV(mapping->host);
+         int error;
+
+         error = dax_writeback_mapping_range(mapping, bdev, wbc);
+         if (error)
+                 return error;
+ }
+ return generic_writepages(mapping, wbc);
+}

Can you remind of the reason for calling generic_writepages() on DAX
enabled address spaces?

Sure.  The initial version of this patch didn't do this, and during testing I
hit a bunch of xfstests failures.  In ext2 at least I believe these were
happening because we were skipping the call into generic_writepages() for DAX
inodes. Without a lot of data to back this up, my guess is that this is due
to metadata inodes or something being marked as DAX (so dax_mapping(mapping)
returns true), but having dirty page cache pages that need to be written back
as part of the writeback.

Changing this so we always call generic_writepages() even in the DAX case
solved the xfstest failures.

If this sounds incorrect, please let me know and I'll go and gather more data.

So I think a more correct fix it to not set S_DAX for inodes that will have
any pagecache pages - e.g. don't set S_DAX for block device inodes when
filesystem is mounted on it (probably the easiest is to just refuse to
mount filesystem on block device which has S_DAX set).

I think we have a wider problem here.  See __blkdev_get, we set S_DAX
on all block devices that have ->direct_access() and have a
page-aligned starting address.  It seems to me we need to modify the
metadata i/o paths to bypass the page cache

Heh, no way to do that easily. All the journalling machinery depends on
buffers and pages...

, or teach the fsync code
how to flush populated data pages out of the radix.

This might be doable but it will be difficult to avoid aliasing issues and
data corruption. And mainly I don't see the point: When you mount a
filesystem on top of block device, you do not want to mess with the block
device directly, even less using DAX. So we just have to find a way how to
set S_DAX for normal open but clear it from fs path. At worst, we could
clear S_DAX on the block device in mount_bdev() or something like that...

								Honza
-- 
Jan Kara [off-list ref]
SUSE Labs, CR

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>