Geert Uytterhoeven
2013-Jul-30 09:07 UTC
btrfs zero divide (was: Re: Linux 3.10 problem reports (yes, plural))
On Tue, 30 Jul 2013, Thorsten Glaser wrote:> NEW problem: btrfs doesn’t work at all. I had to reboot my > buildd into 3.2 using echo s/u/s/o >/proc/sysrq-trigger as > the attempt to mount it left the system hanging there.> [ 0.000000] Linux version 3.10-1-m68k (debian-kernel@lists.debian.org) (gcc version 4.8.1 (Debian 4.8.1-7+m68k.1) ) #1 Debian 3.10.3-1 (2013-07-27)> [ 6.720000] bio: create slab <bio-1> at 1 > [ 6.740000] Btrfs loaded > [ 6.830000] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3 > [ 7.150000] EXT4-fs (nfhd8p1): mounted filesystem with ordered data mode. Opts: (null) > [ 14.520000] udevd[228]: starting version 175 > [ 17.820000] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3 > [ 20.850000] Adding 3670012k swap on /dev/nfhd8p2. Priority:-1 extents:1 across:3670012k > [ 21.380000] EXT4-fs (nfhd8p1): re-mounted. Opts: (null) > [ 31.300000] EXT4-fs (nfhd8p1): re-mounted. Opts: errors=remount-ro > [ 38.460000] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3 > [ 38.530000] btrfs: setting nodatacow, compression disabled > [ 38.540000] btrfs: enabling auto recovery > [ 38.570000] btrfs: disk space caching is enabled > [ 38.600000] *** ZERO DIVIDE *** FORMAT=2 > [ 38.630000] Current process id is 722 > [ 38.660000] BAD KERNEL TRAP: 00000000 > [ 38.680000] Modules linked in: evdev mac_hid ext4 crc16 jbd2 mbcache btrfs xor lzo_compress zlib_deflate raid6_pq crc32c libcrc32c > [ 38.730000] PC: [<319535b2>] __btrfs_map_block+0x11c/0x119a [btrfs]Woops, adding the btrfs devs to CC.> [ 38.770000] SR: 2000 SP: 30c1fab4 a2: 30f0faf0 > [ 38.800000] d0: 00000000 d1: 00001000 d2: 00000000 d3: 00000000 > [ 38.830000] d4: 00010000 d5: 00000000 a0: 3085c72c a1: 3085c72c > [ 38.850000] Process mount (pid: 722, task=30f0faf0) > [ 38.870000] Frame format=2 instr addr=319535ae > [ 38.880000] Stack from 30c1faec: > [ 38.880000] 00000000 00000020 00000000 00001000 00000000 01401000 30253928 300ffc00 > [ 38.880000] 00a843ac 3026f640 00000000 00010000 0009e250 00d106c0 00011220 00000000 > [ 38.880000] 00001000 301c6830 0009e32a 000000ff 00000009 3085c72c 00000000 00000000 > [ 38.880000] 30c1fd14 00000000 00000020 00000000 30c1fd14 0009e26c 00000020 00000003 > [ 38.880000] 00000000 0009dd8a 300b0b6c 30253928 00a843ac 00001000 00000000 00000000 > [ 38.880000] 0000a008 3194e76a 30253928 00a843ac 00001000 00000000 00000000 00000002 > [ 39.190000] Call Trace: [<00001000>] kernel_pg_dir+0x0/0x1000 > [ 39.210000] [<00010000>] res_func+0x1020/0x141a > [ 39.250000] [<0009e250>] bvec_alloc+0xa2/0xbe > [ 39.270000] [<00011220>] sasin+0x87c/0x944 > [ 39.290000] [<00001000>] kernel_pg_dir+0x0/0x1000 > [ 39.330000] [<0009e32a>] bio_alloc_bioset+0xbe/0x12e > [ 39.360000] [<0009e26c>] bio_alloc_bioset+0x0/0x12e > [ 39.380000] [<0009dd8a>] bio_add_page+0x4a/0x58 > [ 39.420000] [<00001000>] kernel_pg_dir+0x0/0x1000 > [ 39.470000] [<0000a008>] via_nubus_irq+0x1c/0xa2 > [ 39.500000] [<3194e76a>] submit_extent_page.isra.44+0x170/0x1bc [btrfs] > [ 39.530000] [<00001000>] kernel_pg_dir+0x0/0x1000 > [ 39.560000] [<00001000>] kernel_pg_dir+0x0/0x1000 > [ 39.600000] [<31959778>] btrfs_map_bio+0x60/0x48c [btrfs] > [ 39.630000] [<31931b72>] btree_submit_bio_hook+0x0/0xae [btrfs] > [ 39.660000] [<3194eaa0>] end_bio_extent_readpage+0x0/0x69c [btrfs] > [ 39.710000] [<00001000>] kernel_pg_dir+0x0/0x1000 > [ 39.730000] [<31931944>] btrfs_bio_wq_end_io+0x16/0x50 [btrfs] > [ 39.760000] [<31931bce>] btree_submit_bio_hook+0x5c/0xae [btrfs] > [ 39.780000] [<3194bd36>] submit_one_bio+0x7c/0xb2 [btrfs] > [ 39.810000] [<3194f174>] __extent_read_full_page+0x0/0x70a [btrfs] > [ 39.830000] [<00058828>] unlock_page+0x0/0x26 > [ 39.840000] [<31951736>] read_extent_buffer_pages+0x1a8/0x218 [btrfs] > [ 39.890000] [<00027d81>] devkmsg_read+0x213/0x39a > [ 39.930000] [<31959006>] btrfs_num_copies+0x0/0x142 [btrfs] > [ 39.970000] [<31930a66>] btree_read_extent_buffer_pages.constprop.52+0x42/0xca [btrfs] > [ 40.030000] [<3192f7c2>] btree_get_extent+0x0/0x102 [btrfs] > [ 40.060000] [<00027d81>] devkmsg_read+0x213/0x39a > [ 40.090000] [<00001000>] kernel_pg_dir+0x0/0x1000 > [ 40.100000] [<3193221e>] read_tree_block+0x38/0x48 [btrfs] > [ 40.130000] [<00027d81>] devkmsg_read+0x213/0x39a > [ 40.140000] [<319321e6>] read_tree_block+0x0/0x48 [btrfs] > [ 40.170000] [<31933d00>] open_ctree+0xe80/0x15e6 [btrfs] > [ 40.200000] [<00001000>] kernel_pg_dir+0x0/0x1000 > [ 40.220000] [<00027d81>] devkmsg_read+0x213/0x39a > [ 40.230000] [<00001000>] kernel_pg_dir+0x0/0x1000 > [ 40.260000] [<000f280a>] resource_string.isra.12+0x2b4/0x2ee > [ 40.280000] [<00001000>] kernel_pg_dir+0x0/0x1000 > [ 40.320000] [<00001000>] kernel_pg_dir+0x0/0x1000 > [ 40.350000] [<000e59ba>] disk_name+0x72/0x80 > [ 40.360000] [<0000aff0>] mac_hwclk.part.0+0xe6/0x174 > [ 40.390000] [<31913ede>] btrfs_mount+0x450/0x73e [btrfs] > [ 40.410000] [<00006ff0>] amiga_get_hardware_list+0x19e/0x44a > [ 40.460000] [<0007acc0>] __kmalloc+0x14/0xac > [ 40.500000] [<000675c6>] kstrdup+0x36/0x48 > [ 40.530000] [<0007fae4>] mount_fs+0x1c/0xc8 > [ 40.560000] [<0008fec8>] vfs_kern_mount+0x44/0xbe > [ 40.580000] [<0008f55c>] put_filesystem+0x0/0x10 > [ 40.620000] [<00085e7e>] kern_path+0x0/0x3c > [ 40.640000] [<00091a96>] do_mount+0x61e/0x6e0 > [ 40.670000] [<0007a73e>] kfree+0x0/0xa2 > [ 40.680000] [<0009144a>] copy_mount_string+0x0/0x2e > [ 40.700000] [<00091bd0>] SyS_mount+0x78/0xb0 > [ 40.730000] [<00002614>] syscall+0x8/0xc > [ 40.750000] [<0008c018>] __d_move+0x46/0x1a8 > [ 40.770000] > [ 40.790000] Code: 222e ff74 2a2e ff5c 2c2e ff60 4c45 1402 <2d40> ff64 2d41 ff68 2205 4c2e 1800 ff68 4c04 0800 2041 d1c0 2206 4c2e 1400 ff68 > [ 40.830000] Disabling lock debugging due to kernel taint0: 222e ff74 movel %fp@(-140),%d1 4: 2a2e ff5c movel %fp@(-164),%d5 8: 2c2e ff60 movel %fp@(-160),%d6 c: 4c45 1402 < divul %d5,%d2,%d1 > 10: 2d40 ff64 movel %d0,%fp@(-156) 14: 2d41 ff68 movel %d1,%fp@(-152) 18: 2205 movel %d5,%d1 1a: 4c2e 1800 ff68 mulsl %fp@(-152),%d1 20: 4c04 0800 mulsl %d4,%d0 24: 2041 moveal %d1,%a0 26: d1c0 addal %d0,%a0 28: 2206 movel %d6,%d1 2a: 4c2e 1400 ff68 mulul %fp@(-152),%d0,%d1 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There''s lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I''m talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- 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
Geert Uytterhoeven dixit:> 0: 222e ff74 movel %fp@(-140),%d1 > 4: 2a2e ff5c movel %fp@(-164),%d5 > 8: 2c2e ff60 movel %fp@(-160),%d6 > c: 4c45 1402 < divul %d5,%d2,%d1 > > 10: 2d40 ff64 movel %d0,%fp@(-156) > 14: 2d41 ff68 movel %d1,%fp@(-152) > 18: 2205 movel %d5,%d1 > 1a: 4c2e 1800 ff68 mulsl %fp@(-152),%d1 > 20: 4c04 0800 mulsl %d4,%d0 > 24: 2041 moveal %d1,%a0 > 26: d1c0 addal %d0,%a0 > 28: 2206 movel %d6,%d1 > 2a: 4c2e 1400 ff68 mulul %fp@(-152),%d0,%d1This is gcc-4.8 compiled, btw… in case that’s a known issue. bye, //mirabilos -- “It is inappropriate to require that a time represented as seconds since the Epoch precisely represent the number of seconds between the referenced time and the Epoch.” -- IEEE Std 1003.1b-1993 (POSIX) Section B.2.2.2
Josef Bacik
2013-Jul-30 17:13 UTC
Re: btrfs zero divide (was: Re: Linux 3.10 problem reports (yes, plural))
On Tue, Jul 30, 2013 at 11:07:30AM +0200, Geert Uytterhoeven wrote:> On Tue, 30 Jul 2013, Thorsten Glaser wrote: > > NEW problem: btrfs doesn’t work at all. I had to reboot my > > buildd into 3.2 using echo s/u/s/o >/proc/sysrq-trigger as > > the attempt to mount it left the system hanging there. > > > [ 0.000000] Linux version 3.10-1-m68k (debian-kernel@lists.debian.org) (gcc version 4.8.1 (Debian 4.8.1-7+m68k.1) ) #1 Debian 3.10.3-1 (2013-07-27) > > > [ 6.720000] bio: create slab <bio-1> at 1 > > [ 6.740000] Btrfs loaded > > [ 6.830000] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3 > > [ 7.150000] EXT4-fs (nfhd8p1): mounted filesystem with ordered data mode. Opts: (null) > > [ 14.520000] udevd[228]: starting version 175 > > [ 17.820000] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3 > > [ 20.850000] Adding 3670012k swap on /dev/nfhd8p2. Priority:-1 extents:1 across:3670012k > > [ 21.380000] EXT4-fs (nfhd8p1): re-mounted. Opts: (null) > > [ 31.300000] EXT4-fs (nfhd8p1): re-mounted. Opts: errors=remount-ro > > [ 38.460000] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3 > > [ 38.530000] btrfs: setting nodatacow, compression disabled > > [ 38.540000] btrfs: enabling auto recovery > > [ 38.570000] btrfs: disk space caching is enabled > > [ 38.600000] *** ZERO DIVIDE *** FORMAT=2 > > [ 38.630000] Current process id is 722 > > [ 38.660000] BAD KERNEL TRAP: 00000000 > > [ 38.680000] Modules linked in: evdev mac_hid ext4 crc16 jbd2 mbcache btrfs xor lzo_compress zlib_deflate raid6_pq crc32c libcrc32c > > [ 38.730000] PC: [<319535b2>] __btrfs_map_block+0x11c/0x119a [btrfs] > > Woops, adding the btrfs devs to CC. >Can you gdb btrfs.ko and do list *(__btrfs_map_block+0x11c) so I can see where this is? I''ve not seen this yet, just so I''m clear this is blowing up because we''re doing blah / 0 right? I''ve looked at all the places we do divides in this function and it doesn''t look like we''re doing this anywhere but I could be blind. Thanks, Josef
Joe Perches
2013-Jul-30 17:36 UTC
Re: btrfs zero divide (was: Re: Linux 3.10 problem reports (yes, plural))
On Tue, 2013-07-30 at 13:13 -0400, Josef Bacik wrote:> I''ve looked at all the places we do divides in this function and it > doesn''t look like we''re doing this anywhere but I could be blind,do_div seems a likely suspect... /* * stripe_nr counts the total number of stripes we have to stride * to get to this block */ do_div(stripe_nr, stripe_len);
Josef Bacik dixit:>Can you gdb btrfs.ko and do > >list *(__btrfs_map_block+0x11c)Not easily (the kernel image is from a .deb package), and even in a compile tree gdb just says: No symbol table is loaded. Use the "file" command. With a bit of cheating and a cross-compiler, this is: (gdb) list *0x0000106e 0x106e is in __btrfs_map_block (/var/lib/gforge/chroot/home/users/tg/Xl/linux-3.10.1/fs/btrfs/volumes.c:4447). 4442 stripe_nr = offset; 4443 /* 4444 * stripe_nr counts the total number of stripes we have to stride 4445 * to get to this block 4446 */ 4447 do_div(stripe_nr, stripe_len); 4448 4449 stripe_offset = stripe_nr * stripe_len; 4450 BUG_ON(offset < stripe_offset); 4451 The code is somewhat matching: 0x00001062 <+268>: movel %fp@(-140),%d1 0x00001066 <+272>: movel %fp@(-164),%d5 0x0000106a <+276>: movel %fp@(-160),%d6 0x0000106e <+280>: divul %d5,%d2,%d1 0x00001072 <+284>: movel %d0,%fp@(-156) 0x00001076 <+288>: movel %d1,%fp@(-152) 0x0000107a <+292>: movel %d5,%d1 0x0000107c <+294>: mulsl %fp@(-152),%d1 0x00001082 <+300>: mulsl %d4,%d0 0x00001086 <+304>: moveal %d1,%a0 0x00001088 <+306>: addal %d0,%a0 0x0000108a <+308>: movel %d6,%d1 0x0000108c <+310>: mulul %fp@(-152),%d0,%d1 According to the registers… [ 38.800000] d0: 00000000 d1: 00001000 d2: 00000000 d3: 00000000 [ 38.830000] d4: 00010000 d5: 00000000 a0: 3085c72c a1: 3085c72c … this is (if I parse this right) 0000000000001000 / 00000000 (64-bit D2:D1 divided by 32-bit D5 store into D1, remainder into D2). Joe Perches dixit quod…> do_div seems a likely suspect...I do admit I don’t understand arch/m68k/include/asm/div64.h being not a real m68k coder, but “it works elsewhere”… (And I loathe GCC inline asm with a passion!) From the code expansion, I assume (__upper = __n.n32[0]) is always zero (as we get only one divul instruction). This looks a bit weird because the numbers in question are all 64 bit (stripe_nr, offset, logical). Hm, actually… from a test program… #include <stdio.h> typedef unsigned long long u64; int main(void) { u64 stripe_nr; u64 stripe_len; stripe_nr = 1234; stripe_len = 2; printf("in : %llu / %llu\n", stripe_nr, stripe_len); ({ union { unsigned long n32[2]; unsigned long long n64; } __n; unsigned long __rem, __upper; __n.n64 = (stripe_nr); if ((__upper = __n.n32[0])) { asm ("divul.l %2,%1:%0" : "=d" (__n.n32[0]), "=d" (__upper) : "d" (stripe_len), "0" (__n.n32[0])); } asm ("divu.l %2,%1:%0" : "=d" (__n.n32[1]), "=d" (__rem) : "d" (stripe_len), "1" (__upper), "0" (__n.n32[1])); (stripe_nr) = __n.n64; __rem; }); printf("out: %llu R %llu\n", stripe_nr, stripe_len); return (0); } … I think we get two divul instructions, just with a lot of moves between them. Hmpf. The frame pointer would be useful to know, to know the proper values used for these operations… … Aaaah okay. Some reading *(gdb.info):: later, indeed: (gdb) info line 4446 Line 4446 of "/var/lib/gforge/chroot/home/users/tg/Xl/linux-3.10.1/fs/btrfs/volumes.c" is at address 0x104a <__btrfs_map_block+244> but contains no code. (gdb) info line 4448 Line 4448 of "/var/lib/gforge/chroot/home/users/tg/Xl/linux-3.10.1/fs/btrfs/volumes.c" is at address 0x107a <__btrfs_map_block+292> but contains no code. (gdb) disas /r 0x104a,0x107a Dump of assembler code from 0x104a to 0x107a: 0x0000104a <__btrfs_map_block+244>: 20 02 movel %d2,%d0 0x0000104c <__btrfs_map_block+246>: 24 2e ff 70 movel %fp@(-144),%d2 0x00001050 <__btrfs_map_block+250>: 4a 80 tstl %d0 0x00001052 <__btrfs_map_block+252>: 67 0e beqs 0x1062 <__btrfs_map_block+268> 0x00001054 <__btrfs_map_block+254>: 20 02 movel %d2,%d0 0x00001056 <__btrfs_map_block+256>: 2c 2e ff 5c movel %fp@(-164),%d6 0x0000105a <__btrfs_map_block+260>: 2e 2e ff 60 movel %fp@(-160),%d7 0x0000105e <__btrfs_map_block+264>: 4c 46 00 02 divull %d6,%d2,%d0 0x00001062 <__btrfs_map_block+268>: 22 2e ff 74 movel %fp@(-140),%d1 0x00001066 <__btrfs_map_block+272>: 2a 2e ff 5c movel %fp@(-164),%d5 0x0000106a <__btrfs_map_block+276>: 2c 2e ff 60 movel %fp@(-160),%d6 0x0000106e <__btrfs_map_block+280>: 4c 45 14 02 divul %d5,%d2,%d1 0x00001072 <__btrfs_map_block+284>: 2d 40 ff 64 movel %d0,%fp@(-156) 0x00001076 <__btrfs_map_block+288>: 2d 41 ff 68 movel %d1,%fp@(-152) End of assembler dump. Now, can anyone more fluent in m68k asm make out a problem with it? bye, //mirabilos -- I believe no one can invent an algorithm. One just happens to hit upon it when God enlightens him. Or only God invents algorithms, we merely copy them. If you don''t believe in God, just consider God as Nature if you won''t deny existence. -- Coywolf Qi Hunt
On Tue, Jul 30, 2013 at 07:02:29PM +0000, Thorsten Glaser wrote:> Josef Bacik dixit: > > >Can you gdb btrfs.ko and do > > > >list *(__btrfs_map_block+0x11c) > > Not easily (the kernel image is from a .deb package), > and even in a compile tree gdb just says: > No symbol table is loaded. Use the "file" command. > > With a bit of cheating and a cross-compiler, this is: > > (gdb) list *0x0000106e > 0x106e is in __btrfs_map_block (/var/lib/gforge/chroot/home/users/tg/Xl/linux-3.10.1/fs/btrfs/volumes.c:4447). > 4442 stripe_nr = offset; > 4443 /* > 4444 * stripe_nr counts the total number of stripes we have to stride > 4445 * to get to this block > 4446 */ > 4447 do_div(stripe_nr, stripe_len); > 4448 > 4449 stripe_offset = stripe_nr * stripe_len; > 4450 BUG_ON(offset < stripe_offset); > 4451 >So stripe_len shouldn''t be 0, if it is you have bigger problems :). Is this a corrupt fs or something? If there was some sort of corruption that occured then I suppose stripe_len could be 0 and we''d need to catch that somewhere higher up the stack and error out. Is there a way you could check and see if that''s the case? Thanks, Josef
On Tue, 2013-07-30 at 16:40 -0400, Josef Bacik wrote:> So stripe_len shouldn''t be 0, if it is you have bigger problems :). Is this a > corrupt fs or something? If there was some sort of corruption that occured then > I suppose stripe_len could be 0 and we''d need to catch that somewhere higher up > the stack and error out. Is there a way you could check and see if that''s the > case? Thanks,Maybe use a temporary check in do_div Something like this maybe. (uncompiled/untested) --- include/asm-generic/div64.h | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h index 8f4e319..cce75fe 100644 --- a/include/asm-generic/div64.h +++ b/include/asm-generic/div64.h @@ -19,16 +19,25 @@ #include <linux/types.h> #include <linux/compiler.h> +#include <linux/bug.h> +#include <linux/printk.h> #if BITS_PER_LONG == 64 -# define do_div(n,base) ({ \ +# define do_div(n, base) \ +({ \ uint32_t __base = (base); \ uint32_t __rem; \ - __rem = ((uint64_t)(n)) % __base; \ - (n) = ((uint64_t)(n)) / __base; \ + if (__base == 0) { \ + WARN(1, "Attempted division by 0\n"); \ + dump_stack(); \ + __rem = 0; \ + } else { \ + __rem = ((uint64_t)(n)) % __base; \ + (n) = ((uint64_t)(n)) / __base; \ + } \ __rem; \ - }) +}) #elif BITS_PER_LONG == 32 @@ -37,16 +46,22 @@ extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor); /* The unnecessary pointer compare is there * to check for type safety (n must be 64bit) */ -# define do_div(n,base) ({ \ - uint32_t __base = (base); \ - uint32_t __rem; \ - (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ - if (likely(((n) >> 32) == 0)) { \ - __rem = (uint32_t)(n) % __base; \ - (n) = (uint32_t)(n) / __base; \ - } else \ - __rem = __div64_32(&(n), __base); \ - __rem; \ +# define do_div(n, base) \ +({ \ + uint32_t __base = (base); \ + uint32_t __rem; \ + (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ + if (__base == 0) { \ + WARN(1, "Attempted division by 0\n"); \ + dump_stack(); \ + __rem = 0; \ + } else if (likely(((n) >> 32) == 0)) { \ + __rem = (uint32_t)(n) % __base; \ + (n) = (uint32_t)(n) / __base; \ + } else { \ + __rem = __div64_32(&(n), __base); \ + } \ + __rem; \ }) #else /* BITS_PER_LONG == ?? */
Josef Bacik dixit:>So stripe_len shouldn''t be 0, if it is you have bigger problems :).☺>Is this a corrupt fs or something? If there was some sort ofI don’t think so, I can access and use that filesystem under 3.2 just fine (it’s what I created it under, too, so it’s possible that it’s indeed corrupt and Linux 3.2 is just the same corrupt to happen to make it work, e.g. wrong endianness used for stripe_len which makes the upper 32 bit of that 64-bit value (usually 0) become the lower 32 bit, or something like that). I have access to that system, and it’s currently running as a Debian/m68k buildd using said filesystem, but I can run commands you tell me to diagnose/analyse it if it won’t get corrupted by those. Joe Perches dixit:>Maybe use a temporary check in do_divMh. If nobody finds anything I’ll try that. (Doing things like compiling a kernel and testing it takes about two days timeboxed and some hours of active human effort, though, so I’d like to avoid guessing. Plus it’ll disrupt running the Debian buildd…) On second thoughts, this sort of check sounds like a good idea to add to that file in general, depending on some debugging CPPFLAG or Kconfig option. But I’m not the authority on that. bye, //mirabilos -- <ch> you introduced a merge commit │<mika> % g rebase -i HEAD^^ <mika> sorry, no idea and rebasing just fscked │<mika> Segmentation <ch> should have cloned into a clean repo │ fault (core dumped) <ch> if I rebase that now, it''s really ugh │<mika:#grml> wuahhhhhh
tl;dr: we got the faulty code pinned down, it''s m68k specific, except the m68k specific part didn’t change from 3.2… Joe Perches dixit:>Something like this maybe. (uncompiled/untested)I tried this: --- div64.h.orig 2013-08-08 19:34:32.663540965 +0000 +++ - 2013-08-08 19:47:30.309776791 +0000 @@ -6,6 +6,8 @@ #else #include <linux/types.h> +#include <linux/bug.h> +#include <linux/printk.h> /* n = n / base; return rem; */ @@ -16,6 +18,11 @@ } __n; \ unsigned long __rem, __upper; \ \ +if (base == 0) { \ +WARN(1, "Attempted division by 0\n"); \ +dump_stack(); \ +__rem = 0; \ +} else { \ __n.n64 = (n); \ if ((__upper = __n.n32[0])) { \ asm ("divul.l %2,%1:%0" \ @@ -26,6 +33,7 @@ : "=d" (__n.n32[1]), "=d" (__rem) \ : "d" (base), "1" (__upper), "0" (__n.n32[1])); \ (n) = __n.n64; \ +} \ __rem; \ }) It didn’t trigger, apparently: [817508.370000] bio: create slab <bio-1> at 1 [817508.510000] Btrfs loaded [817524.110000] loop: module loaded [817534.860000] device fsid 01cfa645-5cde-4e4c-9b0b-df7b37bdc495 devid 1 transid 4 /dev/loop0 [817534.860000] btrfs: disk space caching is enabled [817534.860000] *** ZERO DIVIDE *** FORMAT=2 [817534.860000] Current process id is 32312 [817534.860000] BAD KERNEL TRAP: 00000000 [817534.860000] Modules linked in: loop btrfs lzo_compress zlib_deflate raid6_pq crc32c libcrc32c xor ipv6 evdev mac_hid ext3 mbcache jbd [last unloaded: btrfs] [817534.860000] PC: [<31c46612>] __btrfs_map_block+0x134/0x147a [btrfs] [817534.860000] SR: 2000 SP: 0249fab0 a2: 3010f660 [817534.860000] d0: 00000000 d1: 00022000 d2: 00000000 d3: 00000000 [817534.860000] d4: 00010000 d5: 00010000 a0: 021777a4 a1: 021777a4 [817534.860000] Process mount (pid: 32312, task=3010f660) [817534.860000] Frame format=2 instr addr=31c4660e [817534.860000] Stack from 0249fae8: 00000000 00000020 00000000 00001000 00000000 00022000 0766a928 07621800 00415d84 00000070 077a97c0 00000070 0249fb68 0009e250 00d106c0 00011220 00000070 00000020 00000000 00022000 000000ff 00000009 00001000 00000000 00000000 021777a4 00000000 00000020 00000000 0249fd14 0009e26c 00000020 00000003 00000000 0009dd8a 3007c02c 0766a928 00415d84 00001000 00000000 00000000 00000110 31c417ae 0766a928 00415d84 00001000 00000000 00000000 [817534.860000] Call Trace: [<00001000>] kernel_pg_dir+0x0/0x1000 [817534.860000] [<00022000>] _060_fpsp_effadd+0xb2c0/0xd518 [817534.860000] [<0009e250>] bvec_alloc+0xa2/0xbe [817534.860000] [<00011220>] sasin+0x87c/0x944 [817534.860000] [<00022000>] _060_fpsp_effadd+0xb2c0/0xd518 [817534.860000] [<00001000>] kernel_pg_dir+0x0/0x1000 [817534.860000] [<0009e26c>] bio_alloc_bioset+0x0/0x12e [817534.860000] [<0009dd8a>] bio_add_page+0x4a/0x58 [817534.860000] [<00001000>] kernel_pg_dir+0x0/0x1000 [817534.860000] [<31c417ae>] submit_extent_page.isra.44+0x170/0x1bc [btrfs] [817534.860000] [<00001000>] kernel_pg_dir+0x0/0x1000 [817534.860000] [<00001000>] kernel_pg_dir+0x0/0x1000 [817534.860000] [<31c4cbfe>] btrfs_map_bio+0x60/0x48c [btrfs] [817534.860000] [<00022000>] _060_fpsp_effadd+0xb2c0/0xd518 [817534.860000] [<00022000>] _060_fpsp_effadd+0xb2c0/0xd518 [817534.860000] [<31c24bb2>] btree_submit_bio_hook+0x0/0xae [btrfs] [817534.860000] [<31c41ae4>] end_bio_extent_readpage+0x0/0x69c [btrfs] [817534.860000] [<00001000>] kernel_pg_dir+0x0/0x1000 [817534.860000] [<31c24984>] btrfs_bio_wq_end_io+0x16/0x50 [btrfs] [817534.860000] [<31c24c0e>] btree_submit_bio_hook+0x5c/0xae [btrfs] [817534.870000] [<00022000>] _060_fpsp_effadd+0xb2c0/0xd518 [817534.870000] [<31c3ed7a>] submit_one_bio+0x7c/0xb2 [btrfs] [817534.870000] [<00022000>] _060_fpsp_effadd+0xb2c0/0xd518 [817534.870000] [<31c421b8>] __extent_read_full_page+0x0/0x70a [btrfs] [817534.870000] [<00058828>] unlock_page+0x0/0x26 [817534.870000] [<31c44780>] read_extent_buffer_pages+0x1a8/0x218 [btrfs] [817534.880000] [<31c4c3b2>] btrfs_num_copies+0x0/0x142 [btrfs] [817534.880000] [<31c23aa6>] btree_read_extent_buffer_pages.constprop.52+0x42/0xca [btrfs] [817534.880000] [<31c22802>] btree_get_extent+0x0/0x102 [btrfs] [817534.880000] [<00022000>] _060_fpsp_effadd+0xb2c0/0xd518 [817534.880000] [<00001000>] kernel_pg_dir+0x0/0x1000 [817534.880000] [<31c2525e>] read_tree_block+0x38/0x48 [btrfs] [817534.880000] [<31c25226>] read_tree_block+0x0/0x48 [btrfs] [817534.890000] [<31c26d40>] open_ctree+0xe80/0x15e6 [btrfs] [817534.890000] [<00022000>] _060_fpsp_effadd+0xb2c0/0xd518 [817534.890000] [<00001000>] kernel_pg_dir+0x0/0x1000 [817534.890000] [<00001000>] kernel_pg_dir+0x0/0x1000 [817534.890000] [<00001000>] kernel_pg_dir+0x0/0x1000 [817534.890000] [<00001000>] kernel_pg_dir+0x0/0x1000 [817534.890000] [<000e0000>] blk_stack_limits+0x54/0x2ec [817534.890000] [<0000af71>] mac_hwclk.part.0+0x67/0x174 [817534.890000] [<31c06ede>] btrfs_mount+0x450/0x73e [btrfs] [817534.900000] [<0007acc0>] __kmalloc+0x14/0xac [817534.900000] [<000675c6>] kstrdup+0x36/0x48 [817534.900000] [<0007fae4>] mount_fs+0x1c/0xc8 [817534.900000] [<0008fec8>] vfs_kern_mount+0x44/0xbe [817534.900000] [<0008f55c>] put_filesystem+0x0/0x10 [817534.900000] [<00085e7e>] kern_path+0x0/0x3c [817534.900000] [<00091a96>] do_mount+0x61e/0x6e0 [817534.900000] [<0007a73e>] kfree+0x0/0xa2 [817534.900000] [<0009144a>] copy_mount_string+0x0/0x2e [817534.900000] [<00091bd0>] SyS_mount+0x78/0xb0 [817534.900000] [<00002614>] syscall+0x8/0xc [817534.900000] [<0008c018>] __d_move+0x46/0x1a8 [817534.900000] [817534.900000] Code: 2400 6704 4c46 0002 222e ff7c 4c46 1402 <2d40> ff68 2d41 ff6c 2006 4c2e 0800 ff6c 222e ff68 4c04 1800 2041 d1c0 222e ff6c [817534.900000] Disabling lock debugging due to kernel taint This is stdio of what I did: root@ara3:~ # dd if=/dev/zero of=/butter bs=1048576 count=128 128+0 records in 128+0 records out 134217728 bytes (134 MB) copied, 14.6502 s, 9.2 MB/s root@ara3:~ # modprobe btrfs root@ara3:~ # losetup /dev/loop0 /butter root@ara3:~ # mkfs.btrfs /dev/loop0 WARNING! - Btrfs v0.20-rc1 IS EXPERIMENTAL WARNING! - see http://btrfs.wiki.kernel.org before using SMALL VOLUME: forcing mixed metadata/data groups Created a data/metadata chunk of size 8388608 fs created label (null) on /dev/loop0 nodesize 4096 leafsize 4096 sectorsize 4096 size 128.00MB Btrfs v0.20-rc1 root@ara3:~ # mount -t btrfs /dev/loop0 /mnt Segmentation fault 139|root@ara3:~ # lsmod | fgrep btrfs btrfs 585560 2 lzo_compress 1510 1 btrfs zlib_deflate 15039 1 btrfs raid6_pq 82747 1 btrfs libcrc32c 698 1 btrfs xor 5048 1 btrfs root@ara3:~ # dpkg -l | fgrep btrfs ii btrfs-tools 0.19+20130315-5 m68k Checksumming Copy on Write Filesystem utilities An rmmod at this point does not work, with -f it does. This gives more backtraces. Ooooookay now I’ve done this: […] #if 1 /*def CONFIG_CPU_HAS_NO_MULDIV64*/ #include <asm-generic/div64.h> #else […] And get: root@ara3:~ # losetup /dev/loop1 /butter root@ara3:~ # mount -t btrfs /dev/loop1 /mnt2 [817960.710000] bio: create slab <bio-1> at 1 [817960.710000] Btrfs loaded [817994.120000] device fsid 01cfa645-5cde-4e4c-9b0b-df7b37bdc495 devid 1 transid 4 /dev/loop1 [817994.120000] btrfs: disk space caching is enabled I can also write there. So, my apologies to the btrfs people and a confirmation that your guess seems to have been right at first. The machdep division code appears to be faulty. On the other hand, the code didn’t change from 3.2 only the condition, but we used the asm code in 3.2 already. So either btrfs changed to use do_div more now, or it misuses it (e.g. two 64-bit numbers) and that is not cought by the macro, or it’s a byproduct of us moving to gcc-4.8 and new binutils. Geert et al. is there anything that we can do about this? Thanks, //mirabilos -- FWIW, I''m quite impressed with mksh interactively. I thought it was much *much* more bare bones. But it turns out it beats the living hell out of ksh93 in that respect. I''d even consider it for my daily use if I hadn''t wasted half my life on my zsh setup. :-) -- Frank Terbeck in #!/bin/mksh
Josef Bacik <jbacik@fusionio.com> writes:> So stripe_len shouldn''t be 0, if it is you have bigger problems :).The bigger problem is that stripe_nr is u64, this is completely bogus. The first operand of do_div must be u32. This goes through the whole file. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Andreas Schwab <schwab@linux-m68k.org> writes:> Josef Bacik <jbacik@fusionio.com> writes: > >> So stripe_len shouldn''t be 0, if it is you have bigger problems :). > > The bigger problem is that stripe_nr is u64, this is completely bogus. > The first operand of do_div must be u32. This goes through the whole > file.Of course, what I meant was that the *second* operand must be u32, but that doesn''t change my point. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- 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
On Fri, Aug 09, 2013 at 02:30:38PM +0200, Andreas Schwab wrote:> Andreas Schwab <schwab@linux-m68k.org> writes: > > > Josef Bacik <jbacik@fusionio.com> writes: > > > >> So stripe_len shouldn''t be 0, if it is you have bigger problems :). > > > > The bigger problem is that stripe_nr is u64, this is completely bogus. > > The first operand of do_div must be u32. This goes through the whole > > file. > > Of course, what I meant was that the *second* operand must be u32, but > that doesn''t change my point. >Yeah we can change this. Thanks, Josef
On Fri, Aug 09, 2013 at 02:26:36PM +0200, Andreas Schwab wrote:> Josef Bacik <jbacik@fusionio.com> writes: > > > So stripe_len shouldn''t be 0, if it is you have bigger problems :). > > The bigger problem is that stripe_nr is u64, this is completely bogus. > The first operand of do_div must be u32. This goes through the whole > file.Definitely. Can we get some typeof() tricks in the macros to have the build fail if (when, evidently) someone gets it wrong? - z -- 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
On Fri, Aug 9, 2013 at 2:30 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:> Andreas Schwab <schwab@linux-m68k.org> writes: >> Josef Bacik <jbacik@fusionio.com> writes: >>> So stripe_len shouldn''t be 0, if it is you have bigger problems :).[ lost context: this is about the first do_div() in __btrfs_map_block() ]>> The bigger problem is that stripe_nr is u64, this is completely bogus. >> The first operand of do_div must be u32. This goes through the whole >> file. > > Of course, what I meant was that the *second* operand must be u32, but > that doesn''t change my point.I checked all do_div() implementations, and (unless I missed one) m68k and mn10300 were the only two that didn''t truncate base to 32 bits. Mn10300 is little endian, so I think the problem won''t happen there. Andreas, I''ll apply your patch (http://permalink.gmane.org/gmane.linux.ports.m68k/5008) with the crash log added to the commit message, so it''s appropriate for -stable. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There''s lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I''m talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Fri, 9 Aug 2013, Zach Brown wrote:> On Fri, Aug 09, 2013 at 02:26:36PM +0200, Andreas Schwab wrote: > > Josef Bacik <jbacik@fusionio.com> writes: > > > > > So stripe_len shouldn''t be 0, if it is you have bigger problems :). > > > > The bigger problem is that stripe_nr is u64, this is completely bogus. > > The first operand of do_div must be u32. This goes through the whole > > file.This was introduced by commit 53b381b3abeb86f12787a6c40fee9b2f71edc23b ("Btrfs: RAID5 and RAID6"), which changed the divisor from map->stripe_len (struct map_lookup.stripe_len is int) to a 64-bit temporary.> Definitely. Can we get some typeof() tricks in the macros to have the > build fail if (when, evidently) someone gets it wrong?Not using typeof, as there are way too many callsites where int is used instead of u32. However, checking that sizeof() equals to 4 seems to work. Below is a patch for asm-generic, which is untested, but it works when adding the same checks to arch/m68k/include/asm/div64.h This is not something we just want to drop in, as it has the potential of breaking lots of things (yes, it breaks btrfs :-)>From 7ccabf41beae38da514f3e09624219a9362375d7 Mon Sep 17 00:00:00 2001From: Geert Uytterhoeven <geert@linux-m68k.org> Date: Tue, 13 Aug 2013 18:04:40 +0200 Subject: [PATCH] asm-generic: Check divisor size in do_div() The second parameter of do_div() should be a 32-bit number. Enforce this using BUILD_BUG_ON(). The first parameter of do_div() should be a 64-bit number, enforce this on 64-bit architectures, just like is done on 32-bit architectures. Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> --- include/asm-generic/div64.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h index 8f4e319..69c0307 100644 --- a/include/asm-generic/div64.h +++ b/include/asm-generic/div64.h @@ -19,12 +19,15 @@ #include <linux/types.h> #include <linux/compiler.h> +#include <linux/bug.h> #if BITS_PER_LONG == 64 # define do_div(n,base) ({ \ uint32_t __base = (base); \ uint32_t __rem; \ + (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ + BUILD_BUG_ON(sizeof(base) != 4); \ __rem = ((uint64_t)(n)) % __base; \ (n) = ((uint64_t)(n)) / __base; \ __rem; \ @@ -41,6 +44,7 @@ extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor); uint32_t __base = (base); \ uint32_t __rem; \ (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ + BUILD_BUG_ON(sizeof(base) != 4); \ if (likely(((n) >> 32) == 0)) { \ __rem = (uint32_t)(n) % __base; \ (n) = (uint32_t)(n) / __base; \ -- 1.7.9.5 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There''s lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I''m talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Aug 13, 2013 at 6:32 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:> On Fri, 9 Aug 2013, Zach Brown wrote: >> On Fri, Aug 09, 2013 at 02:26:36PM +0200, Andreas Schwab wrote: >> > Josef Bacik <jbacik@fusionio.com> writes: >> > >> > > So stripe_len shouldn''t be 0, if it is you have bigger problems :). >> > >> > The bigger problem is that stripe_nr is u64, this is completely bogus. >> > The first operand of do_div must be u32. This goes through the whole >> > file. > > This was introduced by commit 53b381b3abeb86f12787a6c40fee9b2f71edc23b > ("Btrfs: RAID5 and RAID6"), which changed the divisor from > map->stripe_len (struct map_lookup.stripe_len is int) to a 64-bit > temporary. > >> Definitely. Can we get some typeof() tricks in the macros to have the >> build fail if (when, evidently) someone gets it wrong? > > Not using typeof, as there are way too many callsites where int is used > instead of u32. > > However, checking that sizeof() equals to 4 seems to work. > Below is a patch for asm-generic, which is untested, but it works when > adding the same checks to arch/m68k/include/asm/div64.h > > This is not something we just want to drop in, as it has the potential of > breaking lots of things (yes, it breaks btrfs :-)Found so far: - Several calls to sector_div() in blkdev_issue_discard() - Two calls to do_div() in sd_completed_bytes() Some of these even operate on dividends that never exceed 32-bit, tss...> >From 7ccabf41beae38da514f3e09624219a9362375d7 Mon Sep 17 00:00:00 2001 > From: Geert Uytterhoeven <geert@linux-m68k.org> > Date: Tue, 13 Aug 2013 18:04:40 +0200 > Subject: [PATCH] asm-generic: Check divisor size in do_div() > > The second parameter of do_div() should be a 32-bit number. > Enforce this using BUILD_BUG_ON(). > > The first parameter of do_div() should be a 64-bit number, > enforce this on 64-bit architectures, just like is done on 32-bit > architectures. > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- > include/asm-generic/div64.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h > index 8f4e319..69c0307 100644 > --- a/include/asm-generic/div64.h > +++ b/include/asm-generic/div64.h > @@ -19,12 +19,15 @@ > > #include <linux/types.h> > #include <linux/compiler.h> > +#include <linux/bug.h> > > #if BITS_PER_LONG == 64 > > # define do_div(n,base) ({ \ > uint32_t __base = (base); \ > uint32_t __rem; \ > + (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ > + BUILD_BUG_ON(sizeof(base) != 4); \ > __rem = ((uint64_t)(n)) % __base; \ > (n) = ((uint64_t)(n)) / __base; \ > __rem; \ > @@ -41,6 +44,7 @@ extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor); > uint32_t __base = (base); \ > uint32_t __rem; \ > (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ > + BUILD_BUG_ON(sizeof(base) != 4); \ > if (likely(((n) >> 32) == 0)) { \ > __rem = (uint32_t)(n) % __base; \ > (n) = (uint32_t)(n) / __base; \ > -- > 1.7.9.5Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There''s lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I''m talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 14, 2013 at 10:40 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:> On Tue, Aug 13, 2013 at 6:32 PM, Geert Uytterhoeven > <geert@linux-m68k.org> wrote: >> On Fri, 9 Aug 2013, Zach Brown wrote: >>> On Fri, Aug 09, 2013 at 02:26:36PM +0200, Andreas Schwab wrote: >>> > Josef Bacik <jbacik@fusionio.com> writes: >>> > >>> > > So stripe_len shouldn''t be 0, if it is you have bigger problems :). >>> > >>> > The bigger problem is that stripe_nr is u64, this is completely bogus. >>> > The first operand of do_div must be u32. This goes through the whole >>> > file. >> >> This was introduced by commit 53b381b3abeb86f12787a6c40fee9b2f71edc23b >> ("Btrfs: RAID5 and RAID6"), which changed the divisor from >> map->stripe_len (struct map_lookup.stripe_len is int) to a 64-bit >> temporary. >> >>> Definitely. Can we get some typeof() tricks in the macros to have the >>> build fail if (when, evidently) someone gets it wrong? >> >> Not using typeof, as there are way too many callsites where int is used >> instead of u32. >> >> However, checking that sizeof() equals to 4 seems to work. >> Below is a patch for asm-generic, which is untested, but it works when >> adding the same checks to arch/m68k/include/asm/div64.h >> >> This is not something we just want to drop in, as it has the potential of >> breaking lots of things (yes, it breaks btrfs :-) > > Found so far: > - Several calls to sector_div() in blkdev_issue_discard() > - Two calls to do_div() in sd_completed_bytes() > > Some of these even operate on dividends that never exceed 32-bit, tss...Two more: drivers/md/dm-cache-target.c:too_many_discard_blocks fs/jfs/jfs_dmap.c:dbDiscardAG These bring in the 64-bit divisor from somewhere else, so they''re less trivial to fix. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There''s lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I''m talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Wed, 2013-08-14 at 10:56 +0200, Geert Uytterhoeven wrote:> These bring in the 64-bit divisor from somewhere else, so they''re less > trivial to fix.Using div64_u64 or div64_s64 could fix it. Maybe that could be added to do_div too.