On Mon, 2012-12-17 at 13:09 +1100, paul.sz...@sydney.edu.au wrote: > Dear Ben, > > In response to your comments: x seems to be in the range [-1,1]. The > returned pos_ratio would be within [0,2] if not for the final *8. > > --- > > [Funny: taking difference of unsigned ints and expect the result to be > negative in some sense. Seems the problem was not with large memory but > with negative numbers. Curious the bug was not noticed before.]
WTF. > Need to cast and sign-extend before taking difference of unsigned > numbers, as the following demonstrates: Sorry, I was thinking they were long and not unsigned long. Again, this happens to work on a 64-bit machine. [...] > Seems a correct patch would be: > > --- old/mm/page-writeback.c 2012-10-17 13:50:15.000000000 +1100 > +++ new/mm/page-writeback.c 2012-12-17 12:25:14.000000000 +1100 > @@ -559,7 +559,7 @@ > * => 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 - (s64)dirty) << RATELIMIT_CALC_SHIFT, > limit - setpoint + 1); > pos_ratio = x; > pos_ratio = pos_ratio * x >> RATELIMIT_CALC_SHIFT; > > However, with that patch in place I still got an OOM crash (log below). > More bugs remain... [...] It's not a crash, though that's kind of an academic distinction. Perhaps you could add some printk() statements to log the results of these various calculations, so you can sanity-check them. You would probably want to make them conditional on the intitial value of x being negative (it's reused for something entirely different later so you would need to assign this condition to a separate variable). Ben. -- Ben Hutchings Life is like a sewer: what you get out of it depends on what you put into it.
signature.asc
Description: This is a digitally signed message part
