| Message ID | e6af669b237690491ecff0717039e28e949208c8.1729825985.git.ritesh.list@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ext4: Add atomic write support for DIO | expand |
On 25/10/2024 04:45, Ritesh Harjani (IBM) wrote: > 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. > > Later patches adds support for bigalloc as well so that ext4 can also > support doing atomic writes for bs = ps systems. > > Co-developed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > --- > fs/ext4/ext4.h | 7 ++++++- > fs/ext4/inode.c | 14 ++++++++++++++ > fs/ext4/super.c | 32 ++++++++++++++++++++++++++++++++ > 3 files changed, 52 insertions(+), 1 deletion(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 44b0d418143c..a41e56c2c628 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -1729,6 +1729,10 @@ struct ext4_sb_info { > */ > struct work_struct s_sb_upd_work; > > + /* Atomic write unit values */ > + unsigned int fs_awu_min; > + unsigned int fs_awu_max; > + > /* Ext4 fast commit sub transaction ID */ > atomic_t s_fc_subtid; > > @@ -1820,7 +1824,8 @@ static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino) > */ > enum { > EXT4_MF_MNTDIR_SAMPLED, > - EXT4_MF_FC_INELIGIBLE /* Fast commit ineligible */ > + EXT4_MF_FC_INELIGIBLE, /* Fast commit ineligible */ > + EXT4_MF_ATOMIC_WRITE /* Supports atomic write */ Does this flag really buy us much? > }; > > static inline void ext4_set_mount_flag(struct super_block *sb, int bit) > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 54bdd4884fe6..897c028d5bc9 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -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_test_mount_flag(inode->i_sb, EXT4_MF_ATOMIC_WRITE)) { I'd use ext4_inode_can_atomicwrite() here, similar to what is done for xfs > + awu_min = sbi->fs_awu_min; > + awu_max = sbi->fs_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; > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 16a4ce704460..f5c075aff060 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -4425,6 +4425,37 @@ static int ext4_handle_clustersize(struct super_block *sb) > return 0; > } > > +/*
John Garry <john.g.garry@oracle.com> writes: > On 25/10/2024 04:45, Ritesh Harjani (IBM) wrote: >> 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. >> >> Later patches adds support for bigalloc as well so that ext4 can also >> support doing atomic writes for bs = ps systems. >> >> Co-developed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> >> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> >> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> >> --- >> fs/ext4/ext4.h | 7 ++++++- >> fs/ext4/inode.c | 14 ++++++++++++++ >> fs/ext4/super.c | 32 ++++++++++++++++++++++++++++++++ >> 3 files changed, 52 insertions(+), 1 deletion(-) >> >> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h >> index 44b0d418143c..a41e56c2c628 100644 >> --- a/fs/ext4/ext4.h >> +++ b/fs/ext4/ext4.h >> @@ -1729,6 +1729,10 @@ struct ext4_sb_info { >> */ >> struct work_struct s_sb_upd_work; >> >> + /* Atomic write unit values */ >> + unsigned int fs_awu_min; >> + unsigned int fs_awu_max; >> + >> /* Ext4 fast commit sub transaction ID */ >> atomic_t s_fc_subtid; >> >> @@ -1820,7 +1824,8 @@ static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino) >> */ >> enum { >> EXT4_MF_MNTDIR_SAMPLED, >> - EXT4_MF_FC_INELIGIBLE /* Fast commit ineligible */ >> + EXT4_MF_FC_INELIGIBLE, /* Fast commit ineligible */ >> + EXT4_MF_ATOMIC_WRITE /* Supports atomic write */ > > Does this flag really buy us much? > I felt it is cleaner this way than comparing non-zero values of fs_awu_min and fs_awu_max. Now that you pointed at it - Maybe a question for others who might have the history of which one to use when - or do we think there is a scope of merging the two into just one as a later cleanup? I know that s_mount_flags was added for fastcommit and it needed the state manipulations to be done in atomic way. Similarly s_ext4_flags also was renamed from s_resize_flags for more general purpose use. Both of these looks like could be merged isn't it? >> }; >> >> static inline void ext4_set_mount_flag(struct super_block *sb, int bit) >> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c >> index 54bdd4884fe6..897c028d5bc9 100644 >> --- a/fs/ext4/inode.c >> +++ b/fs/ext4/inode.c >> @@ -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_test_mount_flag(inode->i_sb, EXT4_MF_ATOMIC_WRITE)) { > > I'd use ext4_inode_can_atomicwrite() here, similar to what is done for xfs > Sure since it is inode operation, we can check against ext4_inode_can_atomicwrite(). >> + awu_min = sbi->fs_awu_min; >> + awu_max = sbi->fs_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; >> diff --git a/fs/ext4/super.c b/fs/ext4/super.c >> index 16a4ce704460..f5c075aff060 100644 >> --- a/fs/ext4/super.c >> +++ b/fs/ext4/super.c >> @@ -4425,6 +4425,37 @@ static int ext4_handle_clustersize(struct super_block *sb) >> return 0; >> } >> >> +/*
On Fri, Oct 25, 2024 at 03:38:03PM +0530, Ritesh Harjani wrote: > John Garry <john.g.garry@oracle.com> writes: > > > On 25/10/2024 04:45, Ritesh Harjani (IBM) wrote: > >> 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. > >> > >> Later patches adds support for bigalloc as well so that ext4 can also > >> support doing atomic writes for bs = ps systems. > >> > >> Co-developed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> > >> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> > >> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > >> --- > >> fs/ext4/ext4.h | 7 ++++++- > >> fs/ext4/inode.c | 14 ++++++++++++++ > >> fs/ext4/super.c | 32 ++++++++++++++++++++++++++++++++ > >> 3 files changed, 52 insertions(+), 1 deletion(-) > >> > >> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > >> index 44b0d418143c..a41e56c2c628 100644 > >> --- a/fs/ext4/ext4.h > >> +++ b/fs/ext4/ext4.h > >> @@ -1729,6 +1729,10 @@ struct ext4_sb_info { > >> */ > >> struct work_struct s_sb_upd_work; > >> > >> + /* Atomic write unit values */ > >> + unsigned int fs_awu_min; > >> + unsigned int fs_awu_max; > >> + > >> /* Ext4 fast commit sub transaction ID */ > >> atomic_t s_fc_subtid; > >> > >> @@ -1820,7 +1824,8 @@ static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino) > >> */ > >> enum { > >> EXT4_MF_MNTDIR_SAMPLED, > >> - EXT4_MF_FC_INELIGIBLE /* Fast commit ineligible */ > >> + EXT4_MF_FC_INELIGIBLE, /* Fast commit ineligible */ > >> + EXT4_MF_ATOMIC_WRITE /* Supports atomic write */ > > > > Does this flag really buy us much? > > > > I felt it is cleaner this way than comparing non-zero values of > fs_awu_min and fs_awu_max. What does it mean when MF_ATOMIC_WRITE is set and fs_awu_* are zero? The awu values don't change at runtime, so I think you can save yourself an atomic test by checking (non-atomically) for awu_min>0. (I don't know anything about the flags, those came after my time iirc.) --D > Now that you pointed at it - Maybe a question for others who might have > the history of which one to use when - or do we think there is a scope > of merging the two into just one as a later cleanup? > > I know that s_mount_flags was added for fastcommit and it needed the > state manipulations to be done in atomic way. Similarly s_ext4_flags > also was renamed from s_resize_flags for more general purpose use. Both > of these looks like could be merged isn't it? > > > > >> }; > >> > >> static inline void ext4_set_mount_flag(struct super_block *sb, int bit) > >> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > >> index 54bdd4884fe6..897c028d5bc9 100644 > >> --- a/fs/ext4/inode.c > >> +++ b/fs/ext4/inode.c > >> @@ -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_test_mount_flag(inode->i_sb, EXT4_MF_ATOMIC_WRITE)) { > > > > I'd use ext4_inode_can_atomicwrite() here, similar to what is done for xfs > > > > Sure since it is inode operation, we can check against ext4_inode_can_atomicwrite(). > > > >> + awu_min = sbi->fs_awu_min; > >> + awu_max = sbi->fs_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; > >> diff --git a/fs/ext4/super.c b/fs/ext4/super.c > >> index 16a4ce704460..f5c075aff060 100644 > >> --- a/fs/ext4/super.c > >> +++ b/fs/ext4/super.c > >> @@ -4425,6 +4425,37 @@ static int ext4_handle_clustersize(struct super_block *sb) > >> return 0; > >> } > >> > >> +/* >
"Darrick J. Wong" <djwong@kernel.org> writes: > On Fri, Oct 25, 2024 at 03:38:03PM +0530, Ritesh Harjani wrote: >> John Garry <john.g.garry@oracle.com> writes: >> >> > On 25/10/2024 04:45, Ritesh Harjani (IBM) wrote: >> >> 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. >> >> >> >> Later patches adds support for bigalloc as well so that ext4 can also >> >> support doing atomic writes for bs = ps systems. >> >> >> >> Co-developed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> >> >> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> >> >> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> >> >> --- >> >> fs/ext4/ext4.h | 7 ++++++- >> >> fs/ext4/inode.c | 14 ++++++++++++++ >> >> fs/ext4/super.c | 32 ++++++++++++++++++++++++++++++++ >> >> 3 files changed, 52 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h >> >> index 44b0d418143c..a41e56c2c628 100644 >> >> --- a/fs/ext4/ext4.h >> >> +++ b/fs/ext4/ext4.h >> >> @@ -1729,6 +1729,10 @@ struct ext4_sb_info { >> >> */ >> >> struct work_struct s_sb_upd_work; >> >> >> >> + /* Atomic write unit values */ >> >> + unsigned int fs_awu_min; >> >> + unsigned int fs_awu_max; >> >> + >> >> /* Ext4 fast commit sub transaction ID */ >> >> atomic_t s_fc_subtid; >> >> >> >> @@ -1820,7 +1824,8 @@ static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino) >> >> */ >> >> enum { >> >> EXT4_MF_MNTDIR_SAMPLED, >> >> - EXT4_MF_FC_INELIGIBLE /* Fast commit ineligible */ >> >> + EXT4_MF_FC_INELIGIBLE, /* Fast commit ineligible */ >> >> + EXT4_MF_ATOMIC_WRITE /* Supports atomic write */ >> > >> > Does this flag really buy us much? >> > >> >> I felt it is cleaner this way than comparing non-zero values of >> fs_awu_min and fs_awu_max. > > What does it mean when MF_ATOMIC_WRITE is set and fs_awu_* are zero? > The awu values don't change at runtime, so I think you can save yourself > an atomic test by checking (non-atomically) for awu_min>0. Sure. I agree with the reasoning that we can just check for awu_min > 0. I can write an inline helper for this. > > (I don't know anything about the flags, those came after my time iirc.) > Thanks for the review :) -ritesh > --D > >> Now that you pointed at it - Maybe a question for others who might have >> the history of which one to use when - or do we think there is a scope >> of merging the two into just one as a later cleanup? >> >> I know that s_mount_flags was added for fastcommit and it needed the >> state manipulations to be done in atomic way. Similarly s_ext4_flags >> also was renamed from s_resize_flags for more general purpose use. Both >> of these looks like could be merged isn't it? >> >> >> >> >> }; >> >> >> >> static inline void ext4_set_mount_flag(struct super_block *sb, int bit) >> >> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c >> >> index 54bdd4884fe6..897c028d5bc9 100644 >> >> --- a/fs/ext4/inode.c >> >> +++ b/fs/ext4/inode.c >> >> @@ -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_test_mount_flag(inode->i_sb, EXT4_MF_ATOMIC_WRITE)) { >> > >> > I'd use ext4_inode_can_atomicwrite() here, similar to what is done for xfs >> > >> >> Sure since it is inode operation, we can check against ext4_inode_can_atomicwrite(). >> >> >> >> + awu_min = sbi->fs_awu_min; >> >> + awu_max = sbi->fs_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; >> >> diff --git a/fs/ext4/super.c b/fs/ext4/super.c >> >> index 16a4ce704460..f5c075aff060 100644 >> >> --- a/fs/ext4/super.c >> >> +++ b/fs/ext4/super.c >> >> @@ -4425,6 +4425,37 @@ static int ext4_handle_clustersize(struct super_block *sb) >> >> return 0; >> >> } >> >> >> >> +/* >>
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 44b0d418143c..a41e56c2c628 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -1729,6 +1729,10 @@ struct ext4_sb_info { */ struct work_struct s_sb_upd_work; + /* Atomic write unit values */ + unsigned int fs_awu_min; + unsigned int fs_awu_max; + /* Ext4 fast commit sub transaction ID */ atomic_t s_fc_subtid; @@ -1820,7 +1824,8 @@ static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino) */ enum { EXT4_MF_MNTDIR_SAMPLED, - EXT4_MF_FC_INELIGIBLE /* Fast commit ineligible */ + EXT4_MF_FC_INELIGIBLE, /* Fast commit ineligible */ + EXT4_MF_ATOMIC_WRITE /* Supports atomic write */ }; static inline void ext4_set_mount_flag(struct super_block *sb, int bit) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 54bdd4884fe6..897c028d5bc9 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -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_test_mount_flag(inode->i_sb, EXT4_MF_ATOMIC_WRITE)) { + awu_min = sbi->fs_awu_min; + awu_max = sbi->fs_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; diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 16a4ce704460..f5c075aff060 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -4425,6 +4425,37 @@ 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->fs_awu_min = max(sb->s_blocksize, + bdev_atomic_write_unit_min_bytes(bdev)); + sbi->fs_awu_max = min(sb->s_blocksize, + bdev_atomic_write_unit_max_bytes(bdev)); + if (sbi->fs_awu_min && sbi->fs_awu_max && + sbi->fs_awu_min <= sbi->fs_awu_max) { + ext4_set_mount_flag(sb, EXT4_MF_ATOMIC_WRITE); + ext4_msg(sb, KERN_NOTICE, "Supports atomic writes awu_min: %u, awu_max: %u", + sbi->fs_awu_min, sbi->fs_awu_max); + } else { + sbi->fs_awu_min = 0; + sbi->fs_awu_max = 0; + } +} + static void ext4_fast_commit_init(struct super_block *sb) { struct ext4_sb_info *sbi = EXT4_SB(sb); @@ -5336,6 +5367,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;