On Fri, Jan 29, 2021 at 07:46:38AM -0500, Brian Foster wrote:
> On Fri, Jan 29, 2021 at 01:10:00PM +0530, Chandan Babu R wrote:
> > 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@xxxxxxxxx>
> > >> > Signed-off-by: Chandan Babu R <chandanrlinux@xxxxxxxxx>
> > >> > ---
> > >> > 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.
> >
>
> Ah, I see. Could you fix the indentation/alignment of the call so it
> looks something like the following?
>
> if (xfs_bmap_extsize_align(mp, &ap->got, &ap->prev, align, 0,
> ap->eof, 0, ap->conv, &ap->offset,
> &ap->length))
> ASSERT(0);
>
> Otherwise the patch seems fine to me.
Yeah, I'll fix it in my tree before I push out for-next again.
--D
> Brian
>
> > --
> > chandan
> >
>
