From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Brian Foster <bfoster@redhat.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 02/27] xfs: query the per-AG reservation counters
Date: Thu, 21 Sep 2017 10:30:06 -0700 [thread overview]
Message-ID: <20170921173006.GJ7112@magnolia> (raw)
In-Reply-To: <20170921143630.GD58956@bfoster.bfoster>
On Thu, Sep 21, 2017 at 10:36:31AM -0400, Brian Foster wrote:
> On Wed, Sep 20, 2017 at 05:17:43PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Establish an ioctl for userspace to query the original and current
> > per-AG reservation counts. This will be used by xfs_scrub to
> > check that the vfs counters are at least somewhat sane.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
>
> The code seems fine, but I'm wondering how much of this would remain as
> is based on Dave's potential switch of the perag res stuff to be a
> hidden reservation. That itself still needs to be posted/reviewed/etc.,
> so might it be smarter to defer defining an interface to export this
> stuff until the core mechanism is worked out?
I think Dave's vision is that we should permanently steal the AG
reservations from the max fs size:
(device size - static metadata - static per-ag reservations)
In which case the ioctl isn't needed at all, since the statfs counters
would simply never report the per-ag reservations at all.
> That aside, I'm also wondering how/what userspace scrub does with the
> overall total values as opposed to individual AG reservation values.
To a rough approximation, xfs_scrub (phase 7) compares the fs geometry
data (used blocks, used inodes) against what it found via getfsmap and
bulkstat and warns about a potentially insufficient scrub if the counts
are off by more than 10% from what the fs reports. The per-AG summary
counter ar_current_resv is added to the statvfs.f_bfree counter since
reserved blocks don't show up in getfsmap.
Since per-AG reservations never use more than 9% of the fs space worst
case (1k blocks) and usually 2% (4k blocks) the current 10% tolerance
built into xfs_scrub is probably good enough for now.
Anyway, I plan to withdraw this patch.
> Are the current "ask" values expected to be uniform across each AG..?
scrub doesn't care.
> For example, what happens if one AG has a bogus "current" value for
> whatever reason (perhaps this is more relevant for both values if we
> do actually end up with on-disk reservations)?
If it exceeds scrub's tolerances it'll warn about the discrepancy.
There's probably no way to fix the summary counters without an
xfs_repair run, unless it's acceptable to freeze the whole fs.
Not going to try to do that. :)
--D
>
> Brian
>
> > fs/xfs/libxfs/xfs_fs.h | 12 ++++++++++++
> > fs/xfs/xfs_fsops.c | 26 ++++++++++++++++++++++++++
> > fs/xfs/xfs_fsops.h | 2 ++
> > fs/xfs/xfs_ioctl.c | 24 ++++++++++++++++++++++++
> > fs/xfs/xfs_ioctl32.c | 1 +
> > 5 files changed, 65 insertions(+)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> > index 8c61f21..2c26c38 100644
> > --- a/fs/xfs/libxfs/xfs_fs.h
> > +++ b/fs/xfs/libxfs/xfs_fs.h
> > @@ -469,6 +469,17 @@ typedef struct xfs_swapext
> > #define XFS_FSOP_GOING_FLAGS_NOLOGFLUSH 0x2 /* don't flush log nor data */
> >
> > /*
> > + * AG reserved block counters
> > + */
> > +struct xfs_fsop_ag_resblks {
> > + __u32 ar_flags; /* output flags, none defined now */
> > + __u32 ar_reserved; /* zero */
> > + __u64 ar_current_resv; /* blocks reserved now */
> > + __u64 ar_mount_resv; /* blocks reserved at mount time */
> > + __u64 ar_reserved2[5]; /* zero */
> > +};
> > +
> > +/*
> > * ioctl limits
> > */
> > #ifdef XATTR_LIST_MAX
> > @@ -543,6 +554,7 @@ typedef struct xfs_swapext
> > #define XFS_IOC_ATTRMULTI_BY_HANDLE _IOW ('X', 123, struct xfs_fsop_attrmulti_handlereq)
> > #define XFS_IOC_FSGEOMETRY _IOR ('X', 124, struct xfs_fsop_geom)
> > #define XFS_IOC_GOINGDOWN _IOR ('X', 125, uint32_t)
> > +#define XFS_IOC_GET_AG_RESBLKS _IOR ('X', 126, struct xfs_fsop_ag_resblks)
> > /* XFS_IOC_GETFSUUID ---------- deprecated 140 */
> >
> >
> > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> > index 8f22fc5..50fb3a2 100644
> > --- a/fs/xfs/xfs_fsops.c
> > +++ b/fs/xfs/xfs_fsops.c
> > @@ -44,6 +44,7 @@
> > #include "xfs_filestream.h"
> > #include "xfs_rmap.h"
> > #include "xfs_ag_resv.h"
> > +#include "xfs_fs.h"
> >
> > /*
> > * File system operations
> > @@ -1046,3 +1047,28 @@ xfs_fs_unreserve_ag_blocks(
> >
> > return error;
> > }
> > +
> > +/* Query the per-AG reservations to see how many blocks we have reserved. */
> > +int
> > +xfs_fs_get_ag_reserve_blocks(
> > + struct xfs_mount *mp,
> > + struct xfs_fsop_ag_resblks *out)
> > +{
> > + struct xfs_ag_resv *r;
> > + struct xfs_perag *pag;
> > + xfs_agnumber_t agno;
> > +
> > + memset(out, 0, sizeof(struct xfs_fsop_ag_resblks));
> > + for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
> > + pag = xfs_perag_get(mp, agno);
> > + r = xfs_perag_resv(pag, XFS_AG_RESV_METADATA);
> > + out->ar_current_resv += r->ar_reserved;
> > + out->ar_mount_resv += r->ar_asked;
> > + r = xfs_perag_resv(pag, XFS_AG_RESV_AGFL);
> > + out->ar_current_resv += r->ar_reserved;
> > + out->ar_mount_resv += r->ar_asked;
> > + xfs_perag_put(pag);
> > + }
> > +
> > + return 0;
> > +}
> > diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
> > index 2954c13..c8f5e26 100644
> > --- a/fs/xfs/xfs_fsops.h
> > +++ b/fs/xfs/xfs_fsops.h
> > @@ -25,6 +25,8 @@ extern int xfs_fs_counts(xfs_mount_t *mp, xfs_fsop_counts_t *cnt);
> > extern int xfs_reserve_blocks(xfs_mount_t *mp, uint64_t *inval,
> > xfs_fsop_resblks_t *outval);
> > extern int xfs_fs_goingdown(xfs_mount_t *mp, uint32_t inflags);
> > +extern int xfs_fs_get_ag_reserve_blocks(struct xfs_mount *mp,
> > + struct xfs_fsop_ag_resblks *out);
> >
> > extern int xfs_fs_reserve_ag_blocks(struct xfs_mount *mp);
> > extern int xfs_fs_unreserve_ag_blocks(struct xfs_mount *mp);
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index 5049e8a..44dc178 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -1782,6 +1782,27 @@ xfs_ioc_swapext(
> > return error;
> > }
> >
> > +static int
> > +xfs_ioc_get_ag_reserve_blocks(
> > + struct xfs_mount *mp,
> > + void __user *arg)
> > +{
> > + struct xfs_fsop_ag_resblks out;
> > + int error;
> > +
> > + if (!capable(CAP_SYS_ADMIN))
> > + return -EPERM;
> > +
> > + error = xfs_fs_get_ag_reserve_blocks(mp, &out);
> > + if (error)
> > + return error;
> > +
> > + if (copy_to_user(arg, &out, sizeof(out)))
> > + return -EFAULT;
> > +
> > + return 0;
> > +}
> > +
> > /*
> > * Note: some of the ioctl's return positive numbers as a
> > * byte count indicating success, such as readlink_by_handle.
> > @@ -1987,6 +2008,9 @@ xfs_file_ioctl(
> > return 0;
> > }
> >
> > + case XFS_IOC_GET_AG_RESBLKS:
> > + return xfs_ioc_get_ag_reserve_blocks(mp, arg);
> > +
> > case XFS_IOC_FSGROWFSDATA: {
> > xfs_growfs_data_t in;
> >
> > diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> > index fa0bc4d..e8b4de3 100644
> > --- a/fs/xfs/xfs_ioctl32.c
> > +++ b/fs/xfs/xfs_ioctl32.c
> > @@ -556,6 +556,7 @@ xfs_file_compat_ioctl(
> > case XFS_IOC_ERROR_INJECTION:
> > case XFS_IOC_ERROR_CLEARALL:
> > case FS_IOC_GETFSMAP:
> > + case XFS_IOC_GET_AG_RESBLKS:
> > return xfs_file_ioctl(filp, cmd, p);
> > #ifndef BROKEN_X86_ALIGNMENT
> > /* These are handled fine if no alignment issues */
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2017-09-21 17:30 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-21 0:17 [PATCH v10 00/27] xfs: online scrub support Darrick J. Wong
2017-09-21 0:17 ` [PATCH 01/27] xfs: return a distinct error code value for IGET_INCORE cache misses Darrick J. Wong
2017-09-21 14:36 ` Brian Foster
2017-09-21 0:17 ` [PATCH 02/27] xfs: query the per-AG reservation counters Darrick J. Wong
2017-09-21 14:36 ` Brian Foster
2017-09-21 17:30 ` Darrick J. Wong [this message]
2017-09-21 0:17 ` [PATCH 03/27] xfs: create an ioctl to scrub AG metadata Darrick J. Wong
2017-09-21 14:36 ` Brian Foster
2017-09-21 17:35 ` Darrick J. Wong
2017-09-21 17:52 ` Brian Foster
2017-09-22 3:26 ` Darrick J. Wong
2017-09-21 0:18 ` [PATCH 04/27] xfs: dispatch metadata scrub subcommands Darrick J. Wong
2017-09-21 14:37 ` Brian Foster
2017-09-21 18:08 ` Darrick J. Wong
2017-09-21 0:18 ` [PATCH 05/27] xfs: test the scrub ioctl Darrick J. Wong
2017-09-21 6:04 ` Dave Chinner
2017-09-21 18:14 ` Darrick J. Wong
2017-09-21 0:18 ` [PATCH 06/27] xfs: create helpers to record and deal with scrub problems Darrick J. Wong
2017-09-22 7:16 ` Dave Chinner
2017-09-22 16:44 ` Darrick J. Wong
2017-09-23 7:22 ` Dave Chinner
2017-09-23 7:24 ` Darrick J. Wong
2017-09-21 0:18 ` [PATCH 07/27] xfs: create helpers to scrub a metadata btree Darrick J. Wong
2017-09-22 7:23 ` Dave Chinner
2017-09-22 16:59 ` Darrick J. Wong
2017-09-21 0:18 ` [PATCH 08/27] xfs: scrub the shape of " Darrick J. Wong
2017-09-22 15:22 ` Brian Foster
2017-09-22 17:22 ` Darrick J. Wong
2017-09-22 19:13 ` Brian Foster
2017-09-22 20:14 ` Darrick J. Wong
2017-09-22 21:15 ` Brian Foster
2017-09-21 0:18 ` [PATCH 09/27] xfs: scrub btree keys and records Darrick J. Wong
2017-09-21 0:18 ` [PATCH 10/27] xfs: create helpers to scan an allocation group Darrick J. Wong
2017-09-21 0:18 ` [PATCH 11/27] xfs: scrub the backup superblocks Darrick J. Wong
2017-09-21 0:18 ` [PATCH 12/27] xfs: scrub AGF and AGFL Darrick J. Wong
2017-09-21 0:18 ` [PATCH 13/27] xfs: scrub the AGI Darrick J. Wong
2017-09-21 0:19 ` [PATCH 14/27] xfs: scrub free space btrees Darrick J. Wong
2017-09-21 0:19 ` [PATCH 15/27] xfs: scrub inode btrees Darrick J. Wong
2017-09-21 0:19 ` [PATCH 16/27] xfs: scrub rmap btrees Darrick J. Wong
2017-09-21 0:19 ` [PATCH 17/27] xfs: scrub refcount btrees Darrick J. Wong
2017-09-21 0:19 ` [PATCH 18/27] xfs: scrub inodes Darrick J. Wong
2017-09-21 0:19 ` [PATCH 19/27] xfs: scrub inode block mappings Darrick J. Wong
2017-09-21 0:19 ` [PATCH 20/27] xfs: scrub directory/attribute btrees Darrick J. Wong
2017-09-21 0:19 ` [PATCH 21/27] xfs: scrub directory metadata Darrick J. Wong
2017-09-21 0:19 ` [PATCH 22/27] xfs: scrub directory freespace Darrick J. Wong
2017-09-21 0:20 ` [PATCH 23/27] xfs: scrub extended attributes Darrick J. Wong
2017-09-21 0:20 ` [PATCH 24/27] xfs: scrub symbolic links Darrick J. Wong
2017-09-21 0:20 ` [PATCH 25/27] xfs: scrub parent pointers Darrick J. Wong
2017-09-21 0:20 ` [PATCH 26/27] xfs: scrub realtime bitmap/summary Darrick J. Wong
2017-09-21 0:20 ` [PATCH 27/27] xfs: scrub quota information Darrick J. Wong
2017-09-22 3:27 ` [PATCH] man: describe the metadata scrubbing ioctl Darrick J. Wong
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=20170921173006.GJ7112@magnolia \
--to=darrick.wong@oracle.com \
--cc=bfoster@redhat.com \
--cc=linux-xfs@vger.kernel.org \
/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