Definition of XFS_DQUOT_LOGRES()

Definition of XFS_DQUOT_LOGRES()

[Michael Nishimoto]

The comment for XFS_DQUOT_LOGRES states that we need to reserve space
for 3 dquots.  I can't figure out why we need to add this amount to *all*
operations and why this amount wasn't added after doing a runtime
quotaon check.

Comments?

     Michael

/*
  * In the worst case, when both user and group quotas are on,
  * we can have a max of three dquots changing in a single transaction.
  */
#define XFS_DQUOT_LOGRES(mp)    (sizeof(xfs_disk_dquot_t) * 3)

Re: Definition of XFS_DQUOT_LOGRES()

[David Chinner]

On Mon, Mar 31, 2008 at 03:45:28PM -0700, Michael Nishimoto wrote:
> The comment for XFS_DQUOT_LOGRES states that we need to reserve space
> for 3 dquots.  I can't figure out why we need to add this amount to *all*
> operations and why this amount wasn't added after doing a runtime
> quotaon check.

It probably could be done that way.  But given that:

> /*
>  * In the worst case, when both user and group quotas are on,
>  * we can have a max of three dquots changing in a single transaction.
>  */
> #define XFS_DQUOT_LOGRES(mp)    (sizeof(xfs_disk_dquot_t) * 3)

sizeof(xfs_disk_dquot_t) = 104 bytes,

the overall addition to the reservations is minor considering:

[0]kdb> xtrres 0xe0000038055ac6c0
write: 109752   truncate: 223672        rename: 305976
link: 153144    remove: 153144  symlink: 158520
create: 158392  mkdir: 158392   ifree: 58936
ichange: 2104   growdata: 45696 swrite: 384
addafork: 70584 writeid: 384    attrinval: 179328
attrset: 22968  attrrm: 90552   clearagi: 1152
growrtalloc: 66048      growrtzero: 4224        growrtfree: 6272
[0]kdb>

on a 14GB filesystem most of the transactions this is added to
are on the far side of 150k and that means we're talking about less
than 0.2% of the entire reservation comes from the dquot. With
larger block sizes and/or larger filesystems, these get much
larger. e.g. same 14GB device, 64k block size instead of 4k:

[0]kdb> xtrres 0xe00000b8027d39f8
write: 987576   truncate: 1977272       rename: 2891064
link: 1445688   remove: 1445688 symlink: 1512504
create: 1511864 mkdir: 1511864  ifree: 470584
ichange: 1592   growdata: 395904        swrite: 384
addafork: 658616        writeid: 384    attrinval: 1581696
attrset: 329656 attrrm: 791480  clearagi: 640
growrtalloc: 592640     growrtzero: 65664       growrtfree: 67200

The rename reservation is *2.8MB* (up from 300k). IOWs, 300 bytes is
really noise when it comes to reservation space. (OT: See why I want to
increase the log size now? :)

Is it worth the complexity of adding this dquot reservation at
runtime for a best case reduction of 0.2% in log space reservation
usage? Probably not, but patches can be convincing ;)

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

Re: Definition of XFS_DQUOT_LOGRES()

[Michael Nishimoto]

David Chinner wrote:
> On Mon, Mar 31, 2008 at 03:45:28PM -0700, Michael Nishimoto wrote:
>> The comment for XFS_DQUOT_LOGRES states that we need to reserve space
>> for 3 dquots.  I can't figure out why we need to add this amount to *all*
>> operations and why this amount wasn't added after doing a runtime
>> quotaon check.
> 
> It probably could be done that way.  But given that:
> 
>> /*
>>  * In the worst case, when both user and group quotas are on,
>>  * we can have a max of three dquots changing in a single transaction.
>>  */
>> #define XFS_DQUOT_LOGRES(mp)    (sizeof(xfs_disk_dquot_t) * 3)
> 
> sizeof(xfs_disk_dquot_t) = 104 bytes,
> 
> the overall addition to the reservations is minor considering:
> 
> [0]kdb> xtrres 0xe0000038055ac6c0
> write: 109752   truncate: 223672        rename: 305976
> link: 153144    remove: 153144  symlink: 158520
> create: 158392  mkdir: 158392   ifree: 58936
> ichange: 2104   growdata: 45696 swrite: 384
> addafork: 70584 writeid: 384    attrinval: 179328
> attrset: 22968  attrrm: 90552   clearagi: 1152
> growrtalloc: 66048      growrtzero: 4224        growrtfree: 6272
> [0]kdb>
> 
> on a 14GB filesystem most of the transactions this is added to
> are on the far side of 150k and that means we're talking about less
> than 0.2% of the entire reservation comes from the dquot. With
> larger block sizes and/or larger filesystems, these get much
> larger. e.g. same 14GB device, 64k block size instead of 4k:
> 
> [0]kdb> xtrres 0xe00000b8027d39f8
> write: 987576   truncate: 1977272       rename: 2891064
> link: 1445688   remove: 1445688 symlink: 1512504
> create: 1511864 mkdir: 1511864  ifree: 470584
> ichange: 1592   growdata: 395904        swrite: 384
> addafork: 658616        writeid: 384    attrinval: 1581696
> attrset: 329656 attrrm: 791480  clearagi: 640
> growrtalloc: 592640     growrtzero: 65664       growrtfree: 67200
> 
> The rename reservation is *2.8MB* (up from 300k). IOWs, 300 bytes is
> really noise when it comes to reservation space. (OT: See why I want to
> increase the log size now? :)
> 
> Is it worth the complexity of adding this dquot reservation at
> runtime for a best case reduction of 0.2% in log space reservation
> usage? Probably not, but patches can be convincing ;)
> 
> Cheers,
> 
> Dave.

