From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 13/15] xfs: simplify xfs_trans_getsb
Date: Tue, 1 Sep 2020 09:58:21 -0700 [thread overview]
Message-ID: <20200901165821.GF6096@magnolia> (raw)
In-Reply-To: <20200901155018.2524-14-hch@lst.de>
On Tue, Sep 01, 2020 at 05:50:16PM +0200, Christoph Hellwig wrote:
> Remove the mp argument as this function is only called in transaction
> context, and open code xfs_getsb given that the function already accesses
> the buffer pointer in the mount point directly.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks pretty straightforward to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/libxfs/xfs_sb.c | 4 ++--
> fs/xfs/xfs_trans.c | 2 +-
> fs/xfs/xfs_trans.h | 2 +-
> fs/xfs/xfs_trans_buf.c | 46 ++++++++++++++----------------------------
> 4 files changed, 19 insertions(+), 35 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index ae9aaf1f34bfcc..15d03d96753769 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -954,7 +954,7 @@ xfs_log_sb(
> struct xfs_trans *tp)
> {
> struct xfs_mount *mp = tp->t_mountp;
> - struct xfs_buf *bp = xfs_trans_getsb(tp, mp);
> + struct xfs_buf *bp = xfs_trans_getsb(tp);
>
> mp->m_sb.sb_icount = percpu_counter_sum(&mp->m_icount);
> mp->m_sb.sb_ifree = percpu_counter_sum(&mp->m_ifree);
> @@ -1084,7 +1084,7 @@ xfs_sync_sb_buf(
> if (error)
> return error;
>
> - bp = xfs_trans_getsb(tp, mp);
> + bp = xfs_trans_getsb(tp);
> xfs_log_sb(tp);
> xfs_trans_bhold(tp, bp);
> xfs_trans_set_sync(tp);
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index ed72867b1a1937..ca18a040336a1d 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -468,7 +468,7 @@ xfs_trans_apply_sb_deltas(
> xfs_buf_t *bp;
> int whole = 0;
>
> - bp = xfs_trans_getsb(tp, tp->t_mountp);
> + bp = xfs_trans_getsb(tp);
> sbp = bp->b_addr;
>
> /*
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index b752501818d257..f46534b7523698 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -209,7 +209,7 @@ xfs_trans_read_buf(
> flags, bpp, ops);
> }
>
> -struct xfs_buf *xfs_trans_getsb(xfs_trans_t *, struct xfs_mount *);
> +struct xfs_buf *xfs_trans_getsb(struct xfs_trans *);
>
> void xfs_trans_brelse(xfs_trans_t *, struct xfs_buf *);
> void xfs_trans_bjoin(xfs_trans_t *, struct xfs_buf *);
> diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
> index 11cd666cd99a63..42d63b830cb9c8 100644
> --- a/fs/xfs/xfs_trans_buf.c
> +++ b/fs/xfs/xfs_trans_buf.c
> @@ -166,50 +166,34 @@ xfs_trans_get_buf_map(
> }
>
> /*
> - * Get and lock the superblock buffer of this file system for the
> - * given transaction.
> - *
> - * We don't need to use incore_match() here, because the superblock
> - * buffer is a private buffer which we keep a pointer to in the
> - * mount structure.
> + * Get and lock the superblock buffer for the given transaction.
> */
> -xfs_buf_t *
> +struct xfs_buf *
> xfs_trans_getsb(
> - xfs_trans_t *tp,
> - struct xfs_mount *mp)
> + struct xfs_trans *tp)
> {
> - xfs_buf_t *bp;
> - struct xfs_buf_log_item *bip;
> + struct xfs_buf *bp = tp->t_mountp->m_sb_bp;
>
> /*
> - * Default to just trying to lock the superblock buffer
> - * if tp is NULL.
> + * Just increment the lock recursion count if the buffer is already
> + * attached to this transaction.
> */
> - if (tp == NULL)
> - return xfs_getsb(mp);
> -
> - /*
> - * If the superblock buffer already has this transaction
> - * pointer in its b_fsprivate2 field, then we know we already
> - * have it locked. In this case we just increment the lock
> - * recursion count and return the buffer to the caller.
> - */
> - bp = mp->m_sb_bp;
> if (bp->b_transp == tp) {
> - bip = bp->b_log_item;
> + struct xfs_buf_log_item *bip = bp->b_log_item;
> +
> ASSERT(bip != NULL);
> ASSERT(atomic_read(&bip->bli_refcount) > 0);
> bip->bli_recur++;
> +
> trace_xfs_trans_getsb_recur(bip);
> - return bp;
> - }
> + } else {
> + xfs_buf_lock(bp);
> + xfs_buf_hold(bp);
> + _xfs_trans_bjoin(tp, bp, 1);
>
> - bp = xfs_getsb(mp);
> - if (bp == NULL)
> - return NULL;
> + trace_xfs_trans_getsb(bp->b_log_item);
> + }
>
> - _xfs_trans_bjoin(tp, bp, 1);
> - trace_xfs_trans_getsb(bp->b_log_item);
> return bp;
> }
>
> --
> 2.28.0
>
next prev parent reply other threads:[~2020-09-01 16:58 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-01 15:50 tidy up the buffer cache implementation v3 Christoph Hellwig
2020-09-01 15:50 ` [PATCH 01/15] xfs: refactor the buf ioend disposition code Christoph Hellwig
2020-09-01 15:50 ` [PATCH 02/15] xfs: mark xfs_buf_ioend static Christoph Hellwig
2020-09-01 15:50 ` [PATCH 03/15] xfs: refactor xfs_buf_ioend Christoph Hellwig
2020-09-01 15:50 ` [PATCH 04/15] xfs: move the buffer retry logic to xfs_buf.c Christoph Hellwig
2020-09-01 15:50 ` [PATCH 05/15] xfs: fold xfs_buf_ioend_finish into xfs_ioend Christoph Hellwig
2020-09-01 15:50 ` [PATCH 06/15] xfs: refactor xfs_buf_ioerror_fail_without_retry Christoph Hellwig
2020-09-01 15:50 ` [PATCH 07/15] xfs: remove xfs_buf_ioerror_retry Christoph Hellwig
2020-09-01 15:50 ` [PATCH 08/15] xfs: lift the XBF_IOEND_FAIL handling into xfs_buf_ioend_disposition Christoph Hellwig
2020-09-01 15:50 ` [PATCH 09/15] xfs: simplify the xfs_buf_ioend_disposition calling convention Christoph Hellwig
2020-09-01 15:50 ` [PATCH 10/15] xfs: use xfs_buf_item_relse in xfs_buf_item_done Christoph Hellwig
2020-09-01 15:50 ` [PATCH 11/15] xfs: clear the read/write flags later in xfs_buf_ioend Christoph Hellwig
2020-09-01 15:50 ` [PATCH 12/15] xfs: remove xlog_recover_iodone Christoph Hellwig
2020-09-01 15:50 ` [PATCH 13/15] xfs: simplify xfs_trans_getsb Christoph Hellwig
2020-09-01 16:58 ` Darrick J. Wong [this message]
2020-09-01 15:50 ` [PATCH 14/15] xfs: remove xfs_getsb Christoph Hellwig
2020-09-01 16:58 ` Darrick J. Wong
2020-09-01 15:50 ` [PATCH 15/15] xfs: reuse _xfs_buf_read for re-reading the superblock Christoph Hellwig
2020-09-01 17:02 ` Darrick J. Wong
2020-09-01 17:12 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200901165821.GF6096@magnolia \
--to=darrick.wong@oracle.com \
--cc=hch@lst.de \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).