- 17 Jul, 2018 3 commits
-
-
Gustavo A. R. Silva authored
Make use of the swap macro and remove some unnecessary variables. This makes the code easier to read and maintain. Also, reduces the stack usage. This code was detected with the help of Coccinelle. Signed-off-by:
Gustavo A. R. Silva <gustavo@embeddedor.com> Reviewed-by:
Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by:
-
Gustavo A. R. Silva authored
Make use of the swap macro and remove some unnecessary variables. This makes the code easier to read and maintain. Also, reduces the stack usage. This code was detected with the help of Coccinelle. Signed-off-by:
-
Darrick J. Wong authored
The original rmap code assumed that there would always be at least one rmap in the rmapbt (the AG sb/agf/agi) and so errored out if it didn't find one. This assumption isn't true for the rmapbt repair function (and it won't be true for realtime rmap either), so remove the check and just deal with the situation. Signed-off-by:
Allison Henderson <allison.henderson@oracle.com>
-
- 12 Jul, 2018 37 commits
-
-
Carlos Maiolino authored
Make sure we initialize *bno and *len, before jumping to out_bad_rec label, and risk calling xfs_warn() with uninitialized variables. Coverity: 100898 Coverity: 1437081 Coverity: 1437129 Coverity: 1437191 Coverity: 1437201 Coverity: 1437212 Coverity: 1437341 Signed-off-by:
Carlos Maiolino <cmaiolino@redhat.com> Reviewed-by:
Brian Foster <bfoster@redhat.com> Reviewed-by:
-
Eric Sandeen authored
Signed-off-by:
Eric Sandeen <sandeen@redhat.com> Reviewed-by:
-
Brian Foster authored
Now that there is only one caller, fold the common submission helper into __xfs_buf_submit(). Suggested-by:
Christoph Hellwig <hch@infradead.org> Signed-off-by:
Christoph Hellwig <hch@lst.de> Reviewed-by:
-
Brian Foster authored
The buffer I/O submission path consists of separate function calls per type. The buffer I/O type is already controlled via buffer state (XBF_ASYNC), however, so there is no real need for separate submission functions. Combine the buffer submission functions into a single function that processes the buffer appropriately based on XBF_ASYNC. Retain an internal helper with a conditional wait parameter to continue to support batched !XBF_ASYNC submission/completion required by delwri queues. Suggested-by:
-
Brian Foster authored
If a delwri queue occurs of a buffer that sits on a delwri queue wait list, the queue sets _XBF_DELWRI_Q without changing the state of ->b_list. This occurs, for example, if another thread beats the current delwri waiter thread to the buffer lock after I/O completion. Once the waiter acquires the lock, it removes the buffer from the wait list and leaves a buffer with _XBF_DELWRI_Q set but not populated on a list. This results in a lost buffer submission and in turn can result in assert failures due to _XBF_DELWRI_Q being set on buffer reclaim or filesystem lockups if the buffer happens to cover an item in the AIL. This problem has been reproduced by repeated iterations of xfs/305 on high CPU count (28xcpu) systems with limited memory (~1GB). Dirty dquot reclaim races with an xfsaild push of a separate dquot backed by the same buffer such that the buffer sits on the reclaim wait list at the time xfsaild attempts to queue it. Since the latter dquot has been flush locked but the underlying buffer not submitted for I/O, the dquot pins the AIL and causes the filesystem to livelock. This race is essentially made possible by the buffer lock cycle involved with waiting on a synchronous delwri queue submission. Close the race by using synchronous buffer I/O for respective delwri queue submission. This means the buffer remains locked across the I/O and so is inaccessible from other contexts while in the intermediate wait list state. The sync buffer I/O wait mechanism is factored into a helper such that sync delwri buffer submission and serialization are batched operations. Designed-by:
Dave Chinner <dchinner@redhat.com> Signed-off-by:
-
Brian Foster authored
Sync and async buffer submission both do generally similar things with a couple odd exceptions. Refactor the core buffer submission code into a common helper to isolate buffer submission from completion handling of synchronous buffer I/O. This patch does not change behavior. It is a step towards support for using synchronous buffer I/O via synchronous delwri queue submission. Designed-by:
-
Brian Foster authored
All but one caller of xfs_defer_init() passes in the ->t_firstblock of the associated transaction. The one outlier is xlog_recover_process_intents(), which simply passes a dummy value because a valid pointer is required. This firstblock variable can simply be removed. At this point we could remove the xfs_defer_init() firstblock parameter and initialize ->t_firstblock directly. Even that is not necessary, however, because ->t_firstblock is automatically reinitialized in the new transaction on a transaction roll. Since xfs_defer_init() should never occur more than once on a particular transaction (since the corresponding finish will roll it), replace the reinit from xfs_defer_init() with an assert that verifies the transaction has a NULLFSBLOCK firstblock. Signed-off-by:
-
Brian Foster authored
Signed-off-by:
-
Brian Foster authored
Signed-off-by:
-
Brian Foster authored
Signed-off-by:
-
Brian Foster authored
xfs_refcount_recover_cow_leftovers() has no need for a firstblock variable and so passes an unrelated xfs_fsblock_t to xfs_defer_init() to avoid declaring one. Replace this no-op initialization with ->t_firstblock. This will be optimized away by the removal of the xfs_defer_init() firstblock param. Signed-off-by:
-
Brian Foster authored
Signed-off-by:
-
Brian Foster authored
The xfs_alloc_arg.firstblock field is used to control the starting agno for an allocation. The structure already carries a pointer to the transaction, which carries the current firstblock value. Remove the field and access ->t_firstblock directly in the allocation code. Signed-off-by:
-
Brian Foster authored
The bmbt cursor private structure has a firstblock field that is used to maintain locking order on bmbt allocations. The field holds an actual firstblock value (as opposed to a pointer), so it is initialized on cursor creation, updated on allocation and then the value is transferred back to the source before the cursor is destroyed. This value is always transferred from and back to the ->t_firstblock field. Since xfs_btree_cur already carries a reference to the transaction, we can remove this field from xfs_btree_cur and the associated copying. The bmbt allocations will update the value in the transaction directly. Signed-off-by:
-
Brian Foster authored
The bmap format helpers receive firstblock via ->t_firstblock. Drop the param and access it directly. Signed-off-by:
-
Brian Foster authored
The add extent helpers all receive firstblock via ->t_firstblock. Drop the parameter and access it directly. Signed-off-by:
-
Brian Foster authored
The xfs_bmalloca.firstblock field carries the firstblock value from the transaction into the bmap infrastructure. It's initialized in one place from ->t_firstblock, so drop the field and access ->t_firstblock directly throughout the bmap code. Signed-off-by:
-
Brian Foster authored
Also remove the unnecessary xfs_bmap_split_extent_at() parameter. Signed-off-by:
-
Brian Foster authored
The only callers pass ->t_firstblock. Signed-off-by:
-
Brian Foster authored
All callers pass ->t_firstblock from the current transaction. Signed-off-by:
-
Brian Foster authored
All callers pass ->t_firstblock from the current transaction. Signed-off-by:
-
Brian Foster authored
Signed-off-by:
-
Brian Foster authored
Signed-off-by:
-
Brian Foster authored
Convert all xfs_bunmapi() callers to ->t_firstblock. Signed-off-by:
-
Brian Foster authored
Convert all xfs_bmapi_write() users to ->t_firstblock. Signed-off-by:
-
Brian Foster authored
Similar to the dirops code, the xattr code uses an on-stack firstblock variable for the various operations. This code rolls the underlying transaction in various places, however, which means we cannot simply replace the local firstblock vars with ->t_firstblock. Doing so (without further changes) would invalidate the memory pointed to by xfs_da_args.firstblock as soon as the first transaction rolls. To avoid this problem, remove xfs_da_args.firstblock and replace all such accesses with ->t_firstblock at the same time. This ensures that accesses to the current firstblock always occur through the current transaction rather than a potentially invalid xfs_da_args pointer. Signed-off-by:
-
Brian Foster authored
Note that this codepath is a user of struct xfs_da_args. Switch it over to ->t_firstblock in preparation to remove xfs_da_args.firstblock. Signed-off-by:
-
Brian Foster authored
All callers of the xfs_dir_*() functions pass ->t_firstblock as the firstblock parameter. Drop the parameter and access ->t_firstblock directly. Signed-off-by:
-
Brian Foster authored
Callers of the xfs_dir_*() functions currently pass an on-stack firstblock variable. While the dirops infrastructure carries a pointer to this variable, it never rolls the transaction and so it is safe to use ->t_firstblock instead. Fix up the various xfs_dir_*() callers to use ->t_firstblock. Also remove the unnecessary parameter for xfs_cross_rename(). Signed-off-by:
-
Brian Foster authored
A firstblock var is typically allocated and initialized along with xfs_defer_ops structures and passed around independent from the associated transaction. To facilitate combining the two, add an optional ->t_firstblock field to xfs_trans that can be used in place of an on-stack variable. The firstblock value follows the lifetime of the transaction, so initialize it on allocation and when a transaction rolls. Signed-off-by:
-
Brian Foster authored
xfs_bmapi_write() always expects a valid firstblock pointer. It immediately dereferences the pointer to help determine how to initialize the bma.minleft field. The remaining accesses are related to modifying btree format forks, which is only relevant for !COW fork callers. The reflink code passes a NULL transaction to xfs_bmapi_write() in a couple places that do COW fork unwritten conversion. The purpose of the firstblock field is to track the first block allocation in the current transaction, so technically firstblock should not be required for these callers either. Tweak xfs_bmapi_write() to initialize the bma correctly without accessing the firstblock pointer if no transaction is provided in the first place. Update the reflink callers to pass NULL instead of otherwise unused firstblock references. Signed-off-by:
-
Brian Foster authored
Most callers of xfs_defer_init() immediately attach the dfops structure to a transaction. Add a transaction parameter to eliminate much of this boilerplate code. This also helps self-document the fact that many codepaths now expect a dfops pointer implicitly via xfs_trans->t_dfops. Signed-off-by:
-
Brian Foster authored
Use ->t_dfops of the leftover COW reservation cleanup transaction. Signed-off-by:
-
Brian Foster authored
Use ->t_dfops of the transaction from the caller. Reset it before we return to avoid leaks of local stack memory. Signed-off-by:
-
Brian Foster authored
xfs_swap_extent_rmap() uses a local dfops instance with a transaction from the caller. Since there is only one caller, pull the dfops structure into the caller and attach it to the transaction. This avoids the need to clear ->t_dfops to prevent invalid stack memory access. Signed-off-by:
-
Brian Foster authored
The xfs_btree_cur.bc_private.a.dfops field is only ever initialized by the refcountbt cursor init function. The only caller of that function with a non-NULL dfops is from deferred completion context, which already has attached to ->t_dfops. In addition to that, the only actual reference of a.dfops is the cursor duplication function, which means the field is effectively unused. Remove the dfops field from the bc_private.a union. Any future users can acquire the dfops from the transaction. This patch does not change behavior. Signed-off-by:
-
Brian Foster authored
All assignments of xfs_btree_cur.bc_private.b.dfops originate from ->t_dfops. Replace accesses of the former with the latter and remove the unnecessary field. This patch does not change behavior. Signed-off-by:
-
