From: Filipe Manana <fdman...@suse.com>

On openSUSE/SLE systems where balance is triggered periodically in the
background, snapshotting happens when doing package installations and
upgrades, and (by default) the root system is organized with multiple
subvolumes, the following warning was triggered often:

[  630.773059] WARNING: CPU: 1 PID: 2549 at fs/btrfs/relocation.c:1848 
replace_path+0x3f0/0x940 [btrfs]
[  630.773060] Modules linked in: af_packet iscsi_ibft iscsi_boot_sysfs xfs 
libcrc32c acpi_cpufreq tpm_tis ppdev tpm parport_pc parport pcspkr e1000
qemu_fw_cfg joydev i2c_piix4 button btrfs xor raid6_pq sr_mod cdrom ata_generic 
virtio_scsi bochs_drm drm_kms_helper syscopyarea sysfillrect sysimgblt
fb_sys_fops ttm ata_piix virtio_pci virtio_ring virtio serio_raw floppy drm sg
[  630.773070] CPU: 1 PID: 2549 Comm: btrfs Tainted: G        W       
4.7.7-2-btrfs+ #2
[  630.773071] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.1-0-gb3ef39f-prebuilt.qemu-project.org 04/01/2014
[  630.773072]  0000000000000000 ffff8801f704b8c8 ffffffff813afd12 
0000000000000000
[  630.773073]  0000000000000000 ffff8801f704b908 ffffffff81081f8b 
0000073800000000
[  630.773075]  0000000000000001 ffff8801e32eb8c0 0000160000000000 
ffff880000000000
[  630.773076] Call Trace:
[  630.773078]  [<ffffffff813afd12>] dump_stack+0x63/0x81
[  630.773079]  [<ffffffff81081f8b>] __warn+0xcb/0xf0
[  630.773080]  [<ffffffff8108207d>] warn_slowpath_null+0x1d/0x20
[  630.773090]  [<ffffffffc01f3310>] replace_path+0x3f0/0x940 [btrfs]
[  630.773092]  [<ffffffff8114bd1e>] ? ring_buffer_unlock_commit+0x3e/0x2a0
[  630.773102]  [<ffffffffc01f8ac4>] merge_reloc_root+0x2b4/0x600 [btrfs]
[  630.773111]  [<ffffffffc01f8f50>] merge_reloc_roots+0x140/0x250 [btrfs]
[  630.773120]  [<ffffffffc01f9377>] relocate_block_group+0x317/0x680 [btrfs]
[  630.773129]  [<ffffffffc01f98ac>] btrfs_relocate_block_group+0x1cc/0x2d0 
[btrfs]
[  630.773139]  [<ffffffffc01ce406>] btrfs_relocate_chunk.isra.40+0x56/0xf0 
[btrfs]
[  630.773149]  [<ffffffffc01cfaa5>] __btrfs_balance+0x8d5/0xbb0 [btrfs]
[  630.773159]  [<ffffffffc01d0050>] btrfs_balance+0x2d0/0x5e0 [btrfs]
[  630.773168]  [<ffffffffc01dbaa3>] btrfs_ioctl_balance+0x383/0x390 [btrfs]
[  630.773178]  [<ffffffffc01df3ef>] btrfs_ioctl+0x90f/0x1fb0 [btrfs]
[  630.773180]  [<ffffffff8106ed03>] ? pte_alloc_one+0x33/0x40
[  630.773182]  [<ffffffff812333d3>] do_vfs_ioctl+0x93/0x5a0
[  630.773183]  [<ffffffff81069803>] ? __do_page_fault+0x203/0x4e0
[  630.773185]  [<ffffffff81233959>] SyS_ioctl+0x79/0x90
[  630.773186]  [<ffffffff816f2ab6>] entry_SYSCALL_64_fastpath+0x1e/0xa8
[  630.773187] ---[ end trace 2cd6167577bf3be7 ]---

It turned out that this warning does not reflect any problem and just
makes users/system administrators worry about something going wrong.
The warning happens because when we create a relocation root (which is
just a snapshot of a subvolume tree) we set its last_snapshot field (as
well as for the subvolume's tree root) to a value corresponding to the
generation of the current transaction minus 1 (we do this at
relocation.c:create_reloc_root()). This means that when we merge the
relocation tree with the corresponding subvolume tree, at
walk_down_reloc_tree() we can catch pointers (bytenr/generation pairs)
with a generation that matches the generation of the transaction where
we created the relocation root, so those pointers correspond to tree
blocks created either before or after the relocation root was created.
If they were created before the relocation root (and in the same
transaction) we hit the warning, which we can safely remove because it
means the tree blocks are accessible from both trees (the subvolume
tree and the relocation tree).

So fix this by removing the warning and adding a couple assertions that
verify the pointer generations match and that their generation matches
the value of the last_snapshot field from the relocation tree plus 1.

Signed-off-by: Filipe Manana <fdman...@suse.com>
---
 fs/btrfs/relocation.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 0ec8ffa..cdc1a1c 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1848,7 +1848,26 @@ again:
                        new_ptr_gen = 0;
                }
 
-               if (WARN_ON(new_bytenr > 0 && new_bytenr == old_bytenr)) {
+               /*
+                * When we create the reloc root (which is a snapshot of the
+                * subvolume tree) we set its last_snapshot field (as well as
+                * for the subvolume's tree root) to the value of the current
+                * transaction generation minus 1 (at create_reloc_root()).
+                * This means that at walk_down_reloc_tree() we can catch
+                * pointers (bytenr/generation pairs) with a generation
+                * matching the generation of the transaction where we created
+                * the reloc root, so those pointers correspond to tree blocks
+                * that were either created before or after the reloc root was
+                * created. If walk_down_reloc_tree() gave us a path that points
+                * to a tree block that was created (or COWed) before the reloc
+                * root was created and in the same transaction where the reloc
+                * root was created, we have nothing to do and can safely return
+                * (the tree block is already in both trees).
+                */
+               if (new_bytenr > 0 && new_bytenr == old_bytenr) {
+                       ASSERT(new_ptr_gen == old_ptr_gen);
+                       ASSERT(new_ptr_gen ==
+                              btrfs_root_last_snapshot(&src->root_item) + 1);
                        ret = level;
                        break;
                }
-- 
2.7.0.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to