| Message ID | 20170408210737.5456-1-kilobyte@angband.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Apr 08, 2017 at 11:07:37PM +0200, Adam Borowski wrote: > Unbreaks ARM and possibly other 32-bit architectures. Turns out those "other 32-bit architectures" happen to include i386. A modular build: ERROR: "__udivdi3" [fs/btrfs/btrfs.ko] undefined! With the patch, i386 builds fine. > Tested on amd64 where all is fine, and on arm (Odroid-U2) where scrub > sometimes works, but, like most operations, randomly dies with some badness > that doesn't look related: io_schedule, kunmap_high. That badness wasn't > there in 4.11-rc5, needs investigating, but since it's not connected to our > issue at hand, I consider this patch sort-of tested. Looks like current -next is pretty broken: while amd64 is ok, on an i386 box (non-NX Pentium 4) it hangs very early during boot, way before filesystem modules would be loaded. Qemu boots but has random hangs. So it looks like it's compile only for now...
On Sun, Apr 09, 2017 at 05:58:54AM +0200, Adam Borowski wrote: > On Sat, Apr 08, 2017 at 11:07:37PM +0200, Adam Borowski wrote: > > Unbreaks ARM and possibly other 32-bit architectures. > > Turns out those "other 32-bit architectures" happen to include i386. > > A modular build: > ERROR: "__udivdi3" [fs/btrfs/btrfs.ko] undefined! > With the patch, i386 builds fine. > > > Tested on amd64 where all is fine, and on arm (Odroid-U2) where scrub > > sometimes works, but, like most operations, randomly dies with some badness > > that doesn't look related: io_schedule, kunmap_high. That badness wasn't > > there in 4.11-rc5, needs investigating, but since it's not connected to our > > issue at hand, I consider this patch sort-of tested. > > Looks like current -next is pretty broken: while amd64 is ok, on an i386 box > (non-NX Pentium 4) it hangs very early during boot, way before filesystem > modules would be loaded. Qemu boots but has random hangs. A non-modular i386_defconfig + btrfs of -next is ok; whatever the problem is, it's not relevant to our division build failure in scrub. But, it looks like parity scrub is ${EXPLETIVE}ed on 32-bit. Not just on -next, also on 4.11-rc5 and 4.9. Test script I used is attached, although it's enough to just scrub a kosher filesystem without even damaging it. On 64-bit it mostly works, but still warns about bogus unrecoverable errors when in fact it succeeded. Thus, I'd recommend: * applying this patch to at least make it compile * taking steps to warn outside people about RAID5/6 Let's discuss the rest in another thread, it's no longer interesting to ARM people, they just want no build failures.
On Sat, Apr 08, 2017 at 11:07:37PM +0200, Adam Borowski wrote: > Unbreaks ARM and possibly other 32-bit architectures. > > Fixes: 7d0ef8b4d: Btrfs: update scrub_parity to use u64 stripe_len > Reported-by: Icenowy Zheng <icenowy@aosc.io> > Signed-off-by: Adam Borowski <kilobyte@angband.pl> > --- > You'd probably want to squash this with Liu's commit, to be nice to future > bisects. Thanks for finding it! > Tested on amd64 where all is fine, and on arm (Odroid-U2) where scrub > sometimes works, but, like most operations, randomly dies with some badness > that doesn't look related: io_schedule, kunmap_high. That badness wasn't > there in 4.11-rc5, needs investigating, but since it's not connected to our > issue at hand, I consider this patch sort-of tested. > > fs/btrfs/scrub.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index b6fe1cd08048..95372e3679f3 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -2407,7 +2407,7 @@ static inline void __scrub_mark_bitmap(struct scrub_parity *sparity, > > start -= sparity->logic_start; > start = div64_u64_rem(start, sparity->stripe_len, &offset); > - offset /= sectorsize; > + do_div(offset, sectorsize); I'll use the div_u64 helper instead, I don't want to reintroduce do_div to fs/btrfs , for-next will be updated in a minute.
On 2017-04-08 17:07, Adam Borowski wrote: > Unbreaks ARM and possibly other 32-bit architectures. > > Fixes: 7d0ef8b4d: Btrfs: update scrub_parity to use u64 stripe_len > Reported-by: Icenowy Zheng <icenowy@aosc.io> > Signed-off-by: Adam Borowski <kilobyte@angband.pl> > --- > You'd probably want to squash this with Liu's commit, to be nice to future > bisects. > > Tested on amd64 where all is fine, and on arm (Odroid-U2) where scrub > sometimes works, but, like most operations, randomly dies with some badness > that doesn't look related: io_schedule, kunmap_high. That badness wasn't > there in 4.11-rc5, needs investigating, but since it's not connected to our > issue at hand, I consider this patch sort-of tested. > > fs/btrfs/scrub.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index b6fe1cd08048..95372e3679f3 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -2407,7 +2407,7 @@ static inline void __scrub_mark_bitmap(struct scrub_parity *sparity, > > start -= sparity->logic_start; > start = div64_u64_rem(start, sparity->stripe_len, &offset); > - offset /= sectorsize; > + do_div(offset, sectorsize); > nsectors = (int)len / sectorsize; > > if (offset + nsectors <= sparity->nsectors) { > Also fixes things on: 32-bit MIPS (eb and el variants) 32-bit SPARC 32-bit PPC You can add my Tested-by if you want.
On Sat, Apr 08, 2017 at 11:07:37PM +0200, Adam Borowski wrote: > Unbreaks ARM and possibly other 32-bit architectures. > Thanks a lot for the fix. Reviewed-by: Liu Bo <bo.li.liu@oracle.com> Thanks, -liubo > Fixes: 7d0ef8b4d: Btrfs: update scrub_parity to use u64 stripe_len > Reported-by: Icenowy Zheng <icenowy@aosc.io> > Signed-off-by: Adam Borowski <kilobyte@angband.pl> > --- > You'd probably want to squash this with Liu's commit, to be nice to future > bisects. > > Tested on amd64 where all is fine, and on arm (Odroid-U2) where scrub > sometimes works, but, like most operations, randomly dies with some badness > that doesn't look related: io_schedule, kunmap_high. That badness wasn't > there in 4.11-rc5, needs investigating, but since it's not connected to our > issue at hand, I consider this patch sort-of tested. > > fs/btrfs/scrub.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index b6fe1cd08048..95372e3679f3 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -2407,7 +2407,7 @@ static inline void __scrub_mark_bitmap(struct scrub_parity *sparity, > > start -= sparity->logic_start; > start = div64_u64_rem(start, sparity->stripe_len, &offset); > - offset /= sectorsize; > + do_div(offset, sectorsize); > nsectors = (int)len / sectorsize; > > if (offset + nsectors <= sparity->nsectors) { > -- > 2.11.0 > > -- > 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
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index b6fe1cd08048..95372e3679f3 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -2407,7 +2407,7 @@ static inline void __scrub_mark_bitmap(struct scrub_parity *sparity, start -= sparity->logic_start; start = div64_u64_rem(start, sparity->stripe_len, &offset); - offset /= sectorsize; + do_div(offset, sectorsize); nsectors = (int)len / sectorsize; if (offset + nsectors <= sparity->nsectors) {
Unbreaks ARM and possibly other 32-bit architectures. Fixes: 7d0ef8b4d: Btrfs: update scrub_parity to use u64 stripe_len Reported-by: Icenowy Zheng <icenowy@aosc.io> Signed-off-by: Adam Borowski <kilobyte@angband.pl> --- You'd probably want to squash this with Liu's commit, to be nice to future bisects. Tested on amd64 where all is fine, and on arm (Odroid-U2) where scrub sometimes works, but, like most operations, randomly dies with some badness that doesn't look related: io_schedule, kunmap_high. That badness wasn't there in 4.11-rc5, needs investigating, but since it's not connected to our issue at hand, I consider this patch sort-of tested. fs/btrfs/scrub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)