const u64 range_end = file_offset + block_size - 1;
const size_t inline_size = size - btrfs_file_extent_calc_inline_size(0);
char *data_start = inline_data + btrfs_file_extent_calc_inline_size(0);
- struct extent_changeset *data_reserved = NULL;
struct folio *folio = NULL;
struct address_space *mapping = inode->vfs_inode.i_mapping;
int ret;
ASSERT(IS_ALIGNED(file_offset, block_size));
/*
- * We have flushed and locked the ranges of the source and destination
- * inodes, we also have locked the inodes, so we are safe to do a
- * reservation here. Also we must not do the reservation while holding
- * a transaction open, otherwise we would deadlock.
+ * Our caller has reserved data and metadata space for our block and
+ * holds an open transaction handle. The reservation can not be done
+ * here since reserving space while holding a transaction open can
+ * deadlock, and the transaction must be started before dirtying the
+ * folio below (see clone_copy_inline_extent()).
*/
- ret = btrfs_delalloc_reserve_space(inode, &data_reserved, file_offset,
- block_size);
- if (ret)
- goto out;
-
folio = __filemap_get_folio(mapping, file_offset >> PAGE_SHIFT,
FGP_LOCK | FGP_ACCESSED | FGP_CREAT,
btrfs_alloc_write_mask(mapping));
goto out_unlock;
/*
- * After dirtying the page our caller will need to start a transaction,
- * and if we are low on metadata free space, that can cause flushing of
- * delalloc for all inodes in order to get metadata space released.
- * However we are holding the range locked for the whole duration of
- * the clone/dedupe operation, so we may deadlock if that happens and no
- * other task releases enough space. So mark this inode as not being
- * possible to flush to avoid such deadlock. We will clear that flag
+ * We are about to dirty a folio in a range we hold locked for the
+ * whole duration of the clone/dedupe operation. If a task flushing
+ * delalloc to release metadata space picks this inode, it would block
+ * on the locked range until we are done, so mark this inode as not
+ * being possible to flush to avoid that. We will clear that flag
* when we finish cloning all extents, since a transaction is started
* after finding each extent to clone.
*/
folio_unlock(folio);
folio_put(folio);
}
- if (ret)
- btrfs_delalloc_release_space(inode, data_reserved, file_offset,
- block_size, true);
- btrfs_delalloc_release_extents(inode, block_size);
-out:
- extent_changeset_free(data_reserved);
return ret;
}
fs_info->sectorsize);
struct btrfs_trans_handle *trans = NULL;
struct btrfs_drop_extents_args drop_args = { 0 };
+ struct extent_changeset *data_reserved = NULL;
int ret;
struct btrfs_key key;
- bool copied_inline_to_page = false;
- if (new_key->offset > 0) {
- ret = copy_inline_to_page(inode, new_key->offset,
- inline_data, size, datal, comp_type);
- copied_inline_to_page = (ret == 0);
- goto out;
- }
+ if (new_key->offset > 0)
+ goto copy_to_page;
key.objectid = btrfs_ino(inode);
key.type = BTRFS_EXTENT_DATA_KEY;
if (unlikely(ret))
btrfs_abort_transaction(trans, ret);
out:
- if (!ret && !trans) {
- if (copied_inline_to_page &&
- new_key->offset + datal > i_size_read(&inode->vfs_inode)) {
- /*
- * If we copied the inline extent data to a page/folio
- * beyond the i_size of the destination inode, then we
- * need to increase the i_size before we start a
- * transaction to update the inode item. This is to
- * prevent a deadlock when the flushoncommit mount
- * option is used, which happens like this:
- *
- * 1) Task A clones an inline extent from inode X to an
- * offset of inode Y that is beyond Y's current
- * i_size. This means we copied the inline extent's
- * data to a folio of inode Y that is beyond its EOF,
- * using the call above to copy_inline_to_page();
- *
- * 2) Task B starts a transaction commit and calls
- * btrfs_start_delalloc_flush() to flush delalloc;
- *
- * 3) The delalloc flushing sees the new dirty folio of
- * inode Y and when it attempts to flush it, it ends
- * up at extent_writepage() and sees that the offset
- * of the folio is beyond the i_size of inode Y, so
- * it attempts to invalidate the folio by calling
- * folio_invalidate(), which ends up at btrfs' folio
- * invalidate callback - btrfs_invalidate_folio().
- * There it tries to lock the folio's range in inode
- * Y's extent io tree, but it blocks since it's
- * currently locked by task A - during reflink we
- * lock the inodes and the source and destination
- * ranges after flushing all delalloc and waiting for
- * ordered extent completion - after that we don't
- * expect to have dirty folios in the ranges, the
- * exception is if we have to copy an inline extent's
- * data (because the destination offset is not zero);
- *
- * 4) Task A then does the 'goto out' below and attempts
- * to start a transaction to update the inode item,
- * and then it's blocked since the current
- * transaction is in the TRANS_STATE_COMMIT_START
- * state. Therefore task A has to wait for the
- * current transaction to become unblocked (its
- * state >= TRANS_STATE_UNBLOCKED).
- *
- * This leads to a deadlock - the task committing the
- * transaction waiting for the delalloc flushing which
- * is blocked during folio invalidation on the inode's
- * extent lock and the reflink task waiting for the
- * current transaction to be unblocked so that it can
- * start a new one to update the inode item (while
- * holding the extent lock).
- */
- i_size_write(&inode->vfs_inode, new_key->offset + datal);
- }
- /*
- * No transaction here means we copied the inline extent into a
- * page of the destination inode.
- *
- * 1 unit to update inode item
- */
- trans = btrfs_start_transaction(root, 1);
- if (IS_ERR(trans)) {
- ret = PTR_ERR(trans);
- trans = NULL;
- }
- }
+ extent_changeset_free(data_reserved);
if (ret && trans)
btrfs_end_transaction(trans);
if (!ret)
copy_to_page:
/*
* Release our path because we don't need it anymore and also because
- * copy_inline_to_page() needs to reserve data and metadata, which may
- * need to flush delalloc when we are low on available space and
- * therefore cause a deadlock if writeback of an inline extent needs to
- * write to the same leaf or an ordered extent completion needs to write
- * to the same leaf.
+ * reserving space for our folio below may need to flush delalloc when
+ * we are low on available space and therefore cause a deadlock if
+ * writeback of an inline extent needs to write to the same leaf or an
+ * ordered extent completion needs to write to the same leaf.
*/
btrfs_release_path(path);
+ /*
+ * Reserve space and start the transaction for the inode item update
+ * BEFORE copying the inline extent's data into a folio and dirtying
+ * it.
+ *
+ * We hold the destination range locked in the inode's io tree for the
+ * whole duration of the clone, so once we dirty a folio in that range
+ * we must not block waiting for a transaction commit: with the
+ * flushoncommit mount option the committing task flushes dirty inodes
+ * through the generic writeback path, which knows nothing about
+ * BTRFS_INODE_NO_DELALLOC_FLUSH, and its flusher then blocks forever
+ * in find_lock_delalloc_range() on our locked range while we wait for
+ * that same commit - a deadlock. This is a variant of the one fixed
+ * by commit b48c980b6a7e ("btrfs: fix deadlock between reflink and
+ * transaction commit when using flushoncommit"), with the folio
+ * inside i_size so the flusher writes it back instead of invalidating
+ * it.
+ *
+ * While the locked range is still clean the flusher has no reason to
+ * touch it, so it is safe to block here, and once we hold the open
+ * transaction handle we can no longer block waiting for a commit.
+ * The space reservation must stay ordered before the transaction
+ * start, since reserving space may flush and wait for a transaction
+ * commit.
+ */
+ ret = btrfs_delalloc_reserve_space(inode, &data_reserved,
+ new_key->offset,
+ fs_info->sectorsize);
+ if (ret)
+ goto out;
+
+ /* 1 unit to update the inode item after copying the inline extent. */
+ trans = btrfs_start_transaction(root, 1);
+ if (IS_ERR(trans)) {
+ ret = PTR_ERR(trans);
+ trans = NULL;
+ btrfs_delalloc_release_space(inode, data_reserved,
+ new_key->offset,
+ fs_info->sectorsize, true);
+ btrfs_delalloc_release_extents(inode, fs_info->sectorsize);
+ goto out;
+ }
+
ret = copy_inline_to_page(inode, new_key->offset,
inline_data, size, datal, comp_type);
- copied_inline_to_page = (ret == 0);
+ if (ret) {
+ btrfs_delalloc_release_space(inode, data_reserved,
+ new_key->offset,
+ fs_info->sectorsize, true);
+ } else if (new_key->offset + datal > i_size_read(&inode->vfs_inode)) {
+ /*
+ * Keep i_size ahead of a folio we dirtied beyond EOF, so that
+ * writeback sends it to disk instead of discarding it through
+ * folio invalidation (see commit b48c980b6a7e).
+ */
+ i_size_write(&inode->vfs_inode, new_key->offset + datal);
+ }
+ btrfs_delalloc_release_extents(inode, fs_info->sectorsize);
goto out;
}
projects / linux / commitdiff