[03/26] libfrog: define LIBFROG_BULKSTAT_CHUNKSIZE to remove dependence on XFS_INODES_PER_CHUNK

Commit Message

Darrick J. Wong Oct. 26, 2020, 11:34 p.m. UTC

From: Darrick J. Wong <[e-mail]>

"Online" XFS programs like scrub have no business importing the internal
disk format headers to discover things like the optimum number of inodes
to request through a bulkstat request.  That number can be derived from
the ioctl definition, so define a new constant in terms of that instead
of pulling in the ondisk format unnecessarily.

Note: This patch will be needed to work around new definitions in the
bigtime patchset that will break scrub builds, so clean this up instead
of adding more #includes to the two scrub source files.

Signed-off-by: Darrick J. Wong <[e-mail]>
---
 include/xfs.h      |    2 ++
 libfrog/bulkstat.h |    4 ++++
 scrub/fscounters.c |    1 -
 scrub/inodes.c     |    5 ++---
 4 files changed, 8 insertions(+), 4 deletions(-)

Comments

Christoph Hellwig Oct. 29, 2020, 9:45 a.m. UTC | #1

Looks good,

Reviewed-by: Christoph Hellwig <[e-mail]>

Christoph Hellwig Oct. 29, 2020, 9:45 a.m. UTC | #2

On Thu, Oct 29, 2020 at 09:45:04AM +0000, Christoph Hellwig wrote:
> Looks good,
> 
> Reviewed-by: Christoph Hellwig <[e-mail]>

And no good reason to use struct timespec64 here, right?

Darrick J. Wong Oct. 29, 2020, 5:25 p.m. UTC | #3

On Thu, Oct 29, 2020 at 09:45:49AM +0000, Christoph Hellwig wrote:
> On Thu, Oct 29, 2020 at 09:45:04AM +0000, Christoph Hellwig wrote:
> > Looks good,
> > 
> > Reviewed-by: Christoph Hellwig <[e-mail]>
> 
> And no good reason to use struct timespec64 here, right?

Huh?  There's no mention of a timespec in this patch at all...?

<confused>

--D

Christoph Hellwig Oct. 30, 2020, 8:20 a.m. UTC | #4

On Thu, Oct 29, 2020 at 10:25:57AM -0700, Darrick J. Wong wrote:
> On Thu, Oct 29, 2020 at 09:45:49AM +0000, Christoph Hellwig wrote:
> > On Thu, Oct 29, 2020 at 09:45:04AM +0000, Christoph Hellwig wrote:
> > > Looks good,
> > > 
> > > Reviewed-by: Christoph Hellwig <[e-mail]>
> > 
> > And no good reason to use struct timespec64 here, right?
> 
> Huh?  There's no mention of a timespec in this patch at all...?

No, but a few patches earlier timespec64 is added with the rationale
that we can't rely on a 64-bit time type otherwise, while we use time64_t
here.

Patch

diff --git a/include/xfs.h b/include/xfs.h
index af0d36cef361..e97158c8d223 100644
--- a/include/xfs.h
+++ b/include/xfs.h
@@ -38,6 +38,8 @@  extern int xfs_assert_largefile[sizeof(off_t)-8];
 #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
 #endif
 
+#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
+
 #include <xfs/xfs_types.h>
 /* Include deprecated/compat pre-vfs xfs-specific symbols */
 #include <xfs/xfs_fs_compat.h>
diff --git a/libfrog/bulkstat.h b/libfrog/bulkstat.h
index 56ef7f9a8237..2f440b14f93d 100644
--- a/libfrog/bulkstat.h
+++ b/libfrog/bulkstat.h
@@ -6,6 +6,10 @@ 
 #ifndef __LIBFROG_BULKSTAT_H__
 #define __LIBFROG_BULKSTAT_H__
 
+/* This is the minimum reasonable size of a bulkstat request. */
+#define LIBFROG_BULKSTAT_CHUNKSIZE \
+		(NBBY * sizeof_field(struct xfs_inumbers, xi_allocmask))
+
 /* Bulkstat wrappers */
 struct xfs_bstat;
 int xfrog_bulkstat_single(struct xfs_fd *xfd, uint64_t ino, unsigned int flags,
diff --git a/scrub/fscounters.c b/scrub/fscounters.c
index 9a240d49477b..f21b24e0935c 100644
--- a/scrub/fscounters.c
+++ b/scrub/fscounters.c
@@ -9,7 +9,6 @@ 
 #include <sys/statvfs.h>
 #include "platform_defs.h"
 #include "xfs_arch.h"
-#include "xfs_format.h"
 #include "libfrog/paths.h"
 #include "libfrog/workqueue.h"
 #include "xfs_scrub.h"
diff --git a/scrub/inodes.c b/scrub/inodes.c
index 5ef752fe39d3..bdc12df31479 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -10,7 +10,6 @@ 
 #include <sys/statvfs.h>
 #include "platform_defs.h"
 #include "xfs_arch.h"
-#include "xfs_format.h"
 #include "handle.h"
 #include "libfrog/paths.h"
 #include "libfrog/workqueue.h"
@@ -74,7 +73,7 @@  bulkstat_for_inumbers(
 	 * Check each of the stats we got back to make sure we got the inodes
 	 * we asked for.
 	 */
-	for (i = 0, bs = bstat; i < XFS_INODES_PER_CHUNK; i++) {
+	for (i = 0, bs = bstat; i < LIBFROG_BULKSTAT_CHUNKSIZE; i++) {
 		if (!(inumbers->xi_allocmask & (1ULL << i)))
 			continue;
 		if (bs->bs_ino == inumbers->xi_startino + i) {
@@ -134,7 +133,7 @@  scan_ag_inodes(
 			sizeof(handle.ha_fid.fid_len);
 	handle.ha_fid.fid_pad = 0;
 
-	error = -xfrog_bulkstat_alloc_req(XFS_INODES_PER_CHUNK, 0, &breq);
+	error = -xfrog_bulkstat_alloc_req(LIBFROG_BULKSTAT_CHUNKSIZE, 0, &breq);
 	if (error) {
 		str_liberror(ctx, error, descr);
 		si->aborted = true;