[T][SRU][PATCH 0/1] Btrfs: send, don't send rmdir for same target multiple times

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[T][SRU][PATCH 0/1] Btrfs: send, don't send rmdir for same target multiple times

Po-Hsu Lin (Sam)
BugLink: https://bugs.launchpad.net/bugs/1809868

== Justification ==
When doing an incremental send on a Btrfs filesystem, if we delete a directory
that has N > 1 hardlinks for the same file and that file has the highest inode
number inside the directory contents, an incremental send would send N times
rmdir operation against the directory. This made the btrfs receive command fail
on the second rmdir instruction, as the target directory didn't exist anymore.

This issue can be triggered with 29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5
test in ubuntu_btrfs_kernel_fixes test suite.


The test will fail with:
  "ERROR: rmdir o259-6-0 failed. No such file or directory" with Trusty kernel.

Performing full device TRIM (1.00GiB) ...
Turning ON incompat feature 'extref': increased hardlink limit per file to 65536

WARNING! - Btrfs v3.12 IS EXPERIMENTAL
WARNING! - see http://btrfs.wiki.kernel.org before using

fs created label (null) on /dev/loop0
 nodesize 16384 leafsize 16384 sectorsize 4096 size 1.00GiB
Btrfs v3.12
Create a readonly snapshot of '/tmp/mnt-29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5' in '/tmp/mnt-29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5/snap1'
At subvol /tmp/mnt-29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5/snap1
Create a readonly snapshot of '/tmp/mnt-29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5' in '/tmp/mnt-29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5/snap2'
At subvol /tmp/mnt-29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5/snap2
Performing full device TRIM (1.00GiB) ...
Turning ON incompat feature 'extref': increased hardlink limit per file to 65536

WARNING! - Btrfs v3.12 IS EXPERIMENTAL
WARNING! - see http://btrfs.wiki.kernel.org before using

fs created label (null) on /dev/loop0
 nodesize 16384 leafsize 16384 sectorsize 4096 size 1.00GiB
Btrfs v3.12
At subvol snap1
At snapshot snap2
ERROR: rmdir o259-6-0 failed. No such file or directory
incremental receive failed


== Fix ==
29d6d30f5 (Btrfs: send, don't send rmdir for same target multiple times)
This patch needs to be backported for a variable difference in the
process_recorded_refs function.

In the Trusty tree:
static int process_recorded_refs(struct send_ctx *sctx)

In the patch:
static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)

As the new *pending_move is unrelated here, we can just backport the patch with
the same logic in this function.

A test kernel could be found here:
http://people.canonical.com/~phlin/kernel/lp-1809868-btrfs-2nd-rmdir/
The 29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5 test will pass with this kernel.


== Regression potential ==
Low,
this patch just adds an extra check to skip unnecessary rmdir operations.


Filipe Manana (1):
  Btrfs: send, don't send rmdir for same target multiple times

 fs/btrfs/send.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[T][SRU][PATCH 1/1] Btrfs: send, don't send rmdir for same target multiple times

Po-Hsu Lin (Sam)
From: Filipe Manana <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1809868

When doing an incremental send, if we delete a directory that has N > 1
hardlinks for the same file and that file has the highest inode number
inside the directory contents, an incremental send would send N times an
rmdir operation against the directory. This made the btrfs receive command
fail on the second rmdir instruction, as the target directory didn't exist
anymore.

Steps to reproduce the issue:

    $ mkfs.btrfs -f /dev/sdb3
    $ mount /dev/sdb3 /mnt/btrfs
    $ mkdir -p /mnt/btrfs/a/b/c
    $ echo 'ola mundo' > /mnt/btrfs/a/b/c/foo.txt
    $ ln /mnt/btrfs/a/b/c/foo.txt /mnt/btrfs/a/b/c/bar.txt
    $ btrfs subvolume snapshot -r /mnt/btrfs /mnt/btrfs/snap1
    $ btrfs send /mnt/btrfs/snap1 -f /tmp/base.send
    $ rm -f /mnt/btrfs/a/b/c/foo.txt
    $ rm -f /mnt/btrfs/a/b/c/bar.txt
    $ rmdir /mnt/btrfs/a/b/c
    $ btrfs subvolume snapshot -r /mnt/btrfs /mnt/btrfs/snap2
    $ btrfs send -p /mnt/btrfs/snap1 /mnt/btrfs/snap2 -f /tmp/incremental.send

    $ umount /mnt/btrfs
    $ mkfs.btrfs -f /dev/sdb3
    $ mount /dev/sdb3 /mnt/btrfs
    $ btrfs receive /mnt/btrfs -f /tmp/base.send
    $ btrfs receive /mnt/btrfs -f /tmp/incremental.send

