Re: btrfs zero divide

2013-08-14 Thread Geert Uytterhoeven 
On Tue, Aug 13, 2013 at 6:32 PM, Geert Uytterhoeven
ge...@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 jba...@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 ge...@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 ge...@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 -- ge...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: btrfs zero divide

2013-08-14 Thread Geert Uytterhoeven 
On Wed, Aug 14, 2013 at 10:40 AM, Geert Uytterhoeven
ge...@linux-m68k.org wrote:
 On Tue, Aug 13, 2013 at 6:32 PM, Geert Uytterhoeven
 ge...@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 jba...@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 -- ge...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: btrfs zero divide

2013-08-14 Thread Joe Perches 
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.

--
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 zero divide

2013-08-13 Thread Geert Uytterhoeven 
On Fri, Aug 9, 2013 at 2:30 PM, Andreas Schwab sch...@linux-m68k.org wrote:
 Andreas Schwab sch...@linux-m68k.org writes:
 Josef Bacik jba...@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 -- ge...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: btrfs zero divide

2013-08-13 Thread Geert Uytterhoeven 
On Fri, 9 Aug 2013, Zach Brown wrote:
 On Fri, Aug 09, 2013 at 02:26:36PM +0200, Andreas Schwab wrote:
  Josef Bacik jba...@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 2001
From: Geert Uytterhoeven ge...@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 ge...@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 -- ge...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: btrfs zero divide

2013-08-09 Thread Andreas Schwab 
Andreas Schwab sch...@linux-m68k.org writes:

 Josef Bacik jba...@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, sch...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: btrfs zero divide

2013-08-09 Thread Josef Bacik 
On Fri, Aug 09, 2013 at 02:30:38PM +0200, Andreas Schwab wrote:
 Andreas Schwab sch...@linux-m68k.org writes:
 
  Josef Bacik jba...@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
--
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 zero divide

2013-08-09 Thread Zach Brown 
On Fri, Aug 09, 2013 at 02:26:36PM +0200, Andreas Schwab wrote:
 Josef Bacik jba...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: btrfs zero divide

2013-08-08 Thread Thorsten Glaser 
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.orig2013-08-08 19:34:32.663540965 +
+++ -   2013-08-08 19:47:30.309776791 +
@@ -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.37] bio: create slab bio-1 at 1
[817508.51] Btrfs loaded
[817524.11] loop: module loaded
[817534.86] device fsid 01cfa645-5cde-4e4c-9b0b-df7b37bdc495 devid 1 
transid 4 /dev/loop0
[817534.86] btrfs: disk space caching is enabled
[817534.86] *** ZERO DIVIDE ***   FORMAT=2
[817534.86] Current process id is 32312
[817534.86] BAD KERNEL TRAP: 
[817534.86] 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.86] PC: [31c46612] __btrfs_map_block+0x134/0x147a [btrfs]
[817534.86] SR: 2000  SP: 0249fab0  a2: 3010f660
[817534.86] d0: d1: 00022000d2: d3: 
[817534.86] d4: 0001d5: 0001a0: 021777a4a1: 021777a4
[817534.86] Process mount (pid: 32312, task=3010f660)
[817534.86] Frame format=2 instr addr=31c4660e
[817534.86] Stack from 0249fae8:
 0020  1000  00022000 0766a928 07621800
00415d84 0070 077a97c0 0070 0249fb68 0009e250 00d106c0 00011220
0070 0020  00022000 00ff 0009 1000 
 021777a4  0020  0249fd14 0009e26c 0020
0003  0009dd8a 3007c02c 0766a928 00415d84 1000 
 0110 31c417ae 0766a928 00415d84 1000  
