According to https://btrfs.wiki.kernel.org/index.php/Debugging_Btrfs_with_GDB UML did work once. Now it corrupts data and triggers BUG_ON once you start to use it. I tried both 2.6.38 and 2.6.39-rc2 (x86_64) I need some help to track it down. doing ''touch `seq 1 11`; rm 11'' kills the kernel: #run> mount -t btrfs /dev/ubda /mnt/btr/ [ 2.220000] device fsid 754599771c9b69eb-66689f77c1542bb9 devid 1 transid 7 /dev/ubda #status: 0 #run> cd /mnt/btr/ #status: 0 touching files 1 .. 11 #run> touch 1 2 3 4 5 6 7 8 9 10 11 #status: 0 #run> ls [ 2.220000] btrfS: invalid dir item name len: 12594 [ 2.220000] btrfS: invalid dir item name len: 0 [ 2.220000] btrfS: invalid dir item name len: 0 11 #status: 0 Fasten your belts: removing file 11 #run> rm 11 [ 2.220000] btrfs failed to delete reference to 11, inode 267 parent 256 [ 2.220000] Kernel panic - not syncing: Kernel mode signal 4 [ 2.220000] Call Trace: [ 2.220000] 6024b918: [<601b2567>] panic+0xea/0x1dc [ 2.220000] 6024b9c8: [<601b491e>] _raw_spin_unlock_irqrestore+0x18/0x1c [ 2.220000] 6024b9e8: [<60017d00>] free_irqs+0x74/0xde [ 2.220000] 6024ba18: [<60015faa>] relay_signal+0x38/0x79 [ 2.220000] 6024ba28: [<60013c8e>] sigio_handler+0x5a/0x60 [ 2.220000] 6024ba48: [<6001f224>] sig_handler_common+0x84/0x98 [ 2.220000] 6024ba68: [<6001f2d1>] real_alarm_handler+0x3c/0x3e [ 2.220000] 6024baf0: [<600579f7>] get_page_from_freelist+0x129/0x478 [ 2.220000] 6024bb78: [<6001f36a>] sig_handler+0x30/0x3b [ 2.220000] 6024bb98: [<6001f59c>] handle_signal+0x6d/0xa3 [ 2.220000] 6024bbe8: [<600203b0>] hard_handler+0x10/0x14 [ 2.220000] 6024bca8: [<600e04c3>] btrfs_unlink+0x77/0xef = I''ve cooked whole root into small archive (3.1MB): https://slyfox.ath.cx/btrfs/linux-2.6-um-x86_64-fs.tar.gz You just need to start ''./run'' to enter into UML root fs and there to issue ./kill_btr to get fault above. Archive contains minimal .config for kernel 2.6.39-rc2, statically linked busybox binary and fresh image of btrfs. It also has tiny script, which will help you to generate the same rootfs if you are afraid to run suspicious binaries. -- Sergei
On Sun, 10 Apr 2011 13:37:10 +0300 Sergei Trofimovich <slyich@gmail.com> wrote:> According to https://btrfs.wiki.kernel.org/index.php/Debugging_Btrfs_with_GDB > UML did work once. > > Now it corrupts data and triggers BUG_ON once you > start to use it. I tried both 2.6.38 and 2.6.39-rc2 (x86_64) > I need some help to track it down. > > doing ''touch `seq 1 11`; rm 11'' kills the kernel:2.6.36 works 2.6.37 doesn''t. bsecting -- Sergei
> > According to https://btrfs.wiki.kernel.org/index.php/Debugging_Btrfs_with_GDB > > UML did work once. > > > > Now it corrupts data and triggers BUG_ON once you > > start to use it. I tried both 2.6.38 and 2.6.39-rc2 (x86_64) > > I need some help to track it down. > > > > doing ''touch `seq 1 11`; rm 11'' kills the kernel: > > 2.6.36 works 2.6.37 doesn''t. bsectingBisected down to: commit 59daa706fbec745684702741b9f5373142dd9fdc (v2.6.36-rc2-2-g59daa70) Author: Ma Ling <ling.ma@intel.com> Date: Tue Jun 29 03:24:25 2010 +0800 x86, mem: Optimize memcpy by avoiding memory false dependece Which means btrfs passes overlapping areas to memcpy. I''ve added some debug info and found out rough place: touching files 1 .. 11 #run> touch 1 2 3 4 5 6 7 8 9 10 11 [ 2.270000] memcpy overlap detected: memcpy(dst=0000000070654e8a, src=0000000070654ea9, size=171) [delta=31] [ 2.270000] ------------[ cut here ]------------ [ 2.270000] WARNING: at /home/slyfox/linux-2.6/fs/btrfs/memcpy_debug.c:18 btrfs_memcpy+0x52/0x68() [ 2.270000] Call Trace: [ 2.270000] 7064b748: [<600eff46>] map_extent_buffer+0x62/0x9e [ 2.270000] 7064b758: [<60029ad9>] warn_slowpath_common+0x59/0x70 [ 2.270000] 7064b798: [<60029b05>] warn_slowpath_null+0x15/0x17 [ 2.270000] 7064b7a8: [<6011129e>] btrfs_memcpy+0x52/0x68 [ 2.270000] 7064b7d8: [<600efa01>] memcpy_extent_buffer+0x18d/0x1da [ 2.270000] 7064b858: [<600efae2>] memmove_extent_buffer+0x94/0x208 [ 2.270000] 7064b8d8: [<600bc4b0>] setup_items_for_insert+0x2b8/0x426 [ 2.270000] 7064b8e8: [<600bb25a>] btrfs_leaf_free_space+0x62/0xa6 [ 2.270000] 7064b9c8: [<600c13f3>] btrfs_insert_empty_items+0xa3/0xb5 [ 2.270000] 7064ba38: [<600ce690>] insert_with_overflow+0x33/0xf1 [ 2.270000] 7064ba88: [<600ce7d4>] btrfs_insert_dir_item+0x86/0x268 [ 2.270000] 7064bae8: [<601b498b>] _raw_spin_unlock+0x9/0xb [ 2.270000] 7064bb48: [<600ddef1>] btrfs_add_link+0x10d/0x170 [ 2.270000] 7064bbc8: [<600ddf7a>] btrfs_add_nondir+0x26/0x52 [ 2.270000] 7064bc08: [<600de73f>] btrfs_create+0xf2/0x1c0 [ 2.270000] 7064bc18: [<6007ccff>] generic_permission+0x57/0x9d [ 2.270000] 7064bc68: [<6007cf60>] vfs_create+0x6a/0x75 which is in extent_io:copy_pages. I haven''t dig further only made sure the following patch below (practically converts copy_pages to move_pages). It certainly does not look the right thing, but I don''t understand extent_io contents yet to understand what actually happened. diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 20ddb28..4cab7db 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -3893,14 +3893,17 @@ static void copy_pages(struct page *dst_page, struct page *src_page, char *src_kaddr; if (dst_page != src_page) + { src_kaddr = kmap_atomic(src_page, KM_USER1); + memcpy(dst_kaddr + dst_off, src_kaddr + src_off, len); + kunmap_atomic(src_kaddr, KM_USER1); + } else + { src_kaddr = dst_kaddr; - - memcpy(dst_kaddr + dst_off, src_kaddr + src_off, len); + memmove(dst_kaddr + dst_off, src_kaddr + src_off, len); + } kunmap_atomic(dst_kaddr, KM_USER0); - if (dst_page != src_page) - kunmap_atomic(src_kaddr, KM_USER1); } void memcpy_extent_buffer(struct extent_buffer *dst, unsigned long dst_offset, -- Sergei
Sergei Trofimovich
2011-Apr-10 20:24 UTC
[PATCH] Re: btrfs does not work on usermode linux
On Sun, 10 Apr 2011 23:06:22 +0300 Sergei Trofimovich <slyich@gmail.com> wrote:> > > According to https://btrfs.wiki.kernel.org/index.php/Debugging_Btrfs_with_GDB > > > UML did work once. > > > > > > Now it corrupts data and triggers BUG_ON once you > > > start to use it. I tried both 2.6.38 and 2.6.39-rc2 (x86_64) > > > I need some help to track it down. > > > > > > doing ''touch `seq 1 11`; rm 11'' kills the kernel: > > > > 2.6.36 works 2.6.37 doesn''t. bsecting > > Bisected down to: > > commit 59daa706fbec745684702741b9f5373142dd9fdc (v2.6.36-rc2-2-g59daa70) > Author: Ma Ling <ling.ma@intel.com> > Date: Tue Jun 29 03:24:25 2010 +0800 > > x86, mem: Optimize memcpy by avoiding memory false dependece > > Which means btrfs passes overlapping areas to memcpy. I''ve added some debug info > and found out rough place: > touching files 1 .. 11 > #run> touch 1 2 3 4 5 6 7 8 9 10 11 > [ 2.270000] memcpy overlap detected: memcpy(dst=0000000070654e8a, src=0000000070654ea9, size=171) [delta=31] > [ 2.270000] ------------[ cut here ]------------ > [ 2.270000] WARNING: at /home/slyfox/linux-2.6/fs/btrfs/memcpy_debug.c:18 btrfs_memcpy+0x52/0x68() > [ 2.270000] Call Trace: > [ 2.270000] 7064b748: [<600eff46>] map_extent_buffer+0x62/0x9e > [ 2.270000] 7064b758: [<60029ad9>] warn_slowpath_common+0x59/0x70 > [ 2.270000] 7064b798: [<60029b05>] warn_slowpath_null+0x15/0x17 > [ 2.270000] 7064b7a8: [<6011129e>] btrfs_memcpy+0x52/0x68 > [ 2.270000] 7064b7d8: [<600efa01>] memcpy_extent_buffer+0x18d/0x1da > [ 2.270000] 7064b858: [<600efae2>] memmove_extent_buffer+0x94/0x208 > [ 2.270000] 7064b8d8: [<600bc4b0>] setup_items_for_insert+0x2b8/0x426 > [ 2.270000] 7064b8e8: [<600bb25a>] btrfs_leaf_free_space+0x62/0xa6 > [ 2.270000] 7064b9c8: [<600c13f3>] btrfs_insert_empty_items+0xa3/0xb5 > [ 2.270000] 7064ba38: [<600ce690>] insert_with_overflow+0x33/0xf1 > [ 2.270000] 7064ba88: [<600ce7d4>] btrfs_insert_dir_item+0x86/0x268 > [ 2.270000] 7064bae8: [<601b498b>] _raw_spin_unlock+0x9/0xb > [ 2.270000] 7064bb48: [<600ddef1>] btrfs_add_link+0x10d/0x170 > [ 2.270000] 7064bbc8: [<600ddf7a>] btrfs_add_nondir+0x26/0x52 > [ 2.270000] 7064bc08: [<600de73f>] btrfs_create+0xf2/0x1c0 > [ 2.270000] 7064bc18: [<6007ccff>] generic_permission+0x57/0x9d > [ 2.270000] 7064bc68: [<6007cf60>] vfs_create+0x6a/0x75 > > which is in extent_io:copy_pages. I haven''t dig further only made sure the following > patch below (practically converts copy_pages to move_pages). It certainly does not > look the right thing, but I don''t understand extent_io contents yet to understand what > actually happened. > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 20ddb28..4cab7db 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -3893,14 +3893,17 @@ static void copy_pages(struct page *dst_page, struct page *src_page, > char *src_kaddr; > > if (dst_page != src_page) > + { > src_kaddr = kmap_atomic(src_page, KM_USER1); > + memcpy(dst_kaddr + dst_off, src_kaddr + src_off, len); > + kunmap_atomic(src_kaddr, KM_USER1); > + } > else > + { > src_kaddr = dst_kaddr; > - > - memcpy(dst_kaddr + dst_off, src_kaddr + src_off, len); > + memmove(dst_kaddr + dst_off, src_kaddr + src_off, len); > + } > kunmap_atomic(dst_kaddr, KM_USER0); > - if (dst_page != src_page) > - kunmap_atomic(src_kaddr, KM_USER1); > } > > void memcpy_extent_buffer(struct extent_buffer *dst, unsigned long dst_offset,Attached nicer patch. Looks like original logic expected ceritain memcpy copy direction, but there isn''t one! -- Sergei
Sergei Trofimovich
2011-Apr-10 20:58 UTC
[PATCH v2] Re: btrfs does not work on usermode linux
On Sun, 10 Apr 2011 23:24:03 +0300 Sergei Trofimovich <slyich@gmail.com> wrote:> Fix data corruption caused by memcpy() usage on overlapping data. > I''ve observed it first when found out usermode linux crash on btrfs.Changes since v1:> else > src_kaddr = dst_kaddr; > > + BUG_ON(abs(src_off - dst_off) < len); > memcpy(dst_kaddr + dst_off, src_kaddr + src_off, len);Too eager BUG_ON. Now used only for src_page == dst_page.> - if (dst_offset < src_offset) { > + if (abs(dst_offset - src_offset) >= len) {abs() is not a good thing to use un unsigned values. aded helper overlapping_areas. -- Sergei
Josef Bacik
2011-Apr-11 15:37 UTC
Re: [PATCH v2] Re: btrfs does not work on usermode linux
On 04/10/2011 04:58 PM, Sergei Trofimovich wrote:> On Sun, 10 Apr 2011 23:24:03 +0300 > Sergei Trofimovich<slyich@gmail.com> wrote: > >> Fix data corruption caused by memcpy() usage on overlapping data. >> I''ve observed it first when found out usermode linux crash on btrfs. > > Changes since v1: > >> else >> src_kaddr = dst_kaddr; >> >> + BUG_ON(abs(src_off - dst_off)< len); >> memcpy(dst_kaddr + dst_off, src_kaddr + src_off, len); > > Too eager BUG_ON. Now used only for src_page == dst_page. > >> - if (dst_offset< src_offset) { >> + if (abs(dst_offset - src_offset)>= len) { > > abs() is not a good thing to use un unsigned values. aded helper overlapping_areas. >Very nice catch, one nit if (dst_page != src_page) src_kaddr = kmap_atomic(src_page, KM_USER1); else + { src_kaddr = dst_kaddr; + BUG_ON(areas_overlap(src_off, dst_off, len)); + } you will want to turn that into if (dst_page != src_page) { src_kaddr = kmap_atomic(src_page, KM_USER1); } else { src_kaddr = dst_kaddr; BUG_ON(areas_overlap(src_off, dst_off, len)); } Also maybe BUG_ON() is a little strong, since the kernel will do this right, it just screws up UML. So maybe just do a WARN_ON() so we notice it. Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sergei Trofimovich
2011-Apr-11 19:44 UTC
[PATCH v3] Re: btrfs does not work on usermode linux
> Fix data corruption caused by memcpy() usage on overlapping data. > I''ve observed it first when found out usermode linux crash on btrfs.Changes since v2: - Code style cleanup - 2 versions of patch: BUG_ON and WARN_ON variants, _but_ see below why I prefer BUG_ON Changes since v1:> else > src_kaddr = dst_kaddr; > > + BUG_ON(abs(src_off - dst_off) < len); > memcpy(dst_kaddr + dst_off, src_kaddr + src_off, len);Too eager BUG_ON. Now used only for src_page == dst_page.> - if (dst_offset < src_offset) { > + if (abs(dst_offset - src_offset) >= len) {abs() is not a good thing to use un unsigned values. aded helper overlapping_areas. On Mon, 11 Apr 2011 11:37:57 -0400 Josef Bacik <josef@redhat.com> wrote:> + { > you will want to turn that into > > if (dst_page != src_page) {done> Also maybe BUG_ON() is a little strong, since the kernel will do this > right, it just screws up UML. So maybe just do a WARN_ON() so we notice > it. Thanks,I''m afaid I didn''t understand this part. The commit I''ve found a deviation was linux''s implementation of memcpy (UML uses it as kernel does). Why the kernel differs to UML in that respect? Seems I don''t know/understand something fundamental here. So, if data overlaps - it''s a moment before data corruption, thus BUG_ON. Another thought is (if memcpy semantics differ from standard C''s function): does linux''s memcpy guarantee copying direction behaviour? If it does, then it''s really a weird memmove and x86/memcpy_64.S is a bit broken. Attached both patches, I personally like BUG_ON variant. Pick the one you like more :] Thanks for the feedback! -- Sergei
Niklas Schnelle
2011-Apr-11 19:49 UTC
Re: [PATCH v3] Re: btrfs does not work on usermode linux
I think the problem here is that memcpy beahviour changed in recent glibc in this regard see here http://lwn.net/Articles/414467/ On Mon, 2011-04-11 at 22:44 +0300, Sergei Trofimovich wrote:> > Fix data corruption caused by memcpy() usage on overlapping data. > > I''ve observed it first when found out usermode linux crash on btrfs. > > Changes since v2: > - Code style cleanup > - 2 versions of patch: BUG_ON and WARN_ON variants, > _but_ see below why I prefer BUG_ON > > Changes since v1: > > > else > > src_kaddr = dst_kaddr; > > > > + BUG_ON(abs(src_off - dst_off) < len); > > memcpy(dst_kaddr + dst_off, src_kaddr + src_off, len); > > Too eager BUG_ON. Now used only for src_page == dst_page. > > > - if (dst_offset < src_offset) { > > + if (abs(dst_offset - src_offset) >= len) { > > abs() is not a good thing to use un unsigned values. aded helper overlapping_areas. > > On Mon, 11 Apr 2011 11:37:57 -0400 > Josef Bacik <josef@redhat.com> wrote: > > > + { > > you will want to turn that into > > > > if (dst_page != src_page) { > > done > > > Also maybe BUG_ON() is a little strong, since the kernel will do this > > right, it just screws up UML. So maybe just do a WARN_ON() so we notice > > it. Thanks, > > I''m afaid I didn''t understand this part. The commit I''ve found a deviation > was linux''s implementation of memcpy (UML uses it as kernel does). Why the > kernel differs to UML in that respect? Seems I don''t know/understand something > fundamental here. > So, if data overlaps - it''s a moment before data corruption, thus BUG_ON. > > Another thought is (if memcpy semantics differ from standard C''s function): > does linux''s memcpy guarantee copying direction behaviour? > If it does, then it''s really a weird memmove and x86/memcpy_64.S is a bit broken. > > Attached both patches, I personally like BUG_ON variant. > Pick the one you like more :] > > Thanks for the feedback! >
Josef Bacik
2011-Apr-11 19:50 UTC
Re: [PATCH v3] Re: btrfs does not work on usermode linux
On 04/11/2011 03:44 PM, Sergei Trofimovich wrote:>> Fix data corruption caused by memcpy() usage on overlapping data. >> I''ve observed it first when found out usermode linux crash on btrfs. > > Changes since v2: > - Code style cleanup > - 2 versions of patch: BUG_ON and WARN_ON variants, > _but_ see below why I prefer BUG_ON > > Changes since v1: > >> else >> src_kaddr = dst_kaddr; >> >> + BUG_ON(abs(src_off - dst_off)< len); >> memcpy(dst_kaddr + dst_off, src_kaddr + src_off, len); > > Too eager BUG_ON. Now used only for src_page == dst_page. > >> - if (dst_offset< src_offset) { >> + if (abs(dst_offset - src_offset)>= len) { > > abs() is not a good thing to use un unsigned values. aded helper overlapping_areas. > > On Mon, 11 Apr 2011 11:37:57 -0400 > Josef Bacik<josef@redhat.com> wrote: > >> + { >> you will want to turn that into >> >> if (dst_page != src_page) { > > done > >> Also maybe BUG_ON() is a little strong, since the kernel will do this >> right, it just screws up UML. So maybe just do a WARN_ON() so we notice >> it. Thanks, > > I''m afaid I didn''t understand this part. The commit I''ve found a deviation > was linux''s implementation of memcpy (UML uses it as kernel does). Why the > kernel differs to UML in that respect? Seems I don''t know/understand something > fundamental here. > So, if data overlaps - it''s a moment before data corruption, thus BUG_ON. > > Another thought is (if memcpy semantics differ from standard C''s function): > does linux''s memcpy guarantee copying direction behaviour? > If it does, then it''s really a weird memmove and x86/memcpy_64.S is a bit broken. > > Attached both patches, I personally like BUG_ON variant. > Pick the one you like more :] > > Thanks for the feedback! >Fair enough, BUG_ON() it is. Repost that version and you can add my Reviewed-by: Josef Bacik <josef@redhat.com> Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sergei Trofimovich
2011-Apr-12 21:23 UTC
Re: [PATCH v3] Re: btrfs does not work on usermode linux
On Mon, 11 Apr 2011 15:50:48 -0400 Josef Bacik <josef@redhat.com> wrote:> On 04/11/2011 03:44 PM, Sergei Trofimovich wrote: > >> Fix data corruption caused by memcpy() usage on overlapping data. > >> I''ve observed it first when found out usermode linux crash on btrfs....> Fair enough, BUG_ON() it is. Repost that version and you can add my > > Reviewed-by: Josef Bacik <josef@redhat.com>Thank you! Added and resent as: http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg09357.html -- Sergei
Chris Mason
2011-Apr-13 11:32 UTC
Re: [PATCH v3] Re: btrfs does not work on usermode linux
Excerpts from Sergei Trofimovich''s message of 2011-04-12 17:23:33 -0400:> On Mon, 11 Apr 2011 15:50:48 -0400 > Josef Bacik <josef@redhat.com> wrote: > > > On 04/11/2011 03:44 PM, Sergei Trofimovich wrote: > > >> Fix data corruption caused by memcpy() usage on overlapping data. > > >> I''ve observed it first when found out usermode linux crash on btrfs. > > ... > > > Fair enough, BUG_ON() it is. Repost that version and you can add my > > > > Reviewed-by: Josef Bacik <josef@redhat.com> > > Thank you! Added and resent as: > http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg09357.html >This is in the master branch now, please give it another test. Thanks a lot for bisecting down and patching! -chris -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sergei Trofimovich
2011-Apr-13 20:12 UTC
Re: [PATCH v3] Re: btrfs does not work on usermode linux
On Wed, 13 Apr 2011 07:32:59 -0400 Chris Mason <chris.mason@oracle.com> wrote:> This is in the master branch now, please give it another test. Thanks a > lot for bisecting down and patching!Tested on btrfs-unstable/master. Works correctly. Reverting 3387206f26e1b48703e810175b98611a4fd8e8ea (to make sure) on top of master returns panic. Thank you! -- Sergei