Re: btrfs zero divide
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
On Wed, Aug 14, 2013 at 10:40 AM, Geert Uytterhoeven
wrote:
> On Tue, Aug 13, 2013 at 6:32 PM, Geert Uytterhoeven
> wrote:
>> On Fri, 9 Aug 2013, Zach Brown wrote:
>>> On Fri, Aug 09, 2013 at 02:26:36PM +0200, Andreas Schwab wrote:
>>> > Josef Bacik 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
On Tue, Aug 13, 2013 at 6:32 PM, Geert Uytterhoeven
wrote:
> On Fri, 9 Aug 2013, Zach Brown wrote:
>> On Fri, Aug 09, 2013 at 02:26:36PM +0200, Andreas Schwab wrote:
>> > Josef Bacik 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
> 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
> ---
> 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
> #include
> +#include
>
> #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
On Fri, 9 Aug 2013, Zach Brown wrote:
> On Fri, Aug 09, 2013 at 02:26:36PM +0200, Andreas Schwab wrote:
> > Josef Bacik 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
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
---
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
#include
+#include
#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
On Fri, Aug 9, 2013 at 2:30 PM, Andreas Schwab wrote:
> Andreas Schwab writes:
>> Josef Bacik 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
On Fri, Aug 09, 2013 at 02:26:36PM +0200, Andreas Schwab wrote: > Josef Bacik 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
On Fri, Aug 09, 2013 at 02:30:38PM +0200, Andreas Schwab wrote: > Andreas Schwab writes: > > > Josef Bacik 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
Andreas Schwab writes: > Josef Bacik 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
Josef Bacik 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, 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
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
+#include
+#include
/* 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 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
[8175
Re: btrfs zero divide
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 -- you introduced a merge commit│ % g rebase -i HEAD^^ sorry, no idea and rebasing just fscked │ Segmentation should have cloned into a clean repo │ fault (core dumped) if I rebase that now, it's really ugh │ 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
Re: btrfs zero divide
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
#include
+#include
+#include
#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
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
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
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 m
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); -- 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))
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 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
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
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.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 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] [] 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] [<000e
