| Message ID | 20200509170125.952508-8-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/8] libxfs: use tabs instead of spaces in div_u64 | expand |
On Sat, May 09, 2020 at 07:01:24PM +0200, Christoph Hellwig wrote: > No need to have two variables for the AGFL block number array. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > repair/phase5.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/repair/phase5.c b/repair/phase5.c > index 17b57448..677297fe 100644 > --- a/repair/phase5.c > +++ b/repair/phase5.c > @@ -2149,18 +2149,15 @@ build_agf_agfl( > > /* setting to 0xff results in initialisation to NULLAGBLOCK */ > memset(agfl, 0xff, mp->m_sb.sb_sectsize); /me wonders why this memset isn't sufficient to null out the freelist, but a better cleanup would be to rip all this out in favor of adapting the nearly identical functions in xfs_ag.c. In the meantime we don't need duplicate variables, and: Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > + freelist = xfs_buf_to_agfl_bno(agfl_buf); > if (xfs_sb_version_hascrc(&mp->m_sb)) { > - __be32 *agfl_bno = xfs_buf_to_agfl_bno(agfl_buf); > - > agfl->agfl_magicnum = cpu_to_be32(XFS_AGFL_MAGIC); > agfl->agfl_seqno = cpu_to_be32(agno); > platform_uuid_copy(&agfl->agfl_uuid, &mp->m_sb.sb_meta_uuid); > for (i = 0; i < libxfs_agfl_size(mp); i++) > - agfl_bno[i] = cpu_to_be32(NULLAGBLOCK); > + freelist[i] = cpu_to_be32(NULLAGBLOCK); > } > > - freelist = xfs_buf_to_agfl_bno(agfl_buf); > - > /* > * do we have left-over blocks in the btree cursors that should > * be used to fill the AGFL? > -- > 2.26.2 >
On 5/9/20 12:11 PM, Darrick J. Wong wrote: > On Sat, May 09, 2020 at 07:01:24PM +0200, Christoph Hellwig wrote: >> No need to have two variables for the AGFL block number array. >> >> Signed-off-by: Christoph Hellwig <hch@lst.de> >> --- >> repair/phase5.c | 7 ++----- >> 1 file changed, 2 insertions(+), 5 deletions(-) >> >> diff --git a/repair/phase5.c b/repair/phase5.c >> index 17b57448..677297fe 100644 >> --- a/repair/phase5.c >> +++ b/repair/phase5.c >> @@ -2149,18 +2149,15 @@ build_agf_agfl( >> >> /* setting to 0xff results in initialisation to NULLAGBLOCK */ >> memset(agfl, 0xff, mp->m_sb.sb_sectsize); > > /me wonders why this memset isn't sufficient to null out the freelist, > but a better cleanup would be to rip all this out in favor of adapting > the nearly identical functions in xfs_ag.c. probably because xfs_agflblock_init is a) static and b) expects a aghdr_init_data *id arg which isn't too convenient here I guess Might be nice to factor xfs_agflblock_init to call a helper that this and build_agf_agfl can both use though, I'll give that a whirl. -Eric > In the meantime we don't need duplicate variables, and: > > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > > --D > >> + freelist = xfs_buf_to_agfl_bno(agfl_buf); >> if (xfs_sb_version_hascrc(&mp->m_sb)) { >> - __be32 *agfl_bno = xfs_buf_to_agfl_bno(agfl_buf); >> - >> agfl->agfl_magicnum = cpu_to_be32(XFS_AGFL_MAGIC); >> agfl->agfl_seqno = cpu_to_be32(agno); >> platform_uuid_copy(&agfl->agfl_uuid, &mp->m_sb.sb_meta_uuid); >> for (i = 0; i < libxfs_agfl_size(mp); i++) >> - agfl_bno[i] = cpu_to_be32(NULLAGBLOCK); >> + freelist[i] = cpu_to_be32(NULLAGBLOCK); >> } >> >> - freelist = xfs_buf_to_agfl_bno(agfl_buf); >> - >> /* >> * do we have left-over blocks in the btree cursors that should >> * be used to fill the AGFL? >> -- >> 2.26.2 >> >
diff --git a/repair/phase5.c b/repair/phase5.c index 17b57448..677297fe 100644 --- a/repair/phase5.c +++ b/repair/phase5.c @@ -2149,18 +2149,15 @@ build_agf_agfl( /* setting to 0xff results in initialisation to NULLAGBLOCK */ memset(agfl, 0xff, mp->m_sb.sb_sectsize); + freelist = xfs_buf_to_agfl_bno(agfl_buf); if (xfs_sb_version_hascrc(&mp->m_sb)) { - __be32 *agfl_bno = xfs_buf_to_agfl_bno(agfl_buf); - agfl->agfl_magicnum = cpu_to_be32(XFS_AGFL_MAGIC); agfl->agfl_seqno = cpu_to_be32(agno); platform_uuid_copy(&agfl->agfl_uuid, &mp->m_sb.sb_meta_uuid); for (i = 0; i < libxfs_agfl_size(mp); i++) - agfl_bno[i] = cpu_to_be32(NULLAGBLOCK); + freelist[i] = cpu_to_be32(NULLAGBLOCK); } - freelist = xfs_buf_to_agfl_bno(agfl_buf); - /* * do we have left-over blocks in the btree cursors that should * be used to fill the AGFL?
No need to have two variables for the AGFL block number array. Signed-off-by: Christoph Hellwig <hch@lst.de> --- repair/phase5.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)