ext4: Add atomic writes support for DIO
Subsystems (2)
Ritesh Harjani (IBM)
reviewerv2 cover ext4: Add atomic writes support for DIO
This is v2 of atomic write enablement on ext4 for DIO. I have split this series as discussed in v1 [1], so this only enables atomic writes to a single fsblock. That means for now this gets only enabled on bs < ps systems on ext4. Enablement of atomic writes for bigalloc (multi-fsblock support) is still under discussion and may require general consensus within the filesystem community [1].
This series adds the base feature support to enable atomic writes in direct-io path for ext4. We advertise the minimum and the maximum atomic write unit sizes via statx on a regular file.
This series allows users to utilize atomic write support using -
1. on bs < ps systems via - mkfs.ext4 -F -b 16384 /dev/sda
This can then be utilized using -
xfs_io -fdc "pwrite -V 1 -A -b16k 0 16k" /mnt/f1
This is built on top of John's DIO atomic write series for XFS [2]. The VFS and block layer enablement for atomic writes were merged already.
[1]: https://lore.kernel.org/linux-ext4/[email protected]
[2]: https://lore.kernel.org/linux-xfs/[email protected]/
Changelogs:
===========
PATCH -> PATCH v2:
- addressed review comments from John and Darrick.
- renamed ext4_sb_info variables names: fs_awu* -> s_awu*
- [PATCH]: https://lore.kernel.org/linux-ext4/[email protected]/
RFC -> PATCH:
- Dropped RFC tag
- Last RFC was posted a while ago but back then a lot of VFS and block layer interfaces were still not merged. Those are now merged, thanks to John and everyone else.
- [RFC] - https://lore.kernel.org/linux-ext4/[email protected]/
Ritesh Harjani (IBM) (4):
ext4: Add statx support for atomic writes
ext4: Check for atomic writes support in write iter
ext4: Support setting FMODE_CAN_ATOMIC_WRITE
ext4: Do not fallback to buffered-io for DIO atomic write
fs/ext4/ext4.h | 9 +++++++++ fs/ext4/file.c | 29 ++++++++++++++++++++++++++++- fs/ext4/inode.c | 14 ++++++++++++++ fs/ext4/super.c | 31 +++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 1 deletion(-)
--
2.46.0
Ritesh Harjani (IBM)
reviewerPATCH v2 1/4 ext4: Add statx support for atomic writes
This patch adds base support for atomic writes via statx getattr. On bs < ps systems, we can create FS with say bs of 16k. That means both atomic write min and max unit can be set to 16k for supporting atomic writes.
--- fs/ext4/ext4.h | 9 +++++++++ fs/ext4/inode.c | 14 ++++++++++++++ fs/ext4/super.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+)
fs/ext4/ext4.h +9 -0
@@ -1729,6 +1729,10 @@ struct ext4_sb_info {*/ struct work_struct s_sb_upd_work; +/* Atomic write unit values in bytes */ +unsigned int s_awu_min; +unsigned int s_awu_max; +/* Ext4 fast commit sub transaction ID */ atomic_t s_fc_subtid;
@@ -3855,6 +3859,11 @@ static inline int ext4_buffer_uptodate(struct buffer_head *bh)return buffer_uptodate (bh); } +static inline bool ext4_can_atomic_write (struct super_block *sb) +{ +return EXT4_SB (sb)->s_awu_min >0 ; +} +extern int ext4_block_write_begin (handle_t *handle,struct folio *folio,loff_t pos,unsigned len,get_block_t *get_block);
fs/ext4/inode.c +14 -0
@@ -5578,6 +5578,20 @@ int ext4_getattr(struct mnt_idmap *idmap, const struct path *path, } } +if (S_ISREG (inode->i_mode) && (request_mask &STATX_WRITE_ATOMIC )) { +struct ext4_sb_info *sbi =EXT4_SB (inode->i_sb); +unsigned int awu_min, awu_max; + +if (ext4_can_atomic_write (inode->i_sb)) { + awu_min = sbi->s_awu_min; + awu_max = sbi->s_awu_max; + }else { + awu_min = awu_max =0 ; + } + +generic_fill_statx_atomic_writes (stat, awu_min, awu_max); + } + flags = ei->i_flags &EXT4_FL_USER_VISIBLE ;if (flags &EXT4_APPEND_FL ) stat->attributes |=STATX_ATTR_APPEND ;
fs/ext4/super.c +31 -0
@@ -4425,6 +4425,36 @@ static int ext4_handle_clustersize(struct super_block *sb)return 0 ; } +/* +* ext4_atomic_write_init: Initializes filesystem min & max atomic write units. +* @sb: super block +* TODO: Later add support for bigalloc +*/ +static void ext4_atomic_write_init (struct super_block *sb) +{ +struct ext4_sb_info *sbi =EXT4_SB (sb); +struct block_device *bdev = sb->s_bdev; + +if (!bdev_can_atomic_write (bdev)) +return ; + +if (!ext4_has_feature_extents (sb)) +return ; + + sbi->s_awu_min =max (sb->s_blocksize, +bdev_atomic_write_unit_min_bytes (bdev)); + sbi->s_awu_max =min (sb->s_blocksize, +bdev_atomic_write_unit_max_bytes (bdev)); +if (sbi->s_awu_min && sbi->s_awu_max && + sbi->s_awu_min <= sbi->s_awu_max) { +ext4_msg (sb,KERN_NOTICE ,"Supports atomic writes awu_min: %u, awu_max: %u" , + sbi->s_awu_min, sbi->s_awu_max); + }else { + sbi->s_awu_min =0 ; + sbi->s_awu_max =0 ; + } +} +static void ext4_fast_commit_init (struct super_block *sb) {struct ext4_sb_info *sbi =EXT4_SB (sb);
@@ -5336,6 +5366,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)spin_lock_init (&sbi->s_bdev_wb_lock); +ext4_atomic_write_init (sb);ext4_fast_commit_init (sb); sb->s_root =NULL ;
Signature · 1 line
2.46.0
Ritesh Harjani (IBM)
reviewerPATCH v2 2/4 ext4: Check for atomic writes support in write iter
In reply to Ritesh Harjani (IBM)Let's validate the given constraints for atomic write request. Otherwise it will fail with -EINVAL. Currently atomic write is only supported on DIO, so for buffered-io it will return -EOPNOTSUPP.
--- fs/ext4/file.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
fs/ext4/file.c +14 -0
@@ -692,6 +692,20 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)if (IS_DAX (inode))return ext4_dax_write_iter (iocb, from);#endif + +if (iocb->ki_flags &IOCB_ATOMIC ) { +size_t len =iov_iter_count (from); +int ret; + +if (len <EXT4_SB (inode->i_sb)->s_awu_min || + len >EXT4_SB (inode->i_sb)->s_awu_max) +return -EINVAL ; + + ret =generic_atomic_write_valid (iocb, from); +if (ret) +return ret; + } +if (iocb->ki_flags &IOCB_DIRECT )return ext4_dio_write_iter (iocb, from);else
Signature · 1 line
2.46.0
Ritesh Harjani (IBM)
reviewerPATCH v2 3/4 ext4: Support setting FMODE_CAN_ATOMIC_WRITE
In reply to Ritesh Harjani (IBM)FS needs to add the fmode capability in order to support atomic writes during file open (refer kiocb_set_rw_flags()). Set this capability on a regular file if ext4 can do atomic write.
--- fs/ext4/file.c | 3 +++ 1 file changed, 3 insertions(+)
fs/ext4/file.c +3 -0
@@ -898,6 +898,9 @@ static int ext4_file_open(struct inode *inode, struct file *filp)return ret; } +if (S_ISREG (inode->i_mode) &&ext4_can_atomic_write (inode->i_sb)) + filp->f_mode |=FMODE_CAN_ATOMIC_WRITE ; + filp->f_mode |=FMODE_NOWAIT |FMODE_CAN_ODIRECT ;return dquot_file_open (inode, filp); }
Signature · 1 line
2.46.0
Ritesh Harjani (IBM)
reviewerPATCH v2 4/4 ext4: Do not fallback to buffered-io for DIO atomic write
In reply to Ritesh Harjani (IBM)iomap can return -ENOTBLK if pagecache invalidation fails. Let's make sure if -ENOTBLK is ever returned for atomic writes than we fail the write request (-EIO) instead of fallback to buffered-io.
--- fs/ext4/file.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
fs/ext4/file.c +11 -1
@@ -576,8 +576,18 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from) iomap_ops = &ext4_iomap_overwrite_ops; ret =iomap_dio_rw (iocb, from, iomap_ops, &ext4_dio_write_ops, dio_flags,NULL ,0 ); -if (ret == -ENOTBLK ) +if (ret == -ENOTBLK ) { ret =0 ; +/* +* iomap can return -ENOTBLK if pagecache invalidation fails. +* Let's make sure if -ENOTBLK is ever returned for atomic +* writes than we fail the write request instead of fallback +* to buffered-io. +*/ +if (iocb->ki_flags &IOCB_ATOMIC ) + ret = -EIO ; + } +if (extend) {/* * We always perform extending DIO write synchronously so by
--
2.46.0
Re: [PATCH v2 1/4] ext4: Add statx support for atomic writes
In reply to Ritesh Harjani (IBM)Quoted 75 lines · Ritesh Harjani (IBM)
This patch adds base support for atomic writes via statx getattr. On bs < ps systems, we can create FS with say bs of 16k. That means both atomic write min and max unit can be set to 16k for supporting atomic writes.
Co-developed-by Ojaswin Mujoo <[email protected]>Signed-off-by Ojaswin Mujoo <[email protected]>Signed-off-by Ritesh Harjani (IBM) <[email protected]>--- fs/ext4/ext4.h | 9 +++++++++ fs/ext4/inode.c | 14 ++++++++++++++ fs/ext4/super.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+)fs/ext4/ext4.h +9
-0@@ -1729,6 +1729,10 @@ struct ext4_sb_info {*/ struct work_struct s_sb_upd_work; +/* Atomic write unit values in bytes */ +unsigned int s_awu_min; +unsigned int s_awu_max; +/* Ext4 fast commit sub transaction ID */ atomic_t s_fc_subtid;@@ -3855,6 +3859,11 @@ static inline int ext4_buffer_uptodate(struct buffer_head *bh)return buffer_uptodate (bh); } +static inline bool ext4_can_atomic_write (struct super_block *sb) +{ +return EXT4_SB (sb)->s_awu_min >0 ; +} +extern int ext4_block_write_begin (handle_t *handle,struct folio *folio,loff_t pos,unsigned len,get_block_t *get_block);fs/ext4/inode.c +14
-0@@ -5578,6 +5578,20 @@ int ext4_getattr(struct mnt_idmap *idmap, const struct path *path, } } +if (S_ISREG (inode->i_mode) && (request_mask &STATX_WRITE_ATOMIC )) { +struct ext4_sb_info *sbi =EXT4_SB (inode->i_sb); +unsigned int awu_min, awu_max; + +if (ext4_can_atomic_write (inode->i_sb)) { + awu_min = sbi->s_awu_min; + awu_max = sbi->s_awu_max; + }else { + awu_min = awu_max =0 ; + } + +generic_fill_statx_atomic_writes (stat, awu_min, awu_max); + } + flags = ei->i_flags &EXT4_FL_USER_VISIBLE ;if (flags &EXT4_APPEND_FL ) stat->attributes |=STATX_ATTR_APPEND ;fs/ext4/super.c +1
-0@@ -4425,6 +4425,36 @@ static int ext4_handle_clustersize(struct super_block *sb)return 0 ; } +/*
fs/ext4/super.c static int ext4_handle_clustersize(struct super_block *sb) · line 4438
Quoted 10 lines
fs/ext4/super.c +10
-0+* ext4_atomic_write_init: Initializes filesystem min & max atomic write units. +* @sb: super block +* TODO: Later add support for bigalloc +*/ +static void ext4_atomic_write_init (struct super_block *sb) +{ +struct ext4_sb_info *sbi =EXT4_SB (sb); +struct block_device *bdev = sb->s_bdev; + +if (!bdev_can_atomic_write (bdev))
this check is duplicated, since bdev_atomic_write_unit_{min, max}_bytes() has this check
fs/ext4/super.c static int ext4_handle_clustersize(struct super_block *sb) · line 4449
Quoted 11 lines
fs/ext4/super.c +11
-0+return ; + +if (!ext4_has_feature_extents (sb)) +return ; + + sbi->s_awu_min =max (sb->s_blocksize, +bdev_atomic_write_unit_min_bytes (bdev)); + sbi->s_awu_max =min (sb->s_blocksize, +bdev_atomic_write_unit_max_bytes (bdev)); +if (sbi->s_awu_min && sbi->s_awu_max && + sbi->s_awu_min <= sbi->s_awu_max) {
This looks a bit complicated. I would just follow the XFS example and ensure bdev_atomic_write_unit_min_bytes() <= sb->s_blocksize <= bdev_atomic_write_unit_max_bytes() [which you are doing, but in a complicated way]
Quoted 19 lines
fs/ext4/super.c +9
-0+ext4_msg (sb,KERN_NOTICE ,"Supports atomic writes awu_min: %u, awu_max: %u" , + sbi->s_awu_min, sbi->s_awu_max); + }else { + sbi->s_awu_min =0 ; + sbi->s_awu_max =0 ; + } +} +static void ext4_fast_commit_init (struct super_block *sb) {struct ext4_sb_info *sbi =EXT4_SB (sb);@@ -5336,6 +5366,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)spin_lock_init (&sbi->s_bdev_wb_lock); +ext4_atomic_write_init (sb);ext4_fast_commit_init (sb); sb->s_root =NULL ;
Re: [PATCH v2 2/4] ext4: Check for atomic writes support in write iter
In reply to Ritesh Harjani (IBM)Quoted 5 lines · Ritesh Harjani (IBM)
Let's validate the given constraints for atomic write request. Otherwise it will fail with
-EINVAL. Currently atomic write is only supported on DIO, so for buffered-io it will return-EOPNOTSUPP.Signed-off-by Ritesh Harjani (IBM)<[email protected]>
Re: [PATCH v2 3/4] ext4: Support setting FMODE_CAN_ATOMIC_WRITE
In reply to Ritesh Harjani (IBM)Quoted 5 lines · Ritesh Harjani (IBM)
FS needs to add the fmode capability in order to support atomic writes during file open (refer
kiocb_set_rw_flags()). Set this capability on a regular file if ext4 can do atomic write.Signed-off-by Ritesh Harjani (IBM)<[email protected]>
Re: [PATCH v2 4/4] ext4: Do not fallback to buffered-io for DIO atomic write
In reply to Ritesh Harjani (IBM)Quoted 7 lines · Ritesh Harjani (IBM)
iomap can return
-ENOTBLKif pagecache invalidation fails. Let's make sure if-ENOTBLKis ever returned for atomic writes than we fail the write request (-EIO) instead of fallback to buffered-io.Reviewed-by Darrick J. Wong <[email protected]>Signed-off-by Ritesh Harjani (IBM) <[email protected]>
I am not sure if you plan on dropping this patch...
Quoted 31 lines
--- fs/ext4/file.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)fs/ext4/file.c +11
-2@@ -576,8 +576,18 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from) iomap_ops = &ext4_iomap_overwrite_ops; ret =iomap_dio_rw (iocb, from, iomap_ops, &ext4_dio_write_ops, dio_flags,NULL ,0 ); -if (ret == -ENOTBLK ) +if (ret == -ENOTBLK ) { ret =0 ; +/* +* iomap can return -ENOTBLK if pagecache invalidation fails. +* Let's make sure if -ENOTBLK is ever returned for atomic +* writes than we fail the write request instead of fallback +* to buffered-io. +*/ +if (iocb->ki_flags &IOCB_ATOMIC ) + ret = -EIO ; + } +if (extend) {/* * We always perform extending DIO write synchronously so by -- 2.46.0
Ritesh Harjani (IBM)
reviewerRe: [PATCH v2 4/4] ext4: Do not fallback to buffered-io for DIO atomic write
Quoted 11 lines · John Garry
Quoted 7 lines · Ritesh Harjani (IBM)
iomap can return
-ENOTBLKif pagecache invalidation fails. Let's make sure if-ENOTBLKis ever returned for atomic writes than we fail the write request (-EIO) instead of fallback to buffered-io.Reviewed-by Darrick J. Wong <[email protected]>Signed-off-by Ritesh Harjani (IBM) <[email protected]>I am not sure if you plan on dropping this patch...
As discussed in the other thread, EXT4 has got a fallback to buffered-io logic at certain places including during short DIO writes unlike XFS which never fallsback to buffered-io in case of short DIO writes. Let me send another patch for this one.
Thanks for the review.
-ritesh
Ritesh Harjani (IBM)
reviewerRe: [PATCH v2 1/4] ext4: Add statx support for atomic writes
In reply to John GarryHi John,
Quoted 75 lines · John Garry
Quoted 74 lines · Ritesh Harjani (IBM)
This patch adds base support for atomic writes via statx getattr. On bs < ps systems, we can create FS with say bs of 16k. That means both atomic write min and max unit can be set to 16k for supporting atomic writes.
Co-developed-by Ojaswin Mujoo <[email protected]>Signed-off-by Ojaswin Mujoo <[email protected]>Signed-off-by Ritesh Harjani (IBM) <[email protected]>--- fs/ext4/ext4.h | 9 +++++++++ fs/ext4/inode.c | 14 ++++++++++++++ fs/ext4/super.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+)fs/ext4/ext4.h +9
-0@@ -1729,6 +1729,10 @@ struct ext4_sb_info {*/ struct work_struct s_sb_upd_work; +/* Atomic write unit values in bytes */ +unsigned int s_awu_min; +unsigned int s_awu_max; +/* Ext4 fast commit sub transaction ID */ atomic_t s_fc_subtid;@@ -3855,6 +3859,11 @@ static inline int ext4_buffer_uptodate(struct buffer_head *bh)return buffer_uptodate (bh); } +static inline bool ext4_can_atomic_write (struct super_block *sb) +{ +return EXT4_SB (sb)->s_awu_min >0 ; +} +extern int ext4_block_write_begin (handle_t *handle,struct folio *folio,loff_t pos,unsigned len,get_block_t *get_block);fs/ext4/inode.c +14
-0@@ -5578,6 +5578,20 @@ int ext4_getattr(struct mnt_idmap *idmap, const struct path *path, } } +if (S_ISREG (inode->i_mode) && (request_mask &STATX_WRITE_ATOMIC )) { +struct ext4_sb_info *sbi =EXT4_SB (inode->i_sb); +unsigned int awu_min, awu_max; + +if (ext4_can_atomic_write (inode->i_sb)) { + awu_min = sbi->s_awu_min; + awu_max = sbi->s_awu_max; + }else { + awu_min = awu_max =0 ; + } + +generic_fill_statx_atomic_writes (stat, awu_min, awu_max); + } + flags = ei->i_flags &EXT4_FL_USER_VISIBLE ;if (flags &EXT4_APPEND_FL ) stat->attributes |=STATX_ATTR_APPEND ;fs/ext4/super.c +0
-0@@ -4425,6 +4425,36 @@ static int ext4_handle_clustersize(struct super_block *sb)return 0 ; }
Quoted 15 lines
fs/ext4/super.c static int ext4_handle_clustersize(struct super_block *sb) · line 4438
Quoted 11 lines
fs/ext4/super.c +11
-0+/* +* ext4_atomic_write_init: Initializes filesystem min & max atomic write units. +* @sb: super block +* TODO: Later add support for bigalloc +*/ +static void ext4_atomic_write_init (struct super_block *sb) +{ +struct ext4_sb_info *sbi =EXT4_SB (sb); +struct block_device *bdev = sb->s_bdev; + +if (!bdev_can_atomic_write (bdev))this check is duplicated, since bdev_atomic_write_unit_{min, max}_bytes() has this check
Right, yes. I can mention a comment and remove this check perhaps. Looks like XFS also got it duplicated then.
Quoted 8 lines
Quoted 8 lines
fs/ext4/super.c +8
-0+return ; + +if (!ext4_has_feature_extents (sb)) +return ; + + sbi->s_awu_min =max (sb->s_blocksize, +bdev_atomic_write_unit_min_bytes (bdev)); + sbi->s_awu_max = min(sb->s_blocksize,
Quoted 9 lines
fs/ext4/super.c static int ext4_handle_clustersize(struct super_block *sb) · line 4449
Quoted 3 lines
fs/ext4/super.c +3
-0+ bdev_atomic_write_unit_max_bytes(bdev)); +if (sbi->s_awu_min && sbi->s_awu_max && + sbi->s_awu_min <= sbi->s_awu_max) {This looks a bit complicated. I would just follow the XFS example and ensure
bdev_atomic_write_unit_min_bytes()<=sb->s_blocksize<=bdev_atomic_write_unit_max_bytes()[which you are doing, but in a complicated way]
In here we are checking for min and max supported units against fs blocksize at one place during mount time itself and then caching the supported atomic write unit. The supported atomic write unit can change when bigalloc gets introduced.
XFS caches the blockdevice min, max units and then defers these checks against fs blocksize during file open time using xfs_inode_can_atomicwrite().
I just preferrd the 1st aproach in case of EXT4 here because it will be simpler this way for when bigalloc also gets introduced.
BTW none of these are ondisk changes, so whenever extsize gets introduced, we might still have to add something like ext4_inode_can_atomic_write() (similar to XFS) based on inode extsize value. But for now it was not needed in EXT4.
I hope the reasoning is clear and make sense.
Quoted 2 lines
fs/ext4/super.c static int ext4_handle_clustersize(struct super_block *sb) · line 4451
Quoted 2 lines
fs/ext4/super.c +2
-0+ext4_msg (sb,KERN_NOTICE ,"Supports atomic writes awu_min: %u, awu_max: %u" , + sbi->s_awu_min, sbi->s_awu_max);
BTW - I was wondering if we should add "experimental" in above msg.
-ritesh
Quoted 17 lines
Quoted 17 lines
fs/ext4/super.c +7
-0+ }else { + sbi->s_awu_min =0 ; + sbi->s_awu_max =0 ; + } +} +static void ext4_fast_commit_init (struct super_block *sb) {struct ext4_sb_info *sbi =EXT4_SB (sb);@@ -5336,6 +5366,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)spin_lock_init (&sbi->s_bdev_wb_lock); +ext4_atomic_write_init (sb);ext4_fast_commit_init (sb); sb->s_root =NULL ;
Ritesh Harjani (IBM)
reviewerRe: [PATCH v2 1/4] ext4: Add statx support for atomic writes
Quoted 95 lines · Ritesh Harjani (IBM)
Hi John,
Quoted 75 lines · John Garry
Quoted 74 lines · Ritesh Harjani (IBM)
This patch adds base support for atomic writes via statx getattr. On bs < ps systems, we can create FS with say bs of 16k. That means both atomic write min and max unit can be set to 16k for supporting atomic writes.
Co-developed-by Ojaswin Mujoo <[email protected]>Signed-off-by Ojaswin Mujoo <[email protected]>Signed-off-by Ritesh Harjani (IBM) <[email protected]>--- fs/ext4/ext4.h | 9 +++++++++ fs/ext4/inode.c | 14 ++++++++++++++ fs/ext4/super.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+)fs/ext4/ext4.h +9
-0@@ -1729,6 +1729,10 @@ struct ext4_sb_info {*/ struct work_struct s_sb_upd_work; +/* Atomic write unit values in bytes */ +unsigned int s_awu_min; +unsigned int s_awu_max; +/* Ext4 fast commit sub transaction ID */ atomic_t s_fc_subtid;@@ -3855,6 +3859,11 @@ static inline int ext4_buffer_uptodate(struct buffer_head *bh)return buffer_uptodate (bh); } +static inline bool ext4_can_atomic_write (struct super_block *sb) +{ +return EXT4_SB (sb)->s_awu_min >0 ; +} +extern int ext4_block_write_begin (handle_t *handle,struct folio *folio,loff_t pos,unsigned len,get_block_t *get_block);fs/ext4/inode.c +14
-0@@ -5578,6 +5578,20 @@ int ext4_getattr(struct mnt_idmap *idmap, const struct path *path, } } +if (S_ISREG (inode->i_mode) && (request_mask &STATX_WRITE_ATOMIC )) { +struct ext4_sb_info *sbi =EXT4_SB (inode->i_sb); +unsigned int awu_min, awu_max; + +if (ext4_can_atomic_write (inode->i_sb)) { + awu_min = sbi->s_awu_min; + awu_max = sbi->s_awu_max; + }else { + awu_min = awu_max =0 ; + } + +generic_fill_statx_atomic_writes (stat, awu_min, awu_max); + } + flags = ei->i_flags &EXT4_FL_USER_VISIBLE ;if (flags &EXT4_APPEND_FL ) stat->attributes |=STATX_ATTR_APPEND ;fs/ext4/super.c +0
-0@@ -4425,6 +4425,36 @@ static int ext4_handle_clustersize(struct super_block *sb)return 0 ; }Quoted 15 lines
fs/ext4/super.c static int ext4_handle_clustersize(struct super_block *sb) · line 4438
Quoted 11 lines
fs/ext4/super.c +11
-0+/* +* ext4_atomic_write_init: Initializes filesystem min & max atomic write units. +* @sb: super block +* TODO: Later add support for bigalloc +*/ +static void ext4_atomic_write_init (struct super_block *sb) +{ +struct ext4_sb_info *sbi =EXT4_SB (sb); +struct block_device *bdev = sb->s_bdev; + +if (!bdev_can_atomic_write (bdev))this check is duplicated, since bdev_atomic_write_unit_{min, max}_bytes() has this check
Quoted 4 lines
Right, yes. I can mention a comment and remove this check perhaps. Looks like XFS also got it duplicated then.
If we remove above bdev check logic, than we are relying on the min max comparison in below code to decide whether we support atomic writes or not. It's rather cleaner and straight forward to check if bdev supports atomic write in the beginning itself.
So I feel it's better to keep the above check as is since it's only once during mount.
(Note - bdev_can_atomic_write() either ways gets called twice when bdev_atomic_write_unit_min|max_bytes() is getting called).
So I would like to keep the check as is please.
Thanks for the review!
-ritesh
Quoted 61 lines
Quoted 8 lines
Quoted 8 lines
fs/ext4/super.c +8
-0+return ; + +if (!ext4_has_feature_extents (sb)) +return ; + + sbi->s_awu_min =max (sb->s_blocksize, +bdev_atomic_write_unit_min_bytes (bdev)); + sbi->s_awu_max = min(sb->s_blocksize,Quoted 9 lines
fs/ext4/super.c static int ext4_handle_clustersize(struct super_block *sb) · line 4449
Quoted 3 lines
fs/ext4/super.c +3
-0+ bdev_atomic_write_unit_max_bytes(bdev)); +if (sbi->s_awu_min && sbi->s_awu_max && + sbi->s_awu_min <= sbi->s_awu_max) {This looks a bit complicated. I would just follow the XFS example and ensure
bdev_atomic_write_unit_min_bytes()<=sb->s_blocksize<=bdev_atomic_write_unit_max_bytes()[which you are doing, but in a complicated way]In here we are checking for min and max supported units against fs blocksize at one place during mount time itself and then caching the supported atomic write unit. The supported atomic write unit can change when bigalloc gets introduced.
XFS caches the blockdevice min, max units and then defers these checks against fs blocksize during file open time using
xfs_inode_can_atomicwrite().I just preferrd the 1st aproach in case of EXT4 here because it will be simpler this way for when bigalloc also gets introduced.
BTW none of these are ondisk changes, so whenever extsize gets introduced, we might still have to add something like
ext4_inode_can_atomic_write()(similar to XFS) based on inode extsize value. But for now it was not needed in EXT4.I hope the reasoning is clear and make sense.
Quoted 2 lines
fs/ext4/super.c static int ext4_handle_clustersize(struct super_block *sb) · line 4451
Quoted 2 lines
fs/ext4/super.c +2
-0+ext4_msg (sb,KERN_NOTICE ,"Supports atomic writes awu_min: %u, awu_max: %u" , + sbi->s_awu_min, sbi->s_awu_max);BTW - I was wondering if we should add "experimental" in above msg.
-ritesh
Quoted 17 lines
Quoted 17 lines
fs/ext4/super.c +7
-0+ }else { + sbi->s_awu_min =0 ; + sbi->s_awu_max =0 ; + } +} +static void ext4_fast_commit_init (struct super_block *sb) {struct ext4_sb_info *sbi =EXT4_SB (sb);@@ -5336,6 +5366,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)spin_lock_init (&sbi->s_bdev_wb_lock); +ext4_atomic_write_init (sb);ext4_fast_commit_init (sb); sb->s_root =NULL ;