| Message ID | 1503384097-19206-1-git-send-email-yi.zhang@huawei.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue 22-08-17 14:41:37, zhangyi (F) wrote: > Current ext4 quota should always "usage enabled" if the > quota feautre is enabled. But in ext4_orphan_cleanup(), it > turn quota off directly (used for the older quota), so > we cannot turn it on again via "quotaon" unless umount and > remount ext4. > > Simple reproduce: > > mkfs.ext4 -O project,quota /dev/vdb1 > mount -o prjquota /dev/vdb1 /mnt > chattr -p 123 /mnt > chattr +P /mnt > touch /mnt/aa /mnt/bb > exec 100<>/mnt/aa > rm -f /mnt/aa > sync > echo c > /proc/sysrq-trigger > > #reboot and mount > mount -o prjquota /dev/vdb1 /mnt > #query status > quotaon -Ppv /dev/vdb1 > #output > quotaon: Cannot find mountpoint for device /dev/vdb1 > quotaon: No correct mountpoint specified. > > This patch avoid incorrect quotaoff when ext4 has quota > feautre. We also take care about the "quota on" part in > ext4_orphan_cleanup(), it is used for journalled quota > and not invoked if quota feature is enabled, so it's not > affected. > > Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> > --- > fs/ext4/super.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index d61a70e2..c1b23b1 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -2510,10 +2510,12 @@ static void ext4_orphan_cleanup(struct super_block *sb, > ext4_msg(sb, KERN_INFO, "%d truncate%s cleaned up", > PLURAL(nr_truncates)); > #ifdef CONFIG_QUOTA > - /* Turn quotas off */ > - for (i = 0; i < EXT4_MAXQUOTAS; i++) { > - if (sb_dqopt(sb)->files[i]) > - dquot_quota_off(sb, i); > + if (!ext4_has_feature_quota(sb)) { > + /* Turn quotas off */ > + for (i = 0; i < EXT4_MAXQUOTAS; i++) { > + if (sb_dqopt(sb)->files[i]) > + dquot_quota_off(sb, i); > + } So this should work however I'd prefer if we kept the test symmetric to the place where we turn quotas on for orphan cleanup. I.e. something like: for (i = 0; i < EXT4_MAXQUOTAS; i++) { /* Old style journalled quotas enabled for orphan cleanup? */ if (EXT4_SB(sb)->s_qf_names[i] && sb_dqopt(sb)->files[i]) dquot_quota_off(sb, i); } Also when looking at this code I've noticed that quota does not get enabled if we use quota feature and we do orphan replay on read-only filesystem. Could you fix that by handling this case in ext4_orphan_cleanup() as well? Thanks! Honza > } > #endif > sb->s_flags = s_flags; /* Restore MS_RDONLY status */ > -- > 2.5.0 >
On 2017/8/22 17:22, Jan Kara Wrote: > On Tue 22-08-17 14:41:37, zhangyi (F) wrote: >> Current ext4 quota should always "usage enabled" if the >> quota feautre is enabled. But in ext4_orphan_cleanup(), it >> turn quota off directly (used for the older quota), so >> we cannot turn it on again via "quotaon" unless umount and >> remount ext4. >> >> Simple reproduce: >> >> mkfs.ext4 -O project,quota /dev/vdb1 >> mount -o prjquota /dev/vdb1 /mnt >> chattr -p 123 /mnt >> chattr +P /mnt >> touch /mnt/aa /mnt/bb >> exec 100<>/mnt/aa >> rm -f /mnt/aa >> sync >> echo c > /proc/sysrq-trigger >> >> #reboot and mount >> mount -o prjquota /dev/vdb1 /mnt >> #query status >> quotaon -Ppv /dev/vdb1 >> #output >> quotaon: Cannot find mountpoint for device /dev/vdb1 >> quotaon: No correct mountpoint specified. >> >> This patch avoid incorrect quotaoff when ext4 has quota >> feautre. We also take care about the "quota on" part in >> ext4_orphan_cleanup(), it is used for journalled quota >> and not invoked if quota feature is enabled, so it's not >> affected. >> >> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> >> --- >> fs/ext4/super.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/fs/ext4/super.c b/fs/ext4/super.c >> index d61a70e2..c1b23b1 100644 >> --- a/fs/ext4/super.c >> +++ b/fs/ext4/super.c >> @@ -2510,10 +2510,12 @@ static void ext4_orphan_cleanup(struct super_block *sb, >> ext4_msg(sb, KERN_INFO, "%d truncate%s cleaned up", >> PLURAL(nr_truncates)); >> #ifdef CONFIG_QUOTA >> - /* Turn quotas off */ >> - for (i = 0; i < EXT4_MAXQUOTAS; i++) { >> - if (sb_dqopt(sb)->files[i]) >> - dquot_quota_off(sb, i); >> + if (!ext4_has_feature_quota(sb)) { >> + /* Turn quotas off */ >> + for (i = 0; i < EXT4_MAXQUOTAS; i++) { >> + if (sb_dqopt(sb)->files[i]) >> + dquot_quota_off(sb, i); >> + } > > So this should work however I'd prefer if we kept the test symmetric to the > place where we turn quotas on for orphan cleanup. I.e. something like: > > for (i = 0; i < EXT4_MAXQUOTAS; i++) { > /* Old style journalled quotas enabled for orphan cleanup? */ > if (EXT4_SB(sb)->s_qf_names[i] && sb_dqopt(sb)->files[i]) > dquot_quota_off(sb, i); > } > Yes, looks better, good suggestion, thanks! > Also when looking at this code I've noticed that quota does not get enabled > if we use quota feature and we do orphan replay on read-only filesystem. > Could you fix that by handling this case in ext4_orphan_cleanup() as well? > Thanks! Sure, I will fix it as well. Thanks, ZhangYi.
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index d61a70e2..c1b23b1 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -2510,10 +2510,12 @@ static void ext4_orphan_cleanup(struct super_block *sb, ext4_msg(sb, KERN_INFO, "%d truncate%s cleaned up", PLURAL(nr_truncates)); #ifdef CONFIG_QUOTA - /* Turn quotas off */ - for (i = 0; i < EXT4_MAXQUOTAS; i++) { - if (sb_dqopt(sb)->files[i]) - dquot_quota_off(sb, i); + if (!ext4_has_feature_quota(sb)) { + /* Turn quotas off */ + for (i = 0; i < EXT4_MAXQUOTAS; i++) { + if (sb_dqopt(sb)->files[i]) + dquot_quota_off(sb, i); + } } #endif sb->s_flags = s_flags; /* Restore MS_RDONLY status */
Current ext4 quota should always "usage enabled" if the quota feautre is enabled. But in ext4_orphan_cleanup(), it turn quota off directly (used for the older quota), so we cannot turn it on again via "quotaon" unless umount and remount ext4. Simple reproduce: mkfs.ext4 -O project,quota /dev/vdb1 mount -o prjquota /dev/vdb1 /mnt chattr -p 123 /mnt chattr +P /mnt touch /mnt/aa /mnt/bb exec 100<>/mnt/aa rm -f /mnt/aa sync echo c > /proc/sysrq-trigger #reboot and mount mount -o prjquota /dev/vdb1 /mnt #query status quotaon -Ppv /dev/vdb1 #output quotaon: Cannot find mountpoint for device /dev/vdb1 quotaon: No correct mountpoint specified. This patch avoid incorrect quotaoff when ext4 has quota feautre. We also take care about the "quota on" part in ext4_orphan_cleanup(), it is used for journalled quota and not invoked if quota feature is enabled, so it's not affected. Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> --- fs/ext4/super.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)