From: Ira Weiny <[email protected]>
Changes from V2:
Rework DAX exclusivity with verity and encryption based on feedback
from Eric
Enable the same per file DAX support in ext4 as was done for xfs. This series
builds and depends on the V11 series for xfs.[1]
This passes the same xfstests test as XFS.
The only issue is that this modifies the old mount option parsing code rather
than waiting for the new parsing code to be finalized.
This series starts with 3 fixes which include making Verity and Encrypt truly
mutually exclusive from DAX. I think these first 3 patches should be picked up
for 5.8 regardless of what is decided regarding the mount parsing.
[1] https://lore.kernel.org/lkml/[email protected]/
To: [email protected]
Cc: "Darrick J. Wong" <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Dave Chinner <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: "Theodore Y. Ts'o" <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Ira Weiny (8):
fs/ext4: Narrow scope of DAX check in setflags
fs/ext4: Disallow verity if inode is DAX
fs/ext4: Change EXT4_MOUNT_DAX to EXT4_MOUNT_DAX_ALWAYS
fs/ext4: Update ext4_should_use_dax()
fs/ext4: Only change S_DAX on inode load
fs/ext4: Make DAX mount option a tri-state
fs/ext4: Introduce DAX inode flag
Documentation/dax: Update DAX enablement for ext4
Documentation/filesystems/dax.txt | 6 +-
Documentation/filesystems/ext4/verity.rst | 3 +
fs/ext4/ext4.h | 22 +++++--
fs/ext4/ialloc.c | 2 +-
fs/ext4/inode.c | 25 +++++--
fs/ext4/ioctl.c | 41 ++++++++++--
fs/ext4/super.c | 80 ++++++++++++++++++-----
fs/ext4/verity.c | 5 +-
include/uapi/linux/fs.h | 1 +
9 files changed, 148 insertions(+), 37 deletions(-)
--
2.25.1
From: Ira Weiny <[email protected]>
Add a flag to preserve FS_XFLAG_DAX in the ext4 inode.
Set the flag to be user visible and changeable. Set the flag to be
inherited. Allow applications to change the flag at any time with the
exception of if VERITY or ENCRYPT is set.
Disallow setting VERITY or ENCRYPT if DAX is set.
Finally, on regular files, flag the inode to not be cached to facilitate
changing S_DAX on the next creation of the inode.
Signed-off-by: Ira Weiny <[email protected]>
---
Change from V2:
Add in making verity and DAX exclusive.
'Squash' in making encryption and DAX exclusive.
Add in EXT4_INODE_DAX flag definition to be compatible with
ext4_[set|test]_inode_flag() bit operations
Use ext4_[set|test]_inode_flag() bit operations to be consistent
with other code.
Change from V0:
Add FS_DAX_FL to include/uapi/linux/fs.h
to be consistent
Move ext4_dax_dontcache() to ext4_ioctl_setflags()
This ensures that it is only set when the flags are going to be
set and not if there is an error
Also this sets don't cache in the FS_IOC_SETFLAGS case
Change from RFC:
use new d_mark_dontcache()
Allow caching if ALWAYS/NEVER is set
Rebased to latest Linus master
Change flag to unused 0x01000000
update ext4_should_enable_dax()
---
fs/ext4/ext4.h | 14 ++++++++++----
fs/ext4/inode.c | 2 +-
fs/ext4/ioctl.c | 34 +++++++++++++++++++++++++++++++++-
fs/ext4/super.c | 3 +++
fs/ext4/verity.c | 2 +-
include/uapi/linux/fs.h | 1 +
6 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 6235440e4c39..467c30a789b6 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -415,13 +415,16 @@ struct flex_groups {
#define EXT4_VERITY_FL 0x00100000 /* Verity protected inode */
#define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
/* 0x00400000 was formerly EXT4_EOFBLOCKS_FL */
+
+#define EXT4_DAX_FL 0x01000000 /* Inode is DAX */
+
#define EXT4_INLINE_DATA_FL 0x10000000 /* Inode has inline data. */
#define EXT4_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
#define EXT4_CASEFOLD_FL 0x40000000 /* Casefolded file */
#define EXT4_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
-#define EXT4_FL_USER_VISIBLE 0x705BDFFF /* User visible flags */
-#define EXT4_FL_USER_MODIFIABLE 0x604BC0FF /* User modifiable flags */
+#define EXT4_FL_USER_VISIBLE 0x715BDFFF /* User visible flags */
+#define EXT4_FL_USER_MODIFIABLE 0x614BC0FF /* User modifiable flags */
/* Flags we can manipulate with through EXT4_IOC_FSSETXATTR */
#define EXT4_FL_XFLAG_VISIBLE (EXT4_SYNC_FL | \
@@ -429,14 +432,16 @@ struct flex_groups {
EXT4_APPEND_FL | \
EXT4_NODUMP_FL | \
EXT4_NOATIME_FL | \
- EXT4_PROJINHERIT_FL)
+ EXT4_PROJINHERIT_FL | \
+ EXT4_DAX_FL)
/* Flags that should be inherited by new inodes from their parent. */
#define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
EXT4_SYNC_FL | EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
EXT4_NOCOMPR_FL | EXT4_JOURNAL_DATA_FL |\
EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL |\
- EXT4_PROJINHERIT_FL | EXT4_CASEFOLD_FL)
+ EXT4_PROJINHERIT_FL | EXT4_CASEFOLD_FL |\
+ EXT4_DAX_FL)
/* Flags that are appropriate for regular files (all but dir-specific ones). */
#define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL | EXT4_CASEFOLD_FL |\
@@ -488,6 +493,7 @@ enum {
EXT4_INODE_VERITY = 20, /* Verity protected inode */
EXT4_INODE_EA_INODE = 21, /* Inode used for large EA */
/* 22 was formerly EXT4_INODE_EOFBLOCKS */
+ EXT4_INODE_DAX = 24, /* Inode is DAX */
EXT4_INODE_INLINE_DATA = 28, /* Data in inode. */
EXT4_INODE_PROJINHERIT = 29, /* Create with parents projid */
EXT4_INODE_RESERVED = 31, /* reserved for ext4 lib */
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 140b1930e2f4..ae61db8b8bae 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4418,7 +4418,7 @@ static bool ext4_should_enable_dax(struct inode *inode)
if (test_opt(inode->i_sb, DAX_ALWAYS))
return true;
- return false;
+ return ext4_test_inode_flag(inode, EXT4_INODE_DAX);
}
void ext4_set_inode_flags(struct inode *inode, bool init)
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 145083e8cd1e..668b8c17d6eb 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -292,6 +292,21 @@ static int ext4_ioctl_check_immutable(struct inode *inode, __u32 new_projid,
return 0;
}
+static void ext4_dax_dontcache(struct inode *inode, unsigned int flags)
+{
+ struct ext4_inode_info *ei = EXT4_I(inode);
+
+ if (S_ISDIR(inode->i_mode))
+ return;
+
+ if (test_opt2(inode->i_sb, DAX_NEVER) ||
+ test_opt(inode->i_sb, DAX_ALWAYS))
+ return;
+
+ if ((ei->i_flags ^ flags) & EXT4_DAX_FL)
+ d_mark_dontcache(inode);
+}
+
static int ext4_ioctl_setflags(struct inode *inode,
unsigned int flags)
{
@@ -303,6 +318,16 @@ static int ext4_ioctl_setflags(struct inode *inode,
unsigned int jflag;
struct super_block *sb = inode->i_sb;
+ if (ext4_test_inode_flag(inode, EXT4_INODE_DAX)) {
+ if (ext4_test_inode_flag(inode, EXT4_INODE_VERITY) ||
+ ext4_test_inode_flag(inode, EXT4_INODE_ENCRYPT) ||
+ ext4_test_inode_state(inode,
+ EXT4_STATE_VERITY_IN_PROGRESS)) {
+ err = -EOPNOTSUPP;
+ goto flags_out;
+ }
+ }
+
/* Is it quota file? Do not allow user to mess with it */
if (ext4_is_quota_file(inode))
goto flags_out;
@@ -369,6 +394,8 @@ static int ext4_ioctl_setflags(struct inode *inode,
if (err)
goto flags_err;
+ ext4_dax_dontcache(inode, flags);
+
for (i = 0, mask = 1; i < 32; i++, mask <<= 1) {
if (!(mask & EXT4_FL_USER_MODIFIABLE))
continue;
@@ -528,12 +555,15 @@ static inline __u32 ext4_iflags_to_xflags(unsigned long iflags)
xflags |= FS_XFLAG_NOATIME;
if (iflags & EXT4_PROJINHERIT_FL)
xflags |= FS_XFLAG_PROJINHERIT;
+ if (iflags & EXT4_DAX_FL)
+ xflags |= FS_XFLAG_DAX;
return xflags;
}
#define EXT4_SUPPORTED_FS_XFLAGS (FS_XFLAG_SYNC | FS_XFLAG_IMMUTABLE | \
FS_XFLAG_APPEND | FS_XFLAG_NODUMP | \
- FS_XFLAG_NOATIME | FS_XFLAG_PROJINHERIT)
+ FS_XFLAG_NOATIME | FS_XFLAG_PROJINHERIT | \
+ FS_XFLAG_DAX)
/* Transfer xflags flags to internal */
static inline unsigned long ext4_xflags_to_iflags(__u32 xflags)
@@ -552,6 +582,8 @@ static inline unsigned long ext4_xflags_to_iflags(__u32 xflags)
iflags |= EXT4_NOATIME_FL;
if (xflags & FS_XFLAG_PROJINHERIT)
iflags |= EXT4_PROJINHERIT_FL;
+ if (xflags & FS_XFLAG_DAX)
+ iflags |= EXT4_DAX_FL;
return iflags;
}
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 5ba65eb0e2ef..be9713e898eb 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1323,6 +1323,9 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode)))
return -EINVAL;
+ if (ext4_test_inode_flag(inode, EXT4_INODE_DAX))
+ return -EOPNOTSUPP;
+
res = ext4_convert_inline_data(inode);
if (res)
return res;
diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
index 89a155ece323..4fecb3e4e338 100644
--- a/fs/ext4/verity.c
+++ b/fs/ext4/verity.c
@@ -113,7 +113,7 @@ static int ext4_begin_enable_verity(struct file *filp)
handle_t *handle;
int err;
- if (IS_DAX(inode))
+ if (IS_DAX(inode) || ext4_test_inode_flag(inode, EXT4_INODE_DAX))
return -EINVAL;
if (ext4_verity_in_progress(inode))
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 379a612f8f1d..7c5f6eb51e2d 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -262,6 +262,7 @@ struct fsxattr {
#define FS_EA_INODE_FL 0x00200000 /* Inode used for large EA */
#define FS_EOFBLOCKS_FL 0x00400000 /* Reserved for ext4 */
#define FS_NOCOW_FL 0x00800000 /* Do not cow file */
+#define FS_DAX_FL 0x01000000 /* Inode is DAX */
#define FS_INLINE_DATA_FL 0x10000000 /* Reserved for ext4 */
#define FS_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
#define FS_CASEFOLD_FL 0x40000000 /* Folder is case insensitive */
--
2.25.1
From: Ira Weiny <[email protected]>
To prevent complications with in memory inodes we only set S_DAX on
inode load. FS_XFLAG_DAX can be changed at any time and S_DAX will
change after inode eviction and reload.
Add init bool to ext4_set_inode_flags() to indicate if the inode is
being newly initialized.
Assert that S_DAX is not set on an inode which is just being loaded.
Reviewed-by: Jan Kara <[email protected]>
Signed-off-by: Ira Weiny <[email protected]>
---
Changes from V2:
Rework based on moving the encryption patch to the end.
Changes from RFC:
Change J_ASSERT() to WARN_ON_ONCE()
Fix bug which would clear S_DAX incorrectly
---
fs/ext4/ext4.h | 2 +-
fs/ext4/ialloc.c | 2 +-
fs/ext4/inode.c | 13 ++++++++++---
fs/ext4/ioctl.c | 3 ++-
fs/ext4/super.c | 4 ++--
fs/ext4/verity.c | 2 +-
6 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 1a3daf2d18ef..86a0994332ce 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2692,7 +2692,7 @@ extern int ext4_can_truncate(struct inode *inode);
extern int ext4_truncate(struct inode *);
extern int ext4_break_layouts(struct inode *);
extern int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length);
-extern void ext4_set_inode_flags(struct inode *);
+extern void ext4_set_inode_flags(struct inode *, bool init);
extern int ext4_alloc_da_blocks(struct inode *inode);
extern void ext4_set_aops(struct inode *inode);
extern int ext4_writepage_trans_blocks(struct inode *);
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 4b8c9a9bdf0c..7941c140723f 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -1116,7 +1116,7 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
ei->i_block_group = group;
ei->i_last_alloc_group = ~0;
- ext4_set_inode_flags(inode);
+ ext4_set_inode_flags(inode, true);
if (IS_DIRSYNC(inode))
ext4_handle_sync(handle);
if (insert_inode_locked(inode) < 0) {
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index d3a4c2ed7a1c..23e42a223235 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4419,11 +4419,13 @@ static bool ext4_should_enable_dax(struct inode *inode)
return false;
}
-void ext4_set_inode_flags(struct inode *inode)
+void ext4_set_inode_flags(struct inode *inode, bool init)
{
unsigned int flags = EXT4_I(inode)->i_flags;
unsigned int new_fl = 0;
+ WARN_ON_ONCE(IS_DAX(inode) && init);
+
if (flags & EXT4_SYNC_FL)
new_fl |= S_SYNC;
if (flags & EXT4_APPEND_FL)
@@ -4434,8 +4436,13 @@ void ext4_set_inode_flags(struct inode *inode)
new_fl |= S_NOATIME;
if (flags & EXT4_DIRSYNC_FL)
new_fl |= S_DIRSYNC;
- if (ext4_should_enable_dax(inode))
+
+ /* Because of the way inode_set_flags() works we must preserve S_DAX
+ * here if already set. */
+ new_fl |= (inode->i_flags & S_DAX);
+ if (init && ext4_should_enable_dax(inode))
new_fl |= S_DAX;
+
if (flags & EXT4_ENCRYPT_FL)
new_fl |= S_ENCRYPTED;
if (flags & EXT4_CASEFOLD_FL)
@@ -4649,7 +4656,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);
+ ext4_set_inode_flags(inode, true);
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))
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 5813e5e73eab..145083e8cd1e 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -381,7 +381,8 @@ static int ext4_ioctl_setflags(struct inode *inode,
ext4_clear_inode_flag(inode, i);
}
- ext4_set_inode_flags(inode);
+ ext4_set_inode_flags(inode, false);
+
inode->i_ctime = current_time(inode);
err = ext4_mark_iloc_dirty(handle, inode, &iloc);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 7b99c44d0a91..3cb9b48d3cc4 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1348,7 +1348,7 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
* Update inode->i_flags - S_ENCRYPTED will be enabled,
* S_DAX may be disabled
*/
- ext4_set_inode_flags(inode);
+ ext4_set_inode_flags(inode, false);
}
return res;
}
@@ -1375,7 +1375,7 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
* Update inode->i_flags - S_ENCRYPTED will be enabled,
* S_DAX may be disabled
*/
- ext4_set_inode_flags(inode);
+ ext4_set_inode_flags(inode, false);
res = ext4_mark_inode_dirty(handle, inode);
if (res)
EXT4_ERROR_INODE(inode, "Failed to mark inode dirty");
diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
index f05a09fb2ae4..89a155ece323 100644
--- a/fs/ext4/verity.c
+++ b/fs/ext4/verity.c
@@ -244,7 +244,7 @@ static int ext4_end_enable_verity(struct file *filp, const void *desc,
if (err)
goto out_stop;
ext4_set_inode_flag(inode, EXT4_INODE_VERITY);
- ext4_set_inode_flags(inode);
+ ext4_set_inode_flags(inode, false);
err = ext4_mark_iloc_dirty(handle, inode, &iloc);
}
out_stop:
--
2.25.1
From: Ira Weiny <[email protected]>
Update the document to reflect ext4 and xfs now behave the same.
Reviewed-by: Jan Kara <[email protected]>
Signed-off-by: Ira Weiny <[email protected]>
---
Changes from RFC:
Update with ext2 text...
---
Documentation/filesystems/dax.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/filesystems/dax.txt b/Documentation/filesystems/dax.txt
index 735fb4b54117..265c4f808dbf 100644
--- a/Documentation/filesystems/dax.txt
+++ b/Documentation/filesystems/dax.txt
@@ -25,7 +25,7 @@ size when creating the filesystem.
Currently 3 filesystems support DAX: ext2, ext4 and xfs. Enabling DAX on them
is different.
-Enabling DAX on ext4 and ext2
+Enabling DAX on ext2
-----------------------------
When mounting the filesystem, use the "-o dax" option on the command line or
@@ -33,8 +33,8 @@ add 'dax' to the options in /etc/fstab. This works to enable DAX on all files
within the filesystem. It is equivalent to the '-o dax=always' behavior below.
-Enabling DAX on xfs
--------------------
+Enabling DAX on xfs and ext4
+----------------------------
Summary
-------
--
2.25.1
From: Ira Weiny <[email protected]>
In prep for the new tri-state mount option which then introduces
EXT4_MOUNT_DAX_NEVER.
Reviewed-by: Jan Kara <[email protected]>
Signed-off-by: Ira Weiny <[email protected]>
---
Changes:
New patch
---
fs/ext4/ext4.h | 4 ++--
fs/ext4/inode.c | 2 +-
fs/ext4/super.c | 12 ++++++------
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 91eb4381cae5..1a3daf2d18ef 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1123,9 +1123,9 @@ struct ext4_inode_info {
#define EXT4_MOUNT_MINIX_DF 0x00080 /* Mimics the Minix statfs */
#define EXT4_MOUNT_NOLOAD 0x00100 /* Don't use existing journal*/
#ifdef CONFIG_FS_DAX
-#define EXT4_MOUNT_DAX 0x00200 /* Direct Access */
+#define EXT4_MOUNT_DAX_ALWAYS 0x00200 /* Direct Access */
#else
-#define EXT4_MOUNT_DAX 0
+#define EXT4_MOUNT_DAX_ALWAYS 0
#endif
#define EXT4_MOUNT_DATA_FLAGS 0x00C00 /* Mode for data writes: */
#define EXT4_MOUNT_JOURNAL_DATA 0x00400 /* Write data to journal */
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 2a4aae6acdcb..a10ff12194db 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4400,7 +4400,7 @@ int ext4_get_inode_loc(struct inode *inode, struct ext4_iloc *iloc)
static bool ext4_should_use_dax(struct inode *inode)
{
- if (!test_opt(inode->i_sb, DAX))
+ if (!test_opt(inode->i_sb, DAX_ALWAYS))
return false;
if (!S_ISREG(inode->i_mode))
return false;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index bf5fcb477f66..7b99c44d0a91 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1775,7 +1775,7 @@ static const struct mount_opts {
{Opt_min_batch_time, 0, MOPT_GTE0},
{Opt_inode_readahead_blks, 0, MOPT_GTE0},
{Opt_init_itable, 0, MOPT_GTE0},
- {Opt_dax, EXT4_MOUNT_DAX, MOPT_SET},
+ {Opt_dax, EXT4_MOUNT_DAX_ALWAYS, MOPT_SET},
{Opt_stripe, 0, MOPT_GTE0},
{Opt_resuid, 0, MOPT_GTE0},
{Opt_resgid, 0, MOPT_GTE0},
@@ -3982,7 +3982,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
"both data=journal and dioread_nolock");
goto failed_mount;
}
- if (test_opt(sb, DAX)) {
+ if (test_opt(sb, DAX_ALWAYS)) {
ext4_msg(sb, KERN_ERR, "can't mount with "
"both data=journal and dax");
goto failed_mount;
@@ -4092,7 +4092,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
goto failed_mount;
}
- if (sbi->s_mount_opt & EXT4_MOUNT_DAX) {
+ if (sbi->s_mount_opt & EXT4_MOUNT_DAX_ALWAYS) {
if (ext4_has_feature_inline_data(sb)) {
ext4_msg(sb, KERN_ERR, "Cannot use DAX on a filesystem"
" that may contain inline data");
@@ -5412,7 +5412,7 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
err = -EINVAL;
goto restore_opts;
}
- if (test_opt(sb, DAX)) {
+ if (test_opt(sb, DAX_ALWAYS)) {
ext4_msg(sb, KERN_ERR, "can't mount with "
"both data=journal and dax");
err = -EINVAL;
@@ -5433,10 +5433,10 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
goto restore_opts;
}
- if ((sbi->s_mount_opt ^ old_opts.s_mount_opt) & EXT4_MOUNT_DAX) {
+ if ((sbi->s_mount_opt ^ old_opts.s_mount_opt) & EXT4_MOUNT_DAX_ALWAYS) {
ext4_msg(sb, KERN_WARNING, "warning: refusing change of "
"dax flag with busy inodes while remounting");
- sbi->s_mount_opt ^= EXT4_MOUNT_DAX;
+ sbi->s_mount_opt ^= EXT4_MOUNT_DAX_ALWAYS;
}
if (sbi->s_mount_flags & EXT4_MF_FS_ABORTED)
--
2.25.1
From: Ira Weiny <[email protected]>
S_DAX should only be enabled when the underlying block device supports
dax.
Change ext4_should_use_dax() to check for device support prior to the
over riding mount option.
While we are at it change the function to ext4_should_enable_dax() as
this better reflects the ask as well as matches xfs.
Reviewed-by: Jan Kara <[email protected]>
Signed-off-by: Ira Weiny <[email protected]>
---
Changes from RFC
Change function name to 'should enable'
Clean up bool conversion
Reorder this for better bisect-ability
---
fs/ext4/inode.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index a10ff12194db..d3a4c2ed7a1c 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4398,10 +4398,8 @@ int ext4_get_inode_loc(struct inode *inode, struct ext4_iloc *iloc)
!ext4_test_inode_state(inode, EXT4_STATE_XATTR));
}
-static bool ext4_should_use_dax(struct inode *inode)
+static bool ext4_should_enable_dax(struct inode *inode)
{
- if (!test_opt(inode->i_sb, DAX_ALWAYS))
- return false;
if (!S_ISREG(inode->i_mode))
return false;
if (ext4_should_journal_data(inode))
@@ -4412,7 +4410,13 @@ static bool ext4_should_use_dax(struct inode *inode)
return false;
if (ext4_test_inode_flag(inode, EXT4_INODE_VERITY))
return false;
- return true;
+ if (!bdev_dax_supported(inode->i_sb->s_bdev,
+ inode->i_sb->s_blocksize))
+ return false;
+ if (test_opt(inode->i_sb, DAX_ALWAYS))
+ return true;
+
+ return false;
}
void ext4_set_inode_flags(struct inode *inode)
@@ -4430,7 +4434,7 @@ void ext4_set_inode_flags(struct inode *inode)
new_fl |= S_NOATIME;
if (flags & EXT4_DIRSYNC_FL)
new_fl |= S_DIRSYNC;
- if (ext4_should_use_dax(inode))
+ if (ext4_should_enable_dax(inode))
new_fl |= S_DAX;
if (flags & EXT4_ENCRYPT_FL)
new_fl |= S_ENCRYPTED;
--
2.25.1
On Tue 19-05-20 22:57:49, [email protected] wrote:
> From: Ira Weiny <[email protected]>
>
> S_DAX should only be enabled when the underlying block device supports
> dax.
>
> Change ext4_should_use_dax() to check for device support prior to the
> over riding mount option.
>
> While we are at it change the function to ext4_should_enable_dax() as
> this better reflects the ask as well as matches xfs.
>
> Reviewed-by: Jan Kara <[email protected]>
> Signed-off-by: Ira Weiny <[email protected]>
...
> @@ -4412,7 +4410,13 @@ static bool ext4_should_use_dax(struct inode *inode)
> return false;
> if (ext4_test_inode_flag(inode, EXT4_INODE_VERITY))
> return false;
> - return true;
> + if (!bdev_dax_supported(inode->i_sb->s_bdev,
> + inode->i_sb->s_blocksize))
> + return false;
> + if (test_opt(inode->i_sb, DAX_ALWAYS))
> + return true;
> +
> + return false;
> }
Now that I think about it - shouldn't we rather cache the result of
bdev_dax_supported() in sb on mount and then just check the flag here?
Because bdev_dax_supported() isn't exactly cheap (it does a lot of checks
and mappings, tries to read from the pmem, ...).
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Tue 19-05-20 22:57:52, [email protected] wrote:
> From: Ira Weiny <[email protected]>
>
> Add a flag to preserve FS_XFLAG_DAX in the ext4 inode.
>
> Set the flag to be user visible and changeable. Set the flag to be
> inherited. Allow applications to change the flag at any time with the
> exception of if VERITY or ENCRYPT is set.
>
> Disallow setting VERITY or ENCRYPT if DAX is set.
>
> Finally, on regular files, flag the inode to not be cached to facilitate
> changing S_DAX on the next creation of the inode.
>
> Signed-off-by: Ira Weiny <[email protected]>
The patch looks good to me. You can add:
Reviewed-by: Jan Kara <[email protected]>
One comment below:
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 5ba65eb0e2ef..be9713e898eb 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1323,6 +1323,9 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
> if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode)))
> return -EINVAL;
AFAIU this check is here so that fscrypt_inherit_context() is able call us
and we can clear S_DAX flag. So can't we rather move this below the
EXT4_INODE_DAX check and change this to
IS_DAX(inode) && !(inode->i_flags & I_NEW)
? Because as I'm reading the code now, this should never trigger?
>
> + if (ext4_test_inode_flag(inode, EXT4_INODE_DAX))
> + return -EOPNOTSUPP;
> +
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Wed, May 20, 2020 at 04:11:38PM +0200, Jan Kara wrote:
> On Tue 19-05-20 22:57:52, [email protected] wrote:
> > From: Ira Weiny <[email protected]>
> >
> > Add a flag to preserve FS_XFLAG_DAX in the ext4 inode.
> >
> > Set the flag to be user visible and changeable. Set the flag to be
> > inherited. Allow applications to change the flag at any time with the
> > exception of if VERITY or ENCRYPT is set.
> >
> > Disallow setting VERITY or ENCRYPT if DAX is set.
> >
> > Finally, on regular files, flag the inode to not be cached to facilitate
> > changing S_DAX on the next creation of the inode.
> >
> > Signed-off-by: Ira Weiny <[email protected]>
>
> The patch looks good to me. You can add:
>
> Reviewed-by: Jan Kara <[email protected]>
>
> One comment below:
>
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index 5ba65eb0e2ef..be9713e898eb 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -1323,6 +1323,9 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
> > if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode)))
> > return -EINVAL;
>
> AFAIU this check is here so that fscrypt_inherit_context() is able call us
> and we can clear S_DAX flag.
Basically yes that is true. It is IMO somewhat convoluted because I think ext4
probably could have prevented S_DAX from being set in __ext4_new_inode() in the
first place. But that is a clean up I was not prepared to make last night.
> So can't we rather move this below the
> EXT4_INODE_DAX check and change this to
>
> IS_DAX(inode) && !(inode->i_flags & I_NEW)
>
> ? Because as I'm reading the code now, this should never trigger?
I agree this should never trigger. But I don't see how the order of the checks
maters much. But changing this to !new is probably worth doing to make it
clear what we really mean here.
I think that is a follow on patch. In addition, if we don't set S_DAX at all
in __ext4_new_inode() this check could then be what I had originally with the warn on.
if (WARN_ON_ONCE(IS_DAX(inode)))
...
... because it would be considered a bug to be setting DAX on inodes which are
going to be encrypted..
Ira
Something like this: (compiled only)
commit 6cd5aed3cd9e2c10e3fb7c6dd23918580532f256 (HEAD -> lck-4071-b13-v4)
Author: Ira Weiny <[email protected]>
Date: Wed May 20 11:32:50 2020 -0700
RFC: do not set S_DAX on an inode which is going to be encrypted
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 7941c140723f..be80cb639d74 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -844,6 +844,9 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
return ERR_PTR(-ENOMEM);
ei = EXT4_I(inode);
+ if (encrypt)
+ ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
+
/*
* Initialize owners and quota early so that we don't have to account
* for quota initialization worst case in standard inode creating
@@ -1165,6 +1168,7 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
err = fscrypt_inherit_context(dir, inode, handle, true);
if (err)
goto fail_free_drop;
+ ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
}
if (!(ei->i_flags & EXT4_EA_INODE_FL)) {
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index be9713e898eb..099b87864f55 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1320,7 +1320,10 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
if (inode->i_ino == EXT4_ROOT_INO)
return -EPERM;
- if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode)))
+ /* S_DAX should never be set here because encryption is not compatible
+ * with DAX
+ */
+ if (WARN_ON_ONCE(IS_DAX(inode)))
return -EINVAL;
if (ext4_test_inode_flag(inode, EXT4_INODE_DAX))
@@ -1337,22 +1340,11 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
* being set on an existing inode in its own transaction. Only in the
* latter case should the "retry on ENOSPC" logic be used.
*/
-
if (handle) {
res = ext4_xattr_set_handle(handle, inode,
EXT4_XATTR_INDEX_ENCRYPTION,
EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
ctx, len, 0);
- if (!res) {
- ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
- ext4_clear_inode_state(inode,
- EXT4_STATE_MAY_INLINE_DATA);
- /*
- * Update inode->i_flags - S_ENCRYPTED will be enabled,
- * S_DAX may be disabled
- */
- ext4_set_inode_flags(inode, false);
- }
return res;
}
On May 19, 2020, at 11:57 PM, [email protected] wrote:
>
> From: Ira Weiny <[email protected]>
>
> Add a flag to preserve FS_XFLAG_DAX in the ext4 inode.
>
> Set the flag to be user visible and changeable. Set the flag to be
> inherited. Allow applications to change the flag at any time with the
> exception of if VERITY or ENCRYPT is set.
>
> Disallow setting VERITY or ENCRYPT if DAX is set.
>
> Finally, on regular files, flag the inode to not be cached to facilitate
> changing S_DAX on the next creation of the inode.
>
> Signed-off-by: Ira Weiny <[email protected]>
>
> ---
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 6235440e4c39..467c30a789b6 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -415,13 +415,16 @@ struct flex_groups {
> #define EXT4_VERITY_FL 0x00100000 /* Verity protected inode */
> #define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
> /* 0x00400000 was formerly EXT4_EOFBLOCKS_FL */
> +
> +#define EXT4_DAX_FL 0x01000000 /* Inode is DAX */
> +
> #define EXT4_INLINE_DATA_FL 0x10000000 /* Inode has inline data. */
> #define EXT4_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
> #define EXT4_CASEFOLD_FL 0x40000000 /* Casefolded file */
> #define EXT4_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
Hi Ira,
This flag value conflicts with the reserved flag in e2fsprogs for snapshots:
#define EXT4_SNAPFILE_FL 0x01000000 /* Inode is a snapshot */
Please change EXT4_DAX_FL and FS_DAX_FL to use 0x02000000, which is not used
for anything in either case.
Cheers, Andreas
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index 379a612f8f1d..7c5f6eb51e2d 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -262,6 +262,7 @@ struct fsxattr {
> #define FS_EA_INODE_FL 0x00200000 /* Inode used for large EA */
> #define FS_EOFBLOCKS_FL 0x00400000 /* Reserved for ext4 */
> #define FS_NOCOW_FL 0x00800000 /* Do not cow file */
> +#define FS_DAX_FL 0x01000000 /* Inode is DAX */
> #define FS_INLINE_DATA_FL 0x10000000 /* Reserved for ext4 */
> #define FS_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
> #define FS_CASEFOLD_FL 0x40000000 /* Folder is case insensitive */
> --
> 2.25.1
>
Cheers, Andreas
On Wed, May 20, 2020 at 01:26:44PM -0600, Andreas Dilger wrote:
> On May 19, 2020, at 11:57 PM, [email protected] wrote:
> >
> > From: Ira Weiny <[email protected]>
> >
> > Add a flag to preserve FS_XFLAG_DAX in the ext4 inode.
> >
> > Set the flag to be user visible and changeable. Set the flag to be
> > inherited. Allow applications to change the flag at any time with the
> > exception of if VERITY or ENCRYPT is set.
> >
> > Disallow setting VERITY or ENCRYPT if DAX is set.
> >
> > Finally, on regular files, flag the inode to not be cached to facilitate
> > changing S_DAX on the next creation of the inode.
> >
> > Signed-off-by: Ira Weiny <[email protected]>
> >
> > ---
> > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> > index 6235440e4c39..467c30a789b6 100644
> > --- a/fs/ext4/ext4.h
> > +++ b/fs/ext4/ext4.h
> > @@ -415,13 +415,16 @@ struct flex_groups {
> > #define EXT4_VERITY_FL 0x00100000 /* Verity protected inode */
> > #define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
> > /* 0x00400000 was formerly EXT4_EOFBLOCKS_FL */
> > +
> > +#define EXT4_DAX_FL 0x01000000 /* Inode is DAX */
> > +
> > #define EXT4_INLINE_DATA_FL 0x10000000 /* Inode has inline data. */
> > #define EXT4_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
> > #define EXT4_CASEFOLD_FL 0x40000000 /* Casefolded file */
> > #define EXT4_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
>
> Hi Ira,
> This flag value conflicts with the reserved flag in e2fsprogs for snapshots:
>
> #define EXT4_SNAPFILE_FL 0x01000000 /* Inode is a snapshot */
Sure NP but is that new? I'm building off of 5.7-rc4.
Just curious if I completely missed something.
>
> Please change EXT4_DAX_FL and FS_DAX_FL to use 0x02000000, which is not used
> for anything in either case.
NP, thanks!
Ira
>
> Cheers, Andreas
>
>
> > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> > index 379a612f8f1d..7c5f6eb51e2d 100644
> > --- a/include/uapi/linux/fs.h
> > +++ b/include/uapi/linux/fs.h
> > @@ -262,6 +262,7 @@ struct fsxattr {
> > #define FS_EA_INODE_FL 0x00200000 /* Inode used for large EA */
> > #define FS_EOFBLOCKS_FL 0x00400000 /* Reserved for ext4 */
> > #define FS_NOCOW_FL 0x00800000 /* Do not cow file */
> > +#define FS_DAX_FL 0x01000000 /* Inode is DAX */
> > #define FS_INLINE_DATA_FL 0x10000000 /* Reserved for ext4 */
> > #define FS_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
> > #define FS_CASEFOLD_FL 0x40000000 /* Folder is case insensitive */
> > --
> > 2.25.1
> >
>
>
> Cheers, Andreas
>
>
>
>
>
On Wed, May 20, 2020 at 01:02:42PM -0700, Ira Weiny wrote:
> On Wed, May 20, 2020 at 01:26:44PM -0600, Andreas Dilger wrote:
> > On May 19, 2020, at 11:57 PM, [email protected] wrote:
> > >
> > > From: Ira Weiny <[email protected]>
> > >
> > > Add a flag to preserve FS_XFLAG_DAX in the ext4 inode.
> > >
> > > Set the flag to be user visible and changeable. Set the flag to be
> > > inherited. Allow applications to change the flag at any time with the
> > > exception of if VERITY or ENCRYPT is set.
> > >
> > > Disallow setting VERITY or ENCRYPT if DAX is set.
> > >
> > > Finally, on regular files, flag the inode to not be cached to facilitate
> > > changing S_DAX on the next creation of the inode.
> > >
> > > Signed-off-by: Ira Weiny <[email protected]>
> > >
> > > ---
> > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> > > index 6235440e4c39..467c30a789b6 100644
> > > --- a/fs/ext4/ext4.h
> > > +++ b/fs/ext4/ext4.h
> > > @@ -415,13 +415,16 @@ struct flex_groups {
> > > #define EXT4_VERITY_FL 0x00100000 /* Verity protected inode */
> > > #define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
> > > /* 0x00400000 was formerly EXT4_EOFBLOCKS_FL */
> > > +
> > > +#define EXT4_DAX_FL 0x01000000 /* Inode is DAX */
> > > +
> > > #define EXT4_INLINE_DATA_FL 0x10000000 /* Inode has inline data. */
> > > #define EXT4_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
> > > #define EXT4_CASEFOLD_FL 0x40000000 /* Casefolded file */
> > > #define EXT4_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
> >
> > Hi Ira,
> > This flag value conflicts with the reserved flag in e2fsprogs for snapshots:
> >
> > #define EXT4_SNAPFILE_FL 0x01000000 /* Inode is a snapshot */
>
> Sure NP but is that new? I'm building off of 5.7-rc4.
>
> Just curious if I completely missed something.
Yeah, you missed that ... for some reason the kernel ext4 driver is
missing flags that are in e2fsprogs. (huh??)
I would say you could probably just take over the flag because the 2010s
called and they don't want next3 back. I guess that leaves 0x02000000
as the sole unclaimed bit, but this seriously needs some cleaning.
--D
> >
> > Please change EXT4_DAX_FL and FS_DAX_FL to use 0x02000000, which is not used
> > for anything in either case.
>
> NP, thanks!
> Ira
>
> >
> > Cheers, Andreas
> >
> >
> > > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> > > index 379a612f8f1d..7c5f6eb51e2d 100644
> > > --- a/include/uapi/linux/fs.h
> > > +++ b/include/uapi/linux/fs.h
> > > @@ -262,6 +262,7 @@ struct fsxattr {
> > > #define FS_EA_INODE_FL 0x00200000 /* Inode used for large EA */
> > > #define FS_EOFBLOCKS_FL 0x00400000 /* Reserved for ext4 */
> > > #define FS_NOCOW_FL 0x00800000 /* Do not cow file */
> > > +#define FS_DAX_FL 0x01000000 /* Inode is DAX */
> > > #define FS_INLINE_DATA_FL 0x10000000 /* Reserved for ext4 */
> > > #define FS_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
> > > #define FS_CASEFOLD_FL 0x40000000 /* Folder is case insensitive */
> > > --
> > > 2.25.1
> > >
> >
> >
> > Cheers, Andreas
> >
> >
> >
> >
> >
>
>
On May 20, 2020, at 2:55 PM, Darrick J. Wong <[email protected]> wrote:
> On Wed, May 20, 2020 at 01:02:42PM -0700, Ira Weiny wrote:
>> On Wed, May 20, 2020 at 01:26:44PM -0600, Andreas Dilger wrote:
>>> On May 19, 2020, at 11:57 PM, [email protected] wrote:
>>>>
>>>> From: Ira Weiny <[email protected]>
>>>>
>>>> Add a flag to preserve FS_XFLAG_DAX in the ext4 inode.
>>>>
>>>> Set the flag to be user visible and changeable. Set the flag to be
>>>> inherited. Allow applications to change the flag at any time with the
>>>> exception of if VERITY or ENCRYPT is set.
>>>>
>>>> Disallow setting VERITY or ENCRYPT if DAX is set.
>>>>
>>>> Finally, on regular files, flag the inode to not be cached to facilitate
>>>> changing S_DAX on the next creation of the inode.
>>>>
>>>> Signed-off-by: Ira Weiny <[email protected]>
>>>>
>>>> ---
>>>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>>>> index 6235440e4c39..467c30a789b6 100644
>>>> --- a/fs/ext4/ext4.h
>>>> +++ b/fs/ext4/ext4.h
>>>> @@ -415,13 +415,16 @@ struct flex_groups {
>>>> #define EXT4_VERITY_FL 0x00100000 /* Verity protected inode */
>>>> #define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
>>>> /* 0x00400000 was formerly EXT4_EOFBLOCKS_FL */
>>>> +
>>>> +#define EXT4_DAX_FL 0x01000000 /* Inode is DAX */
>>>> +
>>>> #define EXT4_INLINE_DATA_FL 0x10000000 /* Inode has inline data. */
>>>> #define EXT4_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
>>>> #define EXT4_CASEFOLD_FL 0x40000000 /* Casefolded file */
>>>> #define EXT4_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
>>>
>>> Hi Ira,
>>> This flag value conflicts with the reserved flag in e2fsprogs for snapshots:
>>>
>>> #define EXT4_SNAPFILE_FL 0x01000000 /* Inode is a snapshot */
>>
>> Sure NP but is that new? I'm building off of 5.7-rc4.
>>
>> Just curious if I completely missed something.
>
> Yeah, you missed that ... for some reason the kernel ext4 driver is
> missing flags that are in e2fsprogs. (huh??)
It's no different than ext2 not having the full set of bits defined or
in use.
> I would say you could probably just take over the flag because the 2010s
> called and they don't want next3 back. I guess that leaves 0x02000000
> as the sole unclaimed bit, but this seriously needs some cleaning.
Darrick,
we are in the process of updating the snapshot code for ext4, so need to
keep the 0x01000000 bit for snapshots. Since 0x02000000 has never been
used for anything, there is no reason not to use it instead.
If we need to reclaim flags, it would be better to look at "COMPR" flags:
/* Reserved for compression usage... */
#define FS_COMPR_FL 0x00000004 /* Compress file */
#define FS_DIRTY_FL 0x00000100
#define FS_COMPRBLK_FL 0x00000200 /* One or more compressed clusters */
#define FS_NOCOMP_FL 0x00000400 /* Don't compress */
since I don't think they have ever been used. I don't think we need 4x
on-disk state flags for that, especially not as part of the API. It is
enough to have FS_COMPR_FL for the API, and then handle internal state
separately (e.g. compress into a separate on-disk extent and then swap
extents atomically instead of storing transient state on disk).
Cheers, Andreas
On Wed 20-05-20 12:40:50, Ira Weiny wrote:
> On Wed, May 20, 2020 at 03:37:28PM +0200, Jan Kara wrote:
> > On Tue 19-05-20 22:57:49, [email protected] wrote:
> > > From: Ira Weiny <[email protected]>
> > >
> > > S_DAX should only be enabled when the underlying block device supports
> > > dax.
> > >
> > > Change ext4_should_use_dax() to check for device support prior to the
> > > over riding mount option.
> > >
> > > While we are at it change the function to ext4_should_enable_dax() as
> > > this better reflects the ask as well as matches xfs.
> > >
> > > Reviewed-by: Jan Kara <[email protected]>
> > > Signed-off-by: Ira Weiny <[email protected]>
> >
> > ...
> >
> > > @@ -4412,7 +4410,13 @@ static bool ext4_should_use_dax(struct inode *inode)
> > > return false;
> > > if (ext4_test_inode_flag(inode, EXT4_INODE_VERITY))
> > > return false;
> > > - return true;
> > > + if (!bdev_dax_supported(inode->i_sb->s_bdev,
> > > + inode->i_sb->s_blocksize))
> > > + return false;
> > > + if (test_opt(inode->i_sb, DAX_ALWAYS))
> > > + return true;
> > > +
> > > + return false;
> > > }
> >
> > Now that I think about it - shouldn't we rather cache the result of
> > bdev_dax_supported() in sb on mount and then just check the flag here?
> > Because bdev_dax_supported() isn't exactly cheap (it does a lot of checks
> > and mappings, tries to read from the pmem, ...).
>
> Sounds reasonable.
>
> Not sure which flags are appropriate. So add it here?
Yes, sounds good. Thanks!
Honza
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 1a3daf2d18ef..0b4db9ce7756 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1979,6 +1979,7 @@ static inline bool ext4_has_incompat_features(struct super_block *sb)
> */
> #define EXT4_FLAGS_RESIZING 0
> #define EXT4_FLAGS_SHUTDOWN 1
> +#define EXT4_FLAGS_BDEV_IS_DAX 2
>
> static inline int ext4_forced_shutdown(struct ext4_sb_info *sbi)
> {
>
--
Jan Kara <[email protected]>
SUSE Labs, CR