On 07 Feb 2022 at 10:26, Chandan Babu R wrote:
> On 02 Feb 2022 at 00:54, Darrick J. Wong wrote:
>> On Fri, Jan 21, 2022 at 10:48:55AM +0530, Chandan Babu R wrote:
>>> The following changes are made to enable userspace to obtain 64-bit extent
>>> counters,
>>> 1. Carve out a new 64-bit field xfs_bulkstat->bs_extents64 from
>>> xfs_bulkstat->bs_pad[] to hold 64-bit extent counter.
>>> 2. Define the new flag XFS_BULK_IREQ_BULKSTAT for userspace to indicate that
>>> it is capable of receiving 64-bit extent counters.
>>>
>>> Suggested-by: Darrick J. Wong <djwong@xxxxxxxxxx>
>>> Signed-off-by: Chandan Babu R <chandan.babu@xxxxxxxxxx>
>>> ---
>>> fs/xfs/libxfs/xfs_fs.h | 12 ++++++++----
>>> fs/xfs/xfs_ioctl.c | 3 +++
>>> fs/xfs/xfs_itable.c | 27 +++++++++++++++++++++++++--
>>> fs/xfs/xfs_itable.h | 7 ++++++-
>>> fs/xfs/xfs_iwalk.h | 7 +++++--
>>> 5 files changed, 47 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
>>> index 42bc39501d81..4e12530eb518 100644
>>> --- a/fs/xfs/libxfs/xfs_fs.h
>>> +++ b/fs/xfs/libxfs/xfs_fs.h
>>> @@ -393,7 +393,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_extents; /* 32-bit data fork extent counter */
>>> uint32_t bs_aextents; /* attribute number of extents */
>>> uint16_t bs_version; /* structure version */
>>> uint16_t bs_forkoff; /* inode fork offset in bytes */
>>> @@ -402,8 +402,9 @@ struct xfs_bulkstat {
>>> uint16_t bs_checked; /* checked inode metadata */
>>> uint16_t bs_mode; /* type and mode */
>>> uint16_t bs_pad2; /* zeroed */
>>> + uint64_t bs_extents64; /* 64-bit data fork extent counter */
>>>
>>> - uint64_t bs_pad[7]; /* zeroed */
>>> + uint64_t bs_pad[6]; /* zeroed */
>>> };
>>>
>>> #define XFS_BULKSTAT_VERSION_V1 (1)
>>> @@ -484,8 +485,11 @@ struct xfs_bulk_ireq {
>>> */
>>> #define XFS_BULK_IREQ_SPECIAL (1 << 1)
>>>
>>> -#define XFS_BULK_IREQ_FLAGS_ALL (XFS_BULK_IREQ_AGNO | \
>>> - XFS_BULK_IREQ_SPECIAL)
>>> +#define XFS_BULK_IREQ_NREXT64 (1 << 2)
>>
>> This needs a comment specifying the behavior of this flag.
>>
>> If the flag is set and the data fork extent count fits in both fields,
>> will they both be filled out?
>
> If the flag is set, xfs_bulkstat->bs_extents64 field will be assigned the data
> fork extent count and xfs_bulkstat->bs_extents will be set to 0
> (xfs_bulkstat() allocates xfs_bstat_chunk->buf by invoking kmem_zalloc()).
>
> If the flag is not set, xfs_bulkstat->bs_extents field will be assigned the
> data fork extent count and xfs_bulkstat->bs_extents64 will be set to 0.
>
>>
>> If the flag is set but the data fork extent count only fits in
>> bs_extents64, what will be written to bs_extents?
>
> bs_extents will be set to zero.
>
>>
>> If the flag is not set and the data fork extent count won't fit in
>> bs_extents, do we return an error value? Fill it with garbage?
>>
>
> In this case, we return -EOVERFLOW and the contents of bs_extents will be set
> to zero. This happens because xfs_bulkstat() will return success even if
> xfs_iwalk() returned an error provided that we already have collected details
> about one more inodes. The next call to xfs_ioc_bulkstat() will start from the
> problematic inode. Here we allocate xfs_bstat_chunk->buf using kmem_zalloc()
> which zeroes the contents of the allocated memory and returns -EOVERFLOW to
> userspace.
>
Sorry, I forgot to mention that I will add the above data points as a comment
before posting the next version of the patchset.
--
chandan
