[PATCH v3 0/3] reduce reflink src inode lock contention

[PATCH v3 0/3] reduce reflink src inode lock contention

[Boris Burkov]

We currently take the src and dst inode i_rwsem exclusive for the
entirety of a reflink which includes writing back all of both files and
walking all the src extents and duplicating them to dst. This results in
contention on those locks which I think can be reduced.

The basic strategy is:
1. try to do obvious easy src writeback outside the lock.
2. downgrade the src lock to shared where it is safe.

Changelog:
v3:
- use btrfs_wait_ordered_range() instead of the misguided custom walk.
- add a helper to unify range computations.
v2:
- introduce unlocked wb pre-flush instead of a downgrade of src inode lock
  for the whole reflink.
- add the "has writers" pessimization.
- still downgrade after done with wb.

Boris Burkov (3):
  btrfs: pre-flush reflink source before taking inode locks
  btrfs: skip unlocked reflink source flush if the inode has writers
  btrfs: downgrade the reflink source inode lock for tree walking

 fs/btrfs/btrfs_inode.h |  6 +++++
 fs/btrfs/direct-io.c   |  7 ++++++
 fs/btrfs/reflink.c     | 51 +++++++++++++++++++++++++++++++++++++-----
 3 files changed, 58 insertions(+), 6 deletions(-)

-- 
2.55.0

[PATCH v3 1/3] btrfs: pre-flush reflink source before taking inode locks

[Boris Burkov]

Consider the following sketch of a shell script:
  dd if=/dev/urandom of=/mnt/src bs=1M count=4096
  cp --reflink=always /mnt/src /mnt/dst &
  sleep 0.1   # let the clone reach its flush
  time dd if=/mnt/src of=/dev/null bs=4K count=1 iflag=direct

The current logic in reflink ensures the existence and stability of both
the src and destination inode by locking them both and then flushing all
dirty pages / ordered_extents under the inode lock. This blocks
concurrent usage by even readers of the inode locks, like direct reads
or seeks for the duration of writeback on the entirety of the two files.

We observe this particular contention frequently in the Meta fleet.

It is also a relatively common pattern to write the src file, then reflink
it while it is still dirty, so that typical path naturally hits this
contention. This workload motivates a relatively simple optimization:
trigger the unavoidable src flushing outside the locked region.

It is tempting to try to move the writeback out of the locks entirely
but this is fraught with all kinds of consistency errors under various
patterns of concurrent writes. fsync is able to manage such a pattern,
but with significant infrastructure investment I don't think is
justified for reflink.

Therefore, do an optimistic btrfs_wait_ordered_range() without the locks
while relying on the calls to btrfs_wait_ordered_range() under the
locks in btrfs_remap_file_range_prep() to ensure correctness. In the
worst case with concurrent writes while unlocked, we can end up doing
the writeback twice, which I think is a reasonable price to pay to avoid
victimizing innocent readers in a common case.

With and without this patch, the above reproducer runs the reflink in
the same ~0.5s on my system. Without the patch, the direct read blocks
for basically the full duration of the reflink while with the patch it
returns in a few milliseconds.

Signed-off-by: Boris Burkov <boris@bur.io>
---
 fs/btrfs/reflink.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
index d2a4101912bd..24c0b226bc31 100644
--- a/fs/btrfs/reflink.c
+++ b/fs/btrfs/reflink.c
@@ -833,6 +833,17 @@ static noinline int btrfs_clone_files(struct file *file, struct file *file_src,
 	return 0;
 }
 
+static u64 calc_remap_wb_len(struct btrfs_inode *src_inode, loff_t off, loff_t len,
+			     unsigned int remap_flags)
+{
+	const u32 bs = src_inode->root->fs_info->sectorsize;
+	const loff_t isize = i_size_read(&src_inode->vfs_inode);
+
+	if (len == 0 && !(remap_flags & REMAP_FILE_DEDUP))
+		return ALIGN(isize, bs) - ALIGN_DOWN(off, bs);
+	return ALIGN(len, bs);
+}
+
 static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in,
 				       struct file *file_out, loff_t pos_out,
 				       loff_t *len, unsigned int remap_flags)
