• Brian Foster's avatar
    xfs: don't unlock invalidated buf on aborted tx commit · d9183105
    Brian Foster authored
    xfstests generic/388,475 occasionally reproduce assertion failures
    in xfs_buf_item_unpin() when the final bli reference is dropped on
    an invalidated buffer and the buffer is not locked as it is expected
    to be. Invalidated buffers should remain locked on transaction
    commit until the final unpin, at which point the buffer is removed
    from the AIL and the bli is freed since stale buffers are not
    written back.
    
    The assert failures are associated with filesystem shutdown,
    typically due to log I/O errors injected by the test. The
    problematic situation can occur if the shutdown happens to cause a
    race between an active transaction that has invalidated a particular
    buffer and an I/O error on a log buffer that contains the bli
    associated with the same (now stale) buffer.
    
    Both transaction and log contexts acquire a bli reference. If the
    transaction has already invalidated the buffer by the time the I/O
    error occurs and ends up aborting due to shutdown, the transaction
    and log hold the last two references to a stale bli. If the
    transaction cancel occurs first, it treats the buffer as non-stale
    due to the aborted state: the bli reference is dropped and the
    buffer is released/unlocked. The log buffer I/O error handling
    eventually calls into xfs_buf_item_unpin(), drops the final
    reference to the bli and treats it as stale. The buffer wasn't left
    locked by xfs_buf_item_unlock(), however, so the assert fails and
    the buffer is double unlocked. The latter problem is mitigated by
    the fact that the fs is shutdown and no further damage is possible.
    
    ->iop_unlock() of an invalidated buffer should behave consistently
    with respect to the bli refcount, regardless of aborted state. If
    the refcount remains elevated on commit, we know the bli is awaiting
    an unpin (since it can't be in another transaction) and will be
    handled appropriately on log buffer completion. If the final bli
    reference of an invalidated buffer is dropped in ->iop_unlock(), we
    can assume the transaction has aborted because invalidation implies
    a dirty transaction. In the non-abort case, the log would have
    acquired a bli reference in ->iop_pin() and prevented bli release at
    ->iop_unlock() time. In the abort case the item must be freed and
    buffer unlocked because it wasn't pinned by the log.
    
    Rework xfs_buf_item_unlock() to simplify the currently circuitous
    and duplicate logic and leave invalidated buffers locked based on
    bli refcount, regardless of aborted state. This ensures that a
    pinned, stale buffer is always found locked when eventually
    unpinned.
    Signed-off-by: default avatarBrian Foster <bfoster@redhat.com>
    Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
    Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
    d9183105
xfs_buf_item.c 34.6 KB