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) <[email protected]>
---
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);
+ }
}
#endif
sb->s_flags = s_flags; /* Restore MS_RDONLY status */
--
2.5.0
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) <[email protected]>
> ---
> 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
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
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) <[email protected]>
>> ---
>> 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.