[PATCH] btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name

[PATCH] btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

If we are logging a new name make sure our inode has the runtime flag
BTRFS_INODE_COPY_EVERYTHING set so that at btrfs_log_inode() we will find
new inode refs/extrefs in the subvolume tree and copy them into the log
tree.

We are currently doing it when adding a new link but we are missing it
when renaming.

An example where this makes a new name not persisted:

  1) create symlink with name foo in directory A
  2) fsync directory A, which persists the symlink
  3) rename the symlink from foo to bar
  4) fsync directory A to persist the new symlink name

Step 4 isn't working correctly as it's not logging the new name and also
leaving the old inode ref in the log tree, so after a power failure the
symlink still has the old name of "foo". This is because when we first
fsync directoy A we log the symlink's inode (as it's a new entry) and at
btrfs_log_inode() we set the log mode to LOG_INODE_ALL and then because
we are using that mode and the inode has the runtime flag
BTRFS_INODE_NEEDS_FULL_SYNC set, we clear that flag as well as the flag
BTRFS_INODE_COPY_EVERYTHING. That means the next time we log the inode,
during the rename through the call to btrfs_log_new_name() (calling
btrfs_log_inode_parent() and then btrfs_log_inode()), we will not search
the subvolume tree for new refs/extrefs and jump directory to the
'log_extents' label.

Fix this by making sure we set BTRFS_INODE_COPY_EVERYTHING on an inode
when we are about to log a new name. A test case for fstests will follow
soon.

Reported-by: Vyacheslav Kovalevsky <slava.kovalevskiy.2014@gmail.com>
Link: https://lore.kernel.org/linux-btrfs/ac949c74-90c2-4b9a-b7fd-1ffc5c3175c7@gmail.com/
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/inode.c    | 1 -
 fs/btrfs/tree-log.c | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 79732756b87f..03e9c3ac20ed 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6885,7 +6885,6 @@ static int btrfs_link(struct dentry *old_dentry, struct inode *dir,
 	BTRFS_I(inode)->dir_index = 0ULL;
 	inode_inc_iversion(inode);
 	inode_set_ctime_current(inode);
-	set_bit(BTRFS_INODE_COPY_EVERYTHING, &BTRFS_I(inode)->runtime_flags);
 
 	ret = btrfs_add_link(trans, BTRFS_I(dir), BTRFS_I(inode),
 			     &fname.disk_name, 1, index);
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 65079eb651da..8dfd504b37ae 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -7905,6 +7905,9 @@ void btrfs_log_new_name(struct btrfs_trans_handle *trans,
 	bool log_pinned = false;
 	int ret;
 
+	/* The inode has a new name (ref/extref), so make sure we log it. */
+	set_bit(BTRFS_INODE_COPY_EVERYTHING, &inode->runtime_flags);
+
 	btrfs_init_log_ctx(&ctx, inode);
 	ctx.logging_new_name = true;
 
-- 
2.47.2

Re: [PATCH] btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name

[Boris Burkov]

On Fri, Oct 24, 2025 at 12:52:02PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> If we are logging a new name make sure our inode has the runtime flag
> BTRFS_INODE_COPY_EVERYTHING set so that at btrfs_log_inode() we will find
> new inode refs/extrefs in the subvolume tree and copy them into the log
> tree.
> 
> We are currently doing it when adding a new link but we are missing it
> when renaming.
> 
> An example where this makes a new name not persisted:
> 
>   1) create symlink with name foo in directory A
>   2) fsync directory A, which persists the symlink
>   3) rename the symlink from foo to bar
>   4) fsync directory A to persist the new symlink name
> 
> Step 4 isn't working correctly as it's not logging the new name and also
> leaving the old inode ref in the log tree, so after a power failure the
> symlink still has the old name of "foo". This is because when we first
> fsync directoy A we log the symlink's inode (as it's a new entry) and at
> btrfs_log_inode() we set the log mode to LOG_INODE_ALL and then because
> we are using that mode and the inode has the runtime flag
> BTRFS_INODE_NEEDS_FULL_SYNC set, we clear that flag as well as the flag
> BTRFS_INODE_COPY_EVERYTHING. That means the next time we log the inode,
> during the rename through the call to btrfs_log_new_name() (calling
> btrfs_log_inode_parent() and then btrfs_log_inode()), we will not search
> the subvolume tree for new refs/extrefs and jump directory to the
> 'log_extents' label.
> 
> Fix this by making sure we set BTRFS_INODE_COPY_EVERYTHING on an inode
> when we are about to log a new name. A test case for fstests will follow
> soon.
> 
> Reported-by: Vyacheslav Kovalevsky <slava.kovalevskiy.2014@gmail.com>
> Link: https://lore.kernel.org/linux-btrfs/ac949c74-90c2-4b9a-b7fd-1ffc5c3175c7@gmail.com/
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Boris Burkov <boris@bur.io>

