| Message ID | 1602986547-15886-1-git-send-email-dotdot@yandex-team.ru |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] ext4: add helpers for checking whether quota can be enabled/is journalled | expand |
On Sun 18-10-20 05:02:26, Roman Anufriev wrote: > Right now, there are several places, where we check whether fs is > capable of enabling quota or if quota is journalled with quite long > and non-self-descriptive condition statements. > > This patch wraps these statements into helpers for better readability > and easier usage. > > Signed-off-by: Roman Anufriev <dotdot@yandex-team.ru> Looks good to me. You can add: Reviewe-by: Jan Kara <jack@suse.cz> Honza > --- > Changes in v2: > - Fix misleading helper name 'ext4_any_quota_enabled()' -> > 'ext4_quota_capable()'. > > fs/ext4/ext4.h | 15 +++++++++++++++ > fs/ext4/ext4_jbd2.h | 9 +++------ > fs/ext4/super.c | 5 +---- > 3 files changed, 19 insertions(+), 10 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 250e905..897df24 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -3251,6 +3251,21 @@ static inline void ext4_unlock_group(struct super_block *sb, > spin_unlock(ext4_group_lock_ptr(sb, group)); > } > > +#ifdef CONFIG_QUOTA > +static inline bool ext4_quota_capable(struct super_block *sb) > +{ > + return (test_opt(sb, QUOTA) || ext4_has_feature_quota(sb)); > +} > + > +static inline bool ext4_is_quota_journalled(struct super_block *sb) > +{ > + struct ext4_sb_info *sbi = EXT4_SB(sb); > + > + return (ext4_has_feature_quota(sb) || > + sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]); > +} > +#endif > + > /* > * Block validity checking > */ > diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h > index 00dc668..a124c68 100644 > --- a/fs/ext4/ext4_jbd2.h > +++ b/fs/ext4/ext4_jbd2.h > @@ -86,17 +86,14 @@ > #ifdef CONFIG_QUOTA > /* Amount of blocks needed for quota update - we know that the structure was > * allocated so we need to update only data block */ > -#define EXT4_QUOTA_TRANS_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\ > - ext4_has_feature_quota(sb)) ? 1 : 0) > +#define EXT4_QUOTA_TRANS_BLOCKS(sb) ((ext4_quota_capable(sb)) ? 1 : 0) > /* Amount of blocks needed for quota insert/delete - we do some block writes > * but inode, sb and group updates are done only once */ > -#define EXT4_QUOTA_INIT_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\ > - ext4_has_feature_quota(sb)) ?\ > +#define EXT4_QUOTA_INIT_BLOCKS(sb) ((ext4_quota_capable(sb)) ?\ > (DQUOT_INIT_ALLOC*(EXT4_SINGLEDATA_TRANS_BLOCKS(sb)-3)\ > +3+DQUOT_INIT_REWRITE) : 0) > > -#define EXT4_QUOTA_DEL_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\ > - ext4_has_feature_quota(sb)) ?\ > +#define EXT4_QUOTA_DEL_BLOCKS(sb) ((ext4_quota_capable(sb)) ?\ > (DQUOT_DEL_ALLOC*(EXT4_SINGLEDATA_TRANS_BLOCKS(sb)-3)\ > +3+DQUOT_DEL_REWRITE) : 0) > #else > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 9d01318..a988cf3 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -6158,11 +6158,8 @@ static int ext4_release_dquot(struct dquot *dquot) > static int ext4_mark_dquot_dirty(struct dquot *dquot) > { > struct super_block *sb = dquot->dq_sb; > - struct ext4_sb_info *sbi = EXT4_SB(sb); > > - /* Are we journaling quotas? */ > - if (ext4_has_feature_quota(sb) || > - sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) { > + if (ext4_is_quota_journalled(sb)) { > dquot_mark_dquot_dirty(dquot); > return ext4_write_dquot(dquot); > } else { > -- > 2.7.4 >
On Mon 19-10-20 11:37:06, Jan Kara wrote: > On Sun 18-10-20 05:02:26, Roman Anufriev wrote: > > Right now, there are several places, where we check whether fs is > > capable of enabling quota or if quota is journalled with quite long > > and non-self-descriptive condition statements. > > > > This patch wraps these statements into helpers for better readability > > and easier usage. > > > > Signed-off-by: Roman Anufriev <dotdot@yandex-team.ru> > > Looks good to me. You can add: > > Reviewe-by: Jan Kara <jack@suse.cz> Now I've realized that if we run in nojournal mode, quota won't be journalled in any case. Probably not a configuration you run in but still we should get that right. Honza > > --- > > Changes in v2: > > - Fix misleading helper name 'ext4_any_quota_enabled()' -> > > 'ext4_quota_capable()'. > > > > fs/ext4/ext4.h | 15 +++++++++++++++ > > fs/ext4/ext4_jbd2.h | 9 +++------ > > fs/ext4/super.c | 5 +---- > > 3 files changed, 19 insertions(+), 10 deletions(-) > > > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > > index 250e905..897df24 100644 > > --- a/fs/ext4/ext4.h > > +++ b/fs/ext4/ext4.h > > @@ -3251,6 +3251,21 @@ static inline void ext4_unlock_group(struct super_block *sb, > > spin_unlock(ext4_group_lock_ptr(sb, group)); > > } > > > > +#ifdef CONFIG_QUOTA > > +static inline bool ext4_quota_capable(struct super_block *sb) > > +{ > > + return (test_opt(sb, QUOTA) || ext4_has_feature_quota(sb)); > > +} > > + > > +static inline bool ext4_is_quota_journalled(struct super_block *sb) > > +{ > > + struct ext4_sb_info *sbi = EXT4_SB(sb); > > + > > + return (ext4_has_feature_quota(sb) || > > + sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]); > > +} > > +#endif > > + > > /* > > * Block validity checking > > */ > > diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h > > index 00dc668..a124c68 100644 > > --- a/fs/ext4/ext4_jbd2.h > > +++ b/fs/ext4/ext4_jbd2.h > > @@ -86,17 +86,14 @@ > > #ifdef CONFIG_QUOTA > > /* Amount of blocks needed for quota update - we know that the structure was > > * allocated so we need to update only data block */ > > -#define EXT4_QUOTA_TRANS_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\ > > - ext4_has_feature_quota(sb)) ? 1 : 0) > > +#define EXT4_QUOTA_TRANS_BLOCKS(sb) ((ext4_quota_capable(sb)) ? 1 : 0) > > /* Amount of blocks needed for quota insert/delete - we do some block writes > > * but inode, sb and group updates are done only once */ > > -#define EXT4_QUOTA_INIT_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\ > > - ext4_has_feature_quota(sb)) ?\ > > +#define EXT4_QUOTA_INIT_BLOCKS(sb) ((ext4_quota_capable(sb)) ?\ > > (DQUOT_INIT_ALLOC*(EXT4_SINGLEDATA_TRANS_BLOCKS(sb)-3)\ > > +3+DQUOT_INIT_REWRITE) : 0) > > > > -#define EXT4_QUOTA_DEL_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\ > > - ext4_has_feature_quota(sb)) ?\ > > +#define EXT4_QUOTA_DEL_BLOCKS(sb) ((ext4_quota_capable(sb)) ?\ > > (DQUOT_DEL_ALLOC*(EXT4_SINGLEDATA_TRANS_BLOCKS(sb)-3)\ > > +3+DQUOT_DEL_REWRITE) : 0) > > #else > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > > index 9d01318..a988cf3 100644 > > --- a/fs/ext4/super.c > > +++ b/fs/ext4/super.c > > @@ -6158,11 +6158,8 @@ static int ext4_release_dquot(struct dquot *dquot) > > static int ext4_mark_dquot_dirty(struct dquot *dquot) > > { > > struct super_block *sb = dquot->dq_sb; > > - struct ext4_sb_info *sbi = EXT4_SB(sb); > > > > - /* Are we journaling quotas? */ > > - if (ext4_has_feature_quota(sb) || > > - sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) { > > + if (ext4_is_quota_journalled(sb)) { > > dquot_mark_dquot_dirty(dquot); > > return ext4_write_dquot(dquot); > > } else { > > -- > > 2.7.4 > > > -- > Jan Kara <jack@suse.com> > SUSE Labs, CR
On Mon, 19 Oct 2020, Jan Kara wrote: > On Mon 19-10-20 11:37:06, Jan Kara wrote: >> On Sun 18-10-20 05:02:26, Roman Anufriev wrote: >>> Right now, there are several places, where we check whether fs is >>> capable of enabling quota or if quota is journalled with quite long >>> and non-self-descriptive condition statements. >>> >>> This patch wraps these statements into helpers for better readability >>> and easier usage. >>> >>> Signed-off-by: Roman Anufriev <dotdot@yandex-team.ru> >> >> Looks good to me. You can add: >> >> Reviewe-by: Jan Kara <jack@suse.cz> > > Now I've realized that if we run in nojournal mode, quota won't be > journalled in any case. Probably not a configuration you run in but still > we should get that right. I forgot about this case. Fixed in v4: https://lore.kernel.org/linux-ext4/1603336860-16153-1-git-send-email-dotdot@yandex-team.ru/ Roman
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 250e905..897df24 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -3251,6 +3251,21 @@ static inline void ext4_unlock_group(struct super_block *sb, spin_unlock(ext4_group_lock_ptr(sb, group)); } +#ifdef CONFIG_QUOTA +static inline bool ext4_quota_capable(struct super_block *sb) +{ + return (test_opt(sb, QUOTA) || ext4_has_feature_quota(sb)); +} + +static inline bool ext4_is_quota_journalled(struct super_block *sb) +{ + struct ext4_sb_info *sbi = EXT4_SB(sb); + + return (ext4_has_feature_quota(sb) || + sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]); +} +#endif + /* * Block validity checking */ diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h index 00dc668..a124c68 100644 --- a/fs/ext4/ext4_jbd2.h +++ b/fs/ext4/ext4_jbd2.h @@ -86,17 +86,14 @@ #ifdef CONFIG_QUOTA /* Amount of blocks needed for quota update - we know that the structure was * allocated so we need to update only data block */ -#define EXT4_QUOTA_TRANS_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\ - ext4_has_feature_quota(sb)) ? 1 : 0) +#define EXT4_QUOTA_TRANS_BLOCKS(sb) ((ext4_quota_capable(sb)) ? 1 : 0) /* Amount of blocks needed for quota insert/delete - we do some block writes * but inode, sb and group updates are done only once */ -#define EXT4_QUOTA_INIT_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\ - ext4_has_feature_quota(sb)) ?\ +#define EXT4_QUOTA_INIT_BLOCKS(sb) ((ext4_quota_capable(sb)) ?\ (DQUOT_INIT_ALLOC*(EXT4_SINGLEDATA_TRANS_BLOCKS(sb)-3)\ +3+DQUOT_INIT_REWRITE) : 0) -#define EXT4_QUOTA_DEL_BLOCKS(sb) ((test_opt(sb, QUOTA) ||\ - ext4_has_feature_quota(sb)) ?\ +#define EXT4_QUOTA_DEL_BLOCKS(sb) ((ext4_quota_capable(sb)) ?\ (DQUOT_DEL_ALLOC*(EXT4_SINGLEDATA_TRANS_BLOCKS(sb)-3)\ +3+DQUOT_DEL_REWRITE) : 0) #else diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 9d01318..a988cf3 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -6158,11 +6158,8 @@ static int ext4_release_dquot(struct dquot *dquot) static int ext4_mark_dquot_dirty(struct dquot *dquot) { struct super_block *sb = dquot->dq_sb; - struct ext4_sb_info *sbi = EXT4_SB(sb); - /* Are we journaling quotas? */ - if (ext4_has_feature_quota(sb) || - sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) { + if (ext4_is_quota_journalled(sb)) { dquot_mark_dquot_dirty(dquot); return ext4_write_dquot(dquot); } else {
Right now, there are several places, where we check whether fs is capable of enabling quota or if quota is journalled with quite long and non-self-descriptive condition statements. This patch wraps these statements into helpers for better readability and easier usage. Signed-off-by: Roman Anufriev <dotdot@yandex-team.ru> --- Changes in v2: - Fix misleading helper name 'ext4_any_quota_enabled()' -> 'ext4_quota_capable()'. fs/ext4/ext4.h | 15 +++++++++++++++ fs/ext4/ext4_jbd2.h | 9 +++------ fs/ext4/super.c | 5 +---- 3 files changed, 19 insertions(+), 10 deletions(-)