Skip to content
  • Brian Foster's avatar
    xfs: close xc_cil list_empty() races with cil commit sequence · 4703da7b
    Brian Foster authored
    
    
    We have seen somewhat rare reports of the following assert from
    xlog_cil_push_background() failing during ltp tests or somewhat
    innocuous desktop root fs workloads (e.g., virt operations, initramfs
    construction):
    
        ASSERT(!list_empty(&cil->xc_cil));
    
    The reasoning behind the assert is that the transaction has inserted
    items to the CIL and hit background push codepath all with
    cil->xc_ctx_lock held for reading. This locks out background commit from
    emptying the CIL, which acquires the lock for writing. Therefore, the
    reasoning is that the items previously inserted in the CIL should still
    be present.
    
    The cil->xc_ctx_lock read lock is not sufficient to protect the xc_cil
    list, however, due to how CIL insertion is handled.
    xlog_cil_insert_items() inserts and reorders the dirty transaction items
    to the tail of the CIL under xc_cil_lock. It uses list_move_tail() to
    achieve insertion and reordering in the same block of code. This
    function removes and reinserts an item to the tail of the list. If a
    transaction commits an item that was already logged and thus already
    resides in the CIL, and said item is the sole item on the list, the
    removal and reinsertion creates a temporary state where the list is
    actually empty.
    
    This state is not valid and thus should never be observed by concurrent
    transaction commit-side checks in the circumstances outlined above. We
    do not want to acquire the xc_cil_lock in all of these instances as it
    was previously removed and replaced with a separate push lock for
    performance reasons. Therefore, close any races with list_empty() on the
    insertion side by ensuring that the list is never in a transient empty
    state.
    
    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>
    4703da7b