public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] repquota does't report correct space usage
@ 2007-02-28  7:33 Utako Kusaka
  2007-03-01 23:26 ` Donald Douwsma
  0 siblings, 1 reply; 6+ messages in thread
From: Utako Kusaka @ 2007-02-28  7:33 UTC (permalink / raw)
  To: xfs

Hi,

repquota may report incorrect space usage when the filesystem is mounted
repeatedly with different quota options.
The cause of the problem is that xfs_qm_quotacheck() is not called because
the `CHKD' flag in mp->m_qflags is not cleared until it is mounted with 
no quota option. This patch fixes it.

Test case:
mkfs.xfs -f /dev/sda6

mount -t xfs -o usrquota,grpquota /dev/sda6 mpnt/
setquota -u utako 3000 4000 0 0 /dev/sda6
setquota -u xfs 1500 2000 0 0 /dev/sda6
setquota -g users 1000 1500 0 0 /dev/sda6
repquota -ugv -a
umount mpnt/

mount -t xfs -o usrquota /dev/sda6 mpnt/
xfs_mkfile 3m mpnt/file1
xfs_mkfile 1m mpnt/file2
chown utako mpnt/file1
chown xfs mpnt/file2
chgrp users mpnt/file*
repquota -ugv -a
umount mpnt/

mount -t xfs -o grpquota /dev/sda6 mpnt/
repquota -ugv -a
rm mpnt/file1
repquota -ugv -a

Result:
*** Report for group quotas on device /dev/sda6
Block grace time: 7days; Inode grace time: 7days
                        Block limits                File limits
Group           used    soft    hard  grace    used  soft  hard  grace
----------------------------------------------------------------------
root      --       0       0       0              3     0     0
users     +- 18014398509478912    1000    1500  7days 18446744073709551615     0     0

*** Status for group quotas on device /dev/sda6
Accounting: ON; Enforcement: ON
Inode: #132 (2 blocks, 2 extents)


Signed-off-by: Utako Kusaka <utako@tnes.nec.co.jp>
---

