linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Allison Collins <allison.henderson@oracle.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>, sandeen@sandeen.net
Cc: linux-xfs@vger.kernel.org, Dave Chinner <dchinner@redhat.com>
Subject: Re: [PATCH 9/9] libfrog: refactor open-coded INUMBERS calls
Date: Sun, 1 Sep 2019 13:55:30 -0700	[thread overview]
Message-ID: <07c35c39-c320-692c-ba1b-34eb9bc4c07b@oracle.com> (raw)
In-Reply-To: <156713888219.386621.15662668758929652470.stgit@magnolia>

On 8/29/19 9:21 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor all the INUMBERS ioctl callsites into helper functions.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
Looks good!  I think it looks a lot cleaner.  Thanks!

Reviewed-by: Allison Collins <allison.henderson@oracle.com>

> ---
>   include/xfrog.h    |    4 ++++
>   io/imap.c          |   33 ++++++++++++++++-----------------
>   io/open.c          |   24 ++++++++++++------------
>   libfrog/bulkstat.c |   26 ++++++++++++++++++++++++++
>   scrub/fscounters.c |   20 ++++++++------------
>   scrub/inodes.c     |   17 ++++++-----------
>   6 files changed, 72 insertions(+), 52 deletions(-)
> 
> 
> diff --git a/include/xfrog.h b/include/xfrog.h
> index 3a49acc3..d16481ce 100644
> --- a/include/xfrog.h
> +++ b/include/xfrog.h
> @@ -116,4 +116,8 @@ int xfrog_bulkstat_single(struct xfs_fd *xfd, uint64_t ino,
>   int xfrog_bulkstat(struct xfs_fd *xfd, uint64_t *lastino, uint32_t icount,
>   		struct xfs_bstat *ubuffer, uint32_t *ocount);
>   
> +struct xfs_inogrp;
> +int xfrog_inumbers(struct xfs_fd *xfd, uint64_t *lastino, uint32_t icount,
> +		struct xfs_inogrp *ubuffer, uint32_t *ocount);
> +
>   #endif	/* __XFROG_H__ */
> diff --git a/io/imap.c b/io/imap.c
> index 9667289a..053ac28e 100644
> --- a/io/imap.c
> +++ b/io/imap.c
> @@ -8,18 +8,20 @@
>   #include "input.h"
>   #include "init.h"
>   #include "io.h"
> +#include "xfrog.h"
>   
>   static cmdinfo_t imap_cmd;
>   
>   static int
>   imap_f(int argc, char **argv)
>   {
> -	int		count;
> -	int		nent;
> -	int		i;
> -	__u64		last = 0;
> -	struct xfs_inogrp *t;
> -	struct xfs_fsop_bulkreq bulkreq;
> +	struct xfs_fd		xfd = XFS_FD_INIT(file->fd);
> +	struct xfs_inogrp	*t;
> +	uint64_t		last = 0;
> +	uint32_t		count;
> +	uint32_t		nent;
> +	int			i;
> +	int			error;
>   
>   	if (argc != 2)
>   		nent = 1;
> @@ -30,14 +32,8 @@ imap_f(int argc, char **argv)
>   	if (!t)
>   		return 0;
>   
> -	bulkreq.lastip  = &last;
> -	bulkreq.icount  = nent;
> -	bulkreq.ubuffer = (void *)t;
> -	bulkreq.ocount  = &count;
> -
> -	while (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS, &bulkreq) == 0) {
> -		if (count == 0)
> -			goto out_free;
> +	while ((error = xfrog_inumbers(&xfd, &last, nent, t, &count)) == 0 &&
> +	       count > 0) {
>   		for (i = 0; i < count; i++) {
>   			printf(_("ino %10llu count %2d mask %016llx\n"),
>   				(unsigned long long)t[i].xi_startino,
> @@ -45,9 +41,12 @@ imap_f(int argc, char **argv)
>   				(unsigned long long)t[i].xi_allocmask);
>   		}
>   	}
> -	perror("xfsctl(XFS_IOC_FSINUMBERS)");
> -	exitcode = 1;
> -out_free:
> +
> +	if (error) {
> +		errno = error;
> +		perror("xfsctl(XFS_IOC_FSINUMBERS)");
> +		exitcode = 1;
> +	}
>   	free(t);
>   	return 0;
>   }
> diff --git a/io/open.c b/io/open.c
> index 35e6131b..c0d0f1e9 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -675,24 +675,24 @@ inode_help(void)
>   "\n"));
>   }
>   
> +#define IGROUP_NR	(1024)
>   static __u64
>   get_last_inode(void)
>   {
> -	__u64			lastip = 0;
> -	__u64			lastgrp = 0;
> -	__s32			ocount = 0;
> +	struct xfs_fd		xfd = XFS_FD_INIT(file->fd);
> +	uint64_t		lastip = 0;
> +	uint32_t		lastgrp = 0;
> +	uint32_t		ocount = 0;
>   	__u64			last_ino;
> -	struct xfs_inogrp	igroup[1024];
> -	struct xfs_fsop_bulkreq	bulkreq;
> -
> -	bulkreq.lastip = &lastip;
> -	bulkreq.ubuffer = &igroup;
> -	bulkreq.icount = sizeof(igroup) / sizeof(struct xfs_inogrp);
> -	bulkreq.ocount = &ocount;
> +	struct xfs_inogrp	igroup[IGROUP_NR];
>   
>   	for (;;) {
> -		if (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS,
> -				&bulkreq)) {
> +		int		ret;
> +
> +		ret = xfrog_inumbers(&xfd, &lastip, IGROUP_NR, igroup,
> +				&ocount);
> +		if (ret) {
> +			errno = ret;
>   			perror("XFS_IOC_FSINUMBERS");
>   			return 0;
>   		}
> diff --git a/libfrog/bulkstat.c b/libfrog/bulkstat.c
> index 0e11ccea..18fffb20 100644
> --- a/libfrog/bulkstat.c
> +++ b/libfrog/bulkstat.c
> @@ -50,3 +50,29 @@ xfrog_bulkstat(
>   		return errno;
>   	return 0;
>   }
> +
> +/*
> + * Query inode allocation bitmask information.  Returns zero or a positive
> + * error code.
> + */
> +int
> +xfrog_inumbers(
> +	struct xfs_fd		*xfd,
> +	uint64_t		*lastino,
> +	uint32_t		icount,
> +	struct xfs_inogrp	*ubuffer,
> +	uint32_t		*ocount)
> +{
> +	struct xfs_fsop_bulkreq	bulkreq = {
> +		.lastip		= (__u64 *)lastino,
> +		.icount		= icount,
> +		.ubuffer	= ubuffer,
> +		.ocount		= (__s32 *)ocount,
> +	};
> +	int			ret;
> +
> +	ret = ioctl(xfd->fd, XFS_IOC_FSINUMBERS, &bulkreq);
> +	if (ret)
> +		return errno;
> +	return 0;
> +}
> diff --git a/scrub/fscounters.c b/scrub/fscounters.c
> index ea6af156..3915822e 100644
> --- a/scrub/fscounters.c
> +++ b/scrub/fscounters.c
> @@ -15,6 +15,7 @@
>   #include "xfs_scrub.h"
>   #include "common.h"
>   #include "fscounters.h"
> +#include "xfrog.h"
>   
>   /*
>    * Filesystem counter collection routines.  We can count the number of
> @@ -41,30 +42,25 @@ xfs_count_inodes_range(
>   	uint64_t		last_ino,
>   	uint64_t		*count)
>   {
> -	struct xfs_fsop_bulkreq	igrpreq = {NULL};
>   	struct xfs_inogrp	inogrp;
> -	__u64			igrp_ino;
> +	uint64_t		igrp_ino;
>   	uint64_t		nr = 0;
> -	__s32			igrplen = 0;
> +	uint32_t		igrplen = 0;
>   	int			error;
>   
>   	ASSERT(!(first_ino & (XFS_INODES_PER_CHUNK - 1)));
>   	ASSERT((last_ino & (XFS_INODES_PER_CHUNK - 1)));
>   
> -	igrpreq.lastip  = &igrp_ino;
> -	igrpreq.icount  = 1;
> -	igrpreq.ubuffer = &inogrp;
> -	igrpreq.ocount  = &igrplen;
> -
>   	igrp_ino = first_ino;
> -	error = ioctl(ctx->mnt.fd, XFS_IOC_FSINUMBERS, &igrpreq);
> -	while (!error && igrplen && inogrp.xi_startino < last_ino) {
> +	while (!(error = xfrog_inumbers(&ctx->mnt, &igrp_ino, 1, &inogrp,
> +			&igrplen))) {
> +		if (igrplen == 0 || inogrp.xi_startino >= last_ino)
> +			break;
>   		nr += inogrp.xi_alloccount;
> -		error = ioctl(ctx->mnt.fd, XFS_IOC_FSINUMBERS, &igrpreq);
>   	}
>   
>   	if (error) {
> -		str_errno(ctx, descr);
> +		str_liberror(ctx, error, descr);
>   		return false;
>   	}
>   
> diff --git a/scrub/inodes.c b/scrub/inodes.c
> index 413037d8..acad9b16 100644
> --- a/scrub/inodes.c
> +++ b/scrub/inodes.c
> @@ -90,16 +90,15 @@ xfs_iterate_inodes_range(
>   	xfs_inode_iter_fn	fn,
>   	void			*arg)
>   {
> -	struct xfs_fsop_bulkreq	igrpreq = {NULL};
>   	struct xfs_handle	handle;
>   	struct xfs_inogrp	inogrp;
>   	struct xfs_bstat	bstat[XFS_INODES_PER_CHUNK];
>   	char			idescr[DESCR_BUFSZ];
>   	struct xfs_bstat	*bs;
> -	__u64			igrp_ino;
> +	uint64_t		igrp_ino;
>   	uint64_t		ino;
>   	uint32_t		bulklen = 0;
> -	__s32			igrplen = 0;
> +	uint32_t		igrplen = 0;
>   	bool			moveon = true;
>   	int			i;
>   	int			error;
> @@ -108,11 +107,6 @@ xfs_iterate_inodes_range(
>   
>   	memset(bstat, 0, XFS_INODES_PER_CHUNK * sizeof(struct xfs_bstat));
>   
> -	igrpreq.lastip  = &igrp_ino;
> -	igrpreq.icount  = 1;
> -	igrpreq.ubuffer = &inogrp;
> -	igrpreq.ocount  = &igrplen;
> -
>   	memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
>   	handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
>   			sizeof(handle.ha_fid.fid_len);
> @@ -120,7 +114,7 @@ xfs_iterate_inodes_range(
>   
>   	/* Find the inode chunk & alloc mask */
>   	igrp_ino = first_ino;
> -	error = ioctl(ctx->mnt.fd, XFS_IOC_FSINUMBERS, &igrpreq);
> +	error = xfrog_inumbers(&ctx->mnt, &igrp_ino, 1, &inogrp, &igrplen);
>   	while (!error && igrplen) {
>   		/* Load the inodes. */
>   		ino = inogrp.xi_startino - 1;
> @@ -176,12 +170,13 @@ _("Changed too many times during scan; giving up."));
>   
>   		stale_count = 0;
>   igrp_retry:
> -		error = ioctl(ctx->mnt.fd, XFS_IOC_FSINUMBERS, &igrpreq);
> +		error = xfrog_inumbers(&ctx->mnt, &igrp_ino, 1, &inogrp,
> +				&igrplen);
>   	}
>   
>   err:
>   	if (error) {
> -		str_errno(ctx, descr);
> +		str_liberror(ctx, error, descr);
>   		moveon = false;
>   	}
>   out:
> 

      reply	other threads:[~2019-09-01 20:57 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-30  4:20 [PATCH v2 0/9] libxfrog: wrap version ioctl calls Darrick J. Wong
2019-08-30  4:20 ` [PATCH 1/9] libxfs: revert FSGEOMETRY v5 -> v4 hack Darrick J. Wong
2019-08-30 21:03   ` Allison Collins
2019-09-02 22:49   ` Dave Chinner
2019-09-03  6:14     ` Christoph Hellwig
2019-08-30  4:20 ` [PATCH 2/9] xfsprogs: update spdx tags in LICENSES/ Darrick J. Wong
2019-08-30 21:03   ` Allison Collins
2019-08-30  4:20 ` [PATCH 3/9] libfrog: refactor online geometry queries Darrick J. Wong
2019-08-30 21:03   ` Allison Collins
2019-09-03 22:51   ` Dave Chinner
2019-08-30  4:20 ` [PATCH 4/9] libfrog: introduce xfs_fd to wrap an fd to a file on an xfs filesystem Darrick J. Wong
2019-08-30 23:29   ` Allison Collins
2019-08-30 23:31     ` Darrick J. Wong
2019-09-03 22:53   ` Dave Chinner
2019-08-30  4:20 ` [PATCH 5/9] libfrog: store more inode and block geometry in struct xfs_fd Darrick J. Wong
2019-08-31  0:37   ` Allison Collins
2019-08-30  4:21 ` [PATCH 6/9] libfrog: create online fs geometry converters Darrick J. Wong
2019-08-31  5:11   ` Allison Collins
2019-09-03 22:55   ` Dave Chinner
2019-08-30  4:21 ` [PATCH 7/9] libfrog: refactor open-coded bulkstat calls Darrick J. Wong
2019-08-30 23:26   ` Darrick J. Wong
2019-08-31  0:42   ` [PATCH v2 " Darrick J. Wong
2019-09-03 22:57     ` Dave Chinner
2019-09-01 20:55   ` [PATCH " Allison Collins
2019-09-01 21:05     ` Darrick J. Wong
2019-08-30  4:21 ` [PATCH 8/9] libfrog: create xfd_open function Darrick J. Wong
2019-09-01 20:55   ` Allison Collins
2019-09-03 23:01   ` Dave Chinner
2019-09-03 23:37     ` Darrick J. Wong
2019-08-30  4:21 ` [PATCH 9/9] libfrog: refactor open-coded INUMBERS calls Darrick J. Wong
2019-09-01 20:55   ` Allison Collins [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=07c35c39-c320-692c-ba1b-34eb9bc4c07b@oracle.com \
    --to=allison.henderson@oracle.com \
    --cc=darrick.wong@oracle.com \
    --cc=dchinner@redhat.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@sandeen.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).