| Message ID | 20170829174835.2218-4-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
This probably ought to have a justification in the changelog, such as "Use the bmap abstraction instead of open-coding bmbt details here." Code otherwise looks ok. --D On Tue, Aug 29, 2017 at 07:48:30PM +0200, Christoph Hellwig wrote: > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/xfs/libxfs/xfs_bmap.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index bbde43c99323..c944987c485d 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -1359,7 +1359,6 @@ xfs_bmap_first_unused( > xfs_fileoff_t lastaddr; /* last block number seen */ > xfs_fileoff_t lowest; /* lowest useful block */ > xfs_fileoff_t max; /* starting useful block */ > - xfs_fileoff_t off; /* offset for this block */ > xfs_extnum_t nextents; /* number of extent entries */ > > ASSERT(XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_BTREE || > @@ -1376,16 +1375,19 @@ xfs_bmap_first_unused( > lowest = *first_unused; > nextents = xfs_iext_count(ifp); > for (idx = 0, lastaddr = 0, max = lowest; idx < nextents; idx++) { > - xfs_bmbt_rec_host_t *ep = xfs_iext_get_ext(ifp, idx); > - off = xfs_bmbt_get_startoff(ep); > + struct xfs_bmbt_irec got; > + > + xfs_iext_get_extent(ifp, idx, &got); > + > /* > * See if the hole before this extent will work. > */ > - if (off >= lowest + len && off - max >= len) { > + if (got.br_startoff >= lowest + len && > + got.br_startoff - max >= len) { > *first_unused = max; > return 0; > } > - lastaddr = off + xfs_bmbt_get_blockcount(ep); > + lastaddr = got.br_startoff + got.br_blockcount; > max = XFS_FILEOFF_MAX(lastaddr, lowest); > } > *first_unused = max; > -- > 2.11.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 29, 2017 at 11:41:48AM -0700, Darrick J. Wong wrote: > This probably ought to have a justification in the changelog, such as > "Use the bmap abstraction instead of open-coding bmbt details here." > > Code otherwise looks ok. Sure. Do you just want to add the above? -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 01, 2017 at 10:06:21PM +0200, Christoph Hellwig wrote: > On Tue, Aug 29, 2017 at 11:41:48AM -0700, Darrick J. Wong wrote: > > This probably ought to have a justification in the changelog, such as > > "Use the bmap abstraction instead of open-coding bmbt details here." > > > > Code otherwise looks ok. > > Sure. Do you just want to add the above? Done. --D > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index bbde43c99323..c944987c485d 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -1359,7 +1359,6 @@ xfs_bmap_first_unused( xfs_fileoff_t lastaddr; /* last block number seen */ xfs_fileoff_t lowest; /* lowest useful block */ xfs_fileoff_t max; /* starting useful block */ - xfs_fileoff_t off; /* offset for this block */ xfs_extnum_t nextents; /* number of extent entries */ ASSERT(XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_BTREE || @@ -1376,16 +1375,19 @@ xfs_bmap_first_unused( lowest = *first_unused; nextents = xfs_iext_count(ifp); for (idx = 0, lastaddr = 0, max = lowest; idx < nextents; idx++) { - xfs_bmbt_rec_host_t *ep = xfs_iext_get_ext(ifp, idx); - off = xfs_bmbt_get_startoff(ep); + struct xfs_bmbt_irec got; + + xfs_iext_get_extent(ifp, idx, &got); + /* * See if the hole before this extent will work. */ - if (off >= lowest + len && off - max >= len) { + if (got.br_startoff >= lowest + len && + got.br_startoff - max >= len) { *first_unused = max; return 0; } - lastaddr = off + xfs_bmbt_get_blockcount(ep); + lastaddr = got.br_startoff + got.br_blockcount; max = XFS_FILEOFF_MAX(lastaddr, lowest); } *first_unused = max;
Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/libxfs/xfs_bmap.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)