On Thu, Jul 09, 2020 at 05:04:48PM +0200, Christoph Hellwig wrote: > Keep all the error handling code together. > > Signed-off-by: Christoph Hellwig <hch@xxxxxx> > --- > fs/xfs/xfs_buf.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c > index e5592563dda6a1..e3e80615c5ed9e 100644 > --- a/fs/xfs/xfs_buf.c > +++ b/fs/xfs/xfs_buf.c > @@ -1285,6 +1285,14 @@ xfs_buf_ioend_disposition( > } > > /* Still considered a transient error. Caller will schedule retries. */ > + if (bp->b_flags & _XBF_INODES) > + xfs_buf_inode_io_fail(bp); > + else if (bp->b_flags & _XBF_DQUOTS) > + xfs_buf_dquot_io_fail(bp); > + else > + ASSERT(list_empty(&bp->b_li_list)); > + xfs_buf_ioerror(bp, 0); > + xfs_buf_relse(bp); > return XBF_IOEND_FAIL; Hm. I was about to whine about turning a "decide the outcome" function into a "decide the outcome and do some of it" function, but I guess the advantage of lazy reviewing is that I can skip to the last patch and see that this all gets swallowed into xfs_buf_ioend_handle_error, doesn't it? Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> --D > > resubmit: > @@ -1338,14 +1346,6 @@ xfs_buf_ioend( > case XBF_IOEND_DONE: > return; > case XBF_IOEND_FAIL: > - if (bp->b_flags & _XBF_INODES) > - xfs_buf_inode_io_fail(bp); > - else if (bp->b_flags & _XBF_DQUOTS) > - xfs_buf_dquot_io_fail(bp); > - else > - ASSERT(list_empty(&bp->b_li_list)); > - xfs_buf_ioerror(bp, 0); > - xfs_buf_relse(bp); > return; > default: > break; > -- > 2.26.2 >
