On Tue, Jul 27, 2021 at 03:54:00PM -0700, Darrick J. Wong wrote:
> On Mon, Jul 26, 2021 at 05:15:38PM +0530, Chandan Babu R wrote:
> > This commit renames XFS_IOC_BULKSTAT to XFS_IOC_BULKSTAT_V5 to allow a future
> > commit to extend bulkstat facility to support 64-bit extent counters. To this
> > end, this commit also renames xfs_bulkstat->bs_extents field to
> > xfs_bulkstat->bs_extents32.
> >
> > Signed-off-by: Chandan Babu R <chandanrlinux@xxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_fs.h | 4 ++--
> > fs/xfs/xfs_ioctl.c | 27 ++++++++++++++++++++++-----
> > fs/xfs/xfs_ioctl32.c | 7 +++++++
> > fs/xfs/xfs_itable.c | 4 ++--
> > fs/xfs/xfs_itable.h | 1 +
> > 5 files changed, 34 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> > index 2594fb647384..d760a969599e 100644
> > --- a/fs/xfs/libxfs/xfs_fs.h
> > +++ b/fs/xfs/libxfs/xfs_fs.h
> > @@ -394,7 +394,7 @@ struct xfs_bulkstat {
> > uint32_t bs_extsize_blks; /* extent size hint, blocks */
> >
> > uint32_t bs_nlink; /* number of links */
> > - uint32_t bs_extents; /* number of extents */
> > + uint32_t bs_extents32; /* number of extents */
>
> I wish I'd thought of this when we introduced bulkstat v5 so you
> wouldn't have had to do this.
>
> (I might have more to say in the bulkstat v6 patch review.)
> Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
Actually, I take that back, I have things to say /now/. :)
Rather than adding a whole new ioctl definition which (I haven't looked
at the xfsprogs changes) likely creates a bunch of churn in userspace,
what if we added a XFS_IBULK_ flag for supporting large extent counts?
There's also quite a bit of reserved padding space in xfs_bulk_ireq, so
perhaps we should define one of those padding u64 as a op-specific flag
field that would be a way to pass bulkstat-specific flags to the
relevant operations. That way the 64-bit extent counts are merely a
variant on bulkstat v5 instead of a whole new format.
I doubt very many bulkstat users have fully gone over to V5 yet; telling
them we have a V6 already isn't going to be fun... :/
--D
> --D
>
> > uint32_t bs_aextents; /* attribute number of extents */
> > uint16_t bs_version; /* structure version */
> > uint16_t bs_forkoff; /* inode fork offset in bytes */
> > @@ -853,7 +853,7 @@ struct xfs_scrub_metadata {
> > #define XFS_IOC_FSGEOMETRY_V4 _IOR ('X', 124, struct xfs_fsop_geom_v4)
> > #define XFS_IOC_GOINGDOWN _IOR ('X', 125, uint32_t)
> > #define XFS_IOC_FSGEOMETRY _IOR ('X', 126, struct xfs_fsop_geom)
> > -#define XFS_IOC_BULKSTAT _IOR ('X', 127, struct xfs_bulkstat_req)
> > +#define XFS_IOC_BULKSTAT_V5 _IOR ('X', 127, struct xfs_bulkstat_req)
> > #define XFS_IOC_INUMBERS _IOR ('X', 128, struct xfs_inumbers_req)
> > /* FIEXCHANGE_RANGE ----------- hoisted 129 */
> > /* XFS_IOC_GETFSUUID ---------- deprecated 140 */
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index 2da90f81e6e3..85f45340cb16 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -715,6 +715,8 @@ xfs_fsbulkstat_one_fmt(
> > {
> > struct xfs_bstat bs1;
> >
> > + ASSERT(breq->version == XFS_BULKSTAT_VERSION_V1);
> > +
> > xfs_bulkstat_to_bstat(breq->mp, &bs1, bstat);
> > if (copy_to_user(breq->ubuffer, &bs1, sizeof(bs1)))
> > return -EFAULT;
> > @@ -728,6 +730,8 @@ xfs_fsinumbers_fmt(
> > {
> > struct xfs_inogrp ig1;
> >
> > + ASSERT(breq->version == XFS_INUMBERS_VERSION_V1);
> > +
> > xfs_inumbers_to_inogrp(&ig1, igrp);
> > if (copy_to_user(breq->ubuffer, &ig1, sizeof(struct xfs_inogrp)))
> > return -EFAULT;
> > @@ -787,14 +791,17 @@ xfs_ioc_fsbulkstat(
> > */
> > if (cmd == XFS_IOC_FSINUMBERS) {
> > breq.startino = lastino ? lastino + 1 : 0;
> > + breq.version = XFS_INUMBERS_VERSION_V1;
> > error = xfs_inumbers(&breq, xfs_fsinumbers_fmt);
> > lastino = breq.startino - 1;
> > } else if (cmd == XFS_IOC_FSBULKSTAT_SINGLE) {
> > breq.startino = lastino;
> > breq.icount = 1;
> > + breq.version = XFS_BULKSTAT_VERSION_V1;
> > error = xfs_bulkstat_one(&breq, xfs_fsbulkstat_one_fmt);
> > } else { /* XFS_IOC_FSBULKSTAT */
> > breq.startino = lastino ? lastino + 1 : 0;
> > + breq.version = XFS_BULKSTAT_VERSION_V1;
> > error = xfs_bulkstat(&breq, xfs_fsbulkstat_one_fmt);
> > lastino = breq.startino - 1;
> > }
> > @@ -819,6 +826,8 @@ xfs_bulkstat_fmt(
> > struct xfs_ibulk *breq,
> > const struct xfs_bulkstat *bstat)
> > {
> > + ASSERT(breq->version == XFS_BULKSTAT_VERSION_V5);
> > +
> > if (copy_to_user(breq->ubuffer, bstat, sizeof(struct xfs_bulkstat)))
> > return -EFAULT;
> > return xfs_ibulk_advance(breq, sizeof(struct xfs_bulkstat));
> > @@ -918,13 +927,15 @@ STATIC int
> > xfs_ioc_bulkstat(
> > struct file *file,
> > unsigned int cmd,
> > - struct xfs_bulkstat_req __user *arg)
> > + struct xfs_bulkstat_req __user *arg,
> > + int version)
> > {
> > struct xfs_mount *mp = XFS_I(file_inode(file))->i_mount;
> > struct xfs_bulk_ireq hdr;
> > struct xfs_ibulk breq = {
> > .mp = mp,
> > .mnt_userns = file_mnt_user_ns(file),
> > + .version = version,
> > };
> > int error;
> >
> > @@ -960,6 +971,8 @@ xfs_inumbers_fmt(
> > struct xfs_ibulk *breq,
> > const struct xfs_inumbers *igrp)
> > {
> > + ASSERT(breq->version == XFS_INUMBERS_VERSION_V5);
> > +
> > if (copy_to_user(breq->ubuffer, igrp, sizeof(struct xfs_inumbers)))
> > return -EFAULT;
> > return xfs_ibulk_advance(breq, sizeof(struct xfs_inumbers));
> > @@ -970,11 +983,13 @@ STATIC int
> > xfs_ioc_inumbers(
> > struct xfs_mount *mp,
> > unsigned int cmd,
> > - struct xfs_inumbers_req __user *arg)
> > + struct xfs_inumbers_req __user *arg,
> > + int version)
> > {
> > struct xfs_bulk_ireq hdr;
> > struct xfs_ibulk breq = {
> > .mp = mp,
> > + .version = version,
> > };
> > int error;
> >
> > @@ -1882,10 +1897,12 @@ xfs_file_ioctl(
> > case XFS_IOC_FSINUMBERS:
> > return xfs_ioc_fsbulkstat(filp, cmd, arg);
> >
> > - case XFS_IOC_BULKSTAT:
> > - return xfs_ioc_bulkstat(filp, cmd, arg);
> > + case XFS_IOC_BULKSTAT_V5:
> > + return xfs_ioc_bulkstat(filp, cmd, arg,
> > + XFS_BULKSTAT_VERSION_V5);
> > case XFS_IOC_INUMBERS:
> > - return xfs_ioc_inumbers(mp, cmd, arg);
> > + return xfs_ioc_inumbers(mp, cmd, arg,
> > + XFS_INUMBERS_VERSION_V5);
> >
> > case XFS_IOC_FSGEOMETRY_V1:
> > return xfs_ioc_fsgeometry(mp, arg, 3);
> > diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> > index bef19060f4aa..d9a7abc209b5 100644
> > --- a/fs/xfs/xfs_ioctl32.c
> > +++ b/fs/xfs/xfs_ioctl32.c
> > @@ -88,6 +88,8 @@ xfs_fsinumbers_fmt_compat(
> > struct xfs_inogrp ig1;
> > struct xfs_inogrp *igrp = &ig1;
> >
> > + ASSERT(breq->version == XFS_INUMBERS_VERSION_V1);
> > +
> > xfs_inumbers_to_inogrp(&ig1, ig);
> >
> > if (put_user(igrp->xi_startino, &p32->xi_startino) ||
> > @@ -177,6 +179,8 @@ xfs_fsbulkstat_one_fmt_compat(
> > struct xfs_bstat bs1;
> > struct xfs_bstat *buffer = &bs1;
> >
> > + ASSERT(breq->version == XFS_BULKSTAT_VERSION_V1);
> > +
> > xfs_bulkstat_to_bstat(breq->mp, &bs1, bstat);
> >
> > if (put_user(buffer->bs_ino, &p32->bs_ino) ||
> > @@ -293,15 +297,18 @@ xfs_compat_ioc_fsbulkstat(
> > */
> > if (cmd == XFS_IOC_FSINUMBERS_32) {
> > breq.startino = lastino ? lastino + 1 : 0;
> > + breq.version = XFS_INUMBERS_VERSION_V1;
> > error = xfs_inumbers(&breq, inumbers_func);
> > lastino = breq.startino - 1;
> > } else if (cmd == XFS_IOC_FSBULKSTAT_SINGLE_32) {
> > breq.startino = lastino;
> > breq.icount = 1;
> > + breq.version = XFS_BULKSTAT_VERSION_V1;
> > error = xfs_bulkstat_one(&breq, bs_one_func);
> > lastino = breq.startino;
> > } else if (cmd == XFS_IOC_FSBULKSTAT_32) {
> > breq.startino = lastino ? lastino + 1 : 0;
> > + breq.version = XFS_BULKSTAT_VERSION_V1;
> > error = xfs_bulkstat(&breq, bs_one_func);
> > lastino = breq.startino - 1;
> > } else {
> > diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> > index 65c194cdea79..f0daa65e61ff 100644
> > --- a/fs/xfs/xfs_itable.c
> > +++ b/fs/xfs/xfs_itable.c
> > @@ -134,7 +134,7 @@ xfs_bulkstat_one_int(
> >
> > buf->bs_xflags = xfs_ip2xflags(ip);
> > buf->bs_extsize_blks = ip->i_extsize;
> > - buf->bs_extents = xfs_ifork_nextents(&ip->i_df);
> > + buf->bs_extents32 = xfs_ifork_nextents(&ip->i_df);
> > xfs_bulkstat_health(ip, buf);
> > buf->bs_aextents = xfs_ifork_nextents(ip->i_afp);
> > buf->bs_forkoff = XFS_IFORK_BOFF(ip);
> > @@ -356,7 +356,7 @@ xfs_bulkstat_to_bstat(
> > bs1->bs_blocks = bstat->bs_blocks;
> > bs1->bs_xflags = bstat->bs_xflags;
> > bs1->bs_extsize = XFS_FSB_TO_B(mp, bstat->bs_extsize_blks);
> > - bs1->bs_extents = bstat->bs_extents;
> > + bs1->bs_extents = bstat->bs_extents32;
> > bs1->bs_gen = bstat->bs_gen;
> > bs1->bs_projid_lo = bstat->bs_projectid & 0xFFFF;
> > bs1->bs_forkoff = bstat->bs_forkoff;
> > diff --git a/fs/xfs/xfs_itable.h b/fs/xfs/xfs_itable.h
> > index f5a13f69883a..ee4d8d0461ca 100644
> > --- a/fs/xfs/xfs_itable.h
> > +++ b/fs/xfs/xfs_itable.h
> > @@ -14,6 +14,7 @@ struct xfs_ibulk {
> > unsigned int icount; /* number of elements in ubuffer */
> > unsigned int ocount; /* number of records returned */
> > unsigned int flags; /* see XFS_IBULK_FLAG_* */
> > + int version; /* structure version to be returned */
> > };
> >
> > /* Only iterate within the same AG as startino */
> > --
> > 2.30.2
> >