Here is a patch to fix a sign problem when growing the log to 2G.


--- xfs_log.2.c 2008-04-01 11:55:45.000000000 -0700
+++ xfs_log.3.c 2008-04-01 11:56:53.000000000 -0700
@@ -230,20 +230,24 @@
  static void
  xlog_grant_add_space_write(struct log *log, int bytes)
  {
-       log->l_grant_write_bytes += bytes;
-       if (log->l_grant_write_bytes > log->l_logsize) {
-               log->l_grant_write_bytes -= log->l_logsize;
-               log->l_grant_write_cycle++;
+       int __tmp = (log)->l_logsize - (log)->l_grant_write_bytes;
+       if (__tmp > bytes)
+               (log)->l_grant_write_bytes += bytes;
+       else {
+               (log)->l_grant_write_cycle++;
+               (log)->l_grant_write_bytes = bytes - __tmp;
         }
  }

  static void
  xlog_grant_add_space_reserve(struct log *log, int bytes)
  {
-       log->l_grant_reserve_bytes += bytes;
-       if (log->l_grant_reserve_bytes > log->l_logsize) {
-               log->l_grant_reserve_bytes -= log->l_logsize;
-               log->l_grant_reserve_cycle++;
+       int __tmp = (log)->l_logsize - (log)->l_grant_reserve_bytes;
+       if (__tmp > bytes)
+               (log)->l_grant_reserve_bytes += bytes;
+       else {
+               (log)->l_grant_reserve_cycle++;
+               (log)->l_grant_reserve_bytes = bytes - __tmp;
         }
  }

Re: Definition of XFS_DQUOT_LOGRES()

[David Chinner]

On Tue, Apr 01, 2008 at 02:03:29PM -0700, Michael Nishimoto wrote:
> David Chinner wrote:
> >On Mon, Mar 31, 2008 at 03:45:28PM -0700, Michael Nishimoto wrote:
> >>The comment for XFS_DQUOT_LOGRES states that we need to reserve space
> >>for 3 dquots.  I can't figure out why we need to add this amount to *all*
> >>operations and why this amount wasn't added after doing a runtime
> >>quotaon check.
> >
> >It probably could be done that way.  But given that:
> >
> >>/*
> >> * In the worst case, when both user and group quotas are on,
> >> * we can have a max of three dquots changing in a single transaction.
> >> */
> >>#define XFS_DQUOT_LOGRES(mp)    (sizeof(xfs_disk_dquot_t) * 3)
> >
> >sizeof(xfs_disk_dquot_t) = 104 bytes,
> >
> >the overall addition to the reservations is minor considering:
> >
> >[0]kdb> xtrres 0xe0000038055ac6c0
> >write: 109752   truncate: 223672        rename: 305976
> >link: 153144    remove: 153144  symlink: 158520
> >create: 158392  mkdir: 158392   ifree: 58936
> >ichange: 2104   growdata: 45696 swrite: 384
> >addafork: 70584 writeid: 384    attrinval: 179328
> >attrset: 22968  attrrm: 90552   clearagi: 1152
> >growrtalloc: 66048      growrtzero: 4224        growrtfree: 6272
> >[0]kdb>
> >
> >on a 14GB filesystem most of the transactions this is added to
> >are on the far side of 150k and that means we're talking about less
> >than 0.2% of the entire reservation comes from the dquot. With
> >larger block sizes and/or larger filesystems, these get much
> >larger. e.g. same 14GB device, 64k block size instead of 4k:
> >
> >[0]kdb> xtrres 0xe00000b8027d39f8
> >write: 987576   truncate: 1977272       rename: 2891064
> >link: 1445688   remove: 1445688 symlink: 1512504
> >create: 1511864 mkdir: 1511864  ifree: 470584
> >ichange: 1592   growdata: 395904        swrite: 384
> >addafork: 658616        writeid: 384    attrinval: 1581696
> >attrset: 329656 attrrm: 791480  clearagi: 640
> >growrtalloc: 592640     growrtzero: 65664       growrtfree: 67200
> >
> >The rename reservation is *2.8MB* (up from 300k). IOWs, 300 bytes is
> >really noise when it comes to reservation space. (OT: See why I want to
> >increase the log size now? :)
> >
> >Is it worth the complexity of adding this dquot reservation at
> >runtime for a best case reduction of 0.2% in log space reservation
> >usage? Probably not, but patches can be convincing ;)
> >
> >Cheers,
> >
> >Dave.
> 
> Here is a patch to fix a sign problem when growing the log to 2G.
> 

Michael, can you repost this patch with a Signed-off-by tag
and.....

> --- xfs_log.2.c 2008-04-01 11:55:45.000000000 -0700
> +++ xfs_log.3.c 2008-04-01 11:56:53.000000000 -0700
> @@ -230,20 +230,24 @@
>  static void
>  xlog_grant_add_space_write(struct log *log, int bytes)
>  {
> -       log->l_grant_write_bytes += bytes;
> -       if (log->l_grant_write_bytes > log->l_logsize) {
> -               log->l_grant_write_bytes -= log->l_logsize;
> -               log->l_grant_write_cycle++;
> +       int __tmp = (log)->l_logsize - (log)->l_grant_write_bytes;

No need for "__" in the tmp var, nor the () around log...

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group