Bug#695182: linux-image-3.2.0-4-686-pae: Write couple of 1GB files for OOM crash
Dear Ben,
> ... I think the initial overflow occurs when calculating x
> ...
> setpoint and dirty are numbers of pages and are declared as long, so on
> a system with enough memory they can presumably differ by 2^21 or more
> (2^21 pages = 8 GB). Shifting left by RATELIMIT_CALC_SHIFT = 10 can
> then change the sign bit.
>
> Does the attached patch fix this?
>
> ...
>
> Most variables in bdi_position_ratio() are declared long, which is
> enough for a page count. However, when converting (setpoint - dirty)
> to a fixed-point number we left-shift by 10, and on a 32-bit system
> with PAE it is possible to have enough dirty pages that the shift
> overflows into the sign bit. We need to cast to s64 before the
> left-shift.
>
> Reported-by: Paul Szabo <paul.szabo@sydney.edu.au>
> Reference: http://bugs.debian.org/695182
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> mm/page-writeback.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 50f0824..8b5600e 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -559,7 +559,7 @@ static unsigned long bdi_position_ratio(struct backing_dev_i
> nfo *bdi,
> * => fast response on large errors; small oscillation near setpoint
> */
> setpoint = (freerun + limit) / 2;
> - x = div_s64((setpoint - dirty) << RATELIMIT_CALC_SHIFT,
> + x = div_s64((s64)(setpoint - dirty) << RATELIMIT_CALC_SHIFT,
> limit - setpoint + 1);
> pos_ratio = x;
> pos_ratio = pos_ratio * x >> RATELIMIT_CALC_SHIFT;
Thanks for the quick patch. I am about to test it (in a day or so).
Initial (blind, off-the-cuff, uneducated) comments:
- I had "BUG_ON(x<0)" in my code, so unlikely x changed sign.
- Why not use float instead of infinite-precision integer arithmetic?
- Do we need a "smooth" function, or would an easy-to-calculate
step function suffice?
- Is there a check that the returned s64 pos_ratio fits into u32?
Cheers, Paul
Paul Szabo psz@maths.usyd.edu.au http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics University of Sydney Australia
Reply to: