03 Dec, 2011

1 commit

  • When testing the new xfstests --large-fs option that does very large
    file preallocations, this assert was tripped deep in
    xfs_alloc_vextent():

    XFS: Assertion failed: args->minlen maxlen, file: fs/xfs/xfs_alloc.c, line: 2239

    The allocation was trying to allocate a zero length extent because
    the lower 32 bits of the allocation length was zero. The remaining
    length of the allocation to be done was an exact multiple of 2^32 -
    the first case I saw was at 496TB remaining to be allocated.

    This turns out to be an overflow when converting the allocation
    length (a 64 bit quantity) into the extent length to allocate (a 32
    bit quantity), and it requires the length to be allocated an exact
    multiple of 2^32 blocks to trip the assert.

    Fix it by limiting the extent lenth to allocate to MAXEXTLEN.

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

    Dave Chinner
     

12 Oct, 2011

27 commits

  • There is no reason to keep a reference to the inode even if we unlock
    it during transaction commit because we never drop a reference between
    the ijoin and commit. Also use this fact to merge xfs_trans_ijoin_ref
    back into xfs_trans_ijoin - the third argument decides if an unlock
    is needed now.

    I'm actually starting to wonder if allowing inodes to be unlocked
    at transaction commit really is worth the effort. The only real
    benefit is that they can be unlocked earlier when commiting a
    synchronous transactions, but that could be solved by doing the
    log force manually after the unlock, too.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Fix a case in xfs_bmap_add_extent_unwritten_real where we aren't
    passing the returned error on.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • All the parameters passed to xfs_bmap_add_extent_hole_real() are in
    the xfs_bmalloca structure now. Just pass the bmalloca parameter to
    the function instead of 8 separate parameters.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • All the parameters passed to xfs_bmap_add_extent_delay_real() are in
    the xfs_bmalloca structure now. Just pass the bmalloca parameter to
    the function instead of 8 separate parameters.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • Rather than passing the firstblock and freelist structure around,
    embed it into the bmalloca structure and remove it from the function
    parameters.

    This also enables the minleft parameter to be set only once in
    xfs_bmapi_write(), and the freelist cursor directly queried in
    xfs_bmapi_allocate to clear it when the lowspace algorithm is
    activated.

    Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • Rather that putting extent records on the stack and then pointing to
    them in the bmalloca structure which is in the same stack frame, put
    the extent records directly in the bmalloca structure. This reduces
    the number of args that need to be passed around.

    Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • All the variables xfs_bmap_isaeof() is passed are contained within
    the xfs_bmalloca structure. Pass that instead.

    Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • There is no real need to the xfs_bmap_add_extent, as the callers
    know what kind of extents they need to it. Removing it means
    duplicating the extents to btree conversion logic in three places,
    but overall it's still much simpler code and quite a bit less code.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Add a common helper for finding the last extent in a file.

    Largely based on a patch from Dave Chinner.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Now that all the read-only users of xfs_bmapi have been converted to
    use xfs_bmapi_read(), we can remove all the read-only handling cases
    from xfs_bmapi().

    Once this is done, rename xfs_bmapi to xfs_bmapi_write to reflect
    the fact it is for allocation only. This enables us to kill the
    XFS_BMAPI_WRITE flag as well.

    Also clean up xfs_bmapi_write to the style used in the newly added
    xfs_bmapi_read/delay functions.

    Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • To further improve the readability of xfs_bmapi(), factor the
    unwritten extent conversion out into a separate function. This
    removes large block of logic from the xfs_bmapi() code loop and
    makes it easier to see the operational logic flow for xfs_bmapi().

    Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • To further improve the readability of xfs_bmapi(), factor the extent
    allocation out into a separate function. This removes a large block
    of logic from the xfs_bmapi() code loop and makes it easier to see
    the operational logic flow for xfs_bmapi().

    Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • We can just call xfs_bmap_add_extent_hole_delay directly to add a
    delayed allocated regions to the extent tree, instead of going
    through all the complexities of xfs_bmap_add_extent that aren't
    needed for this simple case.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Delalloc reservations are much simpler than allocations, so give
    them a separate bmapi-level interface. Using the previously added
    xfs_bmapi_reserve_delalloc we get a function that is only minimally
    more complicated than xfs_bmapi_read, which is far from the complexity
    in xfs_bmapi. Also remove the XFS_BMAPI_DELAY code after switching
    over the only user to xfs_bmapi_delay.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Move the reservation of delayed allocations, and addition of delalloc
    regions to the extent trees into a new helper function. For now
    this adds some twisted goto logic to xfs_bmapi, but that will be
    cleaned up in the following patches.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Now we have xfs_bmapi_read, there is no need for xfs_bmapi_single().
    Change the remaining caller over and kill the function.

    Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • xfs_bmapi() currently handles both extent map reading and
    allocation. As a result, the code is littered with "if (wr)"
    branches to conditionally do allocation operations if required.
    This makes the code much harder to follow and causes significant
    indent issues with the code.

    Given that read mapping is much simpler than allocation, we can
    split out read mapping from xfs_bmapi() and reuse the logic that
    we have already factored out do do all the hard work of handling the
    extent map manipulations. The results in a much simpler function for
    the common extent read operations, and will allow the allocation
    code to be simplified in another commit.

    Once xfs_bmapi_read() is implemented, convert all the callers of
    xfs_bmapi() that are only reading extents to use the new function.

    Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • To further improve the readability of xfs_bmapi(), factor the pure
    extent map manipulations out into separate functions. This removes
    large blocks of logic from the xfs_bmapi() code loop and makes it
    easier to see the operational logic flow for xfs_bmapi().

    Signed-off-by: Dave Chinner
    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Dave Chinner
     
  • Instead of using a local variable that needs to updated when we modify
    the extent map just check ifp->if_bytes directly where we use it.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • We already have the worst case blocks reserved, so xfs_icsb_modify_counters
    won't fail in xfs_bmap_add_extent_delay_real. In fact we've had an assert
    to catch this case since day and it never triggered. So remove the code
    to try smaller reservations, and just return the error for that case in
    addition to keeping the assert.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Both xfs_bmap_add_extent_hole_delay and xfs_bmap_add_extent_hole_real
    already contain code to handle the case where there is no extent to
    merge with, which is effectively the same as the code duplicated here.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Currently xfs_attr_inactive causes a synchronous transactions if we are
    removing a file that has any extents allocated to the attribute fork, and
    thus makes XFS extremely slow at removing files with out of line extended
    attributes. The code looks a like a relict from the days before the busy
    extent list, but with the busy extent list we avoid reusing data and attr
    extents that have been freed but not commited yet, so this code is just
    as superflous as the synchronous transactions for data blocks.

    Signed-off-by: Christoph Hellwig
    Reported-by: Bernd Schubert
    Reviewed-by: Dave Chinner
    Signed-off-by: Alex Elder

    Christoph Hellwig
     

08 Aug, 2011

1 commit


27 Jul, 2011

1 commit


26 Jul, 2011

1 commit


13 Jul, 2011

1 commit

  • Replace the current mess of dir2 headers with just three that have a clear
    purpose:

    - xfs_dir2_format.h for all format definitions, including the inline helpers
    to access our variable size structures
    - xfs_dir2_priv.h for all prototypes that are internal to the dir2 code
    and not needed by anything outside of the directory code. For this
    purpose xfs_da_btree.c, and phase6.c in xfs_repair are considered part
    of the directory code.
    - xfs_dir2.h for the public interface to the directory code

    In addition to the reshuffle I have also update the comments to not only
    match the new file structure, but also to describe the directory format
    better.

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

    Christoph Hellwig
     

08 Jul, 2011

2 commits

  • Micro-optimize various comparisms by always byteswapping the constant
    instead of the variable, which allows to do the swap at compile instead
    of runtime.

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

    Christoph Hellwig
     
  • Remove the transaction pointer in the inode. It's only used to avoid
    passing down an argument in the bmap code, and for a few asserts in
    the transaction code right now.

    Also use the local variable ip in a few more places in xfs_inode_item_unlock,
    so that it isn't only used for debug builds after the above change.

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

    Christoph Hellwig
     

25 May, 2011

6 commits

  • The code in xfs_bmap_del_extent does not correctly decrement the
    extent buffer index when deleting a whole extent. Most of the time
    this gets caught by checks in xfs_bmapi that work around it and
    decrement it manually and thus wasn't noticed so far.

    Based on an earlier patch from Lachlan McIlroy.

    Signed-off-by: Christoph Hellwig
    Reviewed-by: Lachlan McIlroy
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • We need to call xfs_iext_get_ext for the previous extent to get a
    valid pointer, and can't just do pointer arithmetics as they might
    be in different pages.

    Signed-off-by: Christoph Hellwig
    Reviewed-by: Lachlan McIlroy
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Make sure to only call xfs_iext_get_ext after we've validate the
    extent index when moving on to the next index in xfs_bunmapi. Also
    remove the old workaround for too large indices that has been
    superceeded by the proper fix in xfs_bmap_del_extent.

    Based on an earlier patch from Lachlan McIlroy.

    Signed-off-by: Christoph Hellwig
    Reviewed-by: Lachlan McIlroy
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Make sure to only call xfs_iext_get_ext after we've validate the
    extent index when moving on to the next index in xfs_bmapi.

    Based on an earlier patch from Lachlan McIlroy.

    Signed-off-by: Christoph Hellwig
    Reviewed-by: Lachlan McIlroy
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • Make sure to only call xfs_iext_get_ext after we've validate the
    extent index in the various xfs_bmap_add_extent_* helpers.

    Based on an earlier patch from Lachlan McIlroy.

    Signed-off-by: Christoph Hellwig
    Reviewed-by: Lachlan McIlroy
    Signed-off-by: Alex Elder

    Christoph Hellwig
     
  • The if_lastex field in struct xfs_ifork is only used as a temporary
    index during xfs_bmapi and xfs_bunmapi. Instead of using the inode
    fork to store it keep it local in the callchain. Fortunately this
    is very easy as we already pass a stack copy of it down the whole
    chain which can simplify be changed to be passed by reference.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Alex Elder

    Christoph Hellwig