> ---
>  fs/btrfs/inode.c    | 1 -
>  fs/btrfs/tree-log.c | 3 +++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 79732756b87f..03e9c3ac20ed 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6885,7 +6885,6 @@ static int btrfs_link(struct dentry *old_dentry, struct inode *dir,
>  	BTRFS_I(inode)->dir_index = 0ULL;
>  	inode_inc_iversion(inode);
>  	inode_set_ctime_current(inode);
> -	set_bit(BTRFS_INODE_COPY_EVERYTHING, &BTRFS_I(inode)->runtime_flags);
>  
>  	ret = btrfs_add_link(trans, BTRFS_I(dir), BTRFS_I(inode),
>  			     &fname.disk_name, 1, index);
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 65079eb651da..8dfd504b37ae 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -7905,6 +7905,9 @@ void btrfs_log_new_name(struct btrfs_trans_handle *trans,
>  	bool log_pinned = false;
>  	int ret;
>  
> +	/* The inode has a new name (ref/extref), so make sure we log it. */
> +	set_bit(BTRFS_INODE_COPY_EVERYTHING, &inode->runtime_flags);
> +
>  	btrfs_init_log_ctx(&ctx, inode);
>  	ctx.logging_new_name = true;
>  
> -- 
> 2.47.2
> 

Re: [PATCH] btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name

[Qu Wenruo]

在 2025/10/24 22:22, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> If we are logging a new name make sure our inode has the runtime flag
> BTRFS_INODE_COPY_EVERYTHING set so that at btrfs_log_inode() we will find
> new inode refs/extrefs in the subvolume tree and copy them into the log
> tree.
> 
> We are currently doing it when adding a new link but we are missing it
> when renaming.
> 
> An example where this makes a new name not persisted:
> 
>    1) create symlink with name foo in directory A
>    2) fsync directory A, which persists the symlink
>    3) rename the symlink from foo to bar
>    4) fsync directory A to persist the new symlink name
> 
> Step 4 isn't working correctly as it's not logging the new name and also
> leaving the old inode ref in the log tree, so after a power failure the
> symlink still has the old name of "foo". This is because when we first
> fsync directoy A we log the symlink's inode (as it's a new entry) and at
> btrfs_log_inode() we set the log mode to LOG_INODE_ALL and then because
> we are using that mode and the inode has the runtime flag
> BTRFS_INODE_NEEDS_FULL_SYNC set, we clear that flag as well as the flag
> BTRFS_INODE_COPY_EVERYTHING. That means the next time we log the inode,
> during the rename through the call to btrfs_log_new_name() (calling
> btrfs_log_inode_parent() and then btrfs_log_inode()), we will not search
> the subvolume tree for new refs/extrefs and jump directory to the
> 'log_extents' label.
> 
> Fix this by making sure we set BTRFS_INODE_COPY_EVERYTHING on an inode
> when we are about to log a new name. A test case for fstests will follow
> soon.
> 
> Reported-by: Vyacheslav Kovalevsky <slava.kovalevskiy.2014@gmail.com>
> Link: https://lore.kernel.org/linux-btrfs/ac949c74-90c2-4b9a-b7fd-1ffc5c3175c7@gmail.com/
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/inode.c    | 1 -
>   fs/btrfs/tree-log.c | 3 +++
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 79732756b87f..03e9c3ac20ed 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6885,7 +6885,6 @@ static int btrfs_link(struct dentry *old_dentry, struct inode *dir,
>   	BTRFS_I(inode)->dir_index = 0ULL;
>   	inode_inc_iversion(inode);
>   	inode_set_ctime_current(inode);
> -	set_bit(BTRFS_INODE_COPY_EVERYTHING, &BTRFS_I(inode)->runtime_flags);
>   
>   	ret = btrfs_add_link(trans, BTRFS_I(dir), BTRFS_I(inode),
>   			     &fname.disk_name, 1, index);
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 65079eb651da..8dfd504b37ae 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -7905,6 +7905,9 @@ void btrfs_log_new_name(struct btrfs_trans_handle *trans,
>   	bool log_pinned = false;
>   	int ret;
>   
> +	/* The inode has a new name (ref/extref), so make sure we log it. */
> +	set_bit(BTRFS_INODE_COPY_EVERYTHING, &inode->runtime_flags);
> +
>   	btrfs_init_log_ctx(&ctx, inode);
>   	ctx.logging_new_name = true;
>