All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] xfsprogs: optimize -L/-U range calls for xfs_quota's dump/report
@ 2022-04-20 14:45 Andrey Albershteyn
  2022-04-20 14:45 ` [PATCH v2 1/5] xfs_quota: separate quota info acquisition into get_dquot() Andrey Albershteyn
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Andrey Albershteyn @ 2022-04-20 14:45 UTC (permalink / raw)
  To: linux-xfs; +Cc: Andrey Albershteyn

The xfs_quota's 'report' and 'dump' commands have -L and -U
arguments for restricting quota querying to the range of
UIDs/GIDs/PIDs. The current implementation is using XFS_GETQUOTA to
get every ID in specified range. It doesn't perform well on wider
ranges. Also, the fallback case (UIDs from /etc/passwd) doesn't take
into account range restriction and outputs all users with quota.

First 3 patches do minor refactoring to split acquisition and
outputting of the quota information. This is not that necessary, but
makes it easier to manipulate with acquired data.

The 4th one replaces XFS_GETQUOTA based loop with XFS_GETNEXTQUOTA
one. The latter returns ID of the next user/group/project with
non-empty quota. The ID is then used for further call.

The last patch adds range checks for fallback case when
XFS_GETNEXTQUOTA is not available.

The fallback case will be also executed in case that empty range is
specified (e.g. -L <too high>), but will print nothing.

Changes from v1:
Darrick J. Wong:
- Renamed get_quota() -> get_dquot()
Christoph Hellwig:
- Formatting: if() with tab, operators without spaces, long lines
- Replace 'fs_disk_quota_t' with 'struct fs_disk_quota'
- Merge and then split patch 2 & 3, so dump_file() and
  report_mount() are in separate patches

Andrey Albershteyn (5):
  xfs_quota: separate quota info acquisition into get_dquot()
  xfs_quota: separate get_dquot() and dump_file()
  xfs_quota: separate get_dquot() and report_mount()
  xfs_quota: utilize XFS_GETNEXTQUOTA for ranged calls in report/dump
  xfs_quota: apply -L/-U range limits in uid/gid/pid loops

 quota/report.c | 323 ++++++++++++++++++++++++-------------------------
 1 file changed, 160 insertions(+), 163 deletions(-)

-- 
2.27.0


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

* [PATCH v2 1/5] xfs_quota: separate quota info acquisition into get_dquot()
  2022-04-20 14:45 [PATCH v2 0/5] xfsprogs: optimize -L/-U range calls for xfs_quota's dump/report Andrey Albershteyn
@ 2022-04-20 14:45 ` Andrey Albershteyn
  2022-04-25 16:22   ` Darrick J. Wong
  2022-04-20 14:45 ` [PATCH v2 2/5] xfs_quota: separate get_dquot() and dump_file() Andrey Albershteyn
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Andrey Albershteyn @ 2022-04-20 14:45 UTC (permalink / raw)
  To: linux-xfs; +Cc: Andrey Albershteyn, Christoph Hellwig

Both report_mount() and dump_file() have identical code to get quota
information. This could be used for further separation of the
functions.

Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 quota/report.c | 49 +++++++++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/quota/report.c b/quota/report.c
index 2eb5b5a9..cccc654e 100644
--- a/quota/report.c
+++ b/quota/report.c
@@ -59,16 +59,15 @@ report_help(void)
 "\n"));
 }
 