--- linux-2.6.20-orgn/fs/xfs/quota/xfs_qm.c.orgn        2007-02-22 17:30:07.000000000 +0900
+++ linux-2.6.20/fs/xfs/xfs_qm.c        2007-02-22 17:30:58.000000000 +0900
@@ -1175,8 +1175,6 @@ xfs_qm_init_quotainfo(
        qinf->qi_dqperchunk = BBTOB(qinf->qi_dqchunklen);
        do_div(qinf->qi_dqperchunk, sizeof(xfs_dqblk_t));

-       mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD);
-
        /*
         * We try to get the limits from the superuser's limits fields.
         * This is quite hacky, but it is standard quota practice.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] repquota does't report correct space usage
  2007-02-28  7:33 [PATCH] repquota does't report correct space usage Utako Kusaka
@ 2007-03-01 23:26 ` Donald Douwsma
  2007-03-02  2:59   ` Utako Kusaka
  2007-03-02  6:34   ` [PATCH] repquota doesn't report correct space usage #2 Utako Kusaka
  0 siblings, 2 replies; 6+ messages in thread
From: Donald Douwsma @ 2007-03-01 23:26 UTC (permalink / raw)
  To: Utako Kusaka; +Cc: xfs

Utako Kusaka wrote:
> Hi,
> 
> repquota may report incorrect space usage when the filesystem is mounted
> repeatedly with different quota options.
> The cause of the problem is that xfs_qm_quotacheck() is not called because
> the `CHKD' flag in mp->m_qflags is not cleared until it is mounted with 
> no quota option. This patch fixes it.

Good find, I've heard of some problems with quota 'corruption' that may
actually be caused by this.

> --- linux-2.6.20-orgn/fs/xfs/quota/xfs_qm.c.orgn        2007-02-22 17:30:07.000000000 +0900
> +++ linux-2.6.20/fs/xfs/xfs_qm.c        2007-02-22 17:30:58.000000000 +0900
> @@ -1175,8 +1175,6 @@ xfs_qm_init_quotainfo(
>         qinf->qi_dqperchunk = BBTOB(qinf->qi_dqchunklen);
>         do_div(qinf->qi_dqperchunk, sizeof(xfs_dqblk_t));
> 
> -       mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD);
> -
>         /*
>          * We try to get the limits from the superuser's limits fields.
>          * This is quite hacky, but it is standard quota practice.

This disables the optimization that skips the quota check for the normal case where mount
options have not been changed.

I don't have any quota check performance figures handy but I don't think we can loose this
optimization for people with large filesystems/machines.

I think instead we need to clear the individual quota bit when a filesystem is mounted
without a particular quota type. This will force a quota check but only when the filesystem
is again mounted with that quota type.

Donald

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] repquota does't report correct space usage
  2007-03-01 23:26 ` Donald Douwsma
@ 2007-03-02  2:59   ` Utako Kusaka
  2007-03-02  6:34   ` [PATCH] repquota doesn't report correct space usage #2 Utako Kusaka
  1 sibling, 0 replies; 6+ messages in thread
From: Utako Kusaka @ 2007-03-02  2:59 UTC (permalink / raw)
  To: Donald Douwsma; +Cc: xfs

Hi, Donald

I understand your explanation. I'll think a new patch for this.

Thanks.

Fri, 02 Mar 2007 10:26:48 +1100 Donald Douwsma wrote:
>Utako Kusaka wrote:
>> Hi,
>> 
>> repquota may report incorrect space usage when the filesystem is mounted
>> repeatedly with different quota options.
>> The cause of the problem is that xfs_qm_quotacheck() is not called because
>> the `CHKD' flag in mp->m_qflags is not cleared until it is mounted with 
>> no quota option. This patch fixes it.
>
>Good find, I've heard of some problems with quota 'corruption' that may
>actually be caused by this.
>
>> --- linux-2.6.20-orgn/fs/xfs/quota/xfs_qm.c.orgn        2007-02-22 17:30:07.000000000 +0900
>> +++ linux-2.6.20/fs/xfs/xfs_qm.c        2007-02-22 17:30:58.000000000 +0900
>> @@ -1175,8 +1175,6 @@ xfs_qm_init_quotainfo(
>>         qinf->qi_dqperchunk = BBTOB(qinf->qi_dqchunklen);
>>         do_div(qinf->qi_dqperchunk, sizeof(xfs_dqblk_t));
>> 
>> -       mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD);
>> -
>>         /*
>>          * We try to get the limits from the superuser's limits fields.
>>          * This is quite hacky, but it is standard quota practice.
>
>This disables the optimization that skips the quota check for the normal case where mount
>options have not been changed.
>
>I don't have any quota check performance figures handy but I don't think we can loose this
>optimization for people with large filesystems/machines.
>
>I think instead we need to clear the individual quota bit when a filesystem is mounted
>without a particular quota type. This will force a quota check but only when the filesystem
>is again mounted with that quota type.
>
>Donald
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] repquota doesn't report correct space usage #2
  2007-03-01 23:26 ` Donald Douwsma
  2007-03-02  2:59   ` Utako Kusaka
@ 2007-03-02  6:34   ` Utako Kusaka
  2007-03-05  6:17     ` Donald Douwsma
  1 sibling, 1 reply; 6+ messages in thread
From: Utako Kusaka @ 2007-03-02  6:34 UTC (permalink / raw)
  To: donaldd, xfs

Hi,

This new patch skips the quota check when the filesystem is mounted
with the same quota option. 

Signed-off-by: Utako Kusaka <utako@tnes.nec.co.jp>
---

--- fs/xfs/quota/xfs_qm.c.orgn	2007-02-22 17:30:07.000000000 +0900
+++ fs/xfs/quota/xfs_qm.c	2007-03-02 15:01:44.000000000 +0900
@@ -1175,7 +1175,12 @@ xfs_qm_init_quotainfo(
 	qinf->qi_dqperchunk = BBTOB(qinf->qi_dqchunklen);
 	do_div(qinf->qi_dqperchunk, sizeof(xfs_dqblk_t));
 
-	mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD);
+	if (XFS_IS_UQUOTA_ON(mp) && (mp->m_sb.sb_qflags & XFS_UQUOTA_ACCT))
+		mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_UQUOTA_CHKD);
+	if (XFS_IS_GQUOTA_ON(mp) && (mp->m_sb.sb_qflags & XFS_GQUOTA_ACCT))
+		mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_OQUOTA_CHKD);
+	if (XFS_IS_PQUOTA_ON(mp) && (mp->m_sb.sb_qflags & XFS_PQUOTA_ACCT))
+		mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_OQUOTA_CHKD);
 
 	/*
 	 * We try to get the limits from the superuser's limits fields.

Fri, 02 Mar 2007 10:26:48 +1100 Donald Douwsma wrote:
>Utako Kusaka wrote:
>> Hi,
>> 
>> repquota may report incorrect space usage when the filesystem is mounted
>> repeatedly with different quota options.
>> The cause of the problem is that xfs_qm_quotacheck() is not called because
>> the `CHKD' flag in mp->m_qflags is not cleared until it is mounted with 
>> no quota option. This patch fixes it.
>
>Good find, I've heard of some problems with quota 'corruption' that may
>actually be caused by this.
>
>> --- linux-2.6.20-orgn/fs/xfs/quota/xfs_qm.c.orgn        2007-02-22 17:30:07.000000000 +0900
>> +++ linux-2.6.20/fs/xfs/xfs_qm.c        2007-02-22 17:30:58.000000000 +0900
>> @@ -1175,8 +1175,6 @@ xfs_qm_init_quotainfo(
>>         qinf->qi_dqperchunk = BBTOB(qinf->qi_dqchunklen);
>>         do_div(qinf->qi_dqperchunk, sizeof(xfs_dqblk_t));
>> 
>> -       mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD);
>> -
>>         /*
>>          * We try to get the limits from the superuser's limits fields.
>>          * This is quite hacky, but it is standard quota practice.
>
>This disables the optimization that skips the quota check for the normal case where mount
>options have not been changed.
>
>I don't have any quota check performance figures handy but I don't think we can loose this
>optimization for people with large filesystems/machines.
>
>I think instead we need to clear the individual quota bit when a filesystem is mounted
>without a particular quota type. This will force a quota check but only when the filesystem
>is again mounted with that quota type.
>
>Donald
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] repquota doesn't report correct space usage #2
  2007-03-02  6:34   ` [PATCH] repquota doesn't report correct space usage #2 Utako Kusaka
@ 2007-03-05  6:17     ` Donald Douwsma
  2007-03-05  8:36       ` Utako Kusaka
  0 siblings, 1 reply; 6+ messages in thread
From: Donald Douwsma @ 2007-03-05  6:17 UTC (permalink / raw)
  To: Utako Kusaka; +Cc: xfs

Hi Utako,

That's closer to what I was thinking of but I'd prefer to do the
manipulation separate to init. Putting it in xfs_qm_mount_quotas()
minimizes the number of places changes are made to the superblock.

We dont need to worry about group/project differences as a
quotacheck is forced by XFS_QM_NEED_QUOTACHECK() if there are
incompatibilities.


Signed-off-by: Donald Douwsma <donaldd@sgi.com>

--- a/fs/xfs/quota/xfs_qm.c     2007-03-05 16:50:11.000000000 +1100
+++ b/fs/xfs/quota/xfs_qm.c     2007-03-05 15:36:12.000000000 +1100
@@ -388,6 +388,17 @@ xfs_qm_mount_quotas(
                        return XFS_ERROR(error);
                }
        }
+	/*
+	 * If one type of quotas is off, then it will lose its
+	 * quotachecked status, since we won't be doing accounting for
+	 * that type anymore.
+	 */
+	if (!XFS_IS_UQUOTA_ON(mp)) {
+		mp->m_qflags &= ~XFS_UQUOTA_CHKD;
+	}
+	if (!(XFS_IS_GQUOTA_ON(mp) || XFS_IS_PQUOTA_ON(mp))) {
+		mp->m_qflags &= ~XFS_OQUOTA_CHKD;
+	}

  write_changes:
        /*


Utako Kusaka wrote:
> Hi,
> 
> This new patch skips the quota check when the filesystem is mounted
> with the same quota option. 
> 
> Signed-off-by: Utako Kusaka <utako@tnes.nec.co.jp>
> ---
> 
> --- fs/xfs/quota/xfs_qm.c.orgn	2007-02-22 17:30:07.000000000 +0900
> +++ fs/xfs/quota/xfs_qm.c	2007-03-02 15:01:44.000000000 +0900
> @@ -1175,7 +1175,12 @@ xfs_qm_init_quotainfo(
>  	qinf->qi_dqperchunk = BBTOB(qinf->qi_dqchunklen);
>  	do_div(qinf->qi_dqperchunk, sizeof(xfs_dqblk_t));
>  
> -	mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD);
> +	if (XFS_IS_UQUOTA_ON(mp) && (mp->m_sb.sb_qflags & XFS_UQUOTA_ACCT))
> +		mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_UQUOTA_CHKD);
> +	if (XFS_IS_GQUOTA_ON(mp) && (mp->m_sb.sb_qflags & XFS_GQUOTA_ACCT))
> +		mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_OQUOTA_CHKD);
> +	if (XFS_IS_PQUOTA_ON(mp) && (mp->m_sb.sb_qflags & XFS_PQUOTA_ACCT))
> +		mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_OQUOTA_CHKD);
>  
>  	/*
>  	 * We try to get the limits from the superuser's limits fields.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] repquota doesn't report correct space usage #2
  2007-03-05  6:17     ` Donald Douwsma
@ 2007-03-05  8:36       ` Utako Kusaka
  0 siblings, 0 replies; 6+ messages in thread
From: Utako Kusaka @ 2007-03-05  8:36 UTC (permalink / raw)
  To: donaldd; +Cc: xfs

Hi Donald,

I see. I tested your patch and it was no problem.

Mon, 05 Mar 2007 17:17:55 +1100 Donald Douwsma wrote:
>Hi Utako,
>
>That's closer to what I was thinking of but I'd prefer to do the
>manipulation separate to init. Putting it in xfs_qm_mount_quotas()
>minimizes the number of places changes are made to the superblock.
>
>We dont need to worry about group/project differences as a
>quotacheck is forced by XFS_QM_NEED_QUOTACHECK() if there are
>incompatibilities.
>
>
>Signed-off-by: Donald Douwsma <donaldd@sgi.com>
>
>--- a/fs/xfs/quota/xfs_qm.c     2007-03-05 16:50:11.000000000 +1100
>+++ b/fs/xfs/quota/xfs_qm.c     2007-03-05 15:36:12.000000000 +1100
>@@ -388,6 +388,17 @@ xfs_qm_mount_quotas(
>                        return XFS_ERROR(error);
>                }
>        }
>+	/*
>+	 * If one type of quotas is off, then it will lose its
>+	 * quotachecked status, since we won't be doing accounting for
>+	 * that type anymore.
>+	 */
>+	if (!XFS_IS_UQUOTA_ON(mp)) {
>+		mp->m_qflags &= ~XFS_UQUOTA_CHKD;
>+	}
>+	if (!(XFS_IS_GQUOTA_ON(mp) || XFS_IS_PQUOTA_ON(mp))) {
>+		mp->m_qflags &= ~XFS_OQUOTA_CHKD;
>+	}
>
>  write_changes:
>        /*
>
>
>Utako Kusaka wrote:
>> Hi,
>> 
>> This new patch skips the quota check when the filesystem is mounted
>> with the same quota option. 
>> 
>> Signed-off-by: Utako Kusaka <utako@tnes.nec.co.jp>
>> ---
>> 
>> --- fs/xfs/quota/xfs_qm.c.orgn	2007-02-22 17:30:07.000000000 +0900
>> +++ fs/xfs/quota/xfs_qm.c	2007-03-02 15:01:44.000000000 +0900
>> @@ -1175,7 +1175,12 @@ xfs_qm_init_quotainfo(
>>  	qinf->qi_dqperchunk = BBTOB(qinf->qi_dqchunklen);
>>  	do_div(qinf->qi_dqperchunk, sizeof(xfs_dqblk_t));
>>  
>> -	mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_ALL_QUOTA_CHKD);
>> +	if (XFS_IS_UQUOTA_ON(mp) && (mp->m_sb.sb_qflags & XFS_UQUOTA_ACCT))
>> +		mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_UQUOTA_CHKD);
>> +	if (XFS_IS_GQUOTA_ON(mp) && (mp->m_sb.sb_qflags & XFS_GQUOTA_ACCT))
>> +		mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_OQUOTA_CHKD);
>> +	if (XFS_IS_PQUOTA_ON(mp) && (mp->m_sb.sb_qflags & XFS_PQUOTA_ACCT))
>> +		mp->m_qflags |= (mp->m_sb.sb_qflags & XFS_OQUOTA_CHKD);
>>  
>>  	/*
>>  	 * We try to get the limits from the superuser's limits fields.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-03-05  8:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-02-28  7:33 [PATCH] repquota does't report correct space usage Utako Kusaka
2007-03-01 23:26 ` Donald Douwsma
2007-03-02  2:59   ` Utako Kusaka
2007-03-02  6:34   ` [PATCH] repquota doesn't report correct space usage #2 Utako Kusaka
2007-03-05  6:17     ` Donald Douwsma
2007-03-05  8:36       ` Utako Kusaka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox