Skip to content

Commit

Permalink
btrfs: add wrapper for transaction abort predicate
Browse files Browse the repository at this point in the history
The status of aborted transaction can change between calls and it needs
to be accessed by READ_ONCE. Add a helper that also wraps the unlikely
hint.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
  • Loading branch information
kdave committed Mar 23, 2020
1 parent b908c33 commit bf31f87
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 20 deletions.
2 changes: 1 addition & 1 deletion fs/btrfs/block-group.c
Original file line number Diff line number Diff line change
Expand Up @@ -2345,7 +2345,7 @@ static int cache_save_setup(struct btrfs_block_group *block_group,
return 0;
}

if (trans->aborted)
if (TRANS_ABORTED(trans))
return 0;
again:
inode = lookup_free_space_inode(block_group, path);
Expand Down
2 changes: 1 addition & 1 deletion fs/btrfs/delayed-inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -1139,7 +1139,7 @@ static int __btrfs_run_delayed_items(struct btrfs_trans_handle *trans, int nr)
int ret = 0;
bool count = (nr > 0);

if (trans->aborted)
if (TRANS_ABORTED(trans))
return -EIO;

path = btrfs_alloc_path();
Expand Down
10 changes: 5 additions & 5 deletions fs/btrfs/extent-tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -1583,7 +1583,7 @@ static int run_delayed_extent_op(struct btrfs_trans_handle *trans,
int err = 0;
int metadata = !extent_op->is_data;

if (trans->aborted)
if (TRANS_ABORTED(trans))
return 0;

if (metadata && !btrfs_fs_incompat(fs_info, SKINNY_METADATA))
Expand Down Expand Up @@ -1703,7 +1703,7 @@ static int run_one_delayed_ref(struct btrfs_trans_handle *trans,
{
int ret = 0;

if (trans->aborted) {
if (TRANS_ABORTED(trans)) {
if (insert_reserved)
btrfs_pin_extent(trans->fs_info, node->bytenr,
node->num_bytes, 1);
Expand Down Expand Up @@ -2191,7 +2191,7 @@ int btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
int run_all = count == (unsigned long)-1;

/* We'll clean this up in btrfs_cleanup_transaction */
if (trans->aborted)
if (TRANS_ABORTED(trans))
return 0;

if (test_bit(BTRFS_FS_CREATING_FREE_SPACE_TREE, &fs_info->flags))
Expand Down Expand Up @@ -2913,7 +2913,7 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans)
else
unpin = &fs_info->freed_extents[0];

while (!trans->aborted) {
while (!TRANS_ABORTED(trans)) {
struct extent_state *cached_state = NULL;

mutex_lock(&fs_info->unused_bg_unpin_mutex);
Expand Down Expand Up @@ -2950,7 +2950,7 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans)
u64 trimmed = 0;

ret = -EROFS;
if (!trans->aborted)
if (!TRANS_ABORTED(trans))
ret = btrfs_discard_extent(fs_info,
block_group->start,
block_group->length,
Expand Down
2 changes: 1 addition & 1 deletion fs/btrfs/super.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ void __btrfs_abort_transaction(struct btrfs_trans_handle *trans,
{
struct btrfs_fs_info *fs_info = trans->fs_info;

trans->aborted = errno;
WRITE_ONCE(trans->aborted, errno);
/* Nothing used. The other threads that have joined this
* transaction may be able to continue. */
if (!trans->dirty && list_empty(&trans->new_bgs)) {
Expand Down
25 changes: 13 additions & 12 deletions fs/btrfs/transaction.c
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ static noinline int join_transaction(struct btrfs_fs_info *fs_info,

cur_trans = fs_info->running_transaction;
if (cur_trans) {
if (cur_trans->aborted) {
if (TRANS_ABORTED(cur_trans)) {
spin_unlock(&fs_info->trans_lock);
return cur_trans->aborted;
}
Expand Down Expand Up @@ -459,7 +459,7 @@ static inline int is_transaction_blocked(struct btrfs_transaction *trans)
{
return (trans->state >= TRANS_STATE_COMMIT_START &&
trans->state < TRANS_STATE_UNBLOCKED &&
!trans->aborted);
!TRANS_ABORTED(trans));
}

/* wait for commit against the current transaction to become unblocked
Expand All @@ -478,7 +478,7 @@ static void wait_current_trans(struct btrfs_fs_info *fs_info)

wait_event(fs_info->transaction_wait,
cur_trans->state >= TRANS_STATE_UNBLOCKED ||
cur_trans->aborted);
TRANS_ABORTED(cur_trans));
btrfs_put_transaction(cur_trans);
} else {
spin_unlock(&fs_info->trans_lock);
Expand Down Expand Up @@ -937,7 +937,7 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
if (throttle)
btrfs_run_delayed_iputs(info);

if (trans->aborted ||
if (TRANS_ABORTED(trans) ||
test_bit(BTRFS_FS_STATE_ERROR, &info->fs_state)) {
wake_up_process(info->transaction_kthread);
err = -EIO;
Expand Down Expand Up @@ -1794,7 +1794,8 @@ static void wait_current_trans_commit_start(struct btrfs_fs_info *fs_info,
struct btrfs_transaction *trans)
{
wait_event(fs_info->transaction_blocked_wait,
trans->state >= TRANS_STATE_COMMIT_START || trans->aborted);
trans->state >= TRANS_STATE_COMMIT_START ||
TRANS_ABORTED(trans));
}

/*
Expand All @@ -1806,7 +1807,8 @@ static void wait_current_trans_commit_start_and_unblock(
struct btrfs_transaction *trans)
{
wait_event(fs_info->transaction_wait,
trans->state >= TRANS_STATE_UNBLOCKED || trans->aborted);
trans->state >= TRANS_STATE_UNBLOCKED ||
TRANS_ABORTED(trans));
}

/*
Expand Down Expand Up @@ -2026,7 +2028,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
trans->dirty = true;

/* Stop the commit early if ->aborted is set */
if (unlikely(READ_ONCE(cur_trans->aborted))) {
if (TRANS_ABORTED(cur_trans)) {
ret = cur_trans->aborted;
btrfs_end_transaction(trans);
return ret;
Expand Down Expand Up @@ -2100,7 +2102,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)

wait_for_commit(cur_trans);

if (unlikely(cur_trans->aborted))
if (TRANS_ABORTED(cur_trans))
ret = cur_trans->aborted;

btrfs_put_transaction(cur_trans);
Expand All @@ -2119,7 +2121,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
spin_unlock(&fs_info->trans_lock);

wait_for_commit(prev_trans);
ret = prev_trans->aborted;
ret = READ_ONCE(prev_trans->aborted);

btrfs_put_transaction(prev_trans);
if (ret)
Expand Down Expand Up @@ -2173,8 +2175,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
wait_event(cur_trans->writer_wait,
atomic_read(&cur_trans->num_writers) == 1);

/* ->aborted might be set after the previous check, so check it */
if (unlikely(READ_ONCE(cur_trans->aborted))) {
if (TRANS_ABORTED(cur_trans)) {
ret = cur_trans->aborted;
goto scrub_continue;
}
Expand Down Expand Up @@ -2292,7 +2293,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
* The tasks which save the space cache and inode cache may also
* update ->aborted, check it.
*/
if (unlikely(READ_ONCE(cur_trans->aborted))) {
if (TRANS_ABORTED(cur_trans)) {
ret = cur_trans->aborted;
mutex_unlock(&fs_info->tree_log_mutex);
mutex_unlock(&fs_info->reloc_mutex);
Expand Down
12 changes: 12 additions & 0 deletions fs/btrfs/transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ struct btrfs_trans_handle {
struct btrfs_block_rsv *orig_rsv;
refcount_t use_count;
unsigned int type;
/*
* Error code of transaction abort, set outside of locks and must use
* the READ_ONCE/WRITE_ONCE access
*/
short aborted;
bool adding_csums;
bool allocating_chunk;
Expand All @@ -126,6 +130,14 @@ struct btrfs_trans_handle {
struct list_head new_bgs;
};

/*
* The abort status can be changed between calls and is not protected by locks.
* This accepts btrfs_transaction and btrfs_trans_handle as types. Once it's
* set to a non-zero value it does not change, so the macro should be in checks
* but is not necessary for further reads of the value.
*/
#define TRANS_ABORTED(trans) (unlikely(READ_ONCE((trans)->aborted)))

struct btrfs_pending_snapshot {
struct dentry *dentry;
struct inode *dir;
Expand Down

0 comments on commit bf31f87

Please sign in to comment.