[PATCH 1/2] ext4: fix incorrect quotaoff if the quota feature is enabled

[PATCH 1/2] ext4: fix incorrect quotaoff if the quota feature is enabled

[zhangyi (F)]

Current ext4 quota should always "usage enabled" if the
quota feautre is enabled. But in ext4_orphan_cleanup(), it
turn quotas off directly (used for the older journaled
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 add check for journaled quotas to avoid incorrect
quotaoff when ext4 has quota feautre.

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 fs/ext4/super.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d61a70e2..cade5c8 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2442,7 +2442,7 @@ static void ext4_orphan_cleanup(struct super_block *sb,
 #ifdef CONFIG_QUOTA
 	/* Needed for iput() to work correctly and not trash data */
 	sb->s_flags |= MS_ACTIVE;
-	/* Turn on quotas so that they are updated correctly */
+	/* Turn on journaled quotas so that they are updated correctly */
 	for (i = 0; i < EXT4_MAXQUOTAS; i++) {
 		if (EXT4_SB(sb)->s_qf_names[i]) {
 			int ret = ext4_quota_on_mount(sb, i);
@@ -2510,9 +2510,9 @@ 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 */
+	/* Turn off journaled quotas if they were enabled for orphan cleanup */
 	for (i = 0; i < EXT4_MAXQUOTAS; i++) {
-		if (sb_dqopt(sb)->files[i])
+		if (EXT4_SB(sb)->s_qf_names[i] && sb_dqopt(sb)->files[i])
 			dquot_quota_off(sb, i);
 	}
 #endif
-- 
2.5.0

[PATCH 2/2] ext4: fix quota inconsistency during orphan cleanup for read-only mounts

[zhangyi (F)]

Quota does not get enabled for read-only mounts if filesystem
has quota feature, so that quotas cannot updated during orphan
cleanup, which will lead to quota inconsistency.

This patch turn on quotas during orphan cleanup for this case,
make sure quotas can be updated correctly.

Reported-by: Jan Kara <jack@suse.cz>
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 fs/ext4/super.c | 38 +++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index cade5c8..c9e7be5 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2404,6 +2404,7 @@ static void ext4_orphan_cleanup(struct super_block *sb,
 	unsigned int s_flags = sb->s_flags;
 	int ret, nr_orphans = 0, nr_truncates = 0;
 #ifdef CONFIG_QUOTA
+	int quota_update = 0;
 	int i;
 #endif
 	if (!es->s_last_orphan) {
@@ -2442,14 +2443,32 @@ static void ext4_orphan_cleanup(struct super_block *sb,
 #ifdef CONFIG_QUOTA
 	/* Needed for iput() to work correctly and not trash data */
 	sb->s_flags |= MS_ACTIVE;
-	/* Turn on journaled quotas so that they are updated correctly */
+
+	/*
+	 * Turn on quotas which were not enabled for read-only mounts if
+	 * filesystem has quota feature, so that they are updated correctly.
+	 */
+	if (ext4_has_feature_quota(sb) && (s_flags & MS_RDONLY)) {
+		int ret = ext4_enable_quotas(sb);
+
+		if (!ret)
+			quota_update = 1;
+		else
+			ext4_msg(sb, KERN_ERR,
+				"Cannot turn on quotas: error %d", ret);
+	}
+
+	/* Turn on journaled quotas used for old sytle */
 	for (i = 0; i < EXT4_MAXQUOTAS; i++) {
 		if (EXT4_SB(sb)->s_qf_names[i]) {
 			int ret = ext4_quota_on_mount(sb, i);
-			if (ret < 0)
+
+			if (!ret)
+				quota_update = 1;
+			else
 				ext4_msg(sb, KERN_ERR,
 					"Cannot turn on journaled "
-					"quota: error %d", ret);
+					"quota: type %d: error %d", i, ret);
 		}
 	}
 #endif
@@ -2510,10 +2529,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 off journaled quotas if they were enabled for orphan cleanup */
-	for (i = 0; i < EXT4_MAXQUOTAS; i++) {
-		if (EXT4_SB(sb)->s_qf_names[i] && sb_dqopt(sb)->files[i])
-			dquot_quota_off(sb, i);
+	/* Turn off quotas if they were enabled for orphan cleanup */
+	if (quota_update) {
+		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 */
@@ -5512,6 +5533,9 @@ static int ext4_enable_quotas(struct super_block *sb)
 				DQUOT_USAGE_ENABLED |
 				(quota_mopt[type] ? DQUOT_LIMITS_ENABLED : 0));
 			if (err) {
+				for (type--; type >= 0; type--)
+					dquot_quota_off(sb, type);
+
 				ext4_warning(sb,
 					"Failed to enable quota tracking "
 					"(type=%d, err=%d). Please run "
-- 
2.5.0

Re: [PATCH 1/2] ext4: fix incorrect quotaoff if the quota feature is enabled

[Jan Kara]

On Wed 23-08-17 17:13:57, zhangyi (F) wrote:
> Current ext4 quota should always "usage enabled" if the
> quota feautre is enabled. But in ext4_orphan_cleanup(), it
> turn quotas off directly (used for the older journaled
> 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 add check for journaled quotas to avoid incorrect
> quotaoff when ext4 has quota feautre.
> 
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza


> ---
>  fs/ext4/super.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index d61a70e2..cade5c8 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2442,7 +2442,7 @@ static void ext4_orphan_cleanup(struct super_block *sb,
>  #ifdef CONFIG_QUOTA
>  	/* Needed for iput() to work correctly and not trash data */
>  	sb->s_flags |= MS_ACTIVE;
> -	/* Turn on quotas so that they are updated correctly */
> +	/* Turn on journaled quotas so that they are updated correctly */
>  	for (i = 0; i < EXT4_MAXQUOTAS; i++) {
>  		if (EXT4_SB(sb)->s_qf_names[i]) {
>  			int ret = ext4_quota_on_mount(sb, i);
> @@ -2510,9 +2510,9 @@ 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 */
> +	/* Turn off journaled quotas if they were enabled for orphan cleanup */
>  	for (i = 0; i < EXT4_MAXQUOTAS; i++) {
> -		if (sb_dqopt(sb)->files[i])
> +		if (EXT4_SB(sb)->s_qf_names[i] && sb_dqopt(sb)->files[i])
>  			dquot_quota_off(sb, i);
>  	}
>  #endif
> -- 
> 2.5.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/2] ext4: fix quota inconsistency during orphan cleanup for read-only mounts

[Jan Kara]

On Wed 23-08-17 17:13:58, zhangyi (F) wrote:
> Quota does not get enabled for read-only mounts if filesystem
> has quota feature, so that quotas cannot updated during orphan
> cleanup, which will lead to quota inconsistency.
> 
> This patch turn on quotas during orphan cleanup for this case,
> make sure quotas can be updated correctly.
> 
> Reported-by: Jan Kara <jack@suse.cz>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza


> ---
>  fs/ext4/super.c | 38 +++++++++++++++++++++++++++++++-------
>  1 file changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index cade5c8..c9e7be5 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2404,6 +2404,7 @@ static void ext4_orphan_cleanup(struct super_block *sb,
>  	unsigned int s_flags = sb->s_flags;
>  	int ret, nr_orphans = 0, nr_truncates = 0;
>  #ifdef CONFIG_QUOTA
> +	int quota_update = 0;
>  	int i;
>  #endif
>  	if (!es->s_last_orphan) {
> @@ -2442,14 +2443,32 @@ static void ext4_orphan_cleanup(struct super_block *sb,
>  #ifdef CONFIG_QUOTA
>  	/* Needed for iput() to work correctly and not trash data */
>  	sb->s_flags |= MS_ACTIVE;
> -	/* Turn on journaled quotas so that they are updated correctly */
> +
> +	/*
> +	 * Turn on quotas which were not enabled for read-only mounts if
> +	 * filesystem has quota feature, so that they are updated correctly.
> +	 */
> +	if (ext4_has_feature_quota(sb) && (s_flags & MS_RDONLY)) {
> +		int ret = ext4_enable_quotas(sb);
> +
> +		if (!ret)
> +			quota_update = 1;
> +		else
> +			ext4_msg(sb, KERN_ERR,
> +				"Cannot turn on quotas: error %d", ret);
> +	}
> +
> +	/* Turn on journaled quotas used for old sytle */
>  	for (i = 0; i < EXT4_MAXQUOTAS; i++) {
>  		if (EXT4_SB(sb)->s_qf_names[i]) {
>  			int ret = ext4_quota_on_mount(sb, i);
> -			if (ret < 0)
> +
> +			if (!ret)
> +				quota_update = 1;
> +			else
>  				ext4_msg(sb, KERN_ERR,
>  					"Cannot turn on journaled "
> -					"quota: error %d", ret);
> +					"quota: type %d: error %d", i, ret);
>  		}
>  	}
>  #endif
> @@ -2510,10 +2529,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 off journaled quotas if they were enabled for orphan cleanup */
> -	for (i = 0; i < EXT4_MAXQUOTAS; i++) {
> -		if (EXT4_SB(sb)->s_qf_names[i] && sb_dqopt(sb)->files[i])
> -			dquot_quota_off(sb, i);
> +	/* Turn off quotas if they were enabled for orphan cleanup */
> +	if (quota_update) {
> +		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 */
> @@ -5512,6 +5533,9 @@ static int ext4_enable_quotas(struct super_block *sb)
>  				DQUOT_USAGE_ENABLED |
>  				(quota_mopt[type] ? DQUOT_LIMITS_ENABLED : 0));
>  			if (err) {
> +				for (type--; type >= 0; type--)
> +					dquot_quota_off(sb, type);
> +
>  				ext4_warning(sb,
>  					"Failed to enable quota tracking "
>  					"(type=%d, err=%d). Please run "
> -- 
> 2.5.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 1/2] ext4: fix incorrect quotaoff if the quota feature is enabled

[Theodore Ts'o]

On Wed, Aug 23, 2017 at 12:00:00PM +0200, Jan Kara wrote:
> On Wed 23-08-17 17:13:57, zhangyi (F) wrote:
> > Current ext4 quota should always "usage enabled" if the
> > quota feautre is enabled. But in ext4_orphan_cleanup(), it
> > turn quotas off directly (used for the older journaled
> > 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 add check for journaled quotas to avoid incorrect
> > quotaoff when ext4 has quota feautre.
> > 
> > Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> 
> Looks good. You can add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks, applied.

					- Ted