| Message ID | 20170825150557.43010-6-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Fri, Aug 25, 2017 at 11:05:53AM -0400, Brian Foster wrote: > Ordered buffers are attached to transactions and pushed through the > logging infrastructure just like normal buffers with the exception > that they are not actually written to the log. Therefore, we don't > need to log dirty ranges of ordered buffers. xfs_trans_log_buf() is > called on ordered buffers to set up all of the dirty state on the > transaction, buffer and log item and prepare the buffer for I/O. > > Now that xfs_trans_dirty_buf() is available, call it from > xfs_trans_ordered_buf() so the latter is now mutually exclusive with > xfs_trans_log_buf(). This reflects the implementation of ordered > buffers and helps eliminate confusion over the need to log ranges of > ordered buffers just to set up internal log state. > > Signed-off-by: Brian Foster <bfoster@redhat.com> > Reviewed-by: Allison Henderson <allison.henderson@oracle.com> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/libxfs/xfs_btree.c | 6 ++---- > fs/xfs/libxfs/xfs_ialloc.c | 2 -- > fs/xfs/xfs_trans_buf.c | 26 ++++++++++++++------------ > 3 files changed, 16 insertions(+), 18 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c > index e0bcc4a..0b7905a 100644 > --- a/fs/xfs/libxfs/xfs_btree.c > +++ b/fs/xfs/libxfs/xfs_btree.c > @@ -4464,12 +4464,10 @@ xfs_btree_block_change_owner( > * though, so everything is consistent in memory. > */ > if (bp) { > - if (cur->bc_tp) { > + if (cur->bc_tp) > xfs_trans_ordered_buf(cur->bc_tp, bp); > - xfs_btree_log_block(cur, bp, XFS_BB_OWNER); > - } else { > + else > xfs_buf_delwri_queue(bp, bbcoi->buffer_list); > - } > } else { > ASSERT(cur->bc_flags & XFS_BTREE_ROOT_IN_INODE); > ASSERT(level == cur->bc_nlevels - 1); > diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c > index 1e0658a..988bb3f 100644 > --- a/fs/xfs/libxfs/xfs_ialloc.c > +++ b/fs/xfs/libxfs/xfs_ialloc.c > @@ -378,8 +378,6 @@ xfs_ialloc_inode_init( > * transaction and pin the log appropriately. > */ > xfs_trans_ordered_buf(tp, fbuf); > - xfs_trans_log_buf(tp, fbuf, 0, > - BBTOB(fbuf->b_length) - 1); > } > } else { > fbuf->b_flags |= XBF_DONE; > diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c > index 8c99813..3089e80 100644 > --- a/fs/xfs/xfs_trans_buf.c > +++ b/fs/xfs/xfs_trans_buf.c > @@ -560,16 +560,12 @@ xfs_trans_log_buf( > struct xfs_buf_log_item *bip = bp->b_fspriv; > > ASSERT(first <= last && last < BBTOB(bp->b_length)); > + ASSERT(!(bip->bli_flags & XFS_BLI_ORDERED)); > > xfs_trans_dirty_buf(tp, bp); > > - /* > - * If we have an ordered buffer we are not logging any dirty range but > - * it still needs to be marked dirty and that it has been logged. > - */ > trace_xfs_trans_log_buf(bip); > - if (!(bip->bli_flags & XFS_BLI_ORDERED)) > - xfs_buf_item_log(bip, first, last); > + xfs_buf_item_log(bip, first, last); > } > > > @@ -722,12 +718,11 @@ xfs_trans_inode_alloc_buf( > } > > /* > - * Mark the buffer as ordered for this transaction. This means > - * that the contents of the buffer are not recorded in the transaction > - * but it is tracked in the AIL as though it was. This allows us > - * to record logical changes in transactions rather than the physical > - * changes we make to the buffer without changing writeback ordering > - * constraints of metadata buffers. > + * Mark the buffer as ordered for this transaction. This means that the contents > + * of the buffer are not recorded in the transaction but it is tracked in the > + * AIL as though it was. This allows us to record logical changes in > + * transactions rather than the physical changes we make to the buffer without > + * changing writeback ordering constraints of metadata buffers. > */ > void > xfs_trans_ordered_buf( > @@ -739,9 +734,16 @@ xfs_trans_ordered_buf( > ASSERT(bp->b_transp == tp); > ASSERT(bip != NULL); > ASSERT(atomic_read(&bip->bli_refcount) > 0); > + ASSERT(!xfs_buf_item_dirty_format(bip)); > > bip->bli_flags |= XFS_BLI_ORDERED; > trace_xfs_buf_item_ordered(bip); > + > + /* > + * We don't log a dirty range of an ordered buffer but it still needs > + * to be marked dirty and that it has been logged. > + */ > + xfs_trans_dirty_buf(tp, bp); > } > > /* > -- > 2.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c index e0bcc4a..0b7905a 100644 --- a/fs/xfs/libxfs/xfs_btree.c +++ b/fs/xfs/libxfs/xfs_btree.c @@ -4464,12 +4464,10 @@ xfs_btree_block_change_owner( * though, so everything is consistent in memory. */ if (bp) { - if (cur->bc_tp) { + if (cur->bc_tp) xfs_trans_ordered_buf(cur->bc_tp, bp); - xfs_btree_log_block(cur, bp, XFS_BB_OWNER); - } else { + else xfs_buf_delwri_queue(bp, bbcoi->buffer_list); - } } else { ASSERT(cur->bc_flags & XFS_BTREE_ROOT_IN_INODE); ASSERT(level == cur->bc_nlevels - 1); diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c index 1e0658a..988bb3f 100644 --- a/fs/xfs/libxfs/xfs_ialloc.c +++ b/fs/xfs/libxfs/xfs_ialloc.c @@ -378,8 +378,6 @@ xfs_ialloc_inode_init( * transaction and pin the log appropriately. */ xfs_trans_ordered_buf(tp, fbuf); - xfs_trans_log_buf(tp, fbuf, 0, - BBTOB(fbuf->b_length) - 1); } } else { fbuf->b_flags |= XBF_DONE; diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c index 8c99813..3089e80 100644 --- a/fs/xfs/xfs_trans_buf.c +++ b/fs/xfs/xfs_trans_buf.c @@ -560,16 +560,12 @@ xfs_trans_log_buf( struct xfs_buf_log_item *bip = bp->b_fspriv; ASSERT(first <= last && last < BBTOB(bp->b_length)); + ASSERT(!(bip->bli_flags & XFS_BLI_ORDERED)); xfs_trans_dirty_buf(tp, bp); - /* - * If we have an ordered buffer we are not logging any dirty range but - * it still needs to be marked dirty and that it has been logged. - */ trace_xfs_trans_log_buf(bip); - if (!(bip->bli_flags & XFS_BLI_ORDERED)) - xfs_buf_item_log(bip, first, last); + xfs_buf_item_log(bip, first, last); } @@ -722,12 +718,11 @@ xfs_trans_inode_alloc_buf( } /* - * Mark the buffer as ordered for this transaction. This means - * that the contents of the buffer are not recorded in the transaction - * but it is tracked in the AIL as though it was. This allows us - * to record logical changes in transactions rather than the physical - * changes we make to the buffer without changing writeback ordering - * constraints of metadata buffers. + * Mark the buffer as ordered for this transaction. This means that the contents + * of the buffer are not recorded in the transaction but it is tracked in the + * AIL as though it was. This allows us to record logical changes in + * transactions rather than the physical changes we make to the buffer without + * changing writeback ordering constraints of metadata buffers. */ void xfs_trans_ordered_buf( @@ -739,9 +734,16 @@ xfs_trans_ordered_buf( ASSERT(bp->b_transp == tp); ASSERT(bip != NULL); ASSERT(atomic_read(&bip->bli_refcount) > 0); + ASSERT(!xfs_buf_item_dirty_format(bip)); bip->bli_flags |= XFS_BLI_ORDERED; trace_xfs_buf_item_ordered(bip); + + /* + * We don't log a dirty range of an ordered buffer but it still needs + * to be marked dirty and that it has been logged. + */ + xfs_trans_dirty_buf(tp, bp); } /*