@@ -876,10 +887,7 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in,
 	 *    not for the ordered extents to complete. We need to wait for them
 	 *    to complete so that new file extent items are in the fs tree.
 	 */
-	if (*len == 0 && !(remap_flags & REMAP_FILE_DEDUP))
-		wb_len = ALIGN(inode_in->vfs_inode.i_size, bs) - ALIGN_DOWN(pos_in, bs);
-	else
-		wb_len = ALIGN(*len, bs);
+	wb_len = calc_remap_wb_len(inode_in, pos_in, *len, remap_flags);
 
 	/*
 	 * Workaround to make sure NOCOW buffered write reach disk as NOCOW.
@@ -930,6 +938,7 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
 	struct btrfs_inode *src_inode = BTRFS_I(file_inode(src_file));
 	struct btrfs_inode *dst_inode = BTRFS_I(file_inode(dst_file));
 	bool same_inode = dst_inode == src_inode;
+	u64 wb_start, wb_len;
 	int ret;
 
 	if (btrfs_is_shutdown(src_inode->root->fs_info))
@@ -938,6 +947,17 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
 	if (remap_flags & ~(REMAP_FILE_DEDUP | REMAP_FILE_ADVISORY))
 		return -EINVAL;
 
+	/*
+	 * Optimistically write out the src inode before taking locks.
+	 * Consistency is properly ensured by the btrfs_wait_ordered_range()
+	 * inside btrfs_remap_file_range_prep().
+	 */
+	wb_start = ALIGN_DOWN(off, src_inode->root->fs_info->sectorsize);
+	wb_len = calc_remap_wb_len(src_inode, off, len, remap_flags);
+	ret = btrfs_wait_ordered_range(src_inode, wb_start, wb_len);
+	if (ret < 0)
+		return ret;
+
 	if (same_inode) {
 		btrfs_inode_lock(src_inode, BTRFS_ILOCK_MMAP);
 	} else {
-- 
2.55.0

[PATCH v3 2/3] btrfs: skip unlocked reflink source flush if the inode has writers

[Boris Burkov]

The optimistic unlocked flushing is counter-productive if there is an
active writer dirtying pages, as it results in doubly writing back any
pages that get dirtied again before the locked flush. Therefore,
pessimize it slightly and don't do the unlocked flushing if we have
outstanding writable file descriptors or memory mappings. Since the
inode isn't locked, this is only advisory, any new mapping or fd that
sneaks in after the check but before the locking can still cause extra
writeback. But it trivially helps quite a bit in simple cases with a
long open file descriptor or mapping.

I tested this with two similar workloads. One was keeping and fd open
and doing continuous writes into the src file while cloning it and doing
lseek on it and the other was slightly meaner, constantly opening and
closing the fd to try to race this check.

Workload 1 shows a huge regression in write amplification and writers
and readers who experience userspace wall time stalls of 100ms+ with
just the pre-flush, but recovers to normal with this check.

Workload 2 is a smaller regression which is mostly helped but not fully
resolved with this check. So we must weigh this regression against the
benefit of the unlocked flush in the allegedly more common simpler case.

Workload 1: fd kept open, 60s of clone/write/lseek looping.
kernel        clones  clone mean s  device MiB/clone  writer stalls  reader stalls
for-next         564         0.095               119             10             10
pre-flush (p1)   179         0.338               405             94             93
skip (p1+p2)     540         0.100               116              9              9

Workload 2: same writer, but closing and reopening the fd around each
64 MiB write with some random sleeping to encourage racing.
kernel        clones  clone mean s  device MiB/clone  writer stalls  reader stalls
for-next         740         0.070                83              9              9
pre-flush (p1)   255         0.224               246             27             26
skip (p1+p2)     518         0.107               121             11             11

Signed-off-by: Boris Burkov <boris@bur.io>
---
 fs/btrfs/reflink.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
index 24c0b226bc31..fe765ab27c82 100644
--- a/fs/btrfs/reflink.c
+++ b/fs/btrfs/reflink.c
@@ -950,13 +950,19 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
 	/*
 	 * Optimistically write out the src inode before taking locks.
 	 * Consistency is properly ensured by the btrfs_wait_ordered_range()
-	 * inside btrfs_remap_file_range_prep().
+	 * inside btrfs_remap_file_range_prep(). This optimization causes
+	 * redundant writeback if there is a concurrent writer to the src file
+	 * so try to catch anyone holding the file open for writes and skip
+	 * the optimization in that case.
 	 */
 	wb_start = ALIGN_DOWN(off, src_inode->root->fs_info->sectorsize);
 	wb_len = calc_remap_wb_len(src_inode, off, len, remap_flags);
-	ret = btrfs_wait_ordered_range(src_inode, wb_start, wb_len);
-	if (ret < 0)
-		return ret;
+	if (atomic_read(&src_inode->vfs_inode.i_writecount) <= 0 &&
+	    !mapping_writably_mapped(src_inode->vfs_inode.i_mapping)) {
+		ret = btrfs_wait_ordered_range(src_inode, wb_start, wb_len);
+		if (ret < 0)
+			return ret;
+	}
 
 	if (same_inode) {
 		btrfs_inode_lock(src_inode, BTRFS_ILOCK_MMAP);
-- 
2.55.0

[PATCH v3 3/3] btrfs: downgrade the reflink source inode lock for tree walking

[Boris Burkov]

When doing a reflink, we are not writing to the src inode or modifying
it in any way, so it is counter-intuitive to have to lock it
exclusively. This has the cost of blocking other shared lock users of
the src file for the duration of all the writeback and reflinking.

There are two important gotchas to do with direct io that must be
addressed to make this idea work.

The first gotcha is that dio writes that come in under EOF try to take
inode->i_rwsem shared, which would result in messed up views of the
extents in the reflinked file. Therefore, add "being used as a reflink
src" as another inode flag exception for the dio write shared tests that
already exist to check for other dangerous conditions. In order to
ensure that this flag is visible to the dio writer, reflink must first
take i_rwsem exclusive, then set the flag, then downgrade to shared.

The second is that dio reads increment inode->i_dio_count which blocks
inode_dio_wait() in __generic_remap_file_range_prep(). Therefore, we
must remain exclusive to dio reads through finishing the prep or risk
starving the clone on a stream of dio readers. There is still O(extents)
work in btrfs_clone() which can be done without holding the lock
exclusive, so we should downgrade to shared and admit dio readers (and
other shared lock users like lseek) after we are finished with
inode_dio_wait().

By running concurrent clones and lseeks or dio reads on a src file with
32k extents on a vm, I can demonstrate 100ms+ stalls on the readers
without this patch and they go away entirely with the patch.

Signed-off-by: Boris Burkov <boris@bur.io>
---
 fs/btrfs/btrfs_inode.h |  6 ++++++
 fs/btrfs/direct-io.c   |  7 +++++++
 fs/btrfs/reflink.c     | 17 +++++++++++++++--
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 46c62f980c24..03c00c99099d 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -99,6 +99,12 @@ enum {
 	 * range).
 	 */
 	BTRFS_INODE_COW_WRITE_ERROR,
+	/*
+	 * Set by reflink while it holds the source's VFS inode lock shared.
+	 * Direct IO writes also take that lock shared; one that finds this bit
+	 * set after acquiring it must retry with the exclusive lock.
+	 */
+	BTRFS_INODE_REFLINK_SRC,
 	/*
 	 * Indicate this is a directory that points to a subvolume for which
 	 * there is no root reference item. That's a case like the following:
diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
index 3075d7992713..ed3914257d39 100644
--- a/fs/btrfs/direct-io.c
+++ b/fs/btrfs/direct-io.c
@@ -913,6 +913,13 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
 		goto relock;
 	}
 
+	if ((ilock_flags & BTRFS_ILOCK_SHARED) &&
+	    test_bit(BTRFS_INODE_REFLINK_SRC, &BTRFS_I(inode)->runtime_flags)) {
+		btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
+		ilock_flags &= ~BTRFS_ILOCK_SHARED;
+		goto relock;
+	}
+
 	ret = generic_write_checks(iocb, from);
 	if (ret <= 0) {
 		btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
index fe765ab27c82..72c1e0db57b7 100644
--- a/fs/btrfs/reflink.c
+++ b/fs/btrfs/reflink.c
@@ -939,6 +939,7 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
 	struct btrfs_inode *dst_inode = BTRFS_I(file_inode(dst_file));
 	bool same_inode = dst_inode == src_inode;
 	u64 wb_start, wb_len;
+	bool src_downgraded = false;
 	int ret;
 
 	if (btrfs_is_shutdown(src_inode->root->fs_info))
@@ -976,6 +977,12 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
 	if (ret < 0 || len == 0)
 		goto out_unlock;
 
+	if (!same_inode) {
+		set_bit(BTRFS_INODE_REFLINK_SRC, &src_inode->runtime_flags);
+		downgrade_write(&src_inode->vfs_inode.i_rwsem);
+		src_downgraded = true;
+	}
+
 	if (remap_flags & REMAP_FILE_DEDUP)
 		ret = btrfs_extent_same(src_inode, off, len, dst_inode, destoff);
 	else
@@ -986,8 +993,14 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
 		btrfs_inode_unlock(src_inode, BTRFS_ILOCK_MMAP);
 	} else {
 		btrfs_double_mmap_unlock(src_inode, dst_inode);
-		unlock_two_nondirectories(&src_inode->vfs_inode,
-					  &dst_inode->vfs_inode);
+		if (src_downgraded) {
+			clear_bit(BTRFS_INODE_REFLINK_SRC, &src_inode->runtime_flags);
+			inode_unlock_shared(&src_inode->vfs_inode);
+			inode_unlock(&dst_inode->vfs_inode);
+		} else {
+			unlock_two_nondirectories(&src_inode->vfs_inode,
+						  &dst_inode->vfs_inode);
+		}
 	}
 
 	/*
-- 
2.55.0

Re: [PATCH v3 1/3] btrfs: pre-flush reflink source before taking inode locks

[Filipe Manana]

On Tue, Sep 15, 2026 at 7:05 PM Boris Burkov <boris@bur.io> wrote:
>
> Consider the following sketch of a shell script:

Could you leave a blank line separating regular text from command lines or code?
It improves readability.

>   dd if=/dev/urandom of=/mnt/src bs=1M count=4096
>   cp --reflink=always /mnt/src /mnt/dst &
>   sleep 0.1   # let the clone reach its flush
>   time dd if=/mnt/src of=/dev/null bs=4K count=1 iflag=direct
>
> The current logic in reflink ensures the existence and stability of both
> the src and destination inode by locking them both and then flushing all
> dirty pages / ordered_extents under the inode lock. This blocks
> concurrent usage by even readers of the inode locks, like direct reads
> or seeks for the duration of writeback on the entirety of the two files.
>
> We observe this particular contention frequently in the Meta fleet.
>
> It is also a relatively common pattern to write the src file, then reflink
> it while it is still dirty, so that typical path naturally hits this
> contention. This workload motivates a relatively simple optimization:
> trigger the unavoidable src flushing outside the locked region.
>
> It is tempting to try to move the writeback out of the locks entirely
> but this is fraught with all kinds of consistency errors under various
> patterns of concurrent writes. fsync is able to manage such a pattern,
> but with significant infrastructure investment I don't think is
> justified for reflink.

What significant infrastructure investment does fsync uses?
What is being done in this patch is nearly the same. With fsync, it's
a bit less trivial because there are two modes (full and fast).

I don't think there's a need to mention that or at least not in a way
that is a bit confusing.
Just saying that it follows a similar approach to fsync is enough.

>
> Therefore, do an optimistic btrfs_wait_ordered_range() without the locks
> while relying on the calls to btrfs_wait_ordered_range() under the
> locks in btrfs_remap_file_range_prep() to ensure correctness. In the
> worst case with concurrent writes while unlocked, we can end up doing
> the writeback twice, which I think is a reasonable price to pay to avoid
> victimizing innocent readers in a common case.
>
> With and without this patch, the above reproducer runs the reflink in
> the same ~0.5s on my system. Without the patch, the direct read blocks
> for basically the full duration of the reflink while with the patch it
> returns in a few milliseconds.
>
> Signed-off-by: Boris Burkov <boris@bur.io>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

> ---
>  fs/btrfs/reflink.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
> index d2a4101912bd..24c0b226bc31 100644
> --- a/fs/btrfs/reflink.c
> +++ b/fs/btrfs/reflink.c
> @@ -833,6 +833,17 @@ static noinline int btrfs_clone_files(struct file *file, struct file *file_src,
>         return 0;
>  }
>
> +static u64 calc_remap_wb_len(struct btrfs_inode *src_inode, loff_t off, loff_t len,
> +                            unsigned int remap_flags)
> +{
> +       const u32 bs = src_inode->root->fs_info->sectorsize;
> +       const loff_t isize = i_size_read(&src_inode->vfs_inode);
> +
> +       if (len == 0 && !(remap_flags & REMAP_FILE_DEDUP))
> +               return ALIGN(isize, bs) - ALIGN_DOWN(off, bs);
> +       return ALIGN(len, bs);
> +}
> +
>  static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in,
>                                        struct file *file_out, loff_t pos_out,
>                                        loff_t *len, unsigned int remap_flags)
> @@ -876,10 +887,7 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in,
>          *    not for the ordered extents to complete. We need to wait for them
>          *    to complete so that new file extent items are in the fs tree.
>          */
> -       if (*len == 0 && !(remap_flags & REMAP_FILE_DEDUP))
> -               wb_len = ALIGN(inode_in->vfs_inode.i_size, bs) - ALIGN_DOWN(pos_in, bs);
> -       else
> -               wb_len = ALIGN(*len, bs);
> +       wb_len = calc_remap_wb_len(inode_in, pos_in, *len, remap_flags);
>
>         /*
>          * Workaround to make sure NOCOW buffered write reach disk as NOCOW.
> @@ -930,6 +938,7 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
>         struct btrfs_inode *src_inode = BTRFS_I(file_inode(src_file));
>         struct btrfs_inode *dst_inode = BTRFS_I(file_inode(dst_file));
>         bool same_inode = dst_inode == src_inode;
> +       u64 wb_start, wb_len;
>         int ret;
>
>         if (btrfs_is_shutdown(src_inode->root->fs_info))
> @@ -938,6 +947,17 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
>         if (remap_flags & ~(REMAP_FILE_DEDUP | REMAP_FILE_ADVISORY))
>                 return -EINVAL;
>
> +       /*
> +        * Optimistically write out the src inode before taking locks.
> +        * Consistency is properly ensured by the btrfs_wait_ordered_range()
> +        * inside btrfs_remap_file_range_prep().
> +        */
> +       wb_start = ALIGN_DOWN(off, src_inode->root->fs_info->sectorsize);
> +       wb_len = calc_remap_wb_len(src_inode, off, len, remap_flags);
> +       ret = btrfs_wait_ordered_range(src_inode, wb_start, wb_len);
> +       if (ret < 0)
> +               return ret;
> +
>         if (same_inode) {
>                 btrfs_inode_lock(src_inode, BTRFS_ILOCK_MMAP);
>         } else {
> --
> 2.55.0
>
>

Re: [PATCH v3 2/3] btrfs: skip unlocked reflink source flush if the inode has writers

[Filipe Manana]

On Tue, Sep 15, 2026 at 8:05 PM Boris Burkov <boris@bur.io> wrote:
>
> The optimistic unlocked flushing is counter-productive if there is an
> active writer dirtying pages, as it results in doubly writing back any
> pages that get dirtied again before the locked flush. Therefore,
> pessimize it slightly and don't do the unlocked flushing if we have
> outstanding writable file descriptors or memory mappings. Since the
> inode isn't locked, this is only advisory, any new mapping or fd that
> sneaks in after the check but before the locking can still cause extra
> writeback. But it trivially helps quite a bit in simple cases with a
> long open file descriptor or mapping.
>
> I tested this with two similar workloads. One was keeping and fd open
> and doing continuous writes into the src file while cloning it and doing
> lseek on it and the other was slightly meaner, constantly opening and
> closing the fd to try to race this check.
>
> Workload 1 shows a huge regression in write amplification and writers
> and readers who experience userspace wall time stalls of 100ms+ with
> just the pre-flush, but recovers to normal with this check.
>
> Workload 2 is a smaller regression which is mostly helped but not fully
> resolved with this check. So we must weigh this regression against the
> benefit of the unlocked flush in the allegedly more common simpler case.
>
> Workload 1: fd kept open, 60s of clone/write/lseek looping.
> kernel        clones  clone mean s  device MiB/clone  writer stalls  reader stalls
> for-next         564         0.095               119             10             10
> pre-flush (p1)   179         0.338               405             94             93
> skip (p1+p2)     540         0.100               116              9              9
>
> Workload 2: same writer, but closing and reopening the fd around each
> 64 MiB write with some random sleeping to encourage racing.
> kernel        clones  clone mean s  device MiB/clone  writer stalls  reader stalls
> for-next         740         0.070                83              9              9
> pre-flush (p1)   255         0.224               246             27             26
> skip (p1+p2)     518         0.107               121             11             11
>
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
>  fs/btrfs/reflink.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
> index 24c0b226bc31..fe765ab27c82 100644
> --- a/fs/btrfs/reflink.c
> +++ b/fs/btrfs/reflink.c
> @@ -950,13 +950,19 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
>         /*
>          * Optimistically write out the src inode before taking locks.
>          * Consistency is properly ensured by the btrfs_wait_ordered_range()
> -        * inside btrfs_remap_file_range_prep().
> +        * inside btrfs_remap_file_range_prep(). This optimization causes
> +        * redundant writeback if there is a concurrent writer to the src file
> +        * so try to catch anyone holding the file open for writes and skip
> +        * the optimization in that case.
>          */
>         wb_start = ALIGN_DOWN(off, src_inode->root->fs_info->sectorsize);
>         wb_len = calc_remap_wb_len(src_inode, off, len, remap_flags);
> -       ret = btrfs_wait_ordered_range(src_inode, wb_start, wb_len);
> -       if (ret < 0)
> -               return ret;
> +       if (atomic_read(&src_inode->vfs_inode.i_writecount) <= 0 &&

Instead of reading the counter directly, use inode_is_open_for_write() please.

With that change:

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

> +           !mapping_writably_mapped(src_inode->vfs_inode.i_mapping)) {
> +               ret = btrfs_wait_ordered_range(src_inode, wb_start, wb_len);
> +               if (ret < 0)
> +                       return ret;
> +       }
>
>         if (same_inode) {
>                 btrfs_inode_lock(src_inode, BTRFS_ILOCK_MMAP);
> --
> 2.55.0
>
>

Re: [PATCH v3 3/3] btrfs: downgrade the reflink source inode lock for tree walking

[Filipe Manana]

On Tue, Sep 15, 2026 at 7:08 PM Boris Burkov <boris@bur.io> wrote:
>
> When doing a reflink, we are not writing to the src inode or modifying
> it in any way, so it is counter-intuitive to have to lock it
> exclusively. This has the cost of blocking other shared lock users of
> the src file for the duration of all the writeback and reflinking.

This last sentence is a bit outdated, as we very often no longer wait
for the duration of all writeback, since it usually happens before
locking.

Other than that:

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

>
> There are two important gotchas to do with direct io that must be
> addressed to make this idea work.
>
> The first gotcha is that dio writes that come in under EOF try to take
> inode->i_rwsem shared, which would result in messed up views of the
> extents in the reflinked file. Therefore, add "being used as a reflink
> src" as another inode flag exception for the dio write shared tests that
> already exist to check for other dangerous conditions. In order to
> ensure that this flag is visible to the dio writer, reflink must first
> take i_rwsem exclusive, then set the flag, then downgrade to shared.
>
> The second is that dio reads increment inode->i_dio_count which blocks
> inode_dio_wait() in __generic_remap_file_range_prep(). Therefore, we
> must remain exclusive to dio reads through finishing the prep or risk
> starving the clone on a stream of dio readers. There is still O(extents)
> work in btrfs_clone() which can be done without holding the lock
> exclusive, so we should downgrade to shared and admit dio readers (and
> other shared lock users like lseek) after we are finished with
> inode_dio_wait().
>
> By running concurrent clones and lseeks or dio reads on a src file with
> 32k extents on a vm, I can demonstrate 100ms+ stalls on the readers
> without this patch and they go away entirely with the patch.
>
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
>  fs/btrfs/btrfs_inode.h |  6 ++++++
>  fs/btrfs/direct-io.c   |  7 +++++++
>  fs/btrfs/reflink.c     | 17 +++++++++++++++--
>  3 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index 46c62f980c24..03c00c99099d 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -99,6 +99,12 @@ enum {
>          * range).
>          */
>         BTRFS_INODE_COW_WRITE_ERROR,
> +       /*
> +        * Set by reflink while it holds the source's VFS inode lock shared.
> +        * Direct IO writes also take that lock shared; one that finds this bit
> +        * set after acquiring it must retry with the exclusive lock.
> +        */
> +       BTRFS_INODE_REFLINK_SRC,
>         /*
>          * Indicate this is a directory that points to a subvolume for which
>          * there is no root reference item. That's a case like the following:
> diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
> index 3075d7992713..ed3914257d39 100644
> --- a/fs/btrfs/direct-io.c
> +++ b/fs/btrfs/direct-io.c
> @@ -913,6 +913,13 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
>                 goto relock;
>         }
>
> +       if ((ilock_flags & BTRFS_ILOCK_SHARED) &&
> +           test_bit(BTRFS_INODE_REFLINK_SRC, &BTRFS_I(inode)->runtime_flags)) {
> +               btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
> +               ilock_flags &= ~BTRFS_ILOCK_SHARED;
> +               goto relock;
> +       }
> +
>         ret = generic_write_checks(iocb, from);
>         if (ret <= 0) {
>                 btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
> diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
> index fe765ab27c82..72c1e0db57b7 100644
> --- a/fs/btrfs/reflink.c
> +++ b/fs/btrfs/reflink.c
> @@ -939,6 +939,7 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
>         struct btrfs_inode *dst_inode = BTRFS_I(file_inode(dst_file));
>         bool same_inode = dst_inode == src_inode;
>         u64 wb_start, wb_len;
> +       bool src_downgraded = false;
>         int ret;
>
>         if (btrfs_is_shutdown(src_inode->root->fs_info))
> @@ -976,6 +977,12 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
>         if (ret < 0 || len == 0)
>                 goto out_unlock;
>
> +       if (!same_inode) {
> +               set_bit(BTRFS_INODE_REFLINK_SRC, &src_inode->runtime_flags);
> +               downgrade_write(&src_inode->vfs_inode.i_rwsem);
> +               src_downgraded = true;
> +       }
> +
>         if (remap_flags & REMAP_FILE_DEDUP)
>                 ret = btrfs_extent_same(src_inode, off, len, dst_inode, destoff);
>         else
> @@ -986,8 +993,14 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off,
>                 btrfs_inode_unlock(src_inode, BTRFS_ILOCK_MMAP);
>         } else {
>                 btrfs_double_mmap_unlock(src_inode, dst_inode);
> -               unlock_two_nondirectories(&src_inode->vfs_inode,
> -                                         &dst_inode->vfs_inode);
> +               if (src_downgraded) {
> +                       clear_bit(BTRFS_INODE_REFLINK_SRC, &src_inode->runtime_flags);
> +                       inode_unlock_shared(&src_inode->vfs_inode);
> +                       inode_unlock(&dst_inode->vfs_inode);
> +               } else {
> +                       unlock_two_nondirectories(&src_inode->vfs_inode,
> +                                                 &dst_inode->vfs_inode);
> +               }
>         }
>
>         /*
> --
> 2.55.0
>
>