On Wed, Sep 23, 2020 at 08:16:14AM +0100, Christoph Hellwig wrote:
> On Wed, Sep 16, 2020 at 08:29:30PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > The bmap intent item checking code in xfs_bui_item_recover is spread all
> > over the function. We should check the recovered log item at the top
> > before we allocate any resources or do anything else, so do that.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > fs/xfs/xfs_bmap_item.c | 57 ++++++++++++++++--------------------------------
> > 1 file changed, 19 insertions(+), 38 deletions(-)
> >
> >
> > diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> > index 0a0904a7650a..877afe76d76a 100644
> > --- a/fs/xfs/xfs_bmap_item.c
> > +++ b/fs/xfs/xfs_bmap_item.c
> > @@ -437,17 +437,13 @@ xfs_bui_item_recover(
> > xfs_fsblock_t inode_fsb;
> > xfs_filblks_t count;
> > xfs_exntst_t state;
> > - enum xfs_bmap_intent_type type;
> > - bool op_ok;
> > unsigned int bui_type;
> > int whichfork;
> > int error = 0;
> >
> > /* Only one mapping operation per BUI... */
> > - if (buip->bui_format.bui_nextents != XFS_BUI_MAX_FAST_EXTENTS) {
> > - xfs_bui_release(buip);
> > - return -EFSCORRUPTED;
> > - }
> > + if (buip->bui_format.bui_nextents != XFS_BUI_MAX_FAST_EXTENTS)
> > + goto garbage;
>
> We don't really need the xfs_bui_release any more, and can stick to
> plain "return -EFSCORRUPTED" instead of the goto, but I suspect the
> previous patch has taken care of that and you've rebased already?
Yep.
--D
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
