Commit fd86a3a3 authored by Omar Sandoval's avatar Omar Sandoval Committed by David Sterba
Browse files

Btrfs: fix error handling in btrfs_truncate_inode_items()



btrfs_truncate_inode_items() uses two variables for error handling, ret
and err. These are not handled consistently, leading to a couple of
bugs.

- Errors from btrfs_del_items() are handled but not propagated to the
  caller
- If btrfs_run_delayed_refs() fails and aborts the transaction, we
  continue running

Just use ret everywhere and simplify things a bit, fixing both of these
issues.

Fixes: 79787eaa ("btrfs: replace many BUG_ONs with proper error handling")
Fixes: 1262133b ("Btrfs: account for crcs in delayed ref processing")
Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent d1342aad
Loading
Loading
Loading
Loading
+28 −27
Original line number Original line Diff line number Diff line
@@ -4705,7 +4705,6 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
	int pending_del_slot = 0;
	int pending_del_slot = 0;
	int extent_type = -1;
	int extent_type = -1;
	int ret;
	int ret;
	int err = 0;
	u64 ino = btrfs_ino(BTRFS_I(inode));
	u64 ino = btrfs_ino(BTRFS_I(inode));
	u64 bytes_deleted = 0;
	u64 bytes_deleted = 0;
	bool be_nice = false;
	bool be_nice = false;
@@ -4757,22 +4756,19 @@ search_again:
	 * up a huge file in a single leaf.  Most of the time that
	 * up a huge file in a single leaf.  Most of the time that
	 * bytes_deleted is > 0, it will be huge by the time we get here
	 * bytes_deleted is > 0, it will be huge by the time we get here
	 */
	 */
	if (be_nice && bytes_deleted > SZ_32M) {
	if (be_nice && bytes_deleted > SZ_32M &&
		if (btrfs_should_end_transaction(trans)) {
	    btrfs_should_end_transaction(trans)) {
			err = -EAGAIN;
		ret = -EAGAIN;
			goto error;
		goto out;
		}
	}
	}



	path->leave_spinning = 1;
	path->leave_spinning = 1;
	ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
	ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
	if (ret < 0) {
	if (ret < 0)
		err = ret;
		goto out;
		goto out;
	}


	if (ret > 0) {
	if (ret > 0) {
		ret = 0;
		/* there are no items in the tree for us to truncate, we're
		/* there are no items in the tree for us to truncate, we're
		 * done
		 * done
		 */
		 */
@@ -4883,7 +4879,7 @@ search_again:
				 * We have to bail so the last_size is set to
				 * We have to bail so the last_size is set to
				 * just before this extent.
				 * just before this extent.
				 */
				 */
				err = NEED_TRUNCATE_BLOCK;
				ret = NEED_TRUNCATE_BLOCK;
				break;
				break;
			}
			}


@@ -4950,7 +4946,7 @@ delete:
						pending_del_nr);
						pending_del_nr);
				if (ret) {
				if (ret) {
					btrfs_abort_transaction(trans, ret);
					btrfs_abort_transaction(trans, ret);
					goto error;
					break;
				}
				}
				pending_del_nr = 0;
				pending_del_nr = 0;
			}
			}
@@ -4961,8 +4957,8 @@ delete:
					trans->delayed_ref_updates = 0;
					trans->delayed_ref_updates = 0;
					ret = btrfs_run_delayed_refs(trans,
					ret = btrfs_run_delayed_refs(trans,
								   updates * 2);
								   updates * 2);
					if (ret && !err)
					if (ret)
						err = ret;
						break;
				}
				}
			}
			}
			/*
			/*
@@ -4970,8 +4966,8 @@ delete:
			 * and let the transaction restart
			 * and let the transaction restart
			 */
			 */
			if (should_end) {
			if (should_end) {
				err = -EAGAIN;
				ret = -EAGAIN;
				goto error;
				break;
			}
			}
			goto search_again;
			goto search_again;
		} else {
		} else {
@@ -4979,32 +4975,37 @@ delete:
		}
		}
	}
	}
out:
out:
	if (pending_del_nr) {
	if (ret >= 0 && pending_del_nr) {
		ret = btrfs_del_items(trans, root, path, pending_del_slot,
		int err;

		err = btrfs_del_items(trans, root, path, pending_del_slot,
				      pending_del_nr);
				      pending_del_nr);
		if (ret)
		if (err) {
			btrfs_abort_transaction(trans, ret);
			btrfs_abort_transaction(trans, err);
			ret = err;
		}
	}
	}
error:
	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
		ASSERT(last_size >= new_size);
		ASSERT(last_size >= new_size);
		if (!err && last_size > new_size)
		if (!ret && last_size > new_size)
			last_size = new_size;
			last_size = new_size;
		btrfs_ordered_update_i_size(inode, last_size, NULL);
		btrfs_ordered_update_i_size(inode, last_size, NULL);
	}
	}


	btrfs_free_path(path);
	btrfs_free_path(path);


	if (be_nice && bytes_deleted > SZ_32M) {
	if (be_nice && bytes_deleted > SZ_32M && (ret >= 0 || ret == -EAGAIN)) {
		unsigned long updates = trans->delayed_ref_updates;
		unsigned long updates = trans->delayed_ref_updates;
		int err;

		if (updates) {
		if (updates) {
			trans->delayed_ref_updates = 0;
			trans->delayed_ref_updates = 0;
			ret = btrfs_run_delayed_refs(trans, updates * 2);
			err = btrfs_run_delayed_refs(trans, updates * 2);
			if (ret && !err)
			if (err)
				err = ret;
				ret = err;
		}
		}
	}
	}
	return err;
	return ret;
}
}


/*
/*