03 Aug, 2016

1 commit

  • Refactor the EFI intent item recovery (and cancellation) functions
    into a general function that scans the AIL and an intent item type
    specific handler. Move the function that recovers a single EFI item
    into the extent free item code. We'll want the generalized function
    when we start wiring up more redo item types.

    Furthermore, ensure that log recovery only replays the redo items
    that were in the AIL prior to recovery by checking the item LSN
    against the largest LSN seen during log scanning. As written this
    should never happen, but we can be defensive anyway.

    Signed-off-by: Darrick J. Wong
    Reviewed-by: Brian Foster
    Signed-off-by: Dave Chinner

    Darrick J. Wong
     

19 Aug, 2015

2 commits

  • Freeing an extent in XFS involves logging an EFI (extent free
    intention), freeing the actual extent, and logging an EFD (extent
    free done). The EFI object is created with a reference count of 2:
    one for the current transaction and one for the subsequently created
    EFD. Under normal circumstances, the first reference is dropped when
    the EFI is unpinned and the second reference is dropped when the EFD
    is committed to the on-disk log.

    In event of errors or filesystem shutdown, there are various
    potential cleanup scenarios depending on the state of the EFI/EFD.
    The cleanup scenarios are confusing and racy, as demonstrated by the
    following test sequence:

    # mount $dev $mnt
    # fsstress -d $mnt -n 99999 -p 16 -z -f fallocate=1 \
    -f punch=1 -f creat=1 -f unlink=1 &
    # sleep 5
    # killall -9 fsstress; wait
    # godown -f $mnt
    # umount

    ... in which the final umount can hang due to the AIL being pinned
    indefinitely by one or more EFI items. This can occur due to several
    conditions. For example, if the shutdown occurs after the EFI is
    committed to the on-disk log and the EFD committed to the CIL, but
    before the EFD committed to the log, the EFD iop_committed() abort
    handler does not drop its reference to the EFI. Alternatively,
    manual error injection in the xfs_bmap_finish() codepath shows that
    if an error occurs after the EFI transaction is committed but before
    the EFD is constructed and logged, the EFI is never released from
    the AIL.

    Update the EFI/EFD item handling code to use a more straightforward
    and reliable approach to error handling. If an error occurs after
    the EFI transaction is committed and before the EFD is constructed,
    release the EFI explicitly from xfs_bmap_finish(). If the EFI
    transaction is cancelled, release the EFI in the unlock handler.

    Once the EFD is constructed, it is responsible for releasing the EFI
    under any circumstances (including whether the EFI item aborts due
    to log I/O error). Update the EFD item handlers to release the EFI
    if the transaction is cancelled or aborts due to log I/O error.
    Finally, update xfs_bmap_finish() to log at least one EFD extent to
    the transaction before xfs_free_extent() errors are handled to
    ensure the transaction is dirty and EFD item error handling is
    triggered.

    Signed-off-by: Brian Foster
    Reviewed-by: Dave Chinner
    Signed-off-by: Dave Chinner

    Brian Foster
     
  • Release of the EFI either occurs based on the reference count or the
    extent count. The extent count used is either the count tracked in
    the EFI or EFD, depending on the particular situation. In either
    case, the count is initialized to the final value and thus always
    matches the current efi_next_extent value once the EFI is completely
    constructed. For example, the EFI extent count is increased as the
    extents are logged in xfs_bmap_finish() and the full free list is
    always completely processed. Therefore, the count is guaranteed to
    be complete once the EFI transaction is committed. The EFD uses the
    efd_nextents counter to release the EFI. This counter is initialized
    to the count of the EFI when the EFD is created. Thus the EFD, as
    currently used, has no concept of partial EFI release based on
    extent count.

    Given that the EFI extent count is always released in whole, use of
    the extent count for reference counting is unnecessary. Remove this
    level of the API and release the EFI based on the core reference
    count. The efi_next_extent counter remains because it is still used
    to track the slot to log the next extent to free.

    Signed-off-by: Brian Foster
    Reviewed-by: Dave Chinner
    Signed-off-by: Dave Chinner

    Brian Foster
     

13 Aug, 2013

1 commit


06 Apr, 2013

