* [PATCH] xfs: Fix 'set but not used' warning in xfs_bmap_compute_alignments() @ 2021-01-27 9:05 Chandan Babu R 2021-01-28 15:34 ` Brian Foster 0 siblings, 1 reply; 5+ messages in thread From: Chandan Babu R @ 2021-01-27 9:05 UTC (permalink / raw) To: linux-xfs; +Cc: Chandan Babu R, djwong, allison.henderson, kernel test robot 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); ASSERT(ap->length); } -- 2.29.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: Fix 'set but not used' warning in xfs_bmap_compute_alignments() 2021-01-27 9:05 [PATCH] xfs: Fix 'set but not used' warning in xfs_bmap_compute_alignments() Chandan Babu R @ 2021-01-28 15:34 ` Brian Foster 2021-01-28 17:44 ` Darrick J. Wong 0 siblings, 1 reply; 5+ messages in thread From: Brian Foster @ 2021-01-28 15:34 UTC (permalink / raw) To: Chandan Babu R; +Cc: linux-xfs, djwong, allison.henderson, kernel test robot 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? Brian > ASSERT(ap->length); > } > > -- > 2.29.2 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: Fix 'set but not used' warning in xfs_bmap_compute_alignments() 2021-01-28 15:34 ` Brian Foster @ 2021-01-28 17:44 ` Darrick J. Wong 2021-01-29 7:40 ` Chandan Babu R 0 siblings, 1 reply; 5+ messages in thread From: Darrick J. Wong @ 2021-01-28 17:44 UTC (permalink / raw) To: Brian Foster Cc: Chandan Babu R, linux-xfs, allison.henderson, kernel test robot 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. --D > Brian > > > ASSERT(ap->length); > > } > > > > -- > > 2.29.2 > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xfs: Fix 'set but not used' warning in xfs_bmap_compute_alignments() 2021-01-28 17:44 ` Darrick J. Wong @ 2021-01-29 7:40 ` Chandan Babu R [not found] ` <20210129124638.GA2660974@bfoster> 0 siblings, 1 reply; 5+ messages in thread From: Chandan Babu R @ 2021-01-29 7:40 UTC (permalink / raw) To: Brian Foster Cc: Darrick J. Wong, linux-xfs, allison.henderson, kernel test robot 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <20210129124638.GA2660974@bfoster>]
* Re: [PATCH] xfs: Fix 'set but not used' warning in xfs_bmap_compute_alignments() [not found] ` <20210129124638.GA2660974@bfoster> @ 2021-01-29 17:08 ` Darrick J. Wong 0 siblings, 0 replies; 5+ messages in thread From: Darrick J. Wong @ 2021-01-29 17:08 UTC (permalink / raw) To: Brian Foster Cc: Chandan Babu R, linux-xfs, allison.henderson, kernel test robot 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@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. > > > > 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 > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-29 17:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-27 9:05 [PATCH] xfs: Fix 'set but not used' warning in xfs_bmap_compute_alignments() Chandan Babu R
2021-01-28 15:34 ` Brian Foster
2021-01-28 17:44 ` Darrick J. Wong
2021-01-29 7:40 ` Chandan Babu R
[not found] ` <20210129124638.GA2660974@bfoster>
2021-01-29 17:08 ` Darrick J. Wong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox