On Fri, May 22, 2020 at 6:51 AM Dave Chinner <david@xxxxxxxxxxxxx> wrote:
>
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Inode buffers always have write IO callbacks, so by marking them
> directly we can avoid needing to attach ->b_iodone functions to
> them. This avoids an indirect call, and makes future modifications
> much simpler.
>
> This is largely a rearrangement of the code at this point - no IO
> completion functionality changes at this point, just how the
> code is run is modified.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/xfs_buf.c | 18 +++++++++++++-----
> fs/xfs/xfs_buf.h | 39 ++++++++++++++++++++++++++-------------
> fs/xfs/xfs_buf_item.c | 42 +++++++++++++++++++++++++++++++-----------
> fs/xfs/xfs_buf_item.h | 1 +
> fs/xfs/xfs_inode.c | 2 +-
> fs/xfs/xfs_trans_buf.c | 3 +++
> 6 files changed, 75 insertions(+), 30 deletions(-)
>
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 9c2fbb6bbf89d..6105b97028d6a 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -14,6 +14,8 @@
> #include "xfs_mount.h"
> #include "xfs_trace.h"
> #include "xfs_log.h"
> +#include "xfs_trans.h"
> +#include "xfs_buf_item.h"
> #include "xfs_errortag.h"
> #include "xfs_error.h"
>
> @@ -1202,12 +1204,18 @@ xfs_buf_ioend(
> bp->b_flags |= XBF_DONE;
> }
>
> - if (bp->b_iodone)
> + /* inodes always have a callback on write */
> + if (!read && (bp->b_flags & _XBF_INODES)) {
> + xfs_buf_inode_iodone(bp);
> + return;
> + }
> +
> + if (bp->b_iodone) {
> (*(bp->b_iodone))(bp);
> - else if (bp->b_flags & XBF_ASYNC)
> - xfs_buf_relse(bp);
> - else
> - complete(&bp->b_iowait);
> + return;
> + }
> +
> + xfs_buf_ioend_finish(bp);
> }
>
> static void
> diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
> index 050c53b739e24..b3e5d653d09f1 100644
> --- a/fs/xfs/xfs_buf.h
> +++ b/fs/xfs/xfs_buf.h
> @@ -30,15 +30,19 @@
> #define XBF_STALE (1 << 6) /* buffer has been staled, do not find it */
> #define XBF_WRITE_FAIL (1 << 7) /* async writes have failed on this buffer */
>
> -/* flags used only as arguments to access routines */
> -#define XBF_TRYLOCK (1 << 16)/* lock requested, but do not wait */
> -#define XBF_UNMAPPED (1 << 17)/* do not map the buffer */
> +/* buffer type flags for write callbacks */
> +#define _XBF_INODES (1 << 16)/* inode buffer */
As I wrote on review of another type flag, best add a definition
of XBF_BUFFER_TYPE_MASK and document that buffer type
flags are mutually exclusive, maybe even ASSERT it in some
places.
For not changing logic by rearranging code:
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
Thanks,
Amir.