1 commit

  • Filesystems are occasionally being shut down with this error:

    xfs_trans_ail_delete_bulk: attempting to delete a log item that is
    not in the AIL.

    It was diagnosed to be related to the EFI/EFD commit order when the
    EFI and EFD are in different checkpoints and the EFD is committed
    before the EFI here:

    http://oss.sgi.com/archives/xfs/2013-01/msg00082.html

    The real problem is that a single bit cannot fully describe the
    states that the EFI/EFD processing can be in. These completion
    states are:

    EFI EFI in AIL EFD Result
    committed/unpinned Yes committed OK
    committed/pinned No committed Shutdown
    uncommitted No committed Shutdown

    Note that the "result" field is what should happen, not what does
    happen. The current logic is broken and handles the first two cases
    correctly by luck. That is, the code will free the EFI if the
    XFS_EFI_COMMITTED bit is *not* set, rather than if it is set. The
    inverted logic "works" because if both EFI and EFD are committed,
    then the first __xfs_efi_release() call clears the XFS_EFI_COMMITTED
    bit, and the second frees the EFI item. Hence as long as
    xfs_efi_item_committed() has been called, everything appears to be
    fine.

    It is the third case where the logic fails - where
    xfs_efd_item_committed() is called before xfs_efi_item_committed(),
    and that results in the EFI being freed before it has been
    committed. That is the bug that triggered the shutdown, and hence
    keeping track of whether the EFI has been committed or not is
    insufficient to correctly order the EFI/EFD operations w.r.t. the
    AIL.

    What we really want is this: the EFI is always placed into the
    AIL before the last reference goes away. The only way to guarantee
    that is that the EFI is not freed until after it has been unpinned
    *and* the EFD has been committed. That is, restructure the logic so
    that the only case that can occur is the first case.

    This can be done easily by replacing the XFS_EFI_COMMITTED with an
    EFI reference count. The EFI is initialised with it's own count, and
    that is not released until it is unpinned. However, there is a
    complication to this method - the high level EFI/EFD code in
    xfs_bmap_finish() does not hold direct references to the EFI
    structure, and runs a transaction commit between the EFI and EFD
    processing. Hence the EFI can be freed even before the EFD is
    created using such a method.

    Further, log recovery uses the AIL for tracking EFI/EFDs that need
    to be recovered, but it uses the AIL *differently* to the EFI
    transaction commit. Hence log recovery never pins or unpins EFIs, so
    we can't drop the EFI reference count indirectly to free the EFI.

    However, this doesn't prevent us from using a reference count here.
    There is a 1:1 relationship between EFIs and EFDs, so when we
    initialise the EFI we can take a reference count for the EFD as
    well. This solves the xfs_bmap_finish() issue - the EFI will never
    be freed until the EFD is processed. In terms of log recovery,
    during the committing of the EFD we can look for the
    XFS_EFI_RECOVERED bit being set and drop the EFI reference as well,
    thereby ensuring everything works correctly there as well.

    Signed-off-by: Dave Chinner
    Reviewed-by: Mark Tinguely
    Signed-off-by: Ben Myers

    Dave Chinner
     

20 Dec, 2010

2 commits

  • EFI/EFD interactions are protected from races by the AIL lock. They
    are the only type of log items that require the the AIL lock to
    serialise internal state, so they need to be separated from the AIL
    lock before we can do bulk insert operations on the AIL.

    To acheive this, convert the counter of the number of extents in the
    EFI to an atomic so it can be safely manipulated by EFD processing
    without locks. Also, convert the EFI state flag manipulations to use
    atomic bit operations so no locks are needed to record state
    changes. Finally, use the state bits to determine when it is safe to
    free the EFI and clean up the code to do this neatly.

    Signed-off-by: Dave Chinner
    Reviewed-by: Christoph Hellwig

    Dave Chinner
     
  • XFS_EFI_CANCELED has not been set in the code base since
    xfs_efi_cancel() was removed back in 2006 by commit
    065d312e15902976d256ddaf396a7950ec0350a8 ("[XFS] Remove unused
    iop_abort log item operation), and even then xfs_efi_cancel() was
    never called. I haven't tracked it back further than that (beyond
    git history), but it indicates that the handling of EFIs in
    cancelled transactions has been broken for a long time.

    Basically, when we get an IOP_UNPIN(lip, 1); call from
    xfs_trans_uncommit() (i.e. remove == 1), if we don't free the log
    item descriptor we leak it. Fix the behviour to be correct and kill
    the XFS_EFI_CANCELED flag.

    Signed-off-by: Dave Chinner
    Reviewed-by: Christoph Hellwig

    Dave Chinner
     

22 Jan, 2009

1 commit


28 Sep, 2006

1 commit


09 Jun, 2006

1 commit


02 Nov, 2005

1 commit


21 Jun, 2005

1 commit


17 Apr, 2005

1 commit

  • Initial git repository build. I'm not bothering with the full history,
    even though we have it. We can create a separate "historical" git
    archive of that later if we want to, and in the meantime it's about
    3.2GB when imported into git - space that would just make the early
    git days unnecessarily complicated, when we don't have a lot of good
    infrastructure for it.

    Let it rip!

    Linus Torvalds