The second btrfs receive command failed with:

    ERROR: rmdir o259-6-0 failed. No such file or directory

A test case for xfstests follows.

Signed-off-by: Filipe David Borba Manana <[hidden email]>
Signed-off-by: Josef Bacik <[hidden email]>
(backported from commit 29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5)
Signed-off-by: Po-Hsu Lin <[hidden email]>
---
 fs/btrfs/send.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index d553752..9cc2ef0 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -2686,6 +2686,7 @@ static int process_recorded_refs(struct send_ctx *sctx)
  u64 ow_gen;
  int did_overwrite = 0;
  int is_orphan = 0;
+ u64 last_dir_ino_rm = 0;
 
 verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino);
 
@@ -2941,7 +2942,8 @@ verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino);
  ret = send_utimes(sctx, cur->dir, cur->dir_gen);
  if (ret < 0)
  goto out;
- } else if (ret == inode_state_did_delete) {
+ } else if (ret == inode_state_did_delete &&
+   cur->dir != last_dir_ino_rm) {
  ret = can_rmdir(sctx, cur->dir, sctx->cur_ino);
  if (ret < 0)
  goto out;
@@ -2953,6 +2955,7 @@ verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino);
  ret = send_rmdir(sctx, valid_path);
  if (ret < 0)
  goto out;
+ last_dir_ino_rm = cur->dir;
  }
  }
  }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [T][SRU][PATCH 1/1] Btrfs: send, don't send rmdir for same target multiple times

Kleber Souza
On 1/3/19 12:07 PM, Po-Hsu Lin wrote:

> From: Filipe Manana <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1809868
>
> When doing an incremental send, if we delete a directory that has N > 1
> hardlinks for the same file and that file has the highest inode number
> inside the directory contents, an incremental send would send N times an
> rmdir operation against the directory. This made the btrfs receive command
> fail on the second rmdir instruction, as the target directory didn't exist
> anymore.
>
> Steps to reproduce the issue:
>
>     $ mkfs.btrfs -f /dev/sdb3
>     $ mount /dev/sdb3 /mnt/btrfs
>     $ mkdir -p /mnt/btrfs/a/b/c
>     $ echo 'ola mundo' > /mnt/btrfs/a/b/c/foo.txt
>     $ ln /mnt/btrfs/a/b/c/foo.txt /mnt/btrfs/a/b/c/bar.txt
>     $ btrfs subvolume snapshot -r /mnt/btrfs /mnt/btrfs/snap1
>     $ btrfs send /mnt/btrfs/snap1 -f /tmp/base.send
>     $ rm -f /mnt/btrfs/a/b/c/foo.txt
>     $ rm -f /mnt/btrfs/a/b/c/bar.txt
>     $ rmdir /mnt/btrfs/a/b/c
>     $ btrfs subvolume snapshot -r /mnt/btrfs /mnt/btrfs/snap2
>     $ btrfs send -p /mnt/btrfs/snap1 /mnt/btrfs/snap2 -f /tmp/incremental.send
>
>     $ umount /mnt/btrfs
>     $ mkfs.btrfs -f /dev/sdb3
>     $ mount /dev/sdb3 /mnt/btrfs
>     $ btrfs receive /mnt/btrfs -f /tmp/base.send
>     $ btrfs receive /mnt/btrfs -f /tmp/incremental.send
>
> The second btrfs receive command failed with:
>
>     ERROR: rmdir o259-6-0 failed. No such file or directory
>
> A test case for xfstests follows.
>
> Signed-off-by: Filipe David Borba Manana <[hidden email]>
> Signed-off-by: Josef Bacik <[hidden email]>
> (backported from commit 29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5)

Simple backport, regression potential seems to be low.

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

> Signed-off-by: Po-Hsu Lin <[hidden email]>
> ---
>  fs/btrfs/send.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index d553752..9cc2ef0 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -2686,6 +2686,7 @@ static int process_recorded_refs(struct send_ctx *sctx)
>   u64 ow_gen;
>   int did_overwrite = 0;
>   int is_orphan = 0;
> + u64 last_dir_ino_rm = 0;
>  
>  verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino);
>  
> @@ -2941,7 +2942,8 @@ verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino);
>   ret = send_utimes(sctx, cur->dir, cur->dir_gen);
>   if (ret < 0)
>   goto out;
> - } else if (ret == inode_state_did_delete) {
> + } else if (ret == inode_state_did_delete &&
> +   cur->dir != last_dir_ino_rm) {
>   ret = can_rmdir(sctx, cur->dir, sctx->cur_ino);
>   if (ret < 0)
>   goto out;
> @@ -2953,6 +2955,7 @@ verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino);
>   ret = send_rmdir(sctx, valid_path);
>   if (ret < 0)
>   goto out;
> + last_dir_ino_rm = cur->dir;
>   }
>   }
>   }



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [T][SRU][PATCH 1/1] Btrfs: send, don't send rmdir for same target multiple times

Stefan Bader-2
In reply to this post by Po-Hsu Lin (Sam)
On 03.01.19 12:07, Po-Hsu Lin wrote:

> From: Filipe Manana <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1809868
>
> When doing an incremental send, if we delete a directory that has N > 1
> hardlinks for the same file and that file has the highest inode number
> inside the directory contents, an incremental send would send N times an
> rmdir operation against the directory. This made the btrfs receive command
> fail on the second rmdir instruction, as the target directory didn't exist
> anymore.
>
> Steps to reproduce the issue:
>
>     $ mkfs.btrfs -f /dev/sdb3
>     $ mount /dev/sdb3 /mnt/btrfs
>     $ mkdir -p /mnt/btrfs/a/b/c
>     $ echo 'ola mundo' > /mnt/btrfs/a/b/c/foo.txt
>     $ ln /mnt/btrfs/a/b/c/foo.txt /mnt/btrfs/a/b/c/bar.txt
>     $ btrfs subvolume snapshot -r /mnt/btrfs /mnt/btrfs/snap1
>     $ btrfs send /mnt/btrfs/snap1 -f /tmp/base.send
>     $ rm -f /mnt/btrfs/a/b/c/foo.txt
>     $ rm -f /mnt/btrfs/a/b/c/bar.txt
>     $ rmdir /mnt/btrfs/a/b/c
>     $ btrfs subvolume snapshot -r /mnt/btrfs /mnt/btrfs/snap2
>     $ btrfs send -p /mnt/btrfs/snap1 /mnt/btrfs/snap2 -f /tmp/incremental.send
>
>     $ umount /mnt/btrfs
>     $ mkfs.btrfs -f /dev/sdb3
>     $ mount /dev/sdb3 /mnt/btrfs
>     $ btrfs receive /mnt/btrfs -f /tmp/base.send
>     $ btrfs receive /mnt/btrfs -f /tmp/incremental.send
>
> The second btrfs receive command failed with:
>
>     ERROR: rmdir o259-6-0 failed. No such file or directory
>
> A test case for xfstests follows.
>
> Signed-off-by: Filipe David Borba Manana <[hidden email]>
> Signed-off-by: Josef Bacik <[hidden email]>
> (backported from commit 29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5)
> Signed-off-by: Po-Hsu Lin <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>

