| Message ID | 20200901155018.2524-14-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [01/15] xfs: refactor the buf ioend disposition code | expand |
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 >
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; }
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> --- 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(-)