On 28 Jul 2021 at 04:47, Dave Chinner wrote:
> On Tue, Jul 27, 2021 at 04:00:02PM -0700, Darrick J. Wong wrote:
>> 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.
>
> Yup, this.
>
> The only reason for creating a new revision of the ioctl is if we've
> run out of expansion space in the existing ioctl structures to cater
> for new information we want to export to userspace.
>
Thanks for the suggestion. I will make the relevant changes before posting the
next version of the patchset.
--
chandan
