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

[Chandan Babu R <chandanrlinux@gmail.com>]

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