lkml.org 
[lkml]   [2017]   [Sep]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.13 38/52] xfs: remove unnecessary dirty bli format check for ordered bufs
    Date
    4.13-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Brian Foster <bfoster@redhat.com>

    commit 6453c65d3576bc3e602abb5add15f112755c08ca upstream.

    xfs_buf_item_unlock() historically checked the dirty state of the
    buffer by manually checking the buffer log formats for dirty
    segments. The introduction of ordered buffers invalidated this check
    because ordered buffers have dirty bli's but no dirty (logged)
    segments. The check was updated to accommodate ordered buffers by
    looking at the bli state first and considering the blf only if the
    bli is clean.

    This logic is safe but unnecessary. There is no valid case where the
    bli is clean yet the blf has dirty segments. The bli is set dirty
    whenever the blf is logged (via xfs_trans_log_buf()) and the blf is
    cleared in the only place BLI_DIRTY is cleared (xfs_trans_binval()).

    Remove the conditional blf dirty checks and replace with an assert
    that should catch any discrepencies between bli and blf dirty
    states. Refactor the old blf dirty check into a helper function to
    be used by the assert.

    Signed-off-by: Brian Foster <bfoster@redhat.com>
    Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
    Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    fs/xfs/xfs_buf_item.c | 62 +++++++++++++++++++++++++-------------------------
    fs/xfs/xfs_buf_item.h | 1
    2 files changed, 33 insertions(+), 30 deletions(-)

    --- a/fs/xfs/xfs_buf_item.c
    +++ b/fs/xfs/xfs_buf_item.c
    @@ -575,26 +575,18 @@ xfs_buf_item_unlock(
    {
    struct xfs_buf_log_item *bip = BUF_ITEM(lip);
    struct xfs_buf *bp = bip->bli_buf;
    - bool clean;
    - bool aborted;
    - int flags;
    + bool aborted = !!(lip->li_flags & XFS_LI_ABORTED);
    + bool hold = !!(bip->bli_flags & XFS_BLI_HOLD);
    + bool dirty = !!(bip->bli_flags & XFS_BLI_DIRTY);
    + bool ordered = !!(bip->bli_flags & XFS_BLI_ORDERED);

    /* Clear the buffer's association with this transaction. */
    bp->b_transp = NULL;

    /*
    - * If this is a transaction abort, don't return early. Instead, allow
    - * the brelse to happen. Normally it would be done for stale
    - * (cancelled) buffers at unpin time, but we'll never go through the
    - * pin/unpin cycle if we abort inside commit.
    - */
    - aborted = (lip->li_flags & XFS_LI_ABORTED) ? true : false;
    - /*
    - * Before possibly freeing the buf item, copy the per-transaction state
    - * so we can reference it safely later after clearing it from the
    - * buffer log item.
    + * The per-transaction state has been copied above so clear it from the
    + * bli.
    */
    - flags = bip->bli_flags;
    bip->bli_flags &= ~(XFS_BLI_LOGGED | XFS_BLI_HOLD | XFS_BLI_ORDERED);

    /*
    @@ -602,7 +594,7 @@ xfs_buf_item_unlock(
    * unlock the buffer and free the buf item when the buffer is unpinned
    * for the last time.
    */
    - if (flags & XFS_BLI_STALE) {
    + if (bip->bli_flags & XFS_BLI_STALE) {
    trace_xfs_buf_item_unlock_stale(bip);
    ASSERT(bip->__bli_format.blf_flags & XFS_BLF_CANCEL);
    if (!aborted) {
    @@ -620,20 +612,11 @@ xfs_buf_item_unlock(
    * regardless of whether it is dirty or not. A dirty abort implies a
    * shutdown, anyway.
    *
    - * Ordered buffers are dirty but may have no recorded changes, so ensure
    - * we only release clean items here.
    + * The bli dirty state should match whether the blf has logged segments
    + * except for ordered buffers, where only the bli should be dirty.
    */
    - clean = (flags & XFS_BLI_DIRTY) ? false : true;
    - if (clean) {
    - int i;
    - for (i = 0; i < bip->bli_format_count; i++) {
    - if (!xfs_bitmap_empty(bip->bli_formats[i].blf_data_map,
    - bip->bli_formats[i].blf_map_size)) {
    - clean = false;
    - break;
    - }
    - }
    - }
    + ASSERT((!ordered && dirty == xfs_buf_item_dirty_format(bip)) ||
    + (ordered && dirty && !xfs_buf_item_dirty_format(bip)));

    /*
    * Clean buffers, by definition, cannot be in the AIL. However, aborted
    @@ -652,11 +635,11 @@ xfs_buf_item_unlock(
    ASSERT(XFS_FORCED_SHUTDOWN(lip->li_mountp));
    xfs_trans_ail_remove(lip, SHUTDOWN_LOG_IO_ERROR);
    xfs_buf_item_relse(bp);
    - } else if (clean)
    + } else if (!dirty)
    xfs_buf_item_relse(bp);
    }

    - if (!(flags & XFS_BLI_HOLD))
    + if (!hold)
    xfs_buf_relse(bp);
    }

    @@ -945,6 +928,25 @@ xfs_buf_item_log(
    }


    +/*
    + * Return true if the buffer has any ranges logged/dirtied by a transaction,
    + * false otherwise.
    + */
    +bool
    +xfs_buf_item_dirty_format(
    + struct xfs_buf_log_item *bip)
    +{
    + int i;
    +
    + for (i = 0; i < bip->bli_format_count; i++) {
    + if (!xfs_bitmap_empty(bip->bli_formats[i].blf_data_map,
    + bip->bli_formats[i].blf_map_size))
    + return true;
    + }
    +
    + return false;
    +}
    +
    STATIC void
    xfs_buf_item_free(
    xfs_buf_log_item_t *bip)
    --- a/fs/xfs/xfs_buf_item.h
    +++ b/fs/xfs/xfs_buf_item.h
    @@ -64,6 +64,7 @@ typedef struct xfs_buf_log_item {
    int xfs_buf_item_init(struct xfs_buf *, struct xfs_mount *);
    void xfs_buf_item_relse(struct xfs_buf *);
    void xfs_buf_item_log(xfs_buf_log_item_t *, uint, uint);
    +bool xfs_buf_item_dirty_format(struct xfs_buf_log_item *);
    void xfs_buf_attach_iodone(struct xfs_buf *,
    void(*)(struct xfs_buf *, xfs_log_item_t *),
    xfs_log_item_t *);

    \
     
     \ /
      Last update: 2017-09-18 11:54    [W:2.547 / U:0.544 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site