[817534.86] Call Trace: [1000] kernel_pg_dir+0x0/0x1000
[817534.86]  [00022000] _060_fpsp_effadd+0xb2c0/0xd518
[817534.86]  [0009e250] bvec_alloc+0xa2/0xbe
[817534.86]  [00011220] sasin+0x87c/0x944
[817534.86]  [00022000] _060_fpsp_effadd+0xb2c0/0xd518
[817534.86]  [1000] kernel_pg_dir+0x0/0x1000
[817534.86]  [0009e26c] bio_alloc_bioset+0x0/0x12e
[817534.86]  [0009dd8a] bio_add_page+0x4a/0x58
[817534.86]  [1000] kernel_pg_dir+0x0/0x1000
[817534.86]  [31c417ae] submit_extent_page.isra.44+0x170/0x1bc [btrfs]
[817534.86]  [1000] kernel_pg_dir+0x0/0x1000
[817534.86]  [1000] kernel_pg_dir+0x0/0x1000
[817534.86]  [31c4cbfe] btrfs_map_bio+0x60/0x48c [btrfs]
[817534.86]  [00022000] _060_fpsp_effadd+0xb2c0/0xd518
[817534.86]  [00022000] _060_fpsp_effadd+0xb2c0/0xd518
[817534.86]  [31c24bb2] btree_submit_bio_hook+0x0/0xae [btrfs]
[817534.86]  [31c41ae4] end_bio_extent_readpage+0x0/0x69c [btrfs]
[817534.86]  [1000] kernel_pg_dir+0x0/0x1000
[817534.86]  [31c24984] btrfs_bio_wq_end_io+0x16/0x50 [btrfs]
[817534.86]  [31c24c0e] btree_submit_bio_hook+0x5c/0xae [btrfs]
[817534.87]  [00022000] _060_fpsp_effadd+0xb2c0/0xd518
[817534.87]  [31c3ed7a] submit_one_bio+0x7c/0xb2 [btrfs]
[817534.87]  [00022000] _060_fpsp_effadd+0xb2c0/0xd518
[817534.87]  [31c421b8] __extent_read_full_page+0x0/0x70a [btrfs]
[817534.87]  [00058828] unlock_page+0x0/0x26
[817534.87]  [31c44780] read_extent_buffer_pages+0x1a8/0x218 [btrfs]
[817534.88]  [31c4c3b2] btrfs_num_copies+0x0/0x142 [btrfs]
[817534.88]  [31c23aa6] 
btree_read_extent_buffer_pages.constprop.52+0x42/0xca [btrfs]
[817534.88]  [31c22802] btree_get_extent+0x0/0x102 [btrfs]
[817534.88]  [00022000] _060_fpsp_effadd+0xb2c0/0xd518
[817534.88]  [1000] kernel_pg_dir+0x0/0x1000
[817534.88]  [31c2525e] read_tree_block+0x38/0x48 [btrfs]
[817534.88]  [31c25226] read_tree_block+0x0/0x48 [btrfs]
[817534.89]  [31c26d40] open_ctree+0xe80/0x15e6 [btrfs]
[817534.89]  [00022000] _060_fpsp_effadd+0xb2c0/0xd518
[817534.89]  [1000] kernel_pg_dir+0x0/0x1000
[817534.89]  [1000] kernel_pg_dir+0x0/0x1000
[817534.89]  [1000] kernel_pg_dir+0x0/0x1000

btrfs zero divide (was: Re: Linux 3.10 problem reports (yes, plural))

2013-07-30 Thread Geert Uytterhoeven 
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.00] Linux version 3.10-1-m68k (debian-ker...@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.72] bio: create slab bio-1 at 1
 [6.74] Btrfs loaded
 [6.83] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3
 [7.15] EXT4-fs (nfhd8p1): mounted filesystem with ordered data mode. 
 Opts: (null)
 [   14.52] udevd[228]: starting version 175
 [   17.82] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3
 [   20.85] Adding 3670012k swap on /dev/nfhd8p2.  Priority:-1 extents:1 
 across:3670012k
 [   21.38] EXT4-fs (nfhd8p1): re-mounted. Opts: (null)
 [   31.30] EXT4-fs (nfhd8p1): re-mounted. Opts: errors=remount-ro
 [   38.46] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3
 [   38.53] btrfs: setting nodatacow, compression disabled
 [   38.54] btrfs: enabling auto recovery
 [   38.57] btrfs: disk space caching is enabled
 [   38.60] *** ZERO DIVIDE ***   FORMAT=2
 [   38.63] Current process id is 722
 [   38.66] BAD KERNEL TRAP: 
 [   38.68] Modules linked in: evdev mac_hid ext4 crc16 jbd2 mbcache btrfs 
 xor lzo_compress zlib_deflate raid6_pq crc32c libcrc32c
 [   38.73] PC: [319535b2] __btrfs_map_block+0x11c/0x119a [btrfs]

Woops, adding the btrfs devs to CC.

 [   38.77] SR: 2000  SP: 30c1fab4  a2: 30f0faf0
 [   38.80] d0: d1: 1000d2: d3: 
 [   38.83] d4: 0001d5: a0: 3085c72ca1: 3085c72c
 [   38.85] Process mount (pid: 722, task=30f0faf0)
 [   38.87] Frame format=2 instr addr=319535ae
 [   38.88] Stack from 30c1faec:
 [   38.88]  0020  1000  01401000 
 30253928 300ffc00
 [   38.88] 00a843ac 3026f640  0001 0009e250 00d106c0 
 00011220 
 [   38.88] 1000 301c6830 0009e32a 00ff 0009 3085c72c 
  
 [   38.88] 30c1fd14  0020  30c1fd14 0009e26c 
 0020 0003
 [   38.88]  0009dd8a 300b0b6c 30253928 00a843ac 1000 
  
 [   38.88] a008 3194e76a 30253928 00a843ac 1000  
  0002
 [   39.19] Call Trace: [1000] kernel_pg_dir+0x0/0x1000
 [   39.21]  [0001] res_func+0x1020/0x141a
 [   39.25]  [0009e250] bvec_alloc+0xa2/0xbe
 [   39.27]  [00011220] sasin+0x87c/0x944
 [   39.29]  [1000] kernel_pg_dir+0x0/0x1000
 [   39.33]  [0009e32a] bio_alloc_bioset+0xbe/0x12e
 [   39.36]  [0009e26c] bio_alloc_bioset+0x0/0x12e
 [   39.38]  [0009dd8a] bio_add_page+0x4a/0x58
 [   39.42]  [1000] kernel_pg_dir+0x0/0x1000
 [   39.47]  [a008] via_nubus_irq+0x1c/0xa2
 [   39.50]  [3194e76a] submit_extent_page.isra.44+0x170/0x1bc [btrfs]
 [   39.53]  [1000] kernel_pg_dir+0x0/0x1000
 [   39.56]  [1000] kernel_pg_dir+0x0/0x1000
 [   39.60]  [31959778] btrfs_map_bio+0x60/0x48c [btrfs]
 [   39.63]  [31931b72] btree_submit_bio_hook+0x0/0xae [btrfs]
 [   39.66]  [3194eaa0] end_bio_extent_readpage+0x0/0x69c [btrfs]
 [   39.71]  [1000] kernel_pg_dir+0x0/0x1000
 [   39.73]  [31931944] btrfs_bio_wq_end_io+0x16/0x50 [btrfs]
 [   39.76]  [31931bce] btree_submit_bio_hook+0x5c/0xae [btrfs]
 [   39.78]  [3194bd36] submit_one_bio+0x7c/0xb2 [btrfs]
 [   39.81]  [3194f174] __extent_read_full_page+0x0/0x70a [btrfs]
 [   39.83]  [00058828] unlock_page+0x0/0x26
 [   39.84]  [31951736] read_extent_buffer_pages+0x1a8/0x218 [btrfs]
 [   39.89]  [00027d81] devkmsg_read+0x213/0x39a
 [   39.93]  [31959006] btrfs_num_copies+0x0/0x142 [btrfs]
 [   39.97]  [31930a66] 
 btree_read_extent_buffer_pages.constprop.52+0x42/0xca [btrfs]
 [   40.03]  [3192f7c2] btree_get_extent+0x0/0x102 [btrfs]
 [   40.06]  [00027d81] devkmsg_read+0x213/0x39a
 [   40.09]  [1000] kernel_pg_dir+0x0/0x1000
 [   40.10]  [3193221e] read_tree_block+0x38/0x48 [btrfs]
 [   40.13]  [00027d81] devkmsg_read+0x213/0x39a
 [   40.14]  [319321e6] read_tree_block+0x0/0x48 [btrfs]
 [   40.17]  [31933d00] open_ctree+0xe80/0x15e6 [btrfs]
 [   40.20]  [1000] kernel_pg_dir+0x0/0x1000
 [   40.22]  [00027d81] devkmsg_read+0x213/0x39a
 [   40.23]  [1000] kernel_pg_dir+0x0/0x1000
 [   40.26]  [000f280a] resource_string.isra.12+0x2b4/0x2ee
 [   40.28]  [1000] kernel_pg_dir+0x0/0x1000
 [   40.32]  [1000] kernel_pg_dir+0x0/0x1000
 [   40.35]  [000e59ba] disk_name+0x72/0x80
 [   40.36]  [aff0] mac_hwclk.part.0+0xe6/0x174
 [   40.39]  [31913ede] btrfs_mount+0x450/0x73e [btrfs]
 [   40.41]

Re: btrfs zero divide

2013-07-30 Thread Thorsten Glaser 
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:  2205movel %d5,%d1
  1a:  4c2e 1800 ff68  mulsl %fp@(-152),%d1
  20:  4c04 0800   mulsl %d4,%d0
  24:  2041moveal %d1,%a0
  26:  d1c0addal %d0,%a0
  28:  2206movel %d6,%d1
  2a:  4c2e 1400 ff68  mulul %fp@(-152),%d0,%d1

This 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
--
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 zero divide (was: Re: Linux 3.10 problem reports (yes, plural))

2013-07-30 Thread Josef Bacik 
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.00] Linux version 3.10-1-m68k (debian-ker...@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.72] bio: create slab bio-1 at 1
  [6.74] Btrfs loaded
  [6.83] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3
  [7.15] EXT4-fs (nfhd8p1): mounted filesystem with ordered data 
  mode. Opts: (null)
  [   14.52] udevd[228]: starting version 175
  [   17.82] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3
  [   20.85] Adding 3670012k swap on /dev/nfhd8p2.  Priority:-1 extents:1 
  across:3670012k
  [   21.38] EXT4-fs (nfhd8p1): re-mounted. Opts: (null)
  [   31.30] EXT4-fs (nfhd8p1): re-mounted. Opts: errors=remount-ro
  [   38.46] device label ara5-butter devid 1 transid 376178 /dev/nfhd8p3
  [   38.53] btrfs: setting nodatacow, compression disabled
  [   38.54] btrfs: enabling auto recovery
  [   38.57] btrfs: disk space caching is enabled
  [   38.60] *** ZERO DIVIDE ***   FORMAT=2
  [   38.63] Current process id is 722
  [   38.66] BAD KERNEL TRAP: 
  [   38.68] Modules linked in: evdev mac_hid ext4 crc16 jbd2 mbcache 
  btrfs xor lzo_compress zlib_deflate raid6_pq crc32c libcrc32c
  [   38.73] 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
--
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 zero divide (was: Re: Linux 3.10 problem reports (yes, plural))

2013-07-30 Thread Joe Perches 
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);


--
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 zero divide

2013-07-30 Thread Thorsten Glaser 
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 *0x106e
0x106e is in __btrfs_map_block 
(/var/lib/gforge/chroot/home/users/tg/Xl/linux-3.10.1/fs/btrfs/volumes.c:4447).
4442stripe_nr = offset;
4443/*
 * stripe_nr counts the total number of stripes we have to 
stride
4445 * to get to this block
4446 */
4447do_div(stripe_nr, stripe_len);
4448
4449stripe_offset = stripe_nr * stripe_len;
4450BUG_ON(offset  stripe_offset);
4451

The code is somewhat matching:

   0x1062 +268:   movel %fp@(-140),%d1
   0x1066 +272:   movel %fp@(-164),%d5
   0x106a +276:   movel %fp@(-160),%d6
   0x106e +280:   divul %d5,%d2,%d1
   0x1072 +284:   movel %d0,%fp@(-156)
   0x1076 +288:   movel %d1,%fp@(-152)
   0x107a +292:   movel %d5,%d1
   0x107c +294:   mulsl %fp@(-152),%d1
   0x1082 +300:   mulsl %d4,%d0
   0x1086 +304:   moveal %d1,%a0
   0x1088 +306:   addal %d0,%a0
   0x108a +308:   movel %d6,%d1
   0x108c +310:   mulul %fp@(-152),%d0,%d1

According to the registers…

 [   38.80] d0: d1: 1000d2: d3: 
 [   38.83] d4: 0001d5: a0: 3085c72ca1: 3085c72c

… this is (if I parse this right) 1000 / 
(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:
   0x104a __btrfs_map_block+244:  20 02   movel %d2,%d0
   0x104c __btrfs_map_block+246:  24 2e ff 70 movel %fp@(-144),%d2
   0x1050 __btrfs_map_block+250:  4a 80   tstl %d0
   0x1052 __btrfs_map_block+252:  67 0e   beqs 0x1062 
__btrfs_map_block+268
   0x1054 __btrfs_map_block+254:  20 02   movel %d2,%d0
   0x1056 __btrfs_map_block+256:  2c 2e ff 5c movel %fp@(-164),%d6
   0x105a __btrfs_map_block+260:  2e 2e ff 60 movel %fp@(-160),%d7
   0x105e __btrfs_map_block+264:  4c 46 00 02 divull %d6,%d2,%d0
   0x1062 __btrfs_map_block+268:  22 2e ff 74 movel %fp@(-140),%d1
   0x1066 __btrfs_map_block+272:  2a 2e ff 5c movel %fp@(-164),%d5
   0x106a __btrfs_map_block+276:  2c 2e ff 60 movel %fp@(-160),%d6
   0x106e __btrfs_map_block+280:  4c 45 14 02 divul %d5,%d2,%d1
   0x1072 __btrfs_map_block+284:  2d 40 ff 64 movel %d0,%fp@(-156)
   0x1076 __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
--
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

Re: btrfs zero divide

2013-07-30 Thread Josef Bacik 
On Tue, Jul 30, 2013 at 07:02:29PM +, 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 *0x106e
 0x106e is in __btrfs_map_block 
 (/var/lib/gforge/chroot/home/users/tg/Xl/linux-3.10.1/fs/btrfs/volumes.c:4447).
 4442stripe_nr = offset;
 4443/*
  * stripe_nr counts the total number of stripes we have to 
 stride
 4445 * to get to this block
 4446 */
 4447do_div(stripe_nr, stripe_len);
 4448
 4449stripe_offset = stripe_nr * stripe_len;
 4450BUG_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
--
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 zero divide

2013-07-30 Thread Joe Perches 
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 == ?? */


--
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 zero divide

2013-07-30 Thread Thorsten Glaser 
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 of

I 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_div

Mh. 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 wuahh
--
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