Re: [PATCH] xfs: Fix 'set but not used' warning in xfs_bmap_compute_alignments()

Chandan Babu R
2021-01-29T07:41:14Z

On 28 Jan 2021 at 23:14, Darrick J. Wong wrote:
> On Thu, Jan 28, 2021 at 10:34:12AM -0500, Brian Foster wrote:
>> On Wed, Jan 27, 2021 at 02:35:37PM +0530, Chandan Babu R wrote:
>> > With both CONFIG_XFS_DEBUG and CONFIG_XFS_WARN disabled, the only reference to
>> > local variable "error" in xfs_bmap_compute_alignments() gets eliminated during
>> > pre-processing stage of the compilation process. This causes the compiler to
>> > generate a "set but not used" warning.
>> >
>> > Reported-by: kernel test robot <lkp@intel.com>
>> > Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
>> > ---
>> > This patch is applicable on top of current xfs-linux/for-next branch.
>> >
>> > fs/xfs/libxfs/xfs_bmap.c | 9 ++++-----
>> > 1 file changed, 4 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
>> > index 2cd24bb06040..ba56554e8c05 100644
>> > --- a/fs/xfs/libxfs/xfs_bmap.c
>> > +++ b/fs/xfs/libxfs/xfs_bmap.c
>> > @@ -3471,7 +3471,6 @@ xfs_bmap_compute_alignments(
>> > struct xfs_mount *mp = args->mp;
>> > xfs_extlen_t align = 0; /* minimum allocation alignment */
>> > int stripe_align = 0;
>> > - int error;
>> >
>> > /* stripe alignment for allocation is determined by mount parameters */
>> > if (mp->m_swidth && (mp->m_flags & XFS_MOUNT_SWALLOC))
>> > @@ -3484,10 +3483,10 @@ xfs_bmap_compute_alignments(
>> > else if (ap->datatype & XFS_ALLOC_USERDATA)
>> > align = xfs_get_extsz_hint(ap->ip);
>> > if (align) {
>> > - error = xfs_bmap_extsize_align(mp, &ap->got, &ap->prev,
>> > - align, 0, ap->eof, 0, ap->conv,
>> > - &ap->offset, &ap->length);
>> > - ASSERT(!error);
>> > + if (xfs_bmap_extsize_align(mp, &ap->got, &ap->prev,
>> > + align, 0, ap->eof, 0, ap->conv, &ap->offset,
>> > + &ap->length))
>> > + ASSERT(0);
>>
>> I was wondering if we should just make xfs_bmap_extsize_align() return
>> void and push the asserts down into the function itself, but it looks
>> like xfs_bmap_rtalloc() actually handles the error. Any idea on why we
>> might have that inconsistency?
>
> It only returns nonzero if isrt (the fifth parameter) is nonzero, and
> only if the requested range is still not aligned to the rt extent size
> after aligning it and eliminating any overlaps with existing extents.
>

Adding to what Darrick has mentioned above ...

Space on realtime devices are tracked at a granularity of "rextsize"
bytes. Each bit held in the data blocks of xfs_mount->m_rbmip represents usage
status of a single rextsized block. Most likely this seems to be underlying
reason for strict allocation alignment requirements for realtime files.

--
chandan