> ---
>  fs/btrfs/send.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index d553752..9cc2ef0 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -2686,6 +2686,7 @@ static int process_recorded_refs(struct send_ctx *sctx)
>   u64 ow_gen;
>   int did_overwrite = 0;
>   int is_orphan = 0;
> + u64 last_dir_ino_rm = 0;
>  
>  verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino);
>  
> @@ -2941,7 +2942,8 @@ verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino);
>   ret = send_utimes(sctx, cur->dir, cur->dir_gen);
>   if (ret < 0)
>   goto out;
> - } else if (ret == inode_state_did_delete) {
> + } else if (ret == inode_state_did_delete &&
> +   cur->dir != last_dir_ino_rm) {
>   ret = can_rmdir(sctx, cur->dir, sctx->cur_ino);
>   if (ret < 0)
>   goto out;
> @@ -2953,6 +2955,7 @@ verbose_printk("btrfs: process_recorded_refs %llu\n", sctx->cur_ino);
>   ret = send_rmdir(sctx, valid_path);
>   if (ret < 0)
>   goto out;
> + last_dir_ino_rm = cur->dir;
>   }
>   }
>   }
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED: [T][SRU][PATCH 0/1] Btrfs: send, don't send rmdir for same target multiple times

Khaled Elmously
In reply to this post by Po-Hsu Lin (Sam)
On 2019-01-03 19:07:26 , Po-Hsu Lin wrote:

> BugLink: https://bugs.launchpad.net/bugs/1809868
>
> == Justification ==
> When doing an incremental send on a Btrfs filesystem, if we delete a directory
> that has N > 1 hardlinks for the same file and that file has the highest inode
> number inside the directory contents, an incremental send would send N times
> rmdir operation against the directory. This made the btrfs receive command fail
> on the second rmdir instruction, as the target directory didn't exist anymore.
>
> This issue can be triggered with 29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5
> test in ubuntu_btrfs_kernel_fixes test suite.
>
>
> The test will fail with:
>   "ERROR: rmdir o259-6-0 failed. No such file or directory" with Trusty kernel.
>
> Performing full device TRIM (1.00GiB) ...
> Turning ON incompat feature 'extref': increased hardlink limit per file to 65536
>
> WARNING! - Btrfs v3.12 IS EXPERIMENTAL
> WARNING! - see http://btrfs.wiki.kernel.org before using
>
> fs created label (null) on /dev/loop0
>  nodesize 16384 leafsize 16384 sectorsize 4096 size 1.00GiB
> Btrfs v3.12
> Create a readonly snapshot of '/tmp/mnt-29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5' in '/tmp/mnt-29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5/snap1'
> At subvol /tmp/mnt-29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5/snap1
> Create a readonly snapshot of '/tmp/mnt-29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5' in '/tmp/mnt-29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5/snap2'
> At subvol /tmp/mnt-29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5/snap2
> Performing full device TRIM (1.00GiB) ...
> Turning ON incompat feature 'extref': increased hardlink limit per file to 65536
>
> WARNING! - Btrfs v3.12 IS EXPERIMENTAL
> WARNING! - see http://btrfs.wiki.kernel.org before using
>
> fs created label (null) on /dev/loop0
>  nodesize 16384 leafsize 16384 sectorsize 4096 size 1.00GiB
> Btrfs v3.12
> At subvol snap1
> At snapshot snap2
> ERROR: rmdir o259-6-0 failed. No such file or directory
> incremental receive failed
>
>
> == Fix ==
> 29d6d30f5 (Btrfs: send, don't send rmdir for same target multiple times)
> This patch needs to be backported for a variable difference in the
> process_recorded_refs function.
>
> In the Trusty tree:
> static int process_recorded_refs(struct send_ctx *sctx)
>
> In the patch:
> static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
>
> As the new *pending_move is unrelated here, we can just backport the patch with
> the same logic in this function.
>
> A test kernel could be found here:
> http://people.canonical.com/~phlin/kernel/lp-1809868-btrfs-2nd-rmdir/
> The 29d6d30f5c8aa58b04f40a58442df3bcaae5a1d5 test will pass with this kernel.
>
>
> == Regression potential ==
> Low,
> this patch just adds an extra check to skip unnecessary rmdir operations.
>
>
> Filipe Manana (1):
>   Btrfs: send, don't send rmdir for same target multiple times
>
>  fs/btrfs/send.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> --
> 2.7.4
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team