* [PATCH] xfs_quota: remove calls to XFS_QSYNC
@ 2012-01-30 11:50 Christoph Hellwig
2012-01-30 19:57 ` Nathan Scott
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2012-01-30 11:50 UTC (permalink / raw)
To: xfs; +Cc: nathans
In 2005 commit master-melb:xfs-cmds:23840a from Nathan added calls to
XFS_QSYNC/Q_XQUOTASYNC to xfs_quota, with the following rather sparse
description:
"Issue a quote sync before reporting quota, resolving issue with delayed
allocation."
I can't really see a reason for this - we do quota accounting by the time
we reserve space for the delayed allocation, and while converting the
reservations might change the quota accounting minimally due to the amount
of btree blocks used for the bmap btree on large files in generally this
makes little sense, and on today's large system has a large performance
impact. Also only xfs_quota ever did these calls, the generic quota tool
never did any kind of sync, and of course removing it does not cause
any regressions in xfstests.
Nathan, I've cced you in case you still remember anything about this,
although it's fairly unlikely after 6.5 years. Also if anyone at SGI
can find anything about the above commits in BugWorks additional feedback
would be welcome.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfsprogs-dev/quota/free.c
===================================================================
--- xfsprogs-dev.orig/quota/free.c 2012-01-27 11:10:34.000000000 +0000
+++ xfsprogs-dev/quota/free.c 2012-01-27 11:10:42.000000000 +0000
@@ -167,7 +167,6 @@ projects_free_space_data(
return 0;
}
- xfsquotactl(XFS_QSYNC, dev, type, fsx.fsx_projid, NULL);
if (xfsquotactl(XFS_GETQUOTA, dev, type, fsx.fsx_projid, &d) < 0) {
perror("XFS_GETQUOTA");
close(fd);
Index: xfsprogs-dev/quota/irix.c
===================================================================
--- xfsprogs-dev.orig/quota/irix.c 2012-01-27 11:10:52.000000000 +0000
+++ xfsprogs-dev/quota/irix.c 2012-01-27 11:10:59.000000000 +0000
@@ -45,8 +45,6 @@ xcommand_to_qcommand(
return Q_XGETQSTAT;
case XFS_QUOTARM:
return Q_XQUOTARM;
- case XFS_QSYNC:
- return Q_SYNC;
}
return 0;
}
Index: xfsprogs-dev/quota/linux.c
===================================================================
--- xfsprogs-dev.orig/quota/linux.c 2012-01-27 11:11:02.000000000 +0000
+++ xfsprogs-dev/quota/linux.c 2012-01-27 11:11:05.000000000 +0000
@@ -55,8 +55,6 @@ xcommand_to_qcommand(
return Q_XGETQSTAT;
case XFS_QUOTARM:
return Q_XQUOTARM;
- case XFS_QSYNC:
- return Q_XQUOTASYNC;
}
return 0;
}
Index: xfsprogs-dev/quota/quota.c
===================================================================
--- xfsprogs-dev.orig/quota/quota.c 2012-01-27 11:10:24.000000000 +0000
+++ xfsprogs-dev/quota/quota.c 2012-01-27 11:10:29.000000000 +0000
@@ -64,7 +64,6 @@ quota_mount(
uint qflags;
int count;
- xfsquotactl(XFS_QSYNC, dev, type, 0, NULL);
if (xfsquotactl(XFS_GETQUOTA, dev, type, id, (void *)&d) < 0)
return 0;
Index: xfsprogs-dev/quota/quota.h
===================================================================
--- xfsprogs-dev.orig/quota/quota.h 2012-01-27 11:11:10.000000000 +0000
+++ xfsprogs-dev/quota/quota.h 2012-01-27 11:11:20.000000000 +0000
@@ -41,7 +41,6 @@ enum {
XFS_SETQLIM, /* set disk limits */
XFS_GETQSTAT, /* get quota subsystem status */
XFS_QUOTARM, /* free disk space used by dquots */
- XFS_QSYNC, /* flush delayed allocate space */
};
/*
Index: xfsprogs-dev/quota/report.c
===================================================================
--- xfsprogs-dev.orig/quota/report.c 2012-01-27 11:09:41.000000000 +0000
+++ xfsprogs-dev/quota/report.c 2012-01-27 11:10:19.000000000 +0000
@@ -520,10 +520,6 @@ report_any_type(
if (type & XFS_USER_QUOTA) {
fs_cursor_initialise(dir, FS_MOUNT_POINT, &cursor);
while ((mount = fs_cursor_next_entry(&cursor))) {
- if (xfsquotactl(XFS_QSYNC, mount->fs_name,
- XFS_USER_QUOTA, 0, NULL) < 0
- && errno != ENOENT && errno != ENOSYS)
- perror("XFS_QSYNC user quota");
report_user_mount(fp, form, mount,
lower, upper, flags);
}
@@ -531,10 +527,6 @@ report_any_type(
if (type & XFS_GROUP_QUOTA) {
fs_cursor_initialise(dir, FS_MOUNT_POINT, &cursor);
while ((mount = fs_cursor_next_entry(&cursor))) {
- if (xfsquotactl(XFS_QSYNC, mount->fs_name,
- XFS_GROUP_QUOTA, 0, NULL) < 0
- && errno != ENOENT && errno != ENOSYS)
- perror("XFS_QSYNC group quota");
report_group_mount(fp, form, mount,
lower, upper, flags);
}
@@ -542,10 +534,6 @@ report_any_type(
if (type & XFS_PROJ_QUOTA) {
fs_cursor_initialise(dir, FS_MOUNT_POINT, &cursor);
while ((mount = fs_cursor_next_entry(&cursor))) {
- if (xfsquotactl(XFS_QSYNC, mount->fs_name,
- XFS_PROJ_QUOTA, 0, NULL) < 0
- && errno != ENOENT && errno != ENOSYS)
- perror("XFS_QSYNC proj quota");
report_project_mount(fp, form, mount,
lower, upper, flags);
}
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] xfs_quota: remove calls to XFS_QSYNC
2012-01-30 11:50 [PATCH] xfs_quota: remove calls to XFS_QSYNC Christoph Hellwig
@ 2012-01-30 19:57 ` Nathan Scott
2012-01-31 16:26 ` Ben Myers
0 siblings, 1 reply; 5+ messages in thread
From: Nathan Scott @ 2012-01-30 19:57 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On 30 January 2012 22:50, Christoph Hellwig <hch@infradead.org> wrote:
> ...
> Nathan, I've cced you in case you still remember anything about this,
> although it's fairly unlikely after 6.5 years. Also if anyone at SGI
> can find anything about the above commits in BugWorks additional feedback
> would be welcome.
I can't recall - but usually there was greater detail in the bugworks entry,
I think thats your best source now.
Patch looks OK to me FWIW, assuming nothing comes of the bugworks
archeology exercise.
cheers.
--
Nathan
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs_quota: remove calls to XFS_QSYNC
2012-01-30 19:57 ` Nathan Scott
@ 2012-01-31 16:26 ` Ben Myers
2012-01-31 21:01 ` Dave Chinner
0 siblings, 1 reply; 5+ messages in thread
From: Ben Myers @ 2012-01-31 16:26 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Nathan Scott, xfs
On Tue, Jan 31, 2012 at 06:57:04AM +1100, Nathan Scott wrote:
> On 30 January 2012 22:50, Christoph Hellwig <hch@infradead.org> wrote:
> > ...
> > Nathan, I've cced you in case you still remember anything about this,
> > although it's fairly unlikely after 6.5 years. Also if anyone at SGI
> > can find anything about the above commits in BugWorks additional feedback
> > would be welcome.
>
> I can't recall - but usually there was greater detail in the bugworks entry,
> I think thats your best source now.
>
> Patch looks OK to me FWIW, assuming nothing comes of the bugworks
> archeology exercise.
I'm not a ptools expert so I had to ask around. This turned out to be
PV942815 - XFS quota reporting interacts badly with delalloc
"We get a constant trickle of confused people who've found that the used
space reported by repquota/quota does not get immediately updated after
creating new files / extending existing ones. The problem is a result
of buffered IO in XFS doing delayed allocation - if we have not done an
allocation, the quota accounting has not been updated, so we end up with
effectively stale data being reported.
This affects CXFS too, of course. After discussion a few weeks back on
the XFS conf. call, it was decided to not change CXFS (too expensive to
do a flush on all nodes) but that we can make some simple XFS changes to
make this less of a problem - i.e. flushing delalloc data just before we
extract quota information." -Nathan
Patch looks good to me.
Reviewed-by: Ben Myers <bpm@sgi.com>
-Ben
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs_quota: remove calls to XFS_QSYNC
2012-01-31 16:26 ` Ben Myers
@ 2012-01-31 21:01 ` Dave Chinner
2012-02-01 10:24 ` Christoph Hellwig
0 siblings, 1 reply; 5+ messages in thread
From: Dave Chinner @ 2012-01-31 21:01 UTC (permalink / raw)
To: Ben Myers; +Cc: Christoph Hellwig, Nathan Scott, xfs
On Tue, Jan 31, 2012 at 10:26:17AM -0600, Ben Myers wrote:
> On Tue, Jan 31, 2012 at 06:57:04AM +1100, Nathan Scott wrote:
> > On 30 January 2012 22:50, Christoph Hellwig <hch@infradead.org> wrote:
> > > ...
> > > Nathan, I've cced you in case you still remember anything about this,
> > > although it's fairly unlikely after 6.5 years. Also if anyone at SGI
> > > can find anything about the above commits in BugWorks additional feedback
> > > would be welcome.
> >
> > I can't recall - but usually there was greater detail in the bugworks entry,
> > I think thats your best source now.
> >
> > Patch looks OK to me FWIW, assuming nothing comes of the bugworks
> > archeology exercise.
>
> I'm not a ptools expert so I had to ask around. This turned out to be
>
> PV942815 - XFS quota reporting interacts badly with delalloc
>
> "We get a constant trickle of confused people who've found that the used
> space reported by repquota/quota does not get immediately updated after
> creating new files / extending existing ones. The problem is a result
> of buffered IO in XFS doing delayed allocation - if we have not done an
> allocation, the quota accounting has not been updated, so we end up with
> effectively stale data being reported.
>
> This affects CXFS too, of course. After discussion a few weeks back on
> the XFS conf. call, it was decided to not change CXFS (too expensive to
> do a flush on all nodes) but that we can make some simple XFS changes to
> make this less of a problem - i.e. flushing delalloc data just before we
> extract quota information." -Nathan
So effectively what that says to me is that quota only exports the
real block usage, even though it internally tracks delalloc
reservations. Perhaps an additionaly change to make in this case is
to fold the reserved blocks into what is reported to the quota
utilities?
Indeed, what is exported to userspace via xfs_qm_export_dquot() is
the information in the dquot core - the on-disk information - so
perhaps all we need to do is export dqp->q_res_bcount (the count of
real + reserved blocks) instead of the on-disk info?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs_quota: remove calls to XFS_QSYNC
2012-01-31 21:01 ` Dave Chinner
@ 2012-02-01 10:24 ` Christoph Hellwig
0 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2012-02-01 10:24 UTC (permalink / raw)
To: Dave Chinner; +Cc: Christoph Hellwig, Ben Myers, Nathan Scott, xfs
On Wed, Feb 01, 2012 at 08:01:10AM +1100, Dave Chinner wrote:
> So effectively what that says to me is that quota only exports the
> real block usage, even though it internally tracks delalloc
> reservations. Perhaps an additionaly change to make in this case is
> to fold the reserved blocks into what is reported to the quota
> utilities?
>
> Indeed, what is exported to userspace via xfs_qm_export_dquot() is
> the information in the dquot core - the on-disk information - so
> perhaps all we need to do is export dqp->q_res_bcount (the count of
> real + reserved blocks) instead of the on-disk info?
That seems like a good idea, given that enforcement takes the
reservation into account. To retain compatbility for the case of new
userspace and an old kernel I'd have to disable Q_XQUOTASYNC in the
kernel intead of in the tool, though.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-02-01 10:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-30 11:50 [PATCH] xfs_quota: remove calls to XFS_QSYNC Christoph Hellwig
2012-01-30 19:57 ` Nathan Scott
2012-01-31 16:26 ` Ben Myers
2012-01-31 21:01 ` Dave Chinner
2012-02-01 10:24 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox