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(-)
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 <[email protected]>
Suggested-by: Jan Kara <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: "Theodore Ts'o" <[email protected]>
Cc: Andreas Dilger <[email protected]>
Cc: <[email protected]>
---
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;
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 <[email protected]>
Suggested-by: Jan Kara <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: "Theodore Ts'o" <[email protected]>
Cc: Andreas Dilger <[email protected]>
Cc: <[email protected]>
---
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;
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 <[email protected]>
Suggested-by: Jan Kara <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: "Theodore Ts'o" <[email protected]>
Cc: Andreas Dilger <[email protected]>
Cc: <[email protected]>
---
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,
};
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 <[email protected]>
> Suggested-by: Jan Kara <[email protected]>
> Cc: Jan Kara <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: "Theodore Ts'o" <[email protected]>
> Cc: Andreas Dilger <[email protected]>
> Cc: <[email protected]>
Thanks, applied.
- Ted
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 <[email protected]>
> Suggested-by: Jan Kara <[email protected]>
> Cc: Jan Kara <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: "Theodore Ts'o" <[email protected]>
> Cc: Andreas Dilger <[email protected]>
> Cc: <[email protected]>
Thanks, applied.
- Ted
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 <[email protected]>
> Suggested-by: Jan Kara <[email protected]>
> Cc: Jan Kara <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: "Theodore Ts'o" <[email protected]>
> Cc: Andreas Dilger <[email protected]>
> Cc: <[email protected]>
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 <[email protected]>
SUSE Labs, CR