xfs_quota: remove calls to XFS_QSYNC

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.

Index: xfsprogs-dev/quota/free.c
===================================================================
@@ -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
===================================================================
@@ -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
===================================================================
@@ -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
===================================================================
@@ -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
===================================================================
@@ -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
===================================================================
@@ -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
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

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
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

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.

-Ben

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

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
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

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
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs