[PATCH] Btrfs: ensure btrfs_prev_leaf doesn't miss 1 item
We might have had an item with the previous key in the tree right before we released our path. And after we released our path, that item might have been pushed to the first slot (0) of the leaf we were holding due to a tree balance. Alternatively, an item with the previous key can exist as the only element of a leaf (big fat item). Therefore account for these 2 cases, so that our callers (like btrfs_previous_item) don't miss an existing item with a key matching the previous key we computed above. Signed-off-by: Filipe David Borba Manana fdman...@gmail.com --- fs/btrfs/ctree.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index d99d965..4eada52 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -5097,7 +5097,17 @@ int btrfs_prev_leaf(struct btrfs_root *root, struct btrfs_path *path) return ret; btrfs_item_key(path-nodes[0], found_key, 0); ret = comp_keys(found_key, key); - if (ret 0) + /* +* We might have had an item with the previous key in the tree right +* before we released our path. And after we released our path, that +* item might have been pushed to the first slot (0) of the leaf we +* were holding due to a tree balance. Alternatively, an item with the +* previous key can exist as the only element of a leaf (big fat item). +* Therefore account for these 2 cases, so that our callers (like +* btrfs_previous_item) don't miss an existing item with a key matching +* the previous key we computed above. +*/ + if (ret = 0) return 0; return 1; } -- 1.9.1 -- 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
Re: [PATCH] mount: add btrfs to mount.8
On 6/7/14, 8:41 AM, Christoph Hellwig wrote: On Fri, Jun 06, 2014 at 10:52:48AM -0500, Eric Sandeen wrote: On 6/6/14, 5:03 AM, Karel Zak wrote: On Fri, Jun 06, 2014 at 11:44:28AM +0200, Karel Zak wrote: I personally have no problem to maintain information about arbitrary FS in mount.8, the problem are updates. Unfortunately, kernel FS developers don't care about the man page at all and it's very often not up to date. Hmm.. another possible way would be to create a script for util-linux that will analyze kernel Documentation/filesystems/fsname.txt and report changes that is necessary to make to mount.8. It should be relative simple with git. I'll try it.. I like that idea. Maybe fsname.txt will need a defined format, though, right? Maybe asciidoc? I've still been meaning (in theory) to produce a mount manpage just for xfs. I'm still willing to do that if the above doesn't pan out. I just need to get to it. I'd be happy to do it for extN as well. Autogenerating man pages from an adhoc format sounds like the wrong approach. I'd much rather have proper man paged for every filesystem. With those we could also drop all that information from the kernel Documentation directory, where users won't looks for them anyway. Well, asciidoc wouldn't be ad-hoc, but still... Eric, if you take care of xfs an extN I'll get started on man pages for the various minor filesystems that don't really have active maintainers. Not sure if we should go for mount.fstype.8 man pages or just improve the fstype.5 pages, but I think the second one is more obvious. Since some mount.fstype binaries actually exist, that would probably lead to some confusion. -Eric -- 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
Re: [PATCH] mount: add btrfs to mount.8
On Sat, Jun 07, 2014 at 06:41:50AM -0700, Christoph Hellwig wrote: On Fri, Jun 06, 2014 at 10:52:48AM -0500, Eric Sandeen wrote: On 6/6/14, 5:03 AM, Karel Zak wrote: On Fri, Jun 06, 2014 at 11:44:28AM +0200, Karel Zak wrote: I personally have no problem to maintain information about arbitrary FS in mount.8, the problem are updates. Unfortunately, kernel FS developers don't care about the man page at all and it's very often not up to date. Hmm.. another possible way would be to create a script for util-linux that will analyze kernel Documentation/filesystems/fsname.txt and report changes that is necessary to make to mount.8. It should be relative simple with git. I'll try it.. I like that idea. Maybe fsname.txt will need a defined format, though, right? Maybe asciidoc? I've still been meaning (in theory) to produce a mount manpage just for xfs. I'm still willing to do that if the above doesn't pan out. I just need to get to it. I'd be happy to do it for extN as well. Autogenerating man pages from an adhoc format sounds like the wrong approach. I'd much rather have proper man paged for every filesystem. With those we could also drop all that information from the kernel Documentation directory, where users won't looks for them anyway. Eric, if you take care of xfs an extN I'll get started on man pages for the various minor filesystems that don't really have active maintainers. Not sure if we should go for mount.fstype.8 man pages or just improve the fstype.5 pages, but I think the second one is more obvious. I think fstype.5 provides opportunity to distribute more information about the filesystem than just mount options only. See for example nfs.5 where is complete overview about the filesystem. Karel -- Karel Zak k...@redhat.com http://karelzak.blogspot.com -- 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
re: Btrfs: split up __extent_writepage to lower stack usage
Hello Chris Mason,
The patch 263524b4ac6b: Btrfs: split up __extent_writepage to lower
stack usage from May 21, 2014, leads to the following static checker
warning:
fs/btrfs/extent_io.c:4071 try_release_extent_state()
warn: use 'mask' here instead of GFP_XXX?
fs/btrfs/extent_io.c
4053 static int try_release_extent_state(struct extent_map_tree *map,
4054 struct extent_io_tree *tree,
4055 struct page *page, gfp_t mask)
4056 {
4057 u64 start = page_offset(page);
4058 u64 end = start + PAGE_CACHE_SIZE - 1;
4059 int ret = 1;
4060
4061 if (test_range_bit(tree, start, end,
4062 EXTENT_IOBITS, 0, NULL))
4063 ret = 0;
4064 else {
4065 if ((mask GFP_NOFS) == GFP_NOFS)
4066 mask = GFP_NOFS;
4067 /*
4068 * at this point we can safely clear everything except
the
4069 * locked bit and the nodatasum bit
4070 */
4071 ret = clear_extent_bit(tree, start, end,
4072 ~(EXTENT_LOCKED | EXTENT_NODATASUM),
4073 0, 0, NULL, GFP_ATOMIC);
^^
It upsets the static checkers to keep mask around when we don't use it
anymore.
4074
4075 /* if clear_extent_bit failed for enomem reasons,
4076 * we can't allow the release to continue.
4077 */
4078 if (ret 0)
4079 ret = 0;
4080 else
4081 ret = 1;
4082 }
4083 return ret;
4084 }
regards,
dan carpenter
--
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
Re: Btrfs: split up __extent_writepage to lower stack usage
On 06/09/2014 10:40 AM, Dan Carpenter wrote:
Hello Chris Mason,
The patch 263524b4ac6b: Btrfs: split up __extent_writepage to lower
stack usage from May 21, 2014, leads to the following static checker
warning:
fs/btrfs/extent_io.c:4071 try_release_extent_state()
warn: use 'mask' here instead of GFP_XXX?
fs/btrfs/extent_io.c
4053 static int try_release_extent_state(struct extent_map_tree *map,
4054 struct extent_io_tree *tree,
4055 struct page *page, gfp_t mask)
4056 {
4057 u64 start = page_offset(page);
4058 u64 end = start + PAGE_CACHE_SIZE - 1;
4059 int ret = 1;
4060
4061 if (test_range_bit(tree, start, end,
4062 EXTENT_IOBITS, 0, NULL))
4063 ret = 0;
4064 else {
4065 if ((mask GFP_NOFS) == GFP_NOFS)
4066 mask = GFP_NOFS;
4067 /*
4068 * at this point we can safely clear everything
except the
4069 * locked bit and the nodatasum bit
4070 */
4071 ret = clear_extent_bit(tree, start, end,
4072 ~(EXTENT_LOCKED | EXTENT_NODATASUM),
4073 0, 0, NULL, GFP_ATOMIC);
^^
It upsets the static checkers to keep mask around when we don't use it
anymore.
Thanks Dan, I'll switch this around to make it more clear.
-chris
--
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
btrfs: hang on boot due to tests
Hi all, It seems that some recent changes to btrfs tests make it hang during boot: [ 49.730033] NMI watchdog: BUG: soft lockup - CPU#34 stuck for 23s! [swapper/0:1] [ 49.730033] Modules linked in: [ 49.730033] hardirqs last enabled at (6389143): restore_args (arch/x86/kernel/entry_64.S:829) [ 49.730033] hardirqs last disabled at (6389144): apic_timer_interrupt (arch/x86/kernel/entry_64.S:1021) [ 49.730033] softirqs last enabled at (6389142): __do_softirq (./arch/x86/include/asm/preempt.h:22 kernel/softirq.c:296) [ 49.730033] softirqs last disabled at (6389139): irq_exit (kernel/softirq.c:346 kernel/softirq.c:387) [ 49.730033] CPU: 34 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc8-next-20140606-sasha-00021-ga9d3a0b-dirty #597 [ 49.730033] task: 88062855 ti: 880036218000 task.ti: 880036218000 [ 49.730033] RIP: __write_lock_failed (arch/x86/lib/rwlock.S:20) [ 49.730033] RSP: :88003621b9e8 EFLAGS: 0297 [ 49.730033] RAX: 0022 RBX: 845658fb RCX: 00059e1d8462 [ 49.730033] RDX: 0003 RSI: 85907bf4 RDI: 8800365c00bc [ 49.730033] RBP: 88003621b9e8 R08: 1b48 R09: 819618b8 [ 49.730033] R10: 0001 R11: R12: 88003621b958 [ 49.730033] R13: 0001 R14: 880036218000 R15: 88062855 [ 49.730033] FS: () GS:8800a6e0() knlGS: [ 49.730033] CS: 0010 DS: ES: CR0: 8005003b [ 49.730033] CR2: CR3: 0602e000 CR4: 06a0 [ 49.730033] Stack: [ 49.730033] 88003621ba08 811d18de 88003621ba68 8800365c00b8 [ 49.730033] 88003621ba38 84563ac0 81961d7c 81b33a05 [ 49.730033] 8800365c 8800365c0100 88003621baa8 81961d7c [ 49.730033] Call Trace: [ 49.730033] do_raw_write_lock (kernel/locking/spinlock_debug.c:236 kernel/locking/spinlock_debug.c:280) [ 49.730033] _raw_write_lock (include/linux/rwlock_api_smp.h:211 kernel/locking/spinlock.c:295) [ 49.730033] ? btrfs_tree_lock (./arch/x86/include/asm/atomic.h:27 fs/btrfs/locking.c:219) [ 49.730033] ? delay_tsc (./arch/x86/include/asm/preempt.h:98 arch/x86/lib/delay.c:86) [ 49.730033] btrfs_tree_lock (./arch/x86/include/asm/atomic.h:27 fs/btrfs/locking.c:219) [ 49.730033] ? __const_udelay (arch/x86/lib/delay.c:126) [ 49.730033] ? __rcu_read_unlock (kernel/rcu/update.c:97) [ 49.730033] btrfs_lock_root_node (fs/btrfs/ctree.c:193) [ 49.730033] btrfs_search_slot (fs/btrfs/ctree.c:2768) [ 49.730033] btrfs_insert_empty_items (fs/btrfs/ctree.c:4836) [ 49.730033] ? dlm_init (fs/btrfs/super.c:1914) [ 49.730033] insert_normal_tree_ref.constprop.4 (fs/btrfs/tests/qgroup-tests.c:60) [ 49.730033] ? dlm_init (fs/btrfs/super.c:1914) [ 49.730033] test_no_shared_qgroup (fs/btrfs/tests/qgroup-tests.c:249) [ 49.730033] btrfs_test_qgroups (fs/btrfs/tests/qgroup-tests.c:462) [ 49.730033] init_btrfs_fs (fs/btrfs/super.c:1909 fs/btrfs/super.c:1969) [ 49.730033] ? dlm_init (fs/btrfs/super.c:1914) [ 49.730033] do_one_initcall (init/main.c:791) [ 49.730033] ? parse_args (kernel/params.c:120 kernel/params.c:205) [ 49.730033] kernel_init_freeable (init/main.c:856 init/main.c:865 init/main.c:884 init/main.c:1005) [ 49.730033] ? loglevel (init/main.c:241) [ 49.730033] ? rest_init (init/main.c:932) [ 49.730033] kernel_init (init/main.c:937) [ 49.730033] ret_from_fork (arch/x86/kernel/entry_64.S:349) [ 49.730033] ? rest_init (init/main.c:932) [ 49.730033] Code: 1f 00 48 89 01 31 c0 0f 1f 00 c3 b8 f2 ff ff ff 0f 1f 00 c3 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 48 89 e5 f0 ff 07 f3 90 83 3f 01 75 f9 f0 ff 0f 75 f1 5d c3 66 66 2e 0f 1f 84 00 00 00 All code 0: 1f (bad) 1: 00 48 89add%cl,-0x77(%rax) 4: 01 31 add%esi,(%rcx) 6: c0 0f 1frorb $0x1f,(%rdi) 9: 00 c3 add%al,%bl b: b8 f2 ff ff ff mov$0xfff2,%eax 10: 0f 1f 00nopl (%rax) 13: c3 retq 14: 90 nop 15: 90 nop 16: 90 nop 17: 90 nop 18: 90 nop 19: 90 nop 1a: 90 nop 1b: 90 nop 1c: 90 nop 1d: 90 nop 1e: 90 nop 1f: 90 nop 20: 90 nop 21: 90 nop 22: 55 push %rbp 23: 48 89 e5mov%rsp,%rbp 26: f0 ff 07lock incl (%rdi) 29: f3 90 pause 2b:* 83 3f 01cmpl $0x1,(%rdi) -- trapping
Re: btrfs: hang on boot due to tests
On 06/09/2014 11:16 AM, Sasha Levin wrote: Hi all, It seems that some recent changes to btrfs tests make it hang during boot: [ 49.730033] NMI watchdog: BUG: soft lockup - CPU#34 stuck for 23s! [swapper/0:1] [ 49.730033] Modules linked in: [ 49.730033] hardirqs last enabled at (6389143): restore_args (arch/x86/kernel/entry_64.S:829) [ 49.730033] hardirqs last disabled at (6389144): apic_timer_interrupt (arch/x86/kernel/entry_64.S:1021) [ 49.730033] softirqs last enabled at (6389142): __do_softirq (./arch/x86/include/asm/preempt.h:22 kernel/softirq.c:296) [ 49.730033] softirqs last disabled at (6389139): irq_exit (kernel/softirq.c:346 kernel/softirq.c:387) [ 49.730033] CPU: 34 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc8-next-20140606-sasha-00021-ga9d3a0b-dirty #597 This is 3.15-rc8 + linux-next? I'll try to reproduce here, but the tests were working for me. -chris -- 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
Re: btrfs: hang on boot due to tests
On 06/09/2014 11:59 AM, Chris Mason wrote: On 06/09/2014 11:16 AM, Sasha Levin wrote: Hi all, It seems that some recent changes to btrfs tests make it hang during boot: [ 49.730033] NMI watchdog: BUG: soft lockup - CPU#34 stuck for 23s! [swapper/0:1] [ 49.730033] Modules linked in: [ 49.730033] hardirqs last enabled at (6389143): restore_args (arch/x86/kernel/entry_64.S:829) [ 49.730033] hardirqs last disabled at (6389144): apic_timer_interrupt (arch/x86/kernel/entry_64.S:1021) [ 49.730033] softirqs last enabled at (6389142): __do_softirq (./arch/x86/include/asm/preempt.h:22 kernel/softirq.c:296) [ 49.730033] softirqs last disabled at (6389139): irq_exit (kernel/softirq.c:346 kernel/softirq.c:387) [ 49.730033] CPU: 34 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc8-next-20140606-sasha-00021-ga9d3a0b-dirty #597 This is 3.15-rc8 + linux-next? I'll try to reproduce here, but the tests were working for me. Yes, it's the latest -next tree available. Also note that it doesn't happen every time, so might be some sort of a race? Thanks, Sasha -- 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
btrfs balance crash BUG ON fs/btrfs/relocation.c:1062 or RIP build_backref_tree+0x9fc/0xcc4
I did a balance on a system that had 3.11 (yes, I know, it's old). It hung.
So, I rebooted with 3.13, and it failed in fs/btrfs/relocation
Problem #1: I cannot stop the relocation. It starts on its own as soon as I
mount the FS, and I can't stop it.
Is there a bug to fix that?
Problem #2: I rebooted with 3.15rc5, and now it's worse.
[ 1569.598026] kernel BUG at fs/btrfs/relocation.c:1064!
then leads to
[ 1569.613240] RIP [81267ef0] build_backref_tree+0x9fc/0xcc4
[ 1569.613491] RSP 880fde599ad8
[ 1569.614119] ---[ end trace da0f24875bbde960 ]---
[ 1569.614398] Kernel panic - not syncing: Fatal exception
(full trace below)
I'm sure that filesystem is damaged in some way, but the kernel of course
should not crash.
3.15 dies here:
struct backref_node *build_backref_tree(struct reloc_control *rc,
(...)
if (!RB_EMPTY_NODE(upper-rb_node)) {
if (upper-lowest) {
list_del_init(upper-lower);
upper-lowest = 0;
}
list_add_tail(edge-list[UPPER], upper-lower);
continue;
}
BUG_ON(!upper-checked); here
So I'm sure I hit a bug and my FS has issues, but can't the kernel do something
better
like abort the rebalance instead of crashing?
In the meantime, does anyone want anything off that filesystem before I wipe it?
= Crash on 3.13:
btrfs: found 4930 extents
btrfs: relocating block group 82699091968 flags 1
btrfs: found 3719 extents
[ cut here ]
kernel BUG at /build/buildd/linux-lts-trusty-3.13.0/fs/btrfs/relocation.c:1062!
invalid opcode: [#1] SMP
Modules linked in: rfcomm parport_pc ppdev bnep binfmt_misc rpcsec_gss_krb5
nfsd nfs_acl auth_rpcgss nfs fscache lockd sunrpc snd_hda_codec_hdmi
nvidia(POF) snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_usb_audio
snd_pcm snd_hwdep snd_usbmidi_lib snd_seq_midi snd_seq_midi_event snd_seq btusb
bluetooth uvcvideo videobuf2_core videodev videobuf2_vmalloc videobuf2_memops
snd_rawmidi snd_timer snd_seq_device drm snd psmouse gpio_ich sb_edac hp_wmi
serio_raw edac_core mei_me mei mac_hid soundcore sparse_keymap snd_page_alloc
lpc_ich wmi tpm_infineon lp parport btrfs raid6_pq xor libcrc32c hid_generic
usbhid hid usb_storage firewire_ohci isci e1000e firewire_core libsas crc_itu_t
ptp ahci libahci pps_core scsi_transport_sas
CPU: 4 PID: 1710 Comm: btrfs-balance Tainted: PF O 3.13.0-29-generic
#53~precise1-Ubuntu
Hardware name: Hewlett-Packard HP Z620 Workstation/158A, BIOS J61 v01.17
11/05/2012
task: 881000bec7d0 ti: 8810018a2000 task.ti: 8810018a2000
RIP: 0010:[a01c04a8] [a01c04a8]
build_backref_tree+0x1228/0x1290 [btrfs]
RSP: 0018:8810018a3ab8 EFLAGS: 00010246
RAX: RBX: 880801a8fa20 RCX: 880801d2cf50
RDX: 880801d2c390 RSI: 880801d2c640 RDI: 8807ecad9c80
RBP: 8810018a3bb8 R08: 8807ecad9c80 R09: 0001
R10: 0001 R11: R12: 880801d2c650
R13: 8807ecad9900 R14: R15: 88003582a800
FS: () GS:88080fc8() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 7f10ee50a370 CR3: 01c0d000 CR4: 000407e0
Stack:
8807ecad9780 01ffa01bded0 880801d2c620 88003582a920
8807ecad9c80 8807ff72e800 8807ecad9240 8807ecad9200
880801a8fa20 880801a8fab0 88003582a924 88003582a820
Call Trace:
[a01c064b] relocate_tree_blocks+0x13b/0x1e0 [btrfs]
[a01c12b9] relocate_block_group+0x199/0x550 [btrfs]
[a01c182a] btrfs_relocate_block_group+0x1ba/0x300 [btrfs]
[a01999f6] btrfs_relocate_chunk.isra.62+0x56/0x3f0 [btrfs]
[a01556b3] ? block_group_cache_tree_search+0xb3/0xf0 [btrfs]
[a018dfe6] ? release_extent_buffer+0x36/0xe0 [btrfs]
[a019c60c] __btrfs_balance+0x32c/0x420 [btrfs]
[a019ca38] btrfs_balance+0x338/0x5d0 [btrfs]
[a019cd54] balance_kthread+0x84/0x90 [btrfs]
[a019ccd0] ? btrfs_balance+0x5d0/0x5d0 [btrfs]
[8108f9a9] kthread+0xc9/0xe0
[8108f8e0] ? flush_kthread_worker+0xb0/0xb0
[817665fc] ret_from_fork+0x7c/0xb0
[8108f8e0] ? flush_kthread_worker+0xb0/0xb0
Code: ff ff 48 89 df e8 79 cb f8 ff 48 8b bd 48 ff ff ff e8 6d cb f8 ff 48 83
bd 58 ff ff ff 00 0f 84 62 ef ff ff e9 87 fd ff ff 0f 0b 0f 0b 48 8b 85 20 ff
ff ff 49 8d 7f 20 48 8b 70 18 48 89 c2 e8
RIP [a01c04a8] build_backref_tree+0x1228/0x1290 [btrfs]
RSP 8810018a3ab8
---[ end trace 1b7853634ea4bd18 ]---
= Crash on 3.15:
[ 1565.477358] BTRFS info (device sdb1): disk space caching is enabled
[ 1565.567580] BTRFS: detected SSD devices, enabling SSD mode
[ 1565.739226] BTRFS info (device sdb1): continuing balance
[ 1565.790228] BTRFS info (device sdb1): relocating block group
Re: btrfs balance crash BUG ON fs/btrfs/relocation.c:1062 or RIP build_backref_tree+0x9fc/0xcc4
On Mon, 9 Jun 2014 16:40:07 Marc MERLIN wrote: I did a balance on a system that had 3.11 (yes, I know, it's old). It hung. So, I rebooted with 3.13, and it failed in fs/btrfs/relocation Problem #1: I cannot stop the relocation. It starts on its own as soon as I mount the FS, and I can't stop it. Is there a bug to fix that? The skip_balance mount option prevents a balance from being continued. On all my systems I have rootflags=skip_balance in the GRUB configuration because so far I have never had a system reboot during a balance for any reason other than a system crash caused by the balance. I think it would be good to have a program comparable to tune2fs that allows us to set the state of such things. It would be a lot easier than modifying boot loader configuration on lots of systems. Also rootflags=skip_balance will abort the boot if you run kernel 3.2.0 and that caused me some annoyance when I applied it to all systems including the one with a stock Debian/Wheezy configuration. -- My Main Blog http://etbe.coker.com.au/ My Documents Bloghttp://doc.coker.com.au/ -- 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
Re: [PATCH V3] Btrfs: device_list_add() should not update list when mounted
Original Message
Subject: [PATCH V3] Btrfs: device_list_add() should not update list when
mounted
From: Anand Jain anand.j...@oracle.com
To: linux-btrfs@vger.kernel.org
Date: 2014年06月06日 11:26
(looks like there was some sendmail problem I don't see this in the btrfs list,
sending again. sorry for multiple copies if any).
device_list_add() is called when user runs btrfs dev scan, which would add
any btrfs device into the btrfs_fs_devices list.
Now think of a mounted btrfs. And a new device which contains the a SB
from the mounted btrfs devices.
In this situation when user runs btrfs dev scan, the current code would
just replace existing device with the new device.
Which is to note that old device is neither closed nor gracefully
removed from the btrfs.
The FS is still operational with the old bdev however the device name
is the btrfs_device is new which is provided by the btrfs dev scan.
reproducer:
devmgt[1] detach /dev/sdc
replace the missing disk /dev/sdc
btrfs rep start -f 1 /dev/sde /btrfs
Label: none uuid: 5dc0aaf4-4683-4050-b2d6-5ebe5f5cd120
Total devices 2 FS bytes used 32.00KiB
devid1 size 958.94MiB used 115.88MiB path /dev/sde
devid2 size 958.94MiB used 103.88MiB path /dev/sdd
make /dev/sdc to reappear
devmgt attach host2
btrfs dev scan
btrfs fi show -m
Label: none uuid: 5dc0aaf4-4683-4050-b2d6-5ebe5f5cd120^M
Total devices 2 FS bytes used 32.00KiB^M
devid1 size 958.94MiB used 115.88MiB path /dev/sdc - Wrong.
devid2 size 958.94MiB used 103.88MiB path /dev/sdd
since /dev/sdc has been replaced with /dev/sde, the /dev/sdc shouldn't be
part of the btrfs-fsid when it reappears. If user want it to be part of it
then sys admin should be using btrfs device add instead.
[1] github.com/anajain/devmgt.git
Signed-off-by: Anand Jain anand.j...@oracle.com
---
v2-v3: simpler commit and comment message
v1-v2: remove '---' in commit messages which conflict with git am
fs/btrfs/volumes.c | 33 -
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index bb2cd66..605d9ef 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -472,9 +472,12 @@ static noinline int device_list_add(const char *path,
device = __find_device(fs_devices-devices, devid,
disk_super-dev_item.uuid);
}
+
if (!device) {
- if (fs_devices-opened)
+ if (fs_devices-opened) {
+ printk(KERN_INFO BTRFS: device list add failed, FS is
open);
return -EBUSY;
+ }
device = btrfs_alloc_device(NULL, devid,
disk_super-dev_item.uuid);
@@ -497,6 +500,34 @@ static noinline int device_list_add(const char *path,
device-fs_devices = fs_devices;
} else if (!device-name || strcmp(device-name-str, path)) {
+ /*
+* When FS is already mounted.
+* 1. If you are here and if the device-name is NULL that means
+*this device was missing at time of FS mount.
+* 2. If you are here and if the device-name is different from
'path'
+*that means either
+* a. The same device disappeared and reappeared with
different
+* name. or
+* b. The missing-disk-which-was-replaced, has reappeared
now.
+*
+* We must allow 1 and 2a above. But 2b would be a spurious and
+* unintentional.
+* Further in case of 1 and 2a above, the disk at 'path' would
have
+* missed some transaction when it was away and in case of 2a
+* the stale bdev has to be updated as well.
+* 2b must not be allowed at all time.
+*/
+
+ /*
+* As of now don't allow update to btrfs_fs_device through the
+* btrfs dev scan cli, after FS has been mounted.
+*/
+
+ if (fs_devices-opened) {
+ printk(KERN_INFO BTRFS: device list update failed, FS is
open);
+ return -EBUSY;
+ }
+
The 'if(fs_devices-opened)' block is in both branch of the
'if(!device)' judgement,
it would be better to extract the code block out of the 'if(!device)' block.
Thanks,
Qu
name = rcu_string_strdup(path, GFP_NOFS);
if (!name)
return -ENOMEM;
--
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
Re: [PATCH] btrfs: add regression test for remount with thread_pool resized
On Fri, May 30, 2014 at 04:52:51PM +0800, Xing Gu wrote:
Regression test for resizing 'thread_pool' when remount the fs.
Ping for btrfs test reviewers - is this test useful at all?
Signed-off-by: Xing Gu gux.f...@cn.fujitsu.com
---
tests/btrfs/055 | 55
+
tests/btrfs/055.out | 1 +
tests/btrfs/group | 1 +
3 files changed, 57 insertions(+)
create mode 100755 tests/btrfs/055
create mode 100644 tests/btrfs/055.out
diff --git a/tests/btrfs/055 b/tests/btrfs/055
new file mode 100755
index 000..0a0dd34
--- /dev/null
+++ b/tests/btrfs/055
@@ -0,0 +1,55 @@
+#!/bin/bash
+# FS QA Test No. btrfs/055
+#
+# Regression test for resizing 'thread_pool' when remount the fs.
+#
+#---
+# Copyright (c) 2014 Fujitsu. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+#
+#---
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo QA output created by $seq
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+rm -f $tmp.*
+}
+
+trap _cleanup ; exit \$status 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+
+_scratch_mkfs /dev/null 21
+
+_scratch_mount -o thread_pool=6
+
+_scratch_mount -o remount,thread_pool=10
+
+status=0 ; exit
diff --git a/tests/btrfs/055.out b/tests/btrfs/055.out
new file mode 100644
index 000..2fdd8f4
--- /dev/null
+++ b/tests/btrfs/055.out
@@ -0,0 +1 @@
+QA output created by 055
diff --git a/tests/btrfs/group b/tests/btrfs/group
index b668485..2c10c5b 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -57,3 +57,4 @@
052 auto quick
053 auto quick
054 auto quick
+055 auto quick
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe fstests in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Dave Chinner
da...@fromorbit.com
--
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
Re: [PATCH v5] xfstests: add test for btrfs cloning with file holes
On Tue, Jun 03, 2014 at 03:37:41PM +0100, Filipe David Borba Manana wrote:
Regression test for the btrfs ioctl clone operation when the source range
contains hole(s) and the FS has the NO_HOLES feature enabled (file holes
don't need file extent items in the btree to represent them).
This issue is fixed by the following linux kernel btrfs patch:
Btrfs: fix clone to deal with holes when NO_HOLES feature is enabled
Signed-off-by: Filipe David Borba Manana fdman...@gmail.com
---
V2: Increased test coverage by testing the cases where a hole overlaps
the start and end of the cloning range.
V3: Test the case where the cloning range includes an hole at the end
of the source file and might increase the size of the target file.
V4: Added test for the case where the clone range covers only a hole at
the beginning of the source file.
Made the test be skipped if the available version of mkfs.btrfs
doesn't support the no-holes feature. And when testing the case
where the no-holes feature isn't enabled, explicitly ask mkfs.btrfs
to disable no-holes (future versions of mkfs.btrfs might enable
this feature by default).
V5: Detect if kernel supports NO_HOLES feature too. Added some messages
(echoes) before each od call to make it easier to match output
with each specific test.
common/rc | 25
tests/btrfs/055 | 173 ++
tests/btrfs/055.out | 347
tests/btrfs/group | 1 +
4 files changed, 546 insertions(+)
create mode 100755 tests/btrfs/055
create mode 100644 tests/btrfs/055.out
diff --git a/common/rc b/common/rc
index f27ee53..e2136d0 100644
--- a/common/rc
+++ b/common/rc
@@ -2177,6 +2177,31 @@ _require_btrfs_send_stream_version()
fi
}
+_require_btrfs_mkfs_feature()
+{
+ if [ -z $1 ]; then
+ echo Missing feature name argument for
_require_btrfs_mkfs_feature
+ exit 1
+ fi
+ feat=$1
+ $MKFS_BTRFS_PROG -O list-all 21 | \
+ grep '^[ \t]*'$feat'\b' /dev/null 21
+ [ $? -eq 0 ] || \
+ _notrun Feature $feat not supported in the available version
of mkfs.btrfs
+}
+
+_require_btrfs_fs_feature()
+{
+ if [ -z $1 ]; then
+ echo Missing feature name argument for
_require_btrfs_fs_feature
+ exit 1
+ fi
+ feat=$1
+ modprobe btrfs /dev/null 21
+ [ -e /sys/fs/btrfs/features/$feat ] || \
+ _notrun Feature $feat not supported by the available btrfs
version
+}
+
init_rc()
{
if [ $iam == new ]
diff --git a/tests/btrfs/055 b/tests/btrfs/055
new file mode 100755
index 000..be38d09
--- /dev/null
+++ b/tests/btrfs/055
@@ -0,0 +1,173 @@
+#! /bin/bash
+# FS QA Test No. btrfs/055
+#
+# Regression test for the btrfs ioctl clone operation when the source range
+# contains hole(s) and the FS has the NO_HOLES feature enabled (file holes
+# don't need file extent items in the btree to represent them).
+#
+# This issue is fixed by the following linux kernel btrfs patch:
+#
+#Btrfs: fix clone to deal with holes when NO_HOLES feature is enabled
+#
+#---
+# Copyright (c) 2014 Filipe Manana. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+#---
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo QA output created by $seq
+
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap _cleanup; exit \$status 0 1 2 3 15
+
+_cleanup()
+{
+rm -fr $tmp
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+_require_btrfs_cloner
+_require_btrfs_fs_feature no_holes
+_require_btrfs_mkfs_feature no-holes
Nice kernel/userspace consistency in naming there ;)
+_need_to_be_root
+
+rm -f $seqres.full
+
+test_btrfs_clone_with_holes()
+{
+ _scratch_mkfs $1 /dev/null 21
+ _scratch_mount
+
+ # Create a file with 4 extents and 1 hole, all with a size of 8Kb each.
+ $XFS_IO_PROG -f -c pwrite -S 0x01 -b 8192 0 8192 $SCRATCH_MNT/foo \
+ |
Re: [PATCH V3] Btrfs: device_list_add() should not update list when mounted
On 10/06/14 09:25, Qu Wenruo wrote:
Original Message
Subject: [PATCH V3] Btrfs: device_list_add() should not update list when
mounted
From: Anand Jain anand.j...@oracle.com
To: linux-btrfs@vger.kernel.org
Date: 2014年06月06日 11:26
(looks like there was some sendmail problem I don't see this in the
btrfs list,
sending again. sorry for multiple copies if any).
device_list_add() is called when user runs btrfs dev scan, which would
add
any btrfs device into the btrfs_fs_devices list.
Now think of a mounted btrfs. And a new device which contains the a SB
from the mounted btrfs devices.
In this situation when user runs btrfs dev scan, the current code would
just replace existing device with the new device.
Which is to note that old device is neither closed nor gracefully
removed from the btrfs.
The FS is still operational with the old bdev however the device name
is the btrfs_device is new which is provided by the btrfs dev scan.
reproducer:
devmgt[1] detach /dev/sdc
replace the missing disk /dev/sdc
btrfs rep start -f 1 /dev/sde /btrfs
Label: none uuid: 5dc0aaf4-4683-4050-b2d6-5ebe5f5cd120
Total devices 2 FS bytes used 32.00KiB
devid1 size 958.94MiB used 115.88MiB path /dev/sde
devid2 size 958.94MiB used 103.88MiB path /dev/sdd
make /dev/sdc to reappear
devmgt attach host2
btrfs dev scan
btrfs fi show -m
Label: none uuid: 5dc0aaf4-4683-4050-b2d6-5ebe5f5cd120^M
Total devices 2 FS bytes used 32.00KiB^M
devid1 size 958.94MiB used 115.88MiB path /dev/sdc - Wrong.
devid2 size 958.94MiB used 103.88MiB path /dev/sdd
since /dev/sdc has been replaced with /dev/sde, the /dev/sdc shouldn't be
part of the btrfs-fsid when it reappears. If user want it to be part
of it
then sys admin should be using btrfs device add instead.
[1] github.com/anajain/devmgt.git
Signed-off-by: Anand Jain anand.j...@oracle.com
---
v2-v3: simpler commit and comment message
v1-v2: remove '---' in commit messages which conflict with git am
fs/btrfs/volumes.c | 33 -
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index bb2cd66..605d9ef 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -472,9 +472,12 @@ static noinline int device_list_add(const char
*path,
device = __find_device(fs_devices-devices, devid,
disk_super-dev_item.uuid);
}
+
if (!device) {
-if (fs_devices-opened)
+if (fs_devices-opened) {
+printk(KERN_INFO BTRFS: device list add failed, FS is
open);
return -EBUSY;
+}
device = btrfs_alloc_device(NULL, devid,
disk_super-dev_item.uuid);
@@ -497,6 +500,34 @@ static noinline int device_list_add(const char
*path,
device-fs_devices = fs_devices;
} else if (!device-name || strcmp(device-name-str, path)) {
+/*
+ * When FS is already mounted.
+ * 1. If you are here and if the device-name is NULL that means
+ *this device was missing at time of FS mount.
+ * 2. If you are here and if the device-name is different
from 'path'
+ *that means either
+ * a. The same device disappeared and reappeared with
different
+ * name. or
+ * b. The missing-disk-which-was-replaced, has
reappeared now.
+ *
+ * We must allow 1 and 2a above. But 2b would be a spurious and
+ * unintentional.
+ * Further in case of 1 and 2a above, the disk at 'path'
would have
+ * missed some transaction when it was away and in case of 2a
+ * the stale bdev has to be updated as well.
+ * 2b must not be allowed at all time.
+ */
+
+/*
+ * As of now don't allow update to btrfs_fs_device through the
+ * btrfs dev scan cli, after FS has been mounted.
+ */
+
+if (fs_devices-opened) {
+printk(KERN_INFO BTRFS: device list update failed, FS is
open);
+return -EBUSY;
+}
+
The 'if(fs_devices-opened)' block is in both branch of the
'if(!device)' judgement,
it would be better to extract the code block out of the 'if(!device)'
block.
thanks for the comments Qu.
we have else if. that is when device is found and path is NOT new
(matches with the old) then we shall return success.
Anand
Thanks,
Qu
name = rcu_string_strdup(path, GFP_NOFS);
if (!name)
return -ENOMEM;
--
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
--
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
Re: [PATCH v2] xfstests: add test for btrfs clone + fsync durability
On Mon, Jun 09, 2014 at 03:48:52AM +0100, Filipe David Borba Manana wrote:
Regression test for btrfs ioctl clone operation + fsync + log
recovery. The issue was that doing an fsync after cloning into
a file didn't gave any persistence guarantees as it should.
What happened was that the in memory metadata (extent maps)
weren't updated, which made the fsync code not able to detect
that file data has been changed and must be persisted to the
log.
This issue is fixed by the following linux kernel btrfs patch:
Btrfs: make fsync work after cloning into a file
Signed-off-by: Filipe David Borba Manana fdman...@gmail.com
---
V2: Test small files too, consisting of a single inline extent, as
it triggers different code paths.
tests/btrfs/056 | 150
tests/btrfs/056.out | 129
tests/btrfs/group | 1 +
3 files changed, 280 insertions(+)
create mode 100755 tests/btrfs/056
create mode 100644 tests/btrfs/056.out
diff --git a/tests/btrfs/056 b/tests/btrfs/056
new file mode 100755
index 000..e066442
--- /dev/null
+++ b/tests/btrfs/056
@@ -0,0 +1,150 @@
+#! /bin/bash
+# FS QA Test No. btrfs/056
+#
+# Regression test for btrfs ioctl clone operation + fsync + log recovery.
+# The issue was that doing an fsync after cloning into a file didn't gave any
+# persistence guarantees as it should. What happened was that the in memory
+# metadata (extent maps) weren't updated, which made the fsync code not able
+# to detect that file data has been changed.
+#
+# This issue is fixed by the following linux kernel btrfs patch:
+#
+#Btrfs: make fsync work after cloning into a file
+#
+#---
+# Copyright (c) 2014 Filipe Manana. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+#---
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo QA output created by $seq
+
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap _cleanup; exit \$status 0 1 2 3 15
+
+_cleanup()
+{
+ _cleanup_flakey
+ rm -fr $tmp
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmflakey
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+_require_btrfs_cloner
+_require_btrfs_fs_feature no_holes
+_require_btrfs_mkfs_feature no-holes
+_require_dm_flakey
+_need_to_be_root
+
+rm -f $seqres.full
+
+test_btrfs_clone_fsync_log_recover()
+{
+ _scratch_mkfs $1 /dev/null 21
+ _init_flakey
+ SAVE_MOUNT_OPTIONS=$MOUNT_OPTIONS
+ MOUNT_OPTIONS=$MOUNT_OPTIONS $2
+ _mount_flakey
+
+ # Create a file with 4 extents and 1 hole, all with a size of 8Kb each.
+ # The hole is in the range [16384, 24576[.
+ $XFS_IO_PROG -f -c pwrite -S 0x01 -b 8192 0 8192 \
+ -c fsync \
+ -c pwrite -S 0x02 -b 8192 8192 8192 \
+ -c fsync \
+ -c pwrite -S 0x04 -b 8192 24576 8192 \
+ -c fsync \
+ -c pwrite -S 0x05 -b 8192 32768 8192 \
+ -c fsync \
+ $SCRATCH_MNT/foo | _filter_xfs_io
+
+ # Clone destination file, 1 extent of 96kb.
+ $XFS_IO_PROG -f -c pwrite -S 0xff -b 98304 0 98304 -c fsync \
+ $SCRATCH_MNT/bar | _filter_xfs_io
I've seen this pattern before ;)
Use the -s option to xfs_io rather than repeated pwrite/fsync
pairs.
Otherwise looks fine.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
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
Re: btrfs balance crash BUG ON fs/btrfs/relocation.c:1062 or RIP build_backref_tree+0x9fc/0xcc4
On Tue, Jun 10, 2014 at 10:32:33AM +1000, Russell Coker wrote: On Mon, 9 Jun 2014 16:40:07 Marc MERLIN wrote: I did a balance on a system that had 3.11 (yes, I know, it's old). It hung. So, I rebooted with 3.13, and it failed in fs/btrfs/relocation Problem #1: I cannot stop the relocation. It starts on its own as soon as I mount the FS, and I can't stop it. Is there a bug to fix that? The skip_balance mount option prevents a balance from being continued. On all my systems I have rootflags=skip_balance in the GRUB configuration because so far I have never had a system reboot during a balance for any reason other than a system crash caused by the balance. Aaah, good idea, thank you. I think it would be good to have a program comparable to tune2fs that allows us to set the state of such things. It would be a lot easier than modifying boot loader configuration on lots of systems. Totally agreed. I'll still wait a bit to see if someone wants me to provide more info from the FS, but having balance crash the system and hard lock it, is indeed not good. Thanks, Marc -- A mouse is a device used to point at the xterm you want to type in - A.S.R. Microsoft is to operating systems what McDonalds is to gourmet cooking Home page: http://marc.merlins.org/ | PGP 1024R/763BE901 -- 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