-static int 
-dump_file(
-	FILE		*fp,
+static int
+get_dquot(
+	struct fs_disk_quota *d,
 	uint		id,
 	uint		*oid,
 	uint		type,
 	char		*dev,
 	int		flags)
 {
-	fs_disk_quota_t	d;
 	int		cmd;
 
 	if (flags & GETNEXTQUOTA_FLAG)
@@ -77,7 +76,7 @@ dump_file(
 		cmd = XFS_GETQUOTA;
 
 	/* Fall back silently if XFS_GETNEXTQUOTA fails, warn on XFS_GETQUOTA */
-	if (xfsquotactl(cmd, dev, type, id, (void *)&d) < 0) {
+	if (xfsquotactl(cmd, dev, type, id, (void *)d) < 0) {
 		if (errno != ENOENT && errno != ENOSYS && errno != ESRCH &&
 		    cmd == XFS_GETQUOTA)
 			perror("XFS_GETQUOTA");
@@ -85,12 +84,29 @@ dump_file(
 	}
 
 	if (oid) {
-		*oid = d.d_id;
+		*oid = d->d_id;
 		/* Did kernelspace wrap? */
 		if (*oid < id)
 			return 0;
 	}
 
+	return 1;
+}
+
+static int
+dump_file(
+	FILE		*fp,
+	uint		id,
+	uint		*oid,
+	uint		type,
+	char		*dev,
+	int		flags)
+{
+	fs_disk_quota_t	d;
+
+	if (!get_dquot(&d, id, oid, type, dev, flags))
+		return 0;
+
 	if (!d.d_blk_softlimit && !d.d_blk_hardlimit &&
 	    !d.d_ino_softlimit && !d.d_ino_hardlimit &&
 	    !d.d_rtb_softlimit && !d.d_rtb_hardlimit)
@@ -329,31 +345,12 @@ report_mount(
 {
 	fs_disk_quota_t	d;
 	time64_t	timer;
-	char		*dev = mount->fs_name;
 	char		c[8], h[8], s[8];
 	uint		qflags;
 	int		count;
-	int		cmd;
 
-	if (flags & GETNEXTQUOTA_FLAG)
-		cmd = XFS_GETNEXTQUOTA;
-	else
-		cmd = XFS_GETQUOTA;
-
-	/* Fall back silently if XFS_GETNEXTQUOTA fails, warn on XFS_GETQUOTA*/
-	if (xfsquotactl(cmd, dev, type, id, (void *)&d) < 0) {
-		if (errno != ENOENT && errno != ENOSYS && errno != ESRCH &&
-		    cmd == XFS_GETQUOTA)
-			perror("XFS_GETQUOTA");
+	if (!get_dquot(&d, id, oid, type, mount->fs_name, flags))
 		return 0;
-	}
-
-	if (oid) {
-		*oid = d.d_id;
-		/* Did kernelspace wrap? */
-		if (* oid < id)
-			return 0;
-	}
 
 	if (flags & TERSE_FLAG) {
 		count = 0;
-- 
2.27.0


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

* [PATCH v2 2/5] xfs_quota: separate get_dquot() and dump_file()
  2022-04-20 14:45 [PATCH v2 0/5] xfsprogs: optimize -L/-U range calls for xfs_quota's dump/report Andrey Albershteyn
  2022-04-20 14:45 ` [PATCH v2 1/5] xfs_quota: separate quota info acquisition into get_dquot() Andrey Albershteyn
@ 2022-04-20 14:45 ` Andrey Albershteyn
  2022-04-24  5:29   ` Christoph Hellwig
  2022-04-25 16:28   ` Darrick J. Wong
  2022-04-20 14:45 ` [PATCH v2 3/5] xfs_quota: separate get_dquot() and report_mount() Andrey Albershteyn
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Andrey Albershteyn @ 2022-04-20 14:45 UTC (permalink / raw)
  To: linux-xfs; +Cc: Andrey Albershteyn

Separate quota info acquisition from outputting it to file. This
allows upper functions to filter obtained info (e.g. within specific
ID range).

Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
---
 quota/report.c | 86 ++++++++++++++++++++++++++------------------------
 1 file changed, 45 insertions(+), 41 deletions(-)

diff --git a/quota/report.c b/quota/report.c
index cccc654e..d5c6f84f 100644
--- a/quota/report.c
+++ b/quota/report.c
@@ -96,39 +96,31 @@ get_dquot(
 static int
 dump_file(
 	FILE		*fp,
-	uint		id,
-	uint		*oid,
-	uint		type,
-	char		*dev,
-	int		flags)
+	struct fs_disk_quota *d,
+	char		*dev)
 {
-	fs_disk_quota_t	d;
-
-	if (!get_dquot(&d, id, oid, type, dev, flags))
-		return 0;
-
-	if (!d.d_blk_softlimit && !d.d_blk_hardlimit &&
-	    !d.d_ino_softlimit && !d.d_ino_hardlimit &&
-	    !d.d_rtb_softlimit && !d.d_rtb_hardlimit)
+	if (!d->d_blk_softlimit && !d->d_blk_hardlimit &&
+	    !d->d_ino_softlimit && !d->d_ino_hardlimit &&
+	    !d->d_rtb_softlimit && !d->d_rtb_hardlimit)
 		return 1;
 	fprintf(fp, "fs = %s\n", dev);
 	/* this branch is for backward compatibility reasons */
-	if (d.d_rtb_softlimit || d.d_rtb_hardlimit)
+	if (d->d_rtb_softlimit || d->d_rtb_hardlimit)
 		fprintf(fp, "%-10d %7llu %7llu %7llu %7llu %7llu %7llu\n",
-			d.d_id,
-			(unsigned long long)d.d_blk_softlimit,
-			(unsigned long long)d.d_blk_hardlimit,
-			(unsigned long long)d.d_ino_softlimit,
-			(unsigned long long)d.d_ino_hardlimit,
-			(unsigned long long)d.d_rtb_softlimit,
-			(unsigned long long)d.d_rtb_hardlimit);
+			d->d_id,
+			(unsigned long long)d->d_blk_softlimit,
+			(unsigned long long)d->d_blk_hardlimit,
+			(unsigned long long)d->d_ino_softlimit,
+			(unsigned long long)d->d_ino_hardlimit,
+			(unsigned long long)d->d_rtb_softlimit,
+			(unsigned long long)d->d_rtb_hardlimit);
 	else
 		fprintf(fp, "%-10d %7llu %7llu %7llu %7llu\n",
-			d.d_id,
-			(unsigned long long)d.d_blk_softlimit,
-			(unsigned long long)d.d_blk_hardlimit,
-			(unsigned long long)d.d_ino_softlimit,
-			(unsigned long long)d.d_ino_hardlimit);
+			d->d_id,
+			(unsigned long long)d->d_blk_softlimit,
+			(unsigned long long)d->d_blk_hardlimit,
+			(unsigned long long)d->d_ino_softlimit,
+			(unsigned long long)d->d_ino_hardlimit);
 
 	return 1;
 }
@@ -142,6 +134,7 @@ dump_limits_any_type(
 	uint		upper)
 {
 	fs_path_t	*mount;
+	struct fs_disk_quota d;
 	uint		id = 0, oid;
 
 	if ((mount = fs_table_lookup(dir, FS_MOUNT_POINT)) == NULL) {
@@ -153,46 +146,57 @@ dump_limits_any_type(
 
 	/* Range was specified; query everything in it */
 	if (upper) {
-		for (id = lower; id <= upper; id++)
-			dump_file(fp, id, NULL, type, mount->fs_name, 0);
+		for (id = lower; id <= upper; id++) {
+			get_dquot(&d, id, &oid, type, mount->fs_name, 0);
+			dump_file(fp, &d, mount->fs_name);
+		}
 		return;
 	}
 
 	/* Use GETNEXTQUOTA if it's available */
-	if (dump_file(fp, id, &oid, type, mount->fs_name, GETNEXTQUOTA_FLAG)) {
+	if (get_dquot(&d, id, &oid, type, mount->fs_name, GETNEXTQUOTA_FLAG)) {
+		dump_file(fp, &d, mount->fs_name);
 		id = oid + 1;
-		while (dump_file(fp, id, &oid, type, mount->fs_name,
-				 GETNEXTQUOTA_FLAG))
+		while (get_dquot(&d, id, &oid, type, mount->fs_name,
+					GETNEXTQUOTA_FLAG)) {
+			dump_file(fp, &d, mount->fs_name);
 			id = oid + 1;
+		}
 		return;
-        }
+	}
 
 	/* Otherwise fall back to iterating over each uid/gid/prjid */
 	switch (type) {
 	case XFS_GROUP_QUOTA: {
 			struct group *g;
 			setgrent();
-			while ((g = getgrent()) != NULL)
-				dump_file(fp, g->gr_gid, NULL, type,
-					  mount->fs_name, 0);
+			while ((g = getgrent()) != NULL) {
+				get_dquot(&d, g->gr_gid, NULL, type,
+						mount->fs_name, 0);
+				dump_file(fp, &d, mount->fs_name);
+			}
 			endgrent();
 			break;
 		}
 	case XFS_PROJ_QUOTA: {
 			struct fs_project *p;
 			setprent();
-			while ((p = getprent()) != NULL)
-				dump_file(fp, p->pr_prid, NULL, type,
-					  mount->fs_name, 0);
+			while ((p = getprent()) != NULL) {
+				get_dquot(&d, p->pr_prid, NULL, type,
+						mount->fs_name, 0);
+				dump_file(fp, &d, mount->fs_name);
+			}
 			endprent();
 			break;
 		}
 	case XFS_USER_QUOTA: {
 			struct passwd *u;
 			setpwent();
-			while ((u = getpwent()) != NULL)
-				dump_file(fp, u->pw_uid, NULL, type,
-					  mount->fs_name, 0);
+			while ((u = getpwent()) != NULL) {
+				get_dquot(&d, u->pw_uid, NULL, type,
+						mount->fs_name, 0);
+				dump_file(fp, &d, mount->fs_name);
+			}
 			endpwent();
 			break;
 		}
-- 
2.27.0


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

* [PATCH v2 3/5] xfs_quota: separate get_dquot() and report_mount()
  2022-04-20 14:45 [PATCH v2 0/5] xfsprogs: optimize -L/-U range calls for xfs_quota's dump/report Andrey Albershteyn
  2022-04-20 14:45 ` [PATCH v2 1/5] xfs_quota: separate quota info acquisition into get_dquot() Andrey Albershteyn
  2022-04-20 14:45 ` [PATCH v2 2/5] xfs_quota: separate get_dquot() and dump_file() Andrey Albershteyn
@ 2022-04-20 14:45 ` Andrey Albershteyn
  2022-04-24  5:29   ` Christoph Hellwig
  2022-04-25 16:29   ` Darrick J. Wong
  2022-04-20 14:45 ` [PATCH v2 4/5] xfs_quota: utilize XFS_GETNEXTQUOTA for ranged calls in report/dump Andrey Albershteyn
  2022-04-20 14:45 ` [PATCH v2 5/5] xfs_quota: apply -L/-U range limits in uid/gid/pid loops Andrey Albershteyn
  4 siblings, 2 replies; 16+ messages in thread
From: Andrey Albershteyn @ 2022-04-20 14:45 UTC (permalink / raw)
  To: linux-xfs; +Cc: Andrey Albershteyn

Separate quota info acquisition from outputting. This allows upper
functions to filter obtained info (e.g. within specific ID range).

Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
---
 quota/report.c | 178 ++++++++++++++++++++++++++++---------------------
 1 file changed, 103 insertions(+), 75 deletions(-)

diff --git a/quota/report.c b/quota/report.c
index d5c6f84f..8ca154f0 100644
--- a/quota/report.c
+++ b/quota/report.c
@@ -339,30 +339,25 @@ report_header(
 static int
 report_mount(
 	FILE		*fp,
-	uint32_t	id,
+	struct fs_disk_quota *d,
 	char		*name,
-	uint32_t	*oid,
 	uint		form,
 	uint		type,
 	fs_path_t	*mount,
 	uint		flags)
 {
-	fs_disk_quota_t	d;
 	time64_t	timer;
 	char		c[8], h[8], s[8];
 	uint		qflags;
 	int		count;
 
-	if (!get_dquot(&d, id, oid, type, mount->fs_name, flags))
-		return 0;
-
 	if (flags & TERSE_FLAG) {
 		count = 0;
-		if ((form & XFS_BLOCK_QUOTA) && d.d_bcount)
+		if ((form & XFS_BLOCK_QUOTA) && d->d_bcount)
 			count++;
-		if ((form & XFS_INODE_QUOTA) && d.d_icount)
+		if ((form & XFS_INODE_QUOTA) && d->d_icount)
 			count++;
-		if ((form & XFS_RTBLOCK_QUOTA) && d.d_rtbcount)
+		if ((form & XFS_RTBLOCK_QUOTA) && d->d_rtbcount)
 			count++;
 		if (!count)
 			return 0;
@@ -372,19 +367,19 @@ report_mount(
 		report_header(fp, form, type, mount, flags);
 
 	if (flags & NO_LOOKUP_FLAG) {
-		fprintf(fp, "#%-10u", d.d_id);
+		fprintf(fp, "#%-10u", d->d_id);
 	} else {
 		if (name == NULL) {
 			if (type == XFS_USER_QUOTA) {
-				struct passwd	*u = getpwuid(d.d_id);
+				struct passwd	*u = getpwuid(d->d_id);
 				if (u)
 					name = u->pw_name;
 			} else if (type == XFS_GROUP_QUOTA) {
-				struct group	*g = getgrgid(d.d_id);
+				struct group	*g = getgrgid(d->d_id);
 				if (g)
 					name = g->gr_name;
 			} else if (type == XFS_PROJ_QUOTA) {
-				fs_project_t	*p = getprprid(d.d_id);
+				fs_project_t	*p = getprprid(d->d_id);
 				if (p)
 					name = p->pr_name;
 			}
@@ -393,73 +388,73 @@ report_mount(
 		if (name != NULL)
 			fprintf(fp, "%-10s", name);
 		else
-			fprintf(fp, "#%-9u", d.d_id);
+			fprintf(fp, "#%-9u", d->d_id);
 	}
 
 	if (form & XFS_BLOCK_QUOTA) {
-		timer = decode_timer(&d, d.d_btimer, d.d_btimer_hi);
+		timer = decode_timer(d, d->d_btimer, d->d_btimer_hi);
 		qflags = (flags & HUMAN_FLAG);
-		if (d.d_blk_hardlimit && d.d_bcount > d.d_blk_hardlimit)
+		if (d->d_blk_hardlimit && d->d_bcount > d->d_blk_hardlimit)
 			qflags |= LIMIT_FLAG;
-		if (d.d_blk_softlimit && d.d_bcount > d.d_blk_softlimit)
+		if (d->d_blk_softlimit && d->d_bcount > d->d_blk_softlimit)
 			qflags |= QUOTA_FLAG;
 		if (flags & HUMAN_FLAG)
 			fprintf(fp, " %6s %6s %6s  %02d %8s",
-				bbs_to_string(d.d_bcount, c, sizeof(c)),
-				bbs_to_string(d.d_blk_softlimit, s, sizeof(s)),
-				bbs_to_string(d.d_blk_hardlimit, h, sizeof(h)),
-				d.d_bwarns,
+				bbs_to_string(d->d_bcount, c, sizeof(c)),
+				bbs_to_string(d->d_blk_softlimit, s, sizeof(s)),
+				bbs_to_string(d->d_blk_hardlimit, h, sizeof(h)),
+				d->d_bwarns,
 				time_to_string(timer, qflags));
 		else
 			fprintf(fp, " %10llu %10llu %10llu     %02d %9s",
-				(unsigned long long)d.d_bcount >> 1,
-				(unsigned long long)d.d_blk_softlimit >> 1,
-				(unsigned long long)d.d_blk_hardlimit >> 1,
-				d.d_bwarns,
+				(unsigned long long)d->d_bcount >> 1,
+				(unsigned long long)d->d_blk_softlimit >> 1,
+				(unsigned long long)d->d_blk_hardlimit >> 1,
+				d->d_bwarns,
 				time_to_string(timer, qflags));
 	}
 	if (form & XFS_INODE_QUOTA) {
-		timer = decode_timer(&d, d.d_itimer, d.d_itimer_hi);
+		timer = decode_timer(d, d->d_itimer, d->d_itimer_hi);
 		qflags = (flags & HUMAN_FLAG);
-		if (d.d_ino_hardlimit && d.d_icount > d.d_ino_hardlimit)
+		if (d->d_ino_hardlimit && d->d_icount > d->d_ino_hardlimit)
 			qflags |= LIMIT_FLAG;
-		if (d.d_ino_softlimit && d.d_icount > d.d_ino_softlimit)
+		if (d->d_ino_softlimit && d->d_icount > d->d_ino_softlimit)
 			qflags |= QUOTA_FLAG;
 		if (flags & HUMAN_FLAG)
 			fprintf(fp, " %6s %6s %6s  %02d %8s",
-				num_to_string(d.d_icount, c, sizeof(c)),
-				num_to_string(d.d_ino_softlimit, s, sizeof(s)),
-				num_to_string(d.d_ino_hardlimit, h, sizeof(h)),
-				d.d_iwarns,
+				num_to_string(d->d_icount, c, sizeof(c)),
+				num_to_string(d->d_ino_softlimit, s, sizeof(s)),
+				num_to_string(d->d_ino_hardlimit, h, sizeof(h)),
+				d->d_iwarns,
 				time_to_string(timer, qflags));
 		else
 			fprintf(fp, " %10llu %10llu %10llu     %02d %9s",
-				(unsigned long long)d.d_icount,
-				(unsigned long long)d.d_ino_softlimit,
-				(unsigned long long)d.d_ino_hardlimit,
-				d.d_iwarns,
+				(unsigned long long)d->d_icount,
+				(unsigned long long)d->d_ino_softlimit,
+				(unsigned long long)d->d_ino_hardlimit,
+				d->d_iwarns,
 				time_to_string(timer, qflags));
 	}
 	if (form & XFS_RTBLOCK_QUOTA) {
-		timer = decode_timer(&d, d.d_rtbtimer, d.d_rtbtimer_hi);
+		timer = decode_timer(d, d->d_rtbtimer, d->d_rtbtimer_hi);
 		qflags = (flags & HUMAN_FLAG);
-		if (d.d_rtb_hardlimit && d.d_rtbcount > d.d_rtb_hardlimit)
+		if (d->d_rtb_hardlimit && d->d_rtbcount > d->d_rtb_hardlimit)
 			qflags |= LIMIT_FLAG;
-		if (d.d_rtb_softlimit && d.d_rtbcount > d.d_rtb_softlimit)
+		if (d->d_rtb_softlimit && d->d_rtbcount > d->d_rtb_softlimit)
 			qflags |= QUOTA_FLAG;
 		if (flags & HUMAN_FLAG)
 			fprintf(fp, " %6s %6s %6s  %02d %8s",
-				bbs_to_string(d.d_rtbcount, c, sizeof(c)),
-				bbs_to_string(d.d_rtb_softlimit, s, sizeof(s)),
-				bbs_to_string(d.d_rtb_hardlimit, h, sizeof(h)),
-				d.d_rtbwarns,
+				bbs_to_string(d->d_rtbcount, c, sizeof(c)),
+				bbs_to_string(d->d_rtb_softlimit, s, sizeof(s)),
+				bbs_to_string(d->d_rtb_hardlimit, h, sizeof(h)),
+				d->d_rtbwarns,
 				time_to_string(timer, qflags));
 		else
 			fprintf(fp, " %10llu %10llu %10llu     %02d %9s",
-				(unsigned long long)d.d_rtbcount >> 1,
-				(unsigned long long)d.d_rtb_softlimit >> 1,
-				(unsigned long long)d.d_rtb_hardlimit >> 1,
-				d.d_rtbwarns,
+				(unsigned long long)d->d_rtbcount >> 1,
+				(unsigned long long)d->d_rtb_softlimit >> 1,
+				(unsigned long long)d->d_rtb_hardlimit >> 1,
+				d->d_rtbwarns,
 				time_to_string(timer, qflags));
 	}
 	fputc('\n', fp);
@@ -476,30 +471,40 @@ report_user_mount(
 	uint		flags)
 {
 	struct passwd	*u;
+	struct fs_disk_quota	d;
 	uint		id = 0, oid;
 
 	if (upper) {	/* identifier range specified */
 		for (id = lower; id <= upper; id++) {
-			if (report_mount(fp, id, NULL, NULL,
-					form, XFS_USER_QUOTA, mount, flags))
+			if (get_dquot(&d, id, NULL, XFS_USER_QUOTA,
+						mount->fs_name, flags)) {
+				report_mount(fp, &d, NULL, form, XFS_USER_QUOTA,
+						mount, flags);
 				flags |= NO_HEADER_FLAG;
+			}
 		}
-	} else if (report_mount(fp, id, NULL, &oid, form,
-				XFS_USER_QUOTA, mount,
+	} else if (get_dquot(&d, id, &oid, XFS_USER_QUOTA, mount->fs_name,
 				flags|GETNEXTQUOTA_FLAG)) {
+		report_mount(fp, &d, NULL, form, XFS_USER_QUOTA, mount,
+			flags|GETNEXTQUOTA_FLAG);
 		id = oid + 1;
 		flags |= GETNEXTQUOTA_FLAG;
 		flags |= NO_HEADER_FLAG;
-		while (report_mount(fp, id, NULL, &oid, form, XFS_USER_QUOTA,
-				    mount, flags)) {
+		while (get_dquot(&d, id, &oid, XFS_USER_QUOTA, mount->fs_name,
+				flags)) {
+			report_mount(fp, &d, NULL, form, XFS_USER_QUOTA,
+				mount, flags);
 			id = oid + 1;
 		}
 	} else {
 		setpwent();
 		while ((u = getpwent()) != NULL) {
-			if (report_mount(fp, u->pw_uid, u->pw_name, NULL,
-					form, XFS_USER_QUOTA, mount, flags))
+			if (get_dquot(&d, u->pw_uid, NULL, XFS_USER_QUOTA,
+						mount->fs_name, flags)) {
+				report_mount(fp, &d, u->pw_name, form,
+						XFS_USER_QUOTA, mount, flags);
 				flags |= NO_HEADER_FLAG;
+			}
 		}
 		endpwent();
 	}
@@ -518,30 +523,40 @@ report_group_mount(
 	uint		flags)
 {
 	struct group	*g;
+	struct fs_disk_quota	d;
 	uint		id = 0, oid;
 
 	if (upper) {	/* identifier range specified */
 		for (id = lower; id <= upper; id++) {
-			if (report_mount(fp, id, NULL, NULL,
-					form, XFS_GROUP_QUOTA, mount, flags))
+			if (get_dquot(&d, id, NULL, XFS_GROUP_QUOTA,
+						mount->fs_name, flags)) {
+				report_mount(fp, &d, NULL, form,
+						XFS_GROUP_QUOTA, mount, flags);
 				flags |= NO_HEADER_FLAG;
+			}
 		}
-	} else if (report_mount(fp, id, NULL, &oid, form,
-				XFS_GROUP_QUOTA, mount,
-				flags|GETNEXTQUOTA_FLAG)) {
+	} else if (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
+				mount->fs_name, flags|GETNEXTQUOTA_FLAG)) {
+		report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount,
+				flags|GETNEXTQUOTA_FLAG);
 		id = oid + 1;
 		flags |= GETNEXTQUOTA_FLAG;
 		flags |= NO_HEADER_FLAG;
-		while (report_mount(fp, id, NULL, &oid, form, XFS_GROUP_QUOTA,
-				    mount, flags)) {
+		while (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
+					mount->fs_name, flags)) {
+			report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount,
+					flags);
 			id = oid + 1;
 		}
 	} else {
 		setgrent();
 		while ((g = getgrent()) != NULL) {
-			if (report_mount(fp, g->gr_gid, g->gr_name, NULL,
-					form, XFS_GROUP_QUOTA, mount, flags))
+			if (get_dquot(&d, g->gr_gid, NULL, XFS_GROUP_QUOTA,
+						mount->fs_name, flags)) {
+				report_mount(fp, &d, g->gr_name, form,
+						XFS_GROUP_QUOTA, mount, flags);
 				flags |= NO_HEADER_FLAG;
+			}
 		}
 	}
 	if (flags & NO_HEADER_FLAG)
@@ -559,22 +574,29 @@ report_project_mount(
 	uint		flags)
 {
 	fs_project_t	*p;
+	struct fs_disk_quota	d;
 	uint		id = 0, oid;
 
 	if (upper) {	/* identifier range specified */
 		for (id = lower; id <= upper; id++) {
-			if (report_mount(fp, id, NULL, NULL,
-					form, XFS_PROJ_QUOTA, mount, flags))
+			if (get_dquot(&d, id, NULL, XFS_PROJ_QUOTA,
+						mount->fs_name, flags)) {
+				report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA,
+						mount, flags);
 				flags |= NO_HEADER_FLAG;
+			}
 		}
-	} else if (report_mount(fp, id, NULL, &oid, form,
-				XFS_PROJ_QUOTA, mount,
-				flags|GETNEXTQUOTA_FLAG)) {
+	} else if (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
+				mount->fs_name, flags|GETNEXTQUOTA_FLAG)) {
+		report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount,
+				flags|GETNEXTQUOTA_FLAG);
 		id = oid + 1;
 		flags |= GETNEXTQUOTA_FLAG;
 		flags |= NO_HEADER_FLAG;
-		while (report_mount(fp, id, NULL, &oid, form, XFS_PROJ_QUOTA,
-				    mount, flags)) {
+		while (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
+					mount->fs_name, flags)) {
+			report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount,
+					flags);
 			id = oid + 1;
 		}
 	} else {
@@ -583,16 +605,22 @@ report_project_mount(
 			 * Print default project quota, even if projid 0
 			 * isn't defined
 			 */
-			if (report_mount(fp, 0, NULL, NULL,
-					form, XFS_PROJ_QUOTA, mount, flags))
+			if (get_dquot(&d, 0, NULL, XFS_PROJ_QUOTA,
+						mount->fs_name, flags)) {
+				report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA,
+						mount, flags);
 				flags |= NO_HEADER_FLAG;
+			}
 		}
 
 		setprent();
 		while ((p = getprent()) != NULL) {
-			if (report_mount(fp, p->pr_prid, p->pr_name, NULL,
-					form, XFS_PROJ_QUOTA, mount, flags))
+			if (get_dquot(&d, p->pr_prid, NULL, XFS_PROJ_QUOTA,
+						mount->fs_name, flags)) {
+				report_mount(fp, &d, p->pr_name, form,
+						XFS_PROJ_QUOTA, mount, flags);
 				flags |= NO_HEADER_FLAG;
+			}
 		}
 		endprent();
 	}
-- 
2.27.0


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

* [PATCH v2 4/5] xfs_quota: utilize XFS_GETNEXTQUOTA for ranged calls in report/dump
  2022-04-20 14:45 [PATCH v2 0/5] xfsprogs: optimize -L/-U range calls for xfs_quota's dump/report Andrey Albershteyn
                   ` (2 preceding siblings ...)
  2022-04-20 14:45 ` [PATCH v2 3/5] xfs_quota: separate get_dquot() and report_mount() Andrey Albershteyn
@ 2022-04-20 14:45 ` Andrey Albershteyn
  2022-04-25 16:33   ` Darrick J. Wong
  2022-04-20 14:45 ` [PATCH v2 5/5] xfs_quota: apply -L/-U range limits in uid/gid/pid loops Andrey Albershteyn
  4 siblings, 1 reply; 16+ messages in thread
From: Andrey Albershteyn @ 2022-04-20 14:45 UTC (permalink / raw)
  To: linux-xfs; +Cc: Andrey Albershteyn, Christoph Hellwig

The implementation based on XFS_GETQUOTA call for each ID in range,
specified with -L/-U, is quite slow for wider ranges.

If kernel supports XFS_GETNEXTQUOTA, report_*_mount/dump_any_file
will use that to obtain quota list for the mount. XFS_GETNEXTQUOTA
returns quota of the requested ID and next ID with non-empty quota.

Otherwise, XFS_GETQUOTA will be used for each user/group/project ID
known from password/group/project database.

Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 quota/report.c | 116 +++++++++++++++----------------------------------
 1 file changed, 35 insertions(+), 81 deletions(-)

diff --git a/quota/report.c b/quota/report.c
index 8ca154f0..65d931f3 100644
--- a/quota/report.c
+++ b/quota/report.c
@@ -135,7 +135,7 @@ dump_limits_any_type(
 {
 	fs_path_t	*mount;
 	struct fs_disk_quota d;
-	uint		id = 0, oid;
+	uint		id = lower, oid, flags = 0;
 
 	if ((mount = fs_table_lookup(dir, FS_MOUNT_POINT)) == NULL) {
 		exitcode = 1;
@@ -144,27 +144,17 @@ dump_limits_any_type(
 		return;
 	}
 
-	/* Range was specified; query everything in it */
-	if (upper) {
-		for (id = lower; id <= upper; id++) {
-			get_dquot(&d, id, &oid, type, mount->fs_name, 0);
-			dump_file(fp, &d, mount->fs_name);
-		}
-		return;
-	}
-
-	/* Use GETNEXTQUOTA if it's available */
-	if (get_dquot(&d, id, &oid, type, mount->fs_name, GETNEXTQUOTA_FLAG)) {
+	while (get_dquot(&d, id, &oid, type,
+				mount->fs_name, flags | GETNEXTQUOTA_FLAG) &&
+			!(upper && (d.d_id > upper))) {
 		dump_file(fp, &d, mount->fs_name);
 		id = oid + 1;
-		while (get_dquot(&d, id, &oid, type, mount->fs_name,
-					GETNEXTQUOTA_FLAG)) {
-			dump_file(fp, &d, mount->fs_name);
-			id = oid + 1;
-		}
-		return;
+		flags |= GETNEXTQUOTA_FLAG;
 	}
 
+	if (flags & GETNEXTQUOTA_FLAG)
+		return;
+
 	/* Otherwise fall back to iterating over each uid/gid/prjid */
 	switch (type) {
 	case XFS_GROUP_QUOTA: {
@@ -472,31 +462,19 @@ report_user_mount(
 {
 	struct passwd	*u;
 	struct fs_disk_quota	d;
-	uint		id = 0, oid;
+	uint		id = lower, oid;
 
-	if (upper) {	/* identifier range specified */
-		for (id = lower; id <= upper; id++) {
-			if (get_dquot(&d, id, NULL, XFS_USER_QUOTA,
-						mount->fs_name, flags)) {
-				report_mount(fp, &d, NULL, form, XFS_USER_QUOTA,
-						mount, flags);
-				flags |= NO_HEADER_FLAG;
-			}
-		}
-	} else if (get_dquot(&d, id, &oid, XFS_USER_QUOTA, mount->fs_name,
-				flags|GETNEXTQUOTA_FLAG)) {
-		report_mount(fp, &d, NULL, form, XFS_USER_QUOTA, mount,
-			flags|GETNEXTQUOTA_FLAG);
+	while (get_dquot(&d, id, &oid, XFS_USER_QUOTA,
+				mount->fs_name, flags | GETNEXTQUOTA_FLAG) &&
+			!(upper && (d.d_id > upper))) {
+		report_mount(fp, &d, NULL, form, XFS_USER_QUOTA, mount, flags);
 		id = oid + 1;
 		flags |= GETNEXTQUOTA_FLAG;
 		flags |= NO_HEADER_FLAG;
-		while (get_dquot(&d, id, &oid, XFS_USER_QUOTA, mount->fs_name,
-				flags)) {
-			report_mount(fp, &d, NULL, form, XFS_USER_QUOTA,
-				mount, flags);
-			id = oid + 1;
-		}
-	} else {
+	}
+
+	/* No GETNEXTQUOTA support, iterate over all from password file */
+	if (!(flags & GETNEXTQUOTA_FLAG)) {
 		setpwent();
 		while ((u = getpwent()) != NULL) {
 			if (get_dquot(&d, u->pw_uid, NULL, XFS_USER_QUOTA,
@@ -524,31 +502,19 @@ report_group_mount(
 {
 	struct group	*g;
 	struct fs_disk_quota	d;
-	uint		id = 0, oid;
+	uint		id = lower, oid;
 
-	if (upper) {	/* identifier range specified */
-		for (id = lower; id <= upper; id++) {
-			if (get_dquot(&d, id, NULL, XFS_GROUP_QUOTA,
-						mount->fs_name, flags)) {
-				report_mount(fp, &d, NULL, form,
-						XFS_GROUP_QUOTA, mount, flags);
-				flags |= NO_HEADER_FLAG;
-			}
-		}
-	} else if (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
-				mount->fs_name, flags|GETNEXTQUOTA_FLAG)) {
-		report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount,
-				flags|GETNEXTQUOTA_FLAG);
+	while (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
+				mount->fs_name, flags | GETNEXTQUOTA_FLAG) &&
+			!(upper && (oid > upper))) {
+		report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount, flags);
 		id = oid + 1;
 		flags |= GETNEXTQUOTA_FLAG;
 		flags |= NO_HEADER_FLAG;
-		while (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
-					mount->fs_name, flags)) {
-			report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount,
-					flags);
-			id = oid + 1;
-		}
-	} else {
+	}
+
+	/* No GETNEXTQUOTA support, iterate over all from password file */
+	if (!(flags & GETNEXTQUOTA_FLAG)) {
 		setgrent();
 		while ((g = getgrent()) != NULL) {
 			if (get_dquot(&d, g->gr_gid, NULL, XFS_GROUP_QUOTA,
@@ -575,31 +541,19 @@ report_project_mount(
 {
 	fs_project_t	*p;
 	struct fs_disk_quota	d;
-	uint		id = 0, oid;
+	uint		id = lower, oid;
 
-	if (upper) {	/* identifier range specified */
-		for (id = lower; id <= upper; id++) {
-			if (get_dquot(&d, id, NULL, XFS_PROJ_QUOTA,
-						mount->fs_name, flags)) {
-				report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA,
-						mount, flags);
-				flags |= NO_HEADER_FLAG;
-			}
-		}
-	} else if (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
-				mount->fs_name, flags|GETNEXTQUOTA_FLAG)) {
-		report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount,
-				flags|GETNEXTQUOTA_FLAG);
+	while (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
+				mount->fs_name, flags | GETNEXTQUOTA_FLAG) &&
+			!(upper && (d.d_id > upper))) {
+		report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount, flags);
 		id = oid + 1;
 		flags |= GETNEXTQUOTA_FLAG;
 		flags |= NO_HEADER_FLAG;
-		while (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
-					mount->fs_name, flags)) {
-			report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount,
-					flags);
-			id = oid + 1;
-		}
-	} else {
+	}
+
+	/* No GETNEXTQUOTA support, iterate over all */
+	if (!(flags & GETNEXTQUOTA_FLAG)) {
 		if (!getprprid(0)) {
 			/*
 			 * Print default project quota, even if projid 0
-- 
2.27.0


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

* [PATCH v2 5/5] xfs_quota: apply -L/-U range limits in uid/gid/pid loops
  2022-04-20 14:45 [PATCH v2 0/5] xfsprogs: optimize -L/-U range calls for xfs_quota's dump/report Andrey Albershteyn
                   ` (3 preceding siblings ...)
  2022-04-20 14:45 ` [PATCH v2 4/5] xfs_quota: utilize XFS_GETNEXTQUOTA for ranged calls in report/dump Andrey Albershteyn
@ 2022-04-20 14:45 ` Andrey Albershteyn
  2022-04-24  5:29   ` Christoph Hellwig
  2022-04-25 16:33   ` Darrick J. Wong
  4 siblings, 2 replies; 16+ messages in thread
From: Andrey Albershteyn @ 2022-04-20 14:45 UTC (permalink / raw)
  To: linux-xfs; +Cc: Andrey Albershteyn

In case kernel doesn't support XFS_GETNEXTQUOTA the report/dump
command will fallback to iterating over all known uid/gid/pid.
However, currently it won't take -L/-U range limits into account
(all entities with non-zero qoutas will be outputted). This applies
those limits for fallback case.

Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
---
 quota/report.c | 40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/quota/report.c b/quota/report.c
index 65d931f3..8af763e4 100644
--- a/quota/report.c
+++ b/quota/report.c
@@ -161,9 +161,11 @@ dump_limits_any_type(
 			struct group *g;
 			setgrent();
 			while ((g = getgrent()) != NULL) {
-				get_dquot(&d, g->gr_gid, NULL, type,
-						mount->fs_name, 0);
-				dump_file(fp, &d, mount->fs_name);
+				if (get_dquot(&d, g->gr_gid, NULL, type,
+							mount->fs_name, 0) &&
+						!(lower && (d.d_id < lower)) &&
+						!(upper && (d.d_id > upper)))
+					dump_file(fp, &d, mount->fs_name);
 			}
 			endgrent();
 			break;
@@ -172,9 +174,11 @@ dump_limits_any_type(
 			struct fs_project *p;
 			setprent();
 			while ((p = getprent()) != NULL) {
-				get_dquot(&d, p->pr_prid, NULL, type,
-						mount->fs_name, 0);
-				dump_file(fp, &d, mount->fs_name);
+				if (get_dquot(&d, p->pr_prid, NULL, type,
+							mount->fs_name, 0) &&
+						!(lower && (d.d_id < lower)) &&
+						!(upper && (d.d_id > upper)))
+					dump_file(fp, &d, mount->fs_name);
 			}
 			endprent();
 			break;
@@ -183,9 +187,11 @@ dump_limits_any_type(
 			struct passwd *u;
 			setpwent();
 			while ((u = getpwent()) != NULL) {
-				get_dquot(&d, u->pw_uid, NULL, type,
-						mount->fs_name, 0);
-				dump_file(fp, &d, mount->fs_name);
+				if (get_dquot(&d, u->pw_uid, NULL, type,
+							mount->fs_name, 0) &&
+						!(lower && (d.d_id < lower)) &&
+						!(upper && (d.d_id > upper)))
+					dump_file(fp, &d, mount->fs_name);
 			}
 			endpwent();
 			break;
@@ -478,7 +484,9 @@ report_user_mount(
 		setpwent();
 		while ((u = getpwent()) != NULL) {
 			if (get_dquot(&d, u->pw_uid, NULL, XFS_USER_QUOTA,
-						mount->fs_name, flags)) {
+						mount->fs_name, flags) &&
+					!(lower && (d.d_id < lower)) &&
+					!(upper && (d.d_id > upper))) {
 				report_mount(fp, &d, u->pw_name, form,
 						XFS_USER_QUOTA, mount, flags);
 				flags |= NO_HEADER_FLAG;
@@ -518,7 +526,9 @@ report_group_mount(
 		setgrent();
 		while ((g = getgrent()) != NULL) {
 			if (get_dquot(&d, g->gr_gid, NULL, XFS_GROUP_QUOTA,
-						mount->fs_name, flags)) {
+						mount->fs_name, flags) &&
+					!(lower && (d.d_id < lower)) &&
+					!(upper && (d.d_id > upper))) {
 				report_mount(fp, &d, g->gr_name, form,
 						XFS_GROUP_QUOTA, mount, flags);
 				flags |= NO_HEADER_FLAG;
@@ -560,7 +570,9 @@ report_project_mount(
 			 * isn't defined
 			 */
 			if (get_dquot(&d, 0, NULL, XFS_PROJ_QUOTA,
-						mount->fs_name, flags)) {
+						mount->fs_name, flags) &&
+					!(lower && (d.d_id < lower)) &&
+					!(upper && (d.d_id > upper))) {
 				report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA,
 						mount, flags);
 				flags |= NO_HEADER_FLAG;
@@ -570,7 +582,9 @@ report_project_mount(
 		setprent();
 		while ((p = getprent()) != NULL) {
 			if (get_dquot(&d, p->pr_prid, NULL, XFS_PROJ_QUOTA,
-						mount->fs_name, flags)) {
+						mount->fs_name, flags) &&
+					!(lower && (d.d_id < lower)) &&
+					!(upper && (d.d_id > upper))) {
 				report_mount(fp, &d, p->pr_name, form,
 						XFS_PROJ_QUOTA, mount, flags);
 				flags |= NO_HEADER_FLAG;
-- 
2.27.0


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

* Re: [PATCH v2 2/5] xfs_quota: separate get_dquot() and dump_file()
  2022-04-20 14:45 ` [PATCH v2 2/5] xfs_quota: separate get_dquot() and dump_file() Andrey Albershteyn
@ 2022-04-24  5:29   ` Christoph Hellwig
  2022-04-25 16:28   ` Darrick J. Wong
  1 sibling, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2022-04-24  5:29 UTC (permalink / raw)
  To: Andrey Albershteyn; +Cc: linux-xfs

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH v2 3/5] xfs_quota: separate get_dquot() and report_mount()
  2022-04-20 14:45 ` [PATCH v2 3/5] xfs_quota: separate get_dquot() and report_mount() Andrey Albershteyn
@ 2022-04-24  5:29   ` Christoph Hellwig
  2022-04-25 16:29   ` Darrick J. Wong
  1 sibling, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2022-04-24  5:29 UTC (permalink / raw)
  To: Andrey Albershteyn; +Cc: linux-xfs

On Wed, Apr 20, 2022 at 04:45:06PM +0200, Andrey Albershteyn wrote:
> Separate quota info acquisition from outputting. This allows upper
> functions to filter obtained info (e.g. within specific ID range).

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH v2 5/5] xfs_quota: apply -L/-U range limits in uid/gid/pid loops
  2022-04-20 14:45 ` [PATCH v2 5/5] xfs_quota: apply -L/-U range limits in uid/gid/pid loops Andrey Albershteyn
@ 2022-04-24  5:29   ` Christoph Hellwig
  2022-04-25 16:33   ` Darrick J. Wong
  1 sibling, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2022-04-24  5:29 UTC (permalink / raw)
  To: Andrey Albershteyn; +Cc: linux-xfs

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH v2 1/5] xfs_quota: separate quota info acquisition into get_dquot()
  2022-04-20 14:45 ` [PATCH v2 1/5] xfs_quota: separate quota info acquisition into get_dquot() Andrey Albershteyn
@ 2022-04-25 16:22   ` Darrick J. Wong
  0 siblings, 0 replies; 16+ messages in thread
From: Darrick J. Wong @ 2022-04-25 16:22 UTC (permalink / raw)
  To: Andrey Albershteyn; +Cc: linux-xfs, Christoph Hellwig

On Wed, Apr 20, 2022 at 04:45:04PM +0200, Andrey Albershteyn wrote:
> Both report_mount() and dump_file() have identical code to get quota
> information. This could be used for further separation of the
> functions.
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  quota/report.c | 49 +++++++++++++++++++++++--------------------------
>  1 file changed, 23 insertions(+), 26 deletions(-)
> 
> diff --git a/quota/report.c b/quota/report.c
> index 2eb5b5a9..cccc654e 100644
> --- a/quota/report.c
> +++ b/quota/report.c
> @@ -59,16 +59,15 @@ report_help(void)
>  "\n"));
>  }
>  
> -static int 
> -dump_file(
> -	FILE		*fp,
> +static int
> +get_dquot(
> +	struct fs_disk_quota *d,
>  	uint		id,
>  	uint		*oid,
>  	uint		type,
>  	char		*dev,
>  	int		flags)
>  {
> -	fs_disk_quota_t	d;
>  	int		cmd;
>  
>  	if (flags & GETNEXTQUOTA_FLAG)
> @@ -77,7 +76,7 @@ dump_file(
>  		cmd = XFS_GETQUOTA;
>  
>  	/* Fall back silently if XFS_GETNEXTQUOTA fails, warn on XFS_GETQUOTA */
> -	if (xfsquotactl(cmd, dev, type, id, (void *)&d) < 0) {
> +	if (xfsquotactl(cmd, dev, type, id, (void *)d) < 0) {
>  		if (errno != ENOENT && errno != ENOSYS && errno != ESRCH &&
>  		    cmd == XFS_GETQUOTA)
>  			perror("XFS_GETQUOTA");
> @@ -85,12 +84,29 @@ dump_file(
>  	}
>  
>  	if (oid) {
> -		*oid = d.d_id;
> +		*oid = d->d_id;
>  		/* Did kernelspace wrap? */
>  		if (*oid < id)
>  			return 0;
>  	}
>  
> +	return 1;
> +}
> +
> +static int
> +dump_file(
> +	FILE		*fp,
> +	uint		id,
> +	uint		*oid,
> +	uint		type,
> +	char		*dev,
> +	int		flags)
> +{
> +	fs_disk_quota_t	d;

struct fs_disk_quota	d;

With that nit fixed,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> +
> +	if (!get_dquot(&d, id, oid, type, dev, flags))
> +		return 0;
> +
>  	if (!d.d_blk_softlimit && !d.d_blk_hardlimit &&
>  	    !d.d_ino_softlimit && !d.d_ino_hardlimit &&
>  	    !d.d_rtb_softlimit && !d.d_rtb_hardlimit)
> @@ -329,31 +345,12 @@ report_mount(
>  {
>  	fs_disk_quota_t	d;
>  	time64_t	timer;
> -	char		*dev = mount->fs_name;
>  	char		c[8], h[8], s[8];
>  	uint		qflags;
>  	int		count;
> -	int		cmd;
>  
> -	if (flags & GETNEXTQUOTA_FLAG)
> -		cmd = XFS_GETNEXTQUOTA;
> -	else
> -		cmd = XFS_GETQUOTA;
> -
> -	/* Fall back silently if XFS_GETNEXTQUOTA fails, warn on XFS_GETQUOTA*/
> -	if (xfsquotactl(cmd, dev, type, id, (void *)&d) < 0) {
> -		if (errno != ENOENT && errno != ENOSYS && errno != ESRCH &&
> -		    cmd == XFS_GETQUOTA)
> -			perror("XFS_GETQUOTA");
> +	if (!get_dquot(&d, id, oid, type, mount->fs_name, flags))
>  		return 0;
> -	}
> -
> -	if (oid) {
> -		*oid = d.d_id;
> -		/* Did kernelspace wrap? */
> -		if (* oid < id)
> -			return 0;
> -	}
>  
>  	if (flags & TERSE_FLAG) {
>  		count = 0;
> -- 
> 2.27.0
> 

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

* Re: [PATCH v2 2/5] xfs_quota: separate get_dquot() and dump_file()
  2022-04-20 14:45 ` [PATCH v2 2/5] xfs_quota: separate get_dquot() and dump_file() Andrey Albershteyn
  2022-04-24  5:29   ` Christoph Hellwig
@ 2022-04-25 16:28   ` Darrick J. Wong
  2022-04-28 12:43     ` Andrey Albershteyn
  1 sibling, 1 reply; 16+ messages in thread
From: Darrick J. Wong @ 2022-04-25 16:28 UTC (permalink / raw)
  To: Andrey Albershteyn; +Cc: linux-xfs

On Wed, Apr 20, 2022 at 04:45:05PM +0200, Andrey Albershteyn wrote:
> Separate quota info acquisition from outputting it to file. This
> allows upper functions to filter obtained info (e.g. within specific
> ID range).
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> ---
>  quota/report.c | 86 ++++++++++++++++++++++++++------------------------
>  1 file changed, 45 insertions(+), 41 deletions(-)
> 
> diff --git a/quota/report.c b/quota/report.c
> index cccc654e..d5c6f84f 100644
> --- a/quota/report.c
> +++ b/quota/report.c
> @@ -96,39 +96,31 @@ get_dquot(
>  static int
>  dump_file(

I kinda wonder if this ought to be named 'dump_dquot' since it's not
really dumping a file or even the dquots of a specifc file.  OTOH, the
rest of the utility seems to have similar naming conventions for "report
something to a FILE* stream" so

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>  	FILE		*fp,
> -	uint		id,
> -	uint		*oid,
> -	uint		type,
> -	char		*dev,
> -	int		flags)
> +	struct fs_disk_quota *d,
> +	char		*dev)
>  {
> -	fs_disk_quota_t	d;
> -
> -	if (!get_dquot(&d, id, oid, type, dev, flags))
> -		return 0;
> -
> -	if (!d.d_blk_softlimit && !d.d_blk_hardlimit &&
> -	    !d.d_ino_softlimit && !d.d_ino_hardlimit &&
> -	    !d.d_rtb_softlimit && !d.d_rtb_hardlimit)
> +	if (!d->d_blk_softlimit && !d->d_blk_hardlimit &&
> +	    !d->d_ino_softlimit && !d->d_ino_hardlimit &&
> +	    !d->d_rtb_softlimit && !d->d_rtb_hardlimit)
>  		return 1;
>  	fprintf(fp, "fs = %s\n", dev);
>  	/* this branch is for backward compatibility reasons */
> -	if (d.d_rtb_softlimit || d.d_rtb_hardlimit)
> +	if (d->d_rtb_softlimit || d->d_rtb_hardlimit)
>  		fprintf(fp, "%-10d %7llu %7llu %7llu %7llu %7llu %7llu\n",
> -			d.d_id,
> -			(unsigned long long)d.d_blk_softlimit,
> -			(unsigned long long)d.d_blk_hardlimit,
> -			(unsigned long long)d.d_ino_softlimit,
> -			(unsigned long long)d.d_ino_hardlimit,
> -			(unsigned long long)d.d_rtb_softlimit,
> -			(unsigned long long)d.d_rtb_hardlimit);
> +			d->d_id,
> +			(unsigned long long)d->d_blk_softlimit,
> +			(unsigned long long)d->d_blk_hardlimit,
> +			(unsigned long long)d->d_ino_softlimit,
> +			(unsigned long long)d->d_ino_hardlimit,
> +			(unsigned long long)d->d_rtb_softlimit,
> +			(unsigned long long)d->d_rtb_hardlimit);
>  	else
>  		fprintf(fp, "%-10d %7llu %7llu %7llu %7llu\n",
> -			d.d_id,
> -			(unsigned long long)d.d_blk_softlimit,
> -			(unsigned long long)d.d_blk_hardlimit,
> -			(unsigned long long)d.d_ino_softlimit,
> -			(unsigned long long)d.d_ino_hardlimit);
> +			d->d_id,
> +			(unsigned long long)d->d_blk_softlimit,
> +			(unsigned long long)d->d_blk_hardlimit,
> +			(unsigned long long)d->d_ino_softlimit,
> +			(unsigned long long)d->d_ino_hardlimit);
>  
>  	return 1;
>  }
> @@ -142,6 +134,7 @@ dump_limits_any_type(
>  	uint		upper)
>  {
>  	fs_path_t	*mount;
> +	struct fs_disk_quota d;
>  	uint		id = 0, oid;
>  
>  	if ((mount = fs_table_lookup(dir, FS_MOUNT_POINT)) == NULL) {
> @@ -153,46 +146,57 @@ dump_limits_any_type(
>  
>  	/* Range was specified; query everything in it */
>  	if (upper) {
> -		for (id = lower; id <= upper; id++)
> -			dump_file(fp, id, NULL, type, mount->fs_name, 0);
> +		for (id = lower; id <= upper; id++) {
> +			get_dquot(&d, id, &oid, type, mount->fs_name, 0);
> +			dump_file(fp, &d, mount->fs_name);
> +		}
>  		return;
>  	}
>  
>  	/* Use GETNEXTQUOTA if it's available */
> -	if (dump_file(fp, id, &oid, type, mount->fs_name, GETNEXTQUOTA_FLAG)) {
> +	if (get_dquot(&d, id, &oid, type, mount->fs_name, GETNEXTQUOTA_FLAG)) {
> +		dump_file(fp, &d, mount->fs_name);
>  		id = oid + 1;
> -		while (dump_file(fp, id, &oid, type, mount->fs_name,
> -				 GETNEXTQUOTA_FLAG))
> +		while (get_dquot(&d, id, &oid, type, mount->fs_name,
> +					GETNEXTQUOTA_FLAG)) {
> +			dump_file(fp, &d, mount->fs_name);
>  			id = oid + 1;
> +		}
>  		return;
> -        }
> +	}
>  
>  	/* Otherwise fall back to iterating over each uid/gid/prjid */
>  	switch (type) {
>  	case XFS_GROUP_QUOTA: {
>  			struct group *g;
>  			setgrent();
> -			while ((g = getgrent()) != NULL)
> -				dump_file(fp, g->gr_gid, NULL, type,
> -					  mount->fs_name, 0);
> +			while ((g = getgrent()) != NULL) {
> +				get_dquot(&d, g->gr_gid, NULL, type,
> +						mount->fs_name, 0);
> +				dump_file(fp, &d, mount->fs_name);
> +			}
>  			endgrent();
>  			break;
>  		}
>  	case XFS_PROJ_QUOTA: {
>  			struct fs_project *p;
>  			setprent();
> -			while ((p = getprent()) != NULL)
> -				dump_file(fp, p->pr_prid, NULL, type,
> -					  mount->fs_name, 0);
> +			while ((p = getprent()) != NULL) {
> +				get_dquot(&d, p->pr_prid, NULL, type,
> +						mount->fs_name, 0);
> +				dump_file(fp, &d, mount->fs_name);
> +			}
>  			endprent();
>  			break;
>  		}
>  	case XFS_USER_QUOTA: {
>  			struct passwd *u;
>  			setpwent();
> -			while ((u = getpwent()) != NULL)
> -				dump_file(fp, u->pw_uid, NULL, type,
> -					  mount->fs_name, 0);
> +			while ((u = getpwent()) != NULL) {
> +				get_dquot(&d, u->pw_uid, NULL, type,
> +						mount->fs_name, 0);
> +				dump_file(fp, &d, mount->fs_name);
> +			}
>  			endpwent();
>  			break;
>  		}
> -- 
> 2.27.0
> 

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

* Re: [PATCH v2 3/5] xfs_quota: separate get_dquot() and report_mount()
  2022-04-20 14:45 ` [PATCH v2 3/5] xfs_quota: separate get_dquot() and report_mount() Andrey Albershteyn
  2022-04-24  5:29   ` Christoph Hellwig
@ 2022-04-25 16:29   ` Darrick J. Wong
  1 sibling, 0 replies; 16+ messages in thread
From: Darrick J. Wong @ 2022-04-25 16:29 UTC (permalink / raw)
  To: Andrey Albershteyn; +Cc: linux-xfs

On Wed, Apr 20, 2022 at 04:45:06PM +0200, Andrey Albershteyn wrote:
> Separate quota info acquisition from outputting. This allows upper
> functions to filter obtained info (e.g. within specific ID range).
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  quota/report.c | 178 ++++++++++++++++++++++++++++---------------------
>  1 file changed, 103 insertions(+), 75 deletions(-)
> 
> diff --git a/quota/report.c b/quota/report.c
> index d5c6f84f..8ca154f0 100644
> --- a/quota/report.c
> +++ b/quota/report.c
> @@ -339,30 +339,25 @@ report_header(
>  static int
>  report_mount(
>  	FILE		*fp,
> -	uint32_t	id,
> +	struct fs_disk_quota *d,
>  	char		*name,
> -	uint32_t	*oid,
>  	uint		form,
>  	uint		type,
>  	fs_path_t	*mount,
>  	uint		flags)
>  {
> -	fs_disk_quota_t	d;
>  	time64_t	timer;
>  	char		c[8], h[8], s[8];
>  	uint		qflags;
>  	int		count;
>  
> -	if (!get_dquot(&d, id, oid, type, mount->fs_name, flags))
> -		return 0;
> -
>  	if (flags & TERSE_FLAG) {
>  		count = 0;
> -		if ((form & XFS_BLOCK_QUOTA) && d.d_bcount)
> +		if ((form & XFS_BLOCK_QUOTA) && d->d_bcount)
>  			count++;
> -		if ((form & XFS_INODE_QUOTA) && d.d_icount)
> +		if ((form & XFS_INODE_QUOTA) && d->d_icount)
>  			count++;
> -		if ((form & XFS_RTBLOCK_QUOTA) && d.d_rtbcount)
> +		if ((form & XFS_RTBLOCK_QUOTA) && d->d_rtbcount)
>  			count++;
>  		if (!count)
>  			return 0;
> @@ -372,19 +367,19 @@ report_mount(
>  		report_header(fp, form, type, mount, flags);
>  
>  	if (flags & NO_LOOKUP_FLAG) {
> -		fprintf(fp, "#%-10u", d.d_id);
> +		fprintf(fp, "#%-10u", d->d_id);
>  	} else {
>  		if (name == NULL) {
>  			if (type == XFS_USER_QUOTA) {
> -				struct passwd	*u = getpwuid(d.d_id);
> +				struct passwd	*u = getpwuid(d->d_id);
>  				if (u)
>  					name = u->pw_name;
>  			} else if (type == XFS_GROUP_QUOTA) {
> -				struct group	*g = getgrgid(d.d_id);
> +				struct group	*g = getgrgid(d->d_id);
>  				if (g)
>  					name = g->gr_name;
>  			} else if (type == XFS_PROJ_QUOTA) {
> -				fs_project_t	*p = getprprid(d.d_id);
> +				fs_project_t	*p = getprprid(d->d_id);
>  				if (p)
>  					name = p->pr_name;
>  			}
> @@ -393,73 +388,73 @@ report_mount(
>  		if (name != NULL)
>  			fprintf(fp, "%-10s", name);
>  		else
> -			fprintf(fp, "#%-9u", d.d_id);
> +			fprintf(fp, "#%-9u", d->d_id);
>  	}
>  
>  	if (form & XFS_BLOCK_QUOTA) {
> -		timer = decode_timer(&d, d.d_btimer, d.d_btimer_hi);
> +		timer = decode_timer(d, d->d_btimer, d->d_btimer_hi);
>  		qflags = (flags & HUMAN_FLAG);
> -		if (d.d_blk_hardlimit && d.d_bcount > d.d_blk_hardlimit)
> +		if (d->d_blk_hardlimit && d->d_bcount > d->d_blk_hardlimit)
>  			qflags |= LIMIT_FLAG;
> -		if (d.d_blk_softlimit && d.d_bcount > d.d_blk_softlimit)
> +		if (d->d_blk_softlimit && d->d_bcount > d->d_blk_softlimit)
>  			qflags |= QUOTA_FLAG;
>  		if (flags & HUMAN_FLAG)
>  			fprintf(fp, " %6s %6s %6s  %02d %8s",
> -				bbs_to_string(d.d_bcount, c, sizeof(c)),
> -				bbs_to_string(d.d_blk_softlimit, s, sizeof(s)),
> -				bbs_to_string(d.d_blk_hardlimit, h, sizeof(h)),
> -				d.d_bwarns,
> +				bbs_to_string(d->d_bcount, c, sizeof(c)),
> +				bbs_to_string(d->d_blk_softlimit, s, sizeof(s)),
> +				bbs_to_string(d->d_blk_hardlimit, h, sizeof(h)),
> +				d->d_bwarns,
>  				time_to_string(timer, qflags));
>  		else
>  			fprintf(fp, " %10llu %10llu %10llu     %02d %9s",
> -				(unsigned long long)d.d_bcount >> 1,
> -				(unsigned long long)d.d_blk_softlimit >> 1,
> -				(unsigned long long)d.d_blk_hardlimit >> 1,
> -				d.d_bwarns,
> +				(unsigned long long)d->d_bcount >> 1,
> +				(unsigned long long)d->d_blk_softlimit >> 1,
> +				(unsigned long long)d->d_blk_hardlimit >> 1,
> +				d->d_bwarns,
>  				time_to_string(timer, qflags));
>  	}
>  	if (form & XFS_INODE_QUOTA) {
> -		timer = decode_timer(&d, d.d_itimer, d.d_itimer_hi);
> +		timer = decode_timer(d, d->d_itimer, d->d_itimer_hi);
>  		qflags = (flags & HUMAN_FLAG);
> -		if (d.d_ino_hardlimit && d.d_icount > d.d_ino_hardlimit)
> +		if (d->d_ino_hardlimit && d->d_icount > d->d_ino_hardlimit)
>  			qflags |= LIMIT_FLAG;
> -		if (d.d_ino_softlimit && d.d_icount > d.d_ino_softlimit)
> +		if (d->d_ino_softlimit && d->d_icount > d->d_ino_softlimit)
>  			qflags |= QUOTA_FLAG;
>  		if (flags & HUMAN_FLAG)
>  			fprintf(fp, " %6s %6s %6s  %02d %8s",
> -				num_to_string(d.d_icount, c, sizeof(c)),
> -				num_to_string(d.d_ino_softlimit, s, sizeof(s)),
> -				num_to_string(d.d_ino_hardlimit, h, sizeof(h)),
> -				d.d_iwarns,
> +				num_to_string(d->d_icount, c, sizeof(c)),
> +				num_to_string(d->d_ino_softlimit, s, sizeof(s)),
> +				num_to_string(d->d_ino_hardlimit, h, sizeof(h)),
> +				d->d_iwarns,
>  				time_to_string(timer, qflags));
>  		else
>  			fprintf(fp, " %10llu %10llu %10llu     %02d %9s",
> -				(unsigned long long)d.d_icount,
> -				(unsigned long long)d.d_ino_softlimit,
> -				(unsigned long long)d.d_ino_hardlimit,
> -				d.d_iwarns,
> +				(unsigned long long)d->d_icount,
> +				(unsigned long long)d->d_ino_softlimit,
> +				(unsigned long long)d->d_ino_hardlimit,
> +				d->d_iwarns,
>  				time_to_string(timer, qflags));
>  	}
>  	if (form & XFS_RTBLOCK_QUOTA) {
> -		timer = decode_timer(&d, d.d_rtbtimer, d.d_rtbtimer_hi);
> +		timer = decode_timer(d, d->d_rtbtimer, d->d_rtbtimer_hi);
>  		qflags = (flags & HUMAN_FLAG);
> -		if (d.d_rtb_hardlimit && d.d_rtbcount > d.d_rtb_hardlimit)
> +		if (d->d_rtb_hardlimit && d->d_rtbcount > d->d_rtb_hardlimit)
>  			qflags |= LIMIT_FLAG;
> -		if (d.d_rtb_softlimit && d.d_rtbcount > d.d_rtb_softlimit)
> +		if (d->d_rtb_softlimit && d->d_rtbcount > d->d_rtb_softlimit)
>  			qflags |= QUOTA_FLAG;
>  		if (flags & HUMAN_FLAG)
>  			fprintf(fp, " %6s %6s %6s  %02d %8s",
> -				bbs_to_string(d.d_rtbcount, c, sizeof(c)),
> -				bbs_to_string(d.d_rtb_softlimit, s, sizeof(s)),
> -				bbs_to_string(d.d_rtb_hardlimit, h, sizeof(h)),
> -				d.d_rtbwarns,
> +				bbs_to_string(d->d_rtbcount, c, sizeof(c)),
> +				bbs_to_string(d->d_rtb_softlimit, s, sizeof(s)),
> +				bbs_to_string(d->d_rtb_hardlimit, h, sizeof(h)),
> +				d->d_rtbwarns,
>  				time_to_string(timer, qflags));
>  		else
>  			fprintf(fp, " %10llu %10llu %10llu     %02d %9s",
> -				(unsigned long long)d.d_rtbcount >> 1,
> -				(unsigned long long)d.d_rtb_softlimit >> 1,
> -				(unsigned long long)d.d_rtb_hardlimit >> 1,
> -				d.d_rtbwarns,
> +				(unsigned long long)d->d_rtbcount >> 1,
> +				(unsigned long long)d->d_rtb_softlimit >> 1,
> +				(unsigned long long)d->d_rtb_hardlimit >> 1,
> +				d->d_rtbwarns,
>  				time_to_string(timer, qflags));
>  	}
>  	fputc('\n', fp);
> @@ -476,30 +471,40 @@ report_user_mount(
>  	uint		flags)
>  {
>  	struct passwd	*u;
> +	struct fs_disk_quota	d;
>  	uint		id = 0, oid;
>  
>  	if (upper) {	/* identifier range specified */
>  		for (id = lower; id <= upper; id++) {
> -			if (report_mount(fp, id, NULL, NULL,
> -					form, XFS_USER_QUOTA, mount, flags))
> +			if (get_dquot(&d, id, NULL, XFS_USER_QUOTA,
> +						mount->fs_name, flags)) {
> +				report_mount(fp, &d, NULL, form, XFS_USER_QUOTA,
> +						mount, flags);
>  				flags |= NO_HEADER_FLAG;
> +			}
>  		}
> -	} else if (report_mount(fp, id, NULL, &oid, form,
> -				XFS_USER_QUOTA, mount,
> +	} else if (get_dquot(&d, id, &oid, XFS_USER_QUOTA, mount->fs_name,
>  				flags|GETNEXTQUOTA_FLAG)) {
> +		report_mount(fp, &d, NULL, form, XFS_USER_QUOTA, mount,
> +			flags|GETNEXTQUOTA_FLAG);
>  		id = oid + 1;
>  		flags |= GETNEXTQUOTA_FLAG;
>  		flags |= NO_HEADER_FLAG;
> -		while (report_mount(fp, id, NULL, &oid, form, XFS_USER_QUOTA,
> -				    mount, flags)) {
> +		while (get_dquot(&d, id, &oid, XFS_USER_QUOTA, mount->fs_name,
> +				flags)) {
> +			report_mount(fp, &d, NULL, form, XFS_USER_QUOTA,
> +				mount, flags);
>  			id = oid + 1;
>  		}
>  	} else {
>  		setpwent();
>  		while ((u = getpwent()) != NULL) {
> -			if (report_mount(fp, u->pw_uid, u->pw_name, NULL,
> -					form, XFS_USER_QUOTA, mount, flags))
> +			if (get_dquot(&d, u->pw_uid, NULL, XFS_USER_QUOTA,
> +						mount->fs_name, flags)) {
> +				report_mount(fp, &d, u->pw_name, form,
> +						XFS_USER_QUOTA, mount, flags);
>  				flags |= NO_HEADER_FLAG;
> +			}
>  		}
>  		endpwent();
>  	}
> @@ -518,30 +523,40 @@ report_group_mount(
>  	uint		flags)
>  {
>  	struct group	*g;
> +	struct fs_disk_quota	d;
>  	uint		id = 0, oid;
>  
>  	if (upper) {	/* identifier range specified */
>  		for (id = lower; id <= upper; id++) {
> -			if (report_mount(fp, id, NULL, NULL,
> -					form, XFS_GROUP_QUOTA, mount, flags))
> +			if (get_dquot(&d, id, NULL, XFS_GROUP_QUOTA,
> +						mount->fs_name, flags)) {
> +				report_mount(fp, &d, NULL, form,
> +						XFS_GROUP_QUOTA, mount, flags);
>  				flags |= NO_HEADER_FLAG;
> +			}
>  		}
> -	} else if (report_mount(fp, id, NULL, &oid, form,
> -				XFS_GROUP_QUOTA, mount,
> -				flags|GETNEXTQUOTA_FLAG)) {
> +	} else if (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
> +				mount->fs_name, flags|GETNEXTQUOTA_FLAG)) {
> +		report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount,
> +				flags|GETNEXTQUOTA_FLAG);
>  		id = oid + 1;
>  		flags |= GETNEXTQUOTA_FLAG;
>  		flags |= NO_HEADER_FLAG;
> -		while (report_mount(fp, id, NULL, &oid, form, XFS_GROUP_QUOTA,
> -				    mount, flags)) {
> +		while (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
> +					mount->fs_name, flags)) {
> +			report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount,
> +					flags);
>  			id = oid + 1;
>  		}
>  	} else {
>  		setgrent();
>  		while ((g = getgrent()) != NULL) {
> -			if (report_mount(fp, g->gr_gid, g->gr_name, NULL,
> -					form, XFS_GROUP_QUOTA, mount, flags))
> +			if (get_dquot(&d, g->gr_gid, NULL, XFS_GROUP_QUOTA,
> +						mount->fs_name, flags)) {
> +				report_mount(fp, &d, g->gr_name, form,
> +						XFS_GROUP_QUOTA, mount, flags);
>  				flags |= NO_HEADER_FLAG;
> +			}
>  		}
>  	}
>  	if (flags & NO_HEADER_FLAG)
> @@ -559,22 +574,29 @@ report_project_mount(
>  	uint		flags)
>  {
>  	fs_project_t	*p;
> +	struct fs_disk_quota	d;
>  	uint		id = 0, oid;
>  
>  	if (upper) {	/* identifier range specified */
>  		for (id = lower; id <= upper; id++) {
> -			if (report_mount(fp, id, NULL, NULL,
> -					form, XFS_PROJ_QUOTA, mount, flags))
> +			if (get_dquot(&d, id, NULL, XFS_PROJ_QUOTA,
> +						mount->fs_name, flags)) {
> +				report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA,
> +						mount, flags);
>  				flags |= NO_HEADER_FLAG;
> +			}
>  		}
> -	} else if (report_mount(fp, id, NULL, &oid, form,
> -				XFS_PROJ_QUOTA, mount,
> -				flags|GETNEXTQUOTA_FLAG)) {
> +	} else if (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
> +				mount->fs_name, flags|GETNEXTQUOTA_FLAG)) {
> +		report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount,
> +				flags|GETNEXTQUOTA_FLAG);
>  		id = oid + 1;
>  		flags |= GETNEXTQUOTA_FLAG;
>  		flags |= NO_HEADER_FLAG;
> -		while (report_mount(fp, id, NULL, &oid, form, XFS_PROJ_QUOTA,
> -				    mount, flags)) {
> +		while (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
> +					mount->fs_name, flags)) {
> +			report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount,
> +					flags);
>  			id = oid + 1;
>  		}
>  	} else {
> @@ -583,16 +605,22 @@ report_project_mount(
>  			 * Print default project quota, even if projid 0
>  			 * isn't defined
>  			 */
> -			if (report_mount(fp, 0, NULL, NULL,
> -					form, XFS_PROJ_QUOTA, mount, flags))
> +			if (get_dquot(&d, 0, NULL, XFS_PROJ_QUOTA,
> +						mount->fs_name, flags)) {
> +				report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA,
> +						mount, flags);
>  				flags |= NO_HEADER_FLAG;
> +			}
>  		}
>  
>  		setprent();
>  		while ((p = getprent()) != NULL) {
> -			if (report_mount(fp, p->pr_prid, p->pr_name, NULL,
> -					form, XFS_PROJ_QUOTA, mount, flags))
> +			if (get_dquot(&d, p->pr_prid, NULL, XFS_PROJ_QUOTA,
> +						mount->fs_name, flags)) {
> +				report_mount(fp, &d, p->pr_name, form,
> +						XFS_PROJ_QUOTA, mount, flags);
>  				flags |= NO_HEADER_FLAG;
> +			}
>  		}
>  		endprent();
>  	}
> -- 
> 2.27.0
> 

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

* Re: [PATCH v2 4/5] xfs_quota: utilize XFS_GETNEXTQUOTA for ranged calls in report/dump
  2022-04-20 14:45 ` [PATCH v2 4/5] xfs_quota: utilize XFS_GETNEXTQUOTA for ranged calls in report/dump Andrey Albershteyn
@ 2022-04-25 16:33   ` Darrick J. Wong
  2022-04-28  8:39     ` Andrey Albershteyn
  0 siblings, 1 reply; 16+ messages in thread
From: Darrick J. Wong @ 2022-04-25 16:33 UTC (permalink / raw)
  To: Andrey Albershteyn; +Cc: linux-xfs, Christoph Hellwig

On Wed, Apr 20, 2022 at 04:45:07PM +0200, Andrey Albershteyn wrote:
> The implementation based on XFS_GETQUOTA call for each ID in range,
> specified with -L/-U, is quite slow for wider ranges.
> 
> If kernel supports XFS_GETNEXTQUOTA, report_*_mount/dump_any_file
> will use that to obtain quota list for the mount. XFS_GETNEXTQUOTA
> returns quota of the requested ID and next ID with non-empty quota.
> 
> Otherwise, XFS_GETQUOTA will be used for each user/group/project ID
> known from password/group/project database.
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  quota/report.c | 116 +++++++++++++++----------------------------------
>  1 file changed, 35 insertions(+), 81 deletions(-)
> 
> diff --git a/quota/report.c b/quota/report.c
> index 8ca154f0..65d931f3 100644
> --- a/quota/report.c
> +++ b/quota/report.c
> @@ -135,7 +135,7 @@ dump_limits_any_type(
>  {
>  	fs_path_t	*mount;
>  	struct fs_disk_quota d;
> -	uint		id = 0, oid;
> +	uint		id = lower, oid, flags = 0;
>  
>  	if ((mount = fs_table_lookup(dir, FS_MOUNT_POINT)) == NULL) {
>  		exitcode = 1;
> @@ -144,27 +144,17 @@ dump_limits_any_type(
>  		return;
>  	}
>  
> -	/* Range was specified; query everything in it */
> -	if (upper) {
> -		for (id = lower; id <= upper; id++) {
> -			get_dquot(&d, id, &oid, type, mount->fs_name, 0);
> -			dump_file(fp, &d, mount->fs_name);
> -		}
> -		return;
> -	}
> -
> -	/* Use GETNEXTQUOTA if it's available */
> -	if (get_dquot(&d, id, &oid, type, mount->fs_name, GETNEXTQUOTA_FLAG)) {
> +	while (get_dquot(&d, id, &oid, type,
> +				mount->fs_name, flags | GETNEXTQUOTA_FLAG) &&
> +			!(upper && (d.d_id > upper))) {
>  		dump_file(fp, &d, mount->fs_name);
>  		id = oid + 1;

Just out of curiosity, could this be "id = d.d_id + 1", and then you
don't have to pass around &oid at all?

--D

> -		while (get_dquot(&d, id, &oid, type, mount->fs_name,
> -					GETNEXTQUOTA_FLAG)) {
> -			dump_file(fp, &d, mount->fs_name);
> -			id = oid + 1;
> -		}
> -		return;
> +		flags |= GETNEXTQUOTA_FLAG;
>  	}
>  
> +	if (flags & GETNEXTQUOTA_FLAG)
> +		return;
> +
>  	/* Otherwise fall back to iterating over each uid/gid/prjid */
>  	switch (type) {
>  	case XFS_GROUP_QUOTA: {
> @@ -472,31 +462,19 @@ report_user_mount(
>  {
>  	struct passwd	*u;
>  	struct fs_disk_quota	d;
> -	uint		id = 0, oid;
> +	uint		id = lower, oid;
>  
> -	if (upper) {	/* identifier range specified */
> -		for (id = lower; id <= upper; id++) {
> -			if (get_dquot(&d, id, NULL, XFS_USER_QUOTA,
> -						mount->fs_name, flags)) {
> -				report_mount(fp, &d, NULL, form, XFS_USER_QUOTA,
> -						mount, flags);
> -				flags |= NO_HEADER_FLAG;
> -			}
> -		}
> -	} else if (get_dquot(&d, id, &oid, XFS_USER_QUOTA, mount->fs_name,
> -				flags|GETNEXTQUOTA_FLAG)) {
> -		report_mount(fp, &d, NULL, form, XFS_USER_QUOTA, mount,
> -			flags|GETNEXTQUOTA_FLAG);
> +	while (get_dquot(&d, id, &oid, XFS_USER_QUOTA,
> +				mount->fs_name, flags | GETNEXTQUOTA_FLAG) &&
> +			!(upper && (d.d_id > upper))) {
> +		report_mount(fp, &d, NULL, form, XFS_USER_QUOTA, mount, flags);
>  		id = oid + 1;
>  		flags |= GETNEXTQUOTA_FLAG;
>  		flags |= NO_HEADER_FLAG;
> -		while (get_dquot(&d, id, &oid, XFS_USER_QUOTA, mount->fs_name,
> -				flags)) {
> -			report_mount(fp, &d, NULL, form, XFS_USER_QUOTA,
> -				mount, flags);
> -			id = oid + 1;
> -		}
> -	} else {
> +	}
> +
> +	/* No GETNEXTQUOTA support, iterate over all from password file */
> +	if (!(flags & GETNEXTQUOTA_FLAG)) {
>  		setpwent();
>  		while ((u = getpwent()) != NULL) {
>  			if (get_dquot(&d, u->pw_uid, NULL, XFS_USER_QUOTA,
> @@ -524,31 +502,19 @@ report_group_mount(
>  {
>  	struct group	*g;
>  	struct fs_disk_quota	d;
> -	uint		id = 0, oid;
> +	uint		id = lower, oid;
>  
> -	if (upper) {	/* identifier range specified */
> -		for (id = lower; id <= upper; id++) {
> -			if (get_dquot(&d, id, NULL, XFS_GROUP_QUOTA,
> -						mount->fs_name, flags)) {
> -				report_mount(fp, &d, NULL, form,
> -						XFS_GROUP_QUOTA, mount, flags);
> -				flags |= NO_HEADER_FLAG;
> -			}
> -		}
> -	} else if (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
> -				mount->fs_name, flags|GETNEXTQUOTA_FLAG)) {
> -		report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount,
> -				flags|GETNEXTQUOTA_FLAG);
> +	while (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
> +				mount->fs_name, flags | GETNEXTQUOTA_FLAG) &&
> +			!(upper && (oid > upper))) {
> +		report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount, flags);
>  		id = oid + 1;
>  		flags |= GETNEXTQUOTA_FLAG;
>  		flags |= NO_HEADER_FLAG;
> -		while (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
> -					mount->fs_name, flags)) {
> -			report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount,
> -					flags);
> -			id = oid + 1;
> -		}
> -	} else {
> +	}
> +
> +	/* No GETNEXTQUOTA support, iterate over all from password file */
> +	if (!(flags & GETNEXTQUOTA_FLAG)) {
>  		setgrent();
>  		while ((g = getgrent()) != NULL) {
>  			if (get_dquot(&d, g->gr_gid, NULL, XFS_GROUP_QUOTA,
> @@ -575,31 +541,19 @@ report_project_mount(
>  {
>  	fs_project_t	*p;
>  	struct fs_disk_quota	d;
> -	uint		id = 0, oid;
> +	uint		id = lower, oid;
>  
> -	if (upper) {	/* identifier range specified */
> -		for (id = lower; id <= upper; id++) {
> -			if (get_dquot(&d, id, NULL, XFS_PROJ_QUOTA,
> -						mount->fs_name, flags)) {
> -				report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA,
> -						mount, flags);
> -				flags |= NO_HEADER_FLAG;
> -			}
> -		}
> -	} else if (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
> -				mount->fs_name, flags|GETNEXTQUOTA_FLAG)) {
> -		report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount,
> -				flags|GETNEXTQUOTA_FLAG);
> +	while (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
> +				mount->fs_name, flags | GETNEXTQUOTA_FLAG) &&
> +			!(upper && (d.d_id > upper))) {
> +		report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount, flags);
>  		id = oid + 1;
>  		flags |= GETNEXTQUOTA_FLAG;
>  		flags |= NO_HEADER_FLAG;
> -		while (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
> -					mount->fs_name, flags)) {
> -			report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount,
> -					flags);
> -			id = oid + 1;
> -		}
> -	} else {
> +	}
> +
> +	/* No GETNEXTQUOTA support, iterate over all */
> +	if (!(flags & GETNEXTQUOTA_FLAG)) {
>  		if (!getprprid(0)) {
>  			/*
>  			 * Print default project quota, even if projid 0
> -- 
> 2.27.0
> 

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

* Re: [PATCH v2 5/5] xfs_quota: apply -L/-U range limits in uid/gid/pid loops
  2022-04-20 14:45 ` [PATCH v2 5/5] xfs_quota: apply -L/-U range limits in uid/gid/pid loops Andrey Albershteyn
  2022-04-24  5:29   ` Christoph Hellwig
@ 2022-04-25 16:33   ` Darrick J. Wong
  1 sibling, 0 replies; 16+ messages in thread
From: Darrick J. Wong @ 2022-04-25 16:33 UTC (permalink / raw)
  To: Andrey Albershteyn; +Cc: linux-xfs

On Wed, Apr 20, 2022 at 04:45:08PM +0200, Andrey Albershteyn wrote:
> In case kernel doesn't support XFS_GETNEXTQUOTA the report/dump
> command will fallback to iterating over all known uid/gid/pid.
> However, currently it won't take -L/-U range limits into account
> (all entities with non-zero qoutas will be outputted). This applies
> those limits for fallback case.
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  quota/report.c | 40 +++++++++++++++++++++++++++-------------
>  1 file changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/quota/report.c b/quota/report.c
> index 65d931f3..8af763e4 100644
> --- a/quota/report.c
> +++ b/quota/report.c
> @@ -161,9 +161,11 @@ dump_limits_any_type(
>  			struct group *g;
>  			setgrent();
>  			while ((g = getgrent()) != NULL) {
> -				get_dquot(&d, g->gr_gid, NULL, type,
> -						mount->fs_name, 0);
> -				dump_file(fp, &d, mount->fs_name);
> +				if (get_dquot(&d, g->gr_gid, NULL, type,
> +							mount->fs_name, 0) &&
> +						!(lower && (d.d_id < lower)) &&
> +						!(upper && (d.d_id > upper)))
> +					dump_file(fp, &d, mount->fs_name);
>  			}
>  			endgrent();
>  			break;
> @@ -172,9 +174,11 @@ dump_limits_any_type(
>  			struct fs_project *p;
>  			setprent();
>  			while ((p = getprent()) != NULL) {
> -				get_dquot(&d, p->pr_prid, NULL, type,
> -						mount->fs_name, 0);
> -				dump_file(fp, &d, mount->fs_name);
> +				if (get_dquot(&d, p->pr_prid, NULL, type,
> +							mount->fs_name, 0) &&
> +						!(lower && (d.d_id < lower)) &&
> +						!(upper && (d.d_id > upper)))
> +					dump_file(fp, &d, mount->fs_name);
>  			}
>  			endprent();
>  			break;
> @@ -183,9 +187,11 @@ dump_limits_any_type(
>  			struct passwd *u;
>  			setpwent();
>  			while ((u = getpwent()) != NULL) {
> -				get_dquot(&d, u->pw_uid, NULL, type,
> -						mount->fs_name, 0);
> -				dump_file(fp, &d, mount->fs_name);
> +				if (get_dquot(&d, u->pw_uid, NULL, type,
> +							mount->fs_name, 0) &&
> +						!(lower && (d.d_id < lower)) &&
> +						!(upper && (d.d_id > upper)))
> +					dump_file(fp, &d, mount->fs_name);
>  			}
>  			endpwent();
>  			break;
> @@ -478,7 +484,9 @@ report_user_mount(
>  		setpwent();
>  		while ((u = getpwent()) != NULL) {
>  			if (get_dquot(&d, u->pw_uid, NULL, XFS_USER_QUOTA,
> -						mount->fs_name, flags)) {
> +						mount->fs_name, flags) &&
> +					!(lower && (d.d_id < lower)) &&
> +					!(upper && (d.d_id > upper))) {
>  				report_mount(fp, &d, u->pw_name, form,
>  						XFS_USER_QUOTA, mount, flags);
>  				flags |= NO_HEADER_FLAG;
> @@ -518,7 +526,9 @@ report_group_mount(
>  		setgrent();
>  		while ((g = getgrent()) != NULL) {
>  			if (get_dquot(&d, g->gr_gid, NULL, XFS_GROUP_QUOTA,
> -						mount->fs_name, flags)) {
> +						mount->fs_name, flags) &&
> +					!(lower && (d.d_id < lower)) &&
> +					!(upper && (d.d_id > upper))) {
>  				report_mount(fp, &d, g->gr_name, form,
>  						XFS_GROUP_QUOTA, mount, flags);
>  				flags |= NO_HEADER_FLAG;
> @@ -560,7 +570,9 @@ report_project_mount(
>  			 * isn't defined
>  			 */
>  			if (get_dquot(&d, 0, NULL, XFS_PROJ_QUOTA,
> -						mount->fs_name, flags)) {
> +						mount->fs_name, flags) &&
> +					!(lower && (d.d_id < lower)) &&
> +					!(upper && (d.d_id > upper))) {
>  				report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA,
>  						mount, flags);
>  				flags |= NO_HEADER_FLAG;
> @@ -570,7 +582,9 @@ report_project_mount(
>  		setprent();
>  		while ((p = getprent()) != NULL) {
>  			if (get_dquot(&d, p->pr_prid, NULL, XFS_PROJ_QUOTA,
> -						mount->fs_name, flags)) {
> +						mount->fs_name, flags) &&
> +					!(lower && (d.d_id < lower)) &&
> +					!(upper && (d.d_id > upper))) {
>  				report_mount(fp, &d, p->pr_name, form,
>  						XFS_PROJ_QUOTA, mount, flags);
>  				flags |= NO_HEADER_FLAG;
> -- 
> 2.27.0
> 

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

* Re: [PATCH v2 4/5] xfs_quota: utilize XFS_GETNEXTQUOTA for ranged calls in report/dump
  2022-04-25 16:33   ` Darrick J. Wong
@ 2022-04-28  8:39     ` Andrey Albershteyn
  0 siblings, 0 replies; 16+ messages in thread
From: Andrey Albershteyn @ 2022-04-28  8:39 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Mon, Apr 25, 2022 at 09:33:00AM -0700, Darrick J. Wong wrote:
> On Wed, Apr 20, 2022 at 04:45:07PM +0200, Andrey Albershteyn wrote:
> > The implementation based on XFS_GETQUOTA call for each ID in range,
> > specified with -L/-U, is quite slow for wider ranges.
> > 
> > If kernel supports XFS_GETNEXTQUOTA, report_*_mount/dump_any_file
> > will use that to obtain quota list for the mount. XFS_GETNEXTQUOTA
> > returns quota of the requested ID and next ID with non-empty quota.
> > 
> > Otherwise, XFS_GETQUOTA will be used for each user/group/project ID
> > known from password/group/project database.
> > 
> > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  quota/report.c | 116 +++++++++++++++----------------------------------
> >  1 file changed, 35 insertions(+), 81 deletions(-)
> > 
> > diff --git a/quota/report.c b/quota/report.c
> > index 8ca154f0..65d931f3 100644
> > --- a/quota/report.c
> > +++ b/quota/report.c
> > @@ -135,7 +135,7 @@ dump_limits_any_type(
> >  {
> >  	fs_path_t	*mount;
> >  	struct fs_disk_quota d;
> > -	uint		id = 0, oid;
> > +	uint		id = lower, oid, flags = 0;
> >  
> >  	if ((mount = fs_table_lookup(dir, FS_MOUNT_POINT)) == NULL) {
> >  		exitcode = 1;
> > @@ -144,27 +144,17 @@ dump_limits_any_type(
> >  		return;
> >  	}
> >  
> > -	/* Range was specified; query everything in it */
> > -	if (upper) {
> > -		for (id = lower; id <= upper; id++) {
> > -			get_dquot(&d, id, &oid, type, mount->fs_name, 0);
> > -			dump_file(fp, &d, mount->fs_name);
> > -		}
> > -		return;
> > -	}
> > -
> > -	/* Use GETNEXTQUOTA if it's available */
> > -	if (get_dquot(&d, id, &oid, type, mount->fs_name, GETNEXTQUOTA_FLAG)) {
> > +	while (get_dquot(&d, id, &oid, type,
> > +				mount->fs_name, flags | GETNEXTQUOTA_FLAG) &&
> > +			!(upper && (d.d_id > upper))) {
> >  		dump_file(fp, &d, mount->fs_name);
> >  		id = oid + 1;
> 
> Just out of curiosity, could this be "id = d.d_id + 1", and then you
> don't have to pass around &oid at all?

yeah I think it can be removed (haven't noticed that it's not needed
anymore), I will resend it together with fix to another your comment

> 
> --D
> 
> > -		while (get_dquot(&d, id, &oid, type, mount->fs_name,
> > -					GETNEXTQUOTA_FLAG)) {
> > -			dump_file(fp, &d, mount->fs_name);
> > -			id = oid + 1;
> > -		}
> > -		return;
> > +		flags |= GETNEXTQUOTA_FLAG;
> >  	}
> >  
> > +	if (flags & GETNEXTQUOTA_FLAG)
> > +		return;
> > +
> >  	/* Otherwise fall back to iterating over each uid/gid/prjid */
> >  	switch (type) {
> >  	case XFS_GROUP_QUOTA: {
> > @@ -472,31 +462,19 @@ report_user_mount(
> >  {
> >  	struct passwd	*u;
> >  	struct fs_disk_quota	d;
> > -	uint		id = 0, oid;
> > +	uint		id = lower, oid;
> >  
> > -	if (upper) {	/* identifier range specified */
> > -		for (id = lower; id <= upper; id++) {
> > -			if (get_dquot(&d, id, NULL, XFS_USER_QUOTA,
> > -						mount->fs_name, flags)) {
> > -				report_mount(fp, &d, NULL, form, XFS_USER_QUOTA,
> > -						mount, flags);
> > -				flags |= NO_HEADER_FLAG;
> > -			}
> > -		}
> > -	} else if (get_dquot(&d, id, &oid, XFS_USER_QUOTA, mount->fs_name,
> > -				flags|GETNEXTQUOTA_FLAG)) {
> > -		report_mount(fp, &d, NULL, form, XFS_USER_QUOTA, mount,
> > -			flags|GETNEXTQUOTA_FLAG);
> > +	while (get_dquot(&d, id, &oid, XFS_USER_QUOTA,
> > +				mount->fs_name, flags | GETNEXTQUOTA_FLAG) &&
> > +			!(upper && (d.d_id > upper))) {
> > +		report_mount(fp, &d, NULL, form, XFS_USER_QUOTA, mount, flags);
> >  		id = oid + 1;
> >  		flags |= GETNEXTQUOTA_FLAG;
> >  		flags |= NO_HEADER_FLAG;
> > -		while (get_dquot(&d, id, &oid, XFS_USER_QUOTA, mount->fs_name,
> > -				flags)) {
> > -			report_mount(fp, &d, NULL, form, XFS_USER_QUOTA,
> > -				mount, flags);
> > -			id = oid + 1;
> > -		}
> > -	} else {
> > +	}
> > +
> > +	/* No GETNEXTQUOTA support, iterate over all from password file */
> > +	if (!(flags & GETNEXTQUOTA_FLAG)) {
> >  		setpwent();
> >  		while ((u = getpwent()) != NULL) {
> >  			if (get_dquot(&d, u->pw_uid, NULL, XFS_USER_QUOTA,
> > @@ -524,31 +502,19 @@ report_group_mount(
> >  {
> >  	struct group	*g;
> >  	struct fs_disk_quota	d;
> > -	uint		id = 0, oid;
> > +	uint		id = lower, oid;
> >  
> > -	if (upper) {	/* identifier range specified */
> > -		for (id = lower; id <= upper; id++) {
> > -			if (get_dquot(&d, id, NULL, XFS_GROUP_QUOTA,
> > -						mount->fs_name, flags)) {
> > -				report_mount(fp, &d, NULL, form,
> > -						XFS_GROUP_QUOTA, mount, flags);
> > -				flags |= NO_HEADER_FLAG;
> > -			}
> > -		}
> > -	} else if (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
> > -				mount->fs_name, flags|GETNEXTQUOTA_FLAG)) {
> > -		report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount,
> > -				flags|GETNEXTQUOTA_FLAG);
> > +	while (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
> > +				mount->fs_name, flags | GETNEXTQUOTA_FLAG) &&
> > +			!(upper && (oid > upper))) {
> > +		report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount, flags);
> >  		id = oid + 1;
> >  		flags |= GETNEXTQUOTA_FLAG;
> >  		flags |= NO_HEADER_FLAG;
> > -		while (get_dquot(&d, id, &oid, XFS_GROUP_QUOTA,
> > -					mount->fs_name, flags)) {
> > -			report_mount(fp, &d, NULL, form, XFS_GROUP_QUOTA, mount,
> > -					flags);
> > -			id = oid + 1;
> > -		}
> > -	} else {
> > +	}
> > +
> > +	/* No GETNEXTQUOTA support, iterate over all from password file */
> > +	if (!(flags & GETNEXTQUOTA_FLAG)) {
> >  		setgrent();
> >  		while ((g = getgrent()) != NULL) {
> >  			if (get_dquot(&d, g->gr_gid, NULL, XFS_GROUP_QUOTA,
> > @@ -575,31 +541,19 @@ report_project_mount(
> >  {
> >  	fs_project_t	*p;
> >  	struct fs_disk_quota	d;
> > -	uint		id = 0, oid;
> > +	uint		id = lower, oid;
> >  
> > -	if (upper) {	/* identifier range specified */
> > -		for (id = lower; id <= upper; id++) {
> > -			if (get_dquot(&d, id, NULL, XFS_PROJ_QUOTA,
> > -						mount->fs_name, flags)) {
> > -				report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA,
> > -						mount, flags);
> > -				flags |= NO_HEADER_FLAG;
> > -			}
> > -		}
> > -	} else if (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
> > -				mount->fs_name, flags|GETNEXTQUOTA_FLAG)) {
> > -		report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount,
> > -				flags|GETNEXTQUOTA_FLAG);
> > +	while (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
> > +				mount->fs_name, flags | GETNEXTQUOTA_FLAG) &&
> > +			!(upper && (d.d_id > upper))) {
> > +		report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount, flags);
> >  		id = oid + 1;
> >  		flags |= GETNEXTQUOTA_FLAG;
> >  		flags |= NO_HEADER_FLAG;
> > -		while (get_dquot(&d, id, &oid, XFS_PROJ_QUOTA,
> > -					mount->fs_name, flags)) {
> > -			report_mount(fp, &d, NULL, form, XFS_PROJ_QUOTA, mount,
> > -					flags);
> > -			id = oid + 1;
> > -		}
> > -	} else {
> > +	}
> > +
> > +	/* No GETNEXTQUOTA support, iterate over all */
> > +	if (!(flags & GETNEXTQUOTA_FLAG)) {
> >  		if (!getprprid(0)) {
> >  			/*
> >  			 * Print default project quota, even if projid 0
> > -- 
> > 2.27.0
> > 
> 

-- 
- Andrey


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

* Re: [PATCH v2 2/5] xfs_quota: separate get_dquot() and dump_file()
  2022-04-25 16:28   ` Darrick J. Wong
@ 2022-04-28 12:43     ` Andrey Albershteyn
  0 siblings, 0 replies; 16+ messages in thread
From: Andrey Albershteyn @ 2022-04-28 12:43 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Mon, Apr 25, 2022 at 09:28:36AM -0700, Darrick J. Wong wrote:
> On Wed, Apr 20, 2022 at 04:45:05PM +0200, Andrey Albershteyn wrote:
> > Separate quota info acquisition from outputting it to file. This
> > allows upper functions to filter obtained info (e.g. within specific
> > ID range).
> > 
> > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > ---
> >  quota/report.c | 86 ++++++++++++++++++++++++++------------------------
> >  1 file changed, 45 insertions(+), 41 deletions(-)
> > 
> > diff --git a/quota/report.c b/quota/report.c
> > index cccc654e..d5c6f84f 100644
> > --- a/quota/report.c
> > +++ b/quota/report.c
> > @@ -96,39 +96,31 @@ get_dquot(
> >  static int
> >  dump_file(
> 
> I kinda wonder if this ought to be named 'dump_dquot' since it's not
> really dumping a file or even the dquots of a specifc file.  OTOH, the
> rest of the utility seems to have similar naming conventions for "report
> something to a FILE* stream" so

yeah, I also think that names are not very descriptive, but I didn't
want to change too many things. There's also space to merge report*
functions :)

> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> 
> --D
> 
> >  	FILE		*fp,
> > -	uint		id,
> > -	uint		*oid,
> > -	uint		type,
> > -	char		*dev,
> > -	int		flags)
> > +	struct fs_disk_quota *d,
> > +	char		*dev)
> >  {
> > -	fs_disk_quota_t	d;
> > -
> > -	if (!get_dquot(&d, id, oid, type, dev, flags))
> > -		return 0;
> > -
> > -	if (!d.d_blk_softlimit && !d.d_blk_hardlimit &&
> > -	    !d.d_ino_softlimit && !d.d_ino_hardlimit &&
> > -	    !d.d_rtb_softlimit && !d.d_rtb_hardlimit)
> > +	if (!d->d_blk_softlimit && !d->d_blk_hardlimit &&
> > +	    !d->d_ino_softlimit && !d->d_ino_hardlimit &&
> > +	    !d->d_rtb_softlimit && !d->d_rtb_hardlimit)
> >  		return 1;
> >  	fprintf(fp, "fs = %s\n", dev);
> >  	/* this branch is for backward compatibility reasons */
> > -	if (d.d_rtb_softlimit || d.d_rtb_hardlimit)
> > +	if (d->d_rtb_softlimit || d->d_rtb_hardlimit)
> >  		fprintf(fp, "%-10d %7llu %7llu %7llu %7llu %7llu %7llu\n",
> > -			d.d_id,
> > -			(unsigned long long)d.d_blk_softlimit,
> > -			(unsigned long long)d.d_blk_hardlimit,
> > -			(unsigned long long)d.d_ino_softlimit,
> > -			(unsigned long long)d.d_ino_hardlimit,
> > -			(unsigned long long)d.d_rtb_softlimit,
> > -			(unsigned long long)d.d_rtb_hardlimit);
> > +			d->d_id,
> > +			(unsigned long long)d->d_blk_softlimit,
> > +			(unsigned long long)d->d_blk_hardlimit,
> > +			(unsigned long long)d->d_ino_softlimit,
> > +			(unsigned long long)d->d_ino_hardlimit,
> > +			(unsigned long long)d->d_rtb_softlimit,
> > +			(unsigned long long)d->d_rtb_hardlimit);
> >  	else
> >  		fprintf(fp, "%-10d %7llu %7llu %7llu %7llu\n",
> > -			d.d_id,
> > -			(unsigned long long)d.d_blk_softlimit,
> > -			(unsigned long long)d.d_blk_hardlimit,
> > -			(unsigned long long)d.d_ino_softlimit,
> > -			(unsigned long long)d.d_ino_hardlimit);
> > +			d->d_id,
> > +			(unsigned long long)d->d_blk_softlimit,
> > +			(unsigned long long)d->d_blk_hardlimit,
> > +			(unsigned long long)d->d_ino_softlimit,
> > +			(unsigned long long)d->d_ino_hardlimit);
> >  
> >  	return 1;
> >  }
> > @@ -142,6 +134,7 @@ dump_limits_any_type(
> >  	uint		upper)
> >  {
> >  	fs_path_t	*mount;
> > +	struct fs_disk_quota d;
> >  	uint		id = 0, oid;
> >  
> >  	if ((mount = fs_table_lookup(dir, FS_MOUNT_POINT)) == NULL) {
> > @@ -153,46 +146,57 @@ dump_limits_any_type(
> >  
> >  	/* Range was specified; query everything in it */
> >  	if (upper) {
> > -		for (id = lower; id <= upper; id++)
> > -			dump_file(fp, id, NULL, type, mount->fs_name, 0);
> > +		for (id = lower; id <= upper; id++) {
> > +			get_dquot(&d, id, &oid, type, mount->fs_name, 0);
> > +			dump_file(fp, &d, mount->fs_name);
> > +		}
> >  		return;
> >  	}
> >  
> >  	/* Use GETNEXTQUOTA if it's available */
> > -	if (dump_file(fp, id, &oid, type, mount->fs_name, GETNEXTQUOTA_FLAG)) {
> > +	if (get_dquot(&d, id, &oid, type, mount->fs_name, GETNEXTQUOTA_FLAG)) {
> > +		dump_file(fp, &d, mount->fs_name);
> >  		id = oid + 1;
> > -		while (dump_file(fp, id, &oid, type, mount->fs_name,
> > -				 GETNEXTQUOTA_FLAG))
> > +		while (get_dquot(&d, id, &oid, type, mount->fs_name,
> > +					GETNEXTQUOTA_FLAG)) {
> > +			dump_file(fp, &d, mount->fs_name);
> >  			id = oid + 1;
> > +		}
> >  		return;
> > -        }
> > +	}
> >  
> >  	/* Otherwise fall back to iterating over each uid/gid/prjid */
> >  	switch (type) {
> >  	case XFS_GROUP_QUOTA: {
> >  			struct group *g;
> >  			setgrent();
> > -			while ((g = getgrent()) != NULL)
> > -				dump_file(fp, g->gr_gid, NULL, type,
> > -					  mount->fs_name, 0);
> > +			while ((g = getgrent()) != NULL) {
> > +				get_dquot(&d, g->gr_gid, NULL, type,
> > +						mount->fs_name, 0);
> > +				dump_file(fp, &d, mount->fs_name);
> > +			}
> >  			endgrent();
> >  			break;
> >  		}
> >  	case XFS_PROJ_QUOTA: {
> >  			struct fs_project *p;
> >  			setprent();
> > -			while ((p = getprent()) != NULL)
> > -				dump_file(fp, p->pr_prid, NULL, type,
> > -					  mount->fs_name, 0);
> > +			while ((p = getprent()) != NULL) {
> > +				get_dquot(&d, p->pr_prid, NULL, type,
> > +						mount->fs_name, 0);
> > +				dump_file(fp, &d, mount->fs_name);
> > +			}
> >  			endprent();
> >  			break;
> >  		}
> >  	case XFS_USER_QUOTA: {
> >  			struct passwd *u;
> >  			setpwent();
> > -			while ((u = getpwent()) != NULL)
> > -				dump_file(fp, u->pw_uid, NULL, type,
> > -					  mount->fs_name, 0);
> > +			while ((u = getpwent()) != NULL) {
> > +				get_dquot(&d, u->pw_uid, NULL, type,
> > +						mount->fs_name, 0);
> > +				dump_file(fp, &d, mount->fs_name);
> > +			}
> >  			endpwent();
> >  			break;
> >  		}
> > -- 
> > 2.27.0
> > 
> 

-- 
- Andrey


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

end of thread, other threads:[~2022-04-28 12:43 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-20 14:45 [PATCH v2 0/5] xfsprogs: optimize -L/-U range calls for xfs_quota's dump/report Andrey Albershteyn
2022-04-20 14:45 ` [PATCH v2 1/5] xfs_quota: separate quota info acquisition into get_dquot() Andrey Albershteyn
2022-04-25 16:22   ` Darrick J. Wong
2022-04-20 14:45 ` [PATCH v2 2/5] xfs_quota: separate get_dquot() and dump_file() Andrey Albershteyn
2022-04-24  5:29   ` Christoph Hellwig
2022-04-25 16:28   ` Darrick J. Wong
2022-04-28 12:43     ` Andrey Albershteyn
2022-04-20 14:45 ` [PATCH v2 3/5] xfs_quota: separate get_dquot() and report_mount() Andrey Albershteyn
2022-04-24  5:29   ` Christoph Hellwig
2022-04-25 16:29   ` Darrick J. Wong
2022-04-20 14:45 ` [PATCH v2 4/5] xfs_quota: utilize XFS_GETNEXTQUOTA for ranged calls in report/dump Andrey Albershteyn
2022-04-25 16:33   ` Darrick J. Wong
2022-04-28  8:39     ` Andrey Albershteyn
2022-04-20 14:45 ` [PATCH v2 5/5] xfs_quota: apply -L/-U range limits in uid/gid/pid loops Andrey Albershteyn
2022-04-24  5:29   ` Christoph Hellwig
2022-04-25 16:33   ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.