[PATCH v2 0/3] fix sync to flush processor cache for ext2/4 DAX files

[PATCH v2 0/3] fix sync to flush processor cache for ext2/4 DAX files

[Toshi Kani]

This patchset fixes issues that sync syscall to existing DAX ex2/4
files does not flush processor cache.

Patch 1/3 adds .bmap to ext4_dax_aops so that mount -o dax works for
the journal inode.

Patch 2/3 fixes the ext4 issue by setting i_flags before ext4_set_aops()
in ext4_iget().

Patch 3/3 fixes the ext2 issue by setting i_flags before ext2_set_aops()
in ext2_iget(), i.e. same as ext4.

v2:
 - Add .bmap to ext4_dax_aops, instead of disabling dax for the journal
   inode. (Jan Kara)
 - Initialize i_flags early in ext4_iget(). (Jan Kara)
 - Update Fixes tag and add Cc to Stable. (Dan Williams)
 - Add fix to ext2.

---
Toshi Kani (3):
 1/3 ext4, dax: add ext4_bmap to ext4_dax_aops
 2/3 ext4, dax: set ext4_dax_aops for dax files
 3/3 ext2, dax: set ext2_dax_aops for dax files

---
 fs/ext2/inode.c | 2 +-
 fs/ext4/inode.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

[PATCH v2 1/3] ext4, dax: add ext4_bmap to ext4_dax_aops

[Toshi Kani]

Ext4 mount path calls .bmap to the journal inode. This currently
works for the DAX mount case because ext4_iget() always set
'ext4_da_aops' to any regular files.

In preparation to fix ext4_iget() to set 'ext4_dax_aops' for ext4
DAX files, add ext4_bmap() to 'ext4_dax_aops'. .bmap works for
DAX inodes. [1]

[1]: https://lkml.org/lkml/2018/9/12/803
Fixes: 5f0663bb4a64 ("ext4, dax: introduce ext4_dax_aops")
Signed-off-by: Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org>
Suggested-by: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
Cc: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
Cc: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: "Theodore Ts'o" <tytso-3s7WtUTddSA@public.gmane.org>
Cc: Andreas Dilger <adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
 fs/ext4/inode.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index d0dd585add6a..e4acaa980467 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3948,6 +3948,7 @@ static const struct address_space_operations ext4_dax_aops = {
 	.writepages		= ext4_dax_writepages,
 	.direct_IO		= noop_direct_IO,
 	.set_page_dirty		= noop_set_page_dirty,
+	.bmap			= ext4_bmap,
 	.invalidatepage		= noop_invalidatepage,
 };

Re: [PATCH v2 1/3] ext4, dax: add ext4_bmap to ext4_dax_aops

[Theodore Y. Ts'o]

On Fri, Sep 14, 2018 at 08:51:12AM -0600, Toshi Kani wrote:
> Ext4 mount path calls .bmap to the journal inode. This currently
> works for the DAX mount case because ext4_iget() always set
> 'ext4_da_aops' to any regular files.
> 
> In preparation to fix ext4_iget() to set 'ext4_dax_aops' for ext4
> DAX files, add ext4_bmap() to 'ext4_dax_aops'. .bmap works for
> DAX inodes. [1]
> 
> [1]: https://lkml.org/lkml/2018/9/12/803
> Fixes: 5f0663bb4a64 ("ext4, dax: introduce ext4_dax_aops")
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Suggested-by: Jan Kara <jack@suse.cz>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Andreas Dilger <adilger.kernel@dilger.ca>
> Cc: <stable@vger.kernel.org>

Thanks, applied.

						- Ted

[PATCH v2 2/3] ext4, dax: set ext4_dax_aops for dax files

[Toshi Kani]

Sync syscall to DAX file needs to flush processor cache, but it
currently does not flush to existing DAX files.  This is because
'ext4_da_aops' is set to address_space_operations of existing DAX
files, instead of 'ext4_dax_aops', since S_DAX flag is set after
ext4_set_aops() in the open path.

  New file
  --------
  lookup_open
    ext4_create
      __ext4_new_inode
        ext4_set_inode_flags   // Set S_DAX flag
      ext4_set_aops            // Set aops to ext4_dax_aops

  Existing file
  -------------
  lookup_open
    ext4_lookup
      ext4_iget
        ext4_set_aops          // Set aops to ext4_da_aops
        ext4_set_inode_flags   // Set S_DAX flag

Change ext4_iget() to initialize i_flags before ext4_set_aops().

Fixes: 5f0663bb4a64 ("ext4, dax: introduce ext4_dax_aops")
Signed-off-by: Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org>
Suggested-by: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
Cc: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
Cc: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: "Theodore Ts'o" <tytso-3s7WtUTddSA@public.gmane.org>
Cc: Andreas Dilger <adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
 fs/ext4/inode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index e4acaa980467..b19387b75f2b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4896,6 +4896,7 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
 		 * not initialized on a new filesystem. */
 	}
 	ei->i_flags = le32_to_cpu(raw_inode->i_flags);
+	ext4_set_inode_flags(inode);
 	inode->i_blocks = ext4_inode_blocks(raw_inode, ei);
 	ei->i_file_acl = le32_to_cpu(raw_inode->i_file_acl_lo);
 	if (ext4_has_feature_64bit(sb))
@@ -5042,7 +5043,6 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
 		goto bad_inode;
 	}
 	brelse(iloc.bh);
-	ext4_set_inode_flags(inode);
 
 	unlock_new_inode(inode);
 	return inode;

Re: [PATCH v2 2/3] ext4, dax: set ext4_dax_aops for dax files

[Theodore Y. Ts'o]

On Fri, Sep 14, 2018 at 08:51:13AM -0600, Toshi Kani wrote:
> Sync syscall to DAX file needs to flush processor cache, but it
> currently does not flush to existing DAX files.  This is because
> 'ext4_da_aops' is set to address_space_operations of existing DAX
> files, instead of 'ext4_dax_aops', since S_DAX flag is set after
> ext4_set_aops() in the open path.
> 
>   New file
>   --------
>   lookup_open
>     ext4_create
>       __ext4_new_inode
>         ext4_set_inode_flags   // Set S_DAX flag
>       ext4_set_aops            // Set aops to ext4_dax_aops
> 
>   Existing file
>   -------------
>   lookup_open
>     ext4_lookup
>       ext4_iget
>         ext4_set_aops          // Set aops to ext4_da_aops
>         ext4_set_inode_flags   // Set S_DAX flag
> 
> Change ext4_iget() to initialize i_flags before ext4_set_aops().
> 
> Fixes: 5f0663bb4a64 ("ext4, dax: introduce ext4_dax_aops")
> Signed-off-by: Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org>
> Suggested-by: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
> Cc: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
> Cc: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: "Theodore Ts'o" <tytso-3s7WtUTddSA@public.gmane.org>
> Cc: Andreas Dilger <adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>

Thanks, applied.

					- Ted

[PATCH v2 3/3] ext2, dax: set ext2_dax_aops for dax files

[Toshi Kani]

Sync syscall to DAX file needs to flush processor cache, but it
currently does not flush to existing DAX files.  This is because
'ext2_da_aops' is set to address_space_operations of existing DAX
files, instead of 'ext2_dax_aops', since S_DAX flag is set after
ext2_set_aops() in the open path.

Similar to ext4, change ext2_iget() to initialize i_flags before
ext2_set_aops().

Fixes: fb094c90748f ("ext2, dax: introduce ext2_dax_aops")
Signed-off-by: Toshi Kani <toshi.kani-ZPxbGqLxI0U@public.gmane.org>
Suggested-by: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
Cc: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
Cc: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: "Theodore Ts'o" <tytso-3s7WtUTddSA@public.gmane.org>
Cc: Andreas Dilger <adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
 fs/ext2/inode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 7f7ee18fe179..e4bb9386c045 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1448,6 +1448,7 @@ struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
 	}
 	inode->i_blocks = le32_to_cpu(raw_inode->i_blocks);
 	ei->i_flags = le32_to_cpu(raw_inode->i_flags);
+	ext2_set_inode_flags(inode);
 	ei->i_faddr = le32_to_cpu(raw_inode->i_faddr);
 	ei->i_frag_no = raw_inode->i_frag;
 	ei->i_frag_size = raw_inode->i_fsize;
@@ -1517,7 +1518,6 @@ struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
 			   new_decode_dev(le32_to_cpu(raw_inode->i_block[1])));
 	}
 	brelse (bh);
-	ext2_set_inode_flags(inode);
 	unlock_new_inode(inode);
 	return inode;
 	

Re: [PATCH v2 3/3] ext2, dax: set ext2_dax_aops for dax files

[Jan Kara]

On Fri 14-09-18 08:51:14, Toshi Kani wrote:
> Sync syscall to DAX file needs to flush processor cache, but it
> currently does not flush to existing DAX files.  This is because
> 'ext2_da_aops' is set to address_space_operations of existing DAX
> files, instead of 'ext2_dax_aops', since S_DAX flag is set after
> ext2_set_aops() in the open path.
> 
> Similar to ext4, change ext2_iget() to initialize i_flags before
> ext2_set_aops().
> 
> Fixes: fb094c90748f ("ext2, dax: introduce ext2_dax_aops")
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Suggested-by: Jan Kara <jack@suse.cz>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Andreas Dilger <adilger.kernel@dilger.ca>
> Cc: <stable@vger.kernel.org>

Thanks. I'll take the patch through my tree.

								Honza

> ---
>  fs/ext2/inode.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 7f7ee18fe179..e4bb9386c045 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -1448,6 +1448,7 @@ struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
>  	}
>  	inode->i_blocks = le32_to_cpu(raw_inode->i_blocks);
>  	ei->i_flags = le32_to_cpu(raw_inode->i_flags);
> +	ext2_set_inode_flags(inode);
>  	ei->i_faddr = le32_to_cpu(raw_inode->i_faddr);
>  	ei->i_frag_no = raw_inode->i_frag;
>  	ei->i_frag_size = raw_inode->i_fsize;
> @@ -1517,7 +1518,6 @@ struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
>  			   new_decode_dev(le32_to_cpu(raw_inode->i_block[1])));
>  	}
>  	brelse (bh);
> -	ext2_set_inode_flags(inode);
>  	unlock_new_inode(inode);
>  	return inode;
>  	
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR