Re: [build breakage] Re: [GIT PULL] Btrfs pull round two

From: Linus Torvalds
Date: Fri Aug 19 2011 - 11:14:57 EST


On Fri, Aug 19, 2011 at 6:24 AM, Josh Boyer <jwboyer@xxxxxxxxx> wrote:
>
> FYI, this was noticed and a patch was posted to linux-btrfs:
>
> http://thread.gmane.org/gmane.comp.file-systems.btrfs/12673
>
> Josef bugged them to send it upstream.

Oh Christ!

Please don't send that upstream.

This:

- min_free /= 2;
+ do_div(min_free, 2);

is just moronic. Guys, YOU ARE DIVIDING A UNSIGNED VALUE BY TWO. Has
anybody ever looked at a real computer? Doesn't anybody know how
computer math works any more? Doing that as a (slow) double division
on 32-bit is so stupid that it's past even just "wrong". It's way off
in la-la-land, sitting in a corner, all hopped up on drugs and
painting its nails purple.

Also, the change that is definitely correct:

- min_free /= dev_min;
+ do_div(min_free, dev_min);

should still be something that people think about. Why is "dev_min" a
signed integer? That looks wrong. You wanted an unsigned divide,
didn't you (and that is what "do_div()" is designed for).

So guys, please: when you get an error like this, don't just go into
that mindless place where you paper things over. Think about what is
going on, and WHY you got the error. Big 64-bit divides are bad bad
bad. They are so horrendously bad on 32-bit that we don't even support
them (which is the reason I refuse to put that __udiv into the 32-bit
libraries), but that are often bad on 64-bit too. So you need to spend
some time thinking about it when you get that __udivdi3 error.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/