| Message ID | 20200709150453.109230-4-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/13] xfs: refactor the buf ioend disposition code | expand |
On Thu, Jul 09, 2020 at 05:04:43PM +0200, Christoph Hellwig wrote: > Refactor the logic for the different I/O completions to prepare for > more code sharing. > > Signed-off-by: Christoph Hellwig <hch@lst.de> I think it's worth mentioning in the commit log that this patch leaves the log recovery buffer completion code totally in charge of the buffer state. Not sure where exactly that part is going, though I guess your eventual goal is to remove xlog_recover_iodone (and clean up the buffer log item disposal)...? If so, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/xfs_buf.c | 41 +++++++++++++++++----------------------- > fs/xfs/xfs_log_recover.c | 14 +++++++------- > 2 files changed, 24 insertions(+), 31 deletions(-) > > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c > index 1bce6457a9b943..7c22d63f43f754 100644 > --- a/fs/xfs/xfs_buf.c > +++ b/fs/xfs/xfs_buf.c > @@ -1199,33 +1199,26 @@ xfs_buf_ioend( > if (!bp->b_error) > bp->b_flags |= XBF_DONE; > xfs_buf_ioend_finish(bp); > - return; > - } > - > - if (!bp->b_error) { > - bp->b_flags &= ~XBF_WRITE_FAIL; > - bp->b_flags |= XBF_DONE; > - } > - > - /* > - * If this is a log recovery buffer, we aren't doing transactional IO > - * yet so we need to let it handle IO completions. > - */ > - if (bp->b_flags & _XBF_LOGRECOVERY) { > + } else if (bp->b_flags & _XBF_LOGRECOVERY) { > + /* > + * If this is a log recovery buffer, we aren't doing > + * transactional I/O yet so we need to let the log recovery code > + * handle I/O completions: > + */ > xlog_recover_iodone(bp); > - return; > - } > - > - if (bp->b_flags & _XBF_INODES) { > - xfs_buf_inode_iodone(bp); > - return; > - } > + } else { > + if (!bp->b_error) { > + bp->b_flags &= ~XBF_WRITE_FAIL; > + bp->b_flags |= XBF_DONE; > + } > > - if (bp->b_flags & _XBF_DQUOTS) { > - xfs_buf_dquot_iodone(bp); > - return; > + if (bp->b_flags & _XBF_INODES) > + xfs_buf_inode_iodone(bp); > + else if (bp->b_flags & _XBF_DQUOTS) > + xfs_buf_dquot_iodone(bp); > + else > + xfs_buf_iodone(bp); > } > - xfs_buf_iodone(bp); > } > > static void > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > index 52a65a74208ffe..cf6dabb98f2327 100644 > --- a/fs/xfs/xfs_log_recover.c > +++ b/fs/xfs/xfs_log_recover.c > @@ -269,15 +269,15 @@ void > xlog_recover_iodone( > struct xfs_buf *bp) > { > - if (bp->b_error) { > + if (!bp->b_error) { > + bp->b_flags |= XBF_DONE; > + } else if (!XFS_FORCED_SHUTDOWN(bp->b_mount)) { > /* > - * We're not going to bother about retrying > - * this during recovery. One strike! > + * We're not going to bother about retrying this during > + * recovery. One strike! > */ > - if (!XFS_FORCED_SHUTDOWN(bp->b_mount)) { > - xfs_buf_ioerror_alert(bp, __this_address); > - xfs_force_shutdown(bp->b_mount, SHUTDOWN_META_IO_ERROR); > - } > + xfs_buf_ioerror_alert(bp, __this_address); > + xfs_force_shutdown(bp->b_mount, SHUTDOWN_META_IO_ERROR); > } > > /* > -- > 2.26.2 >
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index 1bce6457a9b943..7c22d63f43f754 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -1199,33 +1199,26 @@ xfs_buf_ioend( if (!bp->b_error) bp->b_flags |= XBF_DONE; xfs_buf_ioend_finish(bp); - return; - } - - if (!bp->b_error) { - bp->b_flags &= ~XBF_WRITE_FAIL; - bp->b_flags |= XBF_DONE; - } - - /* - * If this is a log recovery buffer, we aren't doing transactional IO - * yet so we need to let it handle IO completions. - */ - if (bp->b_flags & _XBF_LOGRECOVERY) { + } else if (bp->b_flags & _XBF_LOGRECOVERY) { + /* + * If this is a log recovery buffer, we aren't doing + * transactional I/O yet so we need to let the log recovery code + * handle I/O completions: + */ xlog_recover_iodone(bp); - return; - } - - if (bp->b_flags & _XBF_INODES) { - xfs_buf_inode_iodone(bp); - return; - } + } else { + if (!bp->b_error) { + bp->b_flags &= ~XBF_WRITE_FAIL; + bp->b_flags |= XBF_DONE; + } - if (bp->b_flags & _XBF_DQUOTS) { - xfs_buf_dquot_iodone(bp); - return; + if (bp->b_flags & _XBF_INODES) + xfs_buf_inode_iodone(bp); + else if (bp->b_flags & _XBF_DQUOTS) + xfs_buf_dquot_iodone(bp); + else + xfs_buf_iodone(bp); } - xfs_buf_iodone(bp); } static void diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 52a65a74208ffe..cf6dabb98f2327 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -269,15 +269,15 @@ void xlog_recover_iodone( struct xfs_buf *bp) { - if (bp->b_error) { + if (!bp->b_error) { + bp->b_flags |= XBF_DONE; + } else if (!XFS_FORCED_SHUTDOWN(bp->b_mount)) { /* - * We're not going to bother about retrying - * this during recovery. One strike! + * We're not going to bother about retrying this during + * recovery. One strike! */ - if (!XFS_FORCED_SHUTDOWN(bp->b_mount)) { - xfs_buf_ioerror_alert(bp, __this_address); - xfs_force_shutdown(bp->b_mount, SHUTDOWN_META_IO_ERROR); - } + xfs_buf_ioerror_alert(bp, __this_address); + xfs_force_shutdown(bp->b_mount, SHUTDOWN_META_IO_ERROR); } /*
Refactor the logic for the different I/O completions to prepare for more code sharing. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_buf.c | 41 +++++++++++++++++----------------------- fs/xfs/xfs_log_recover.c | 14 +++++++------- 2 files changed, 24 insertions(+), 31 deletions(-)