On Fri, Aug 12, 2011 at 05:45:33PM +0800, Peter Zijlstra wrote:
> On Fri, 2011-08-12 at 13:45 +0800, Wu Fengguang wrote:
> > Code is
> >
> > unsigned long freerun = (thresh + bg_thresh) / 2;
> >
> > setpoint = (limit + freerun) / 2;
> > pos_ratio = abs(dirty - setpoint);
> > pos_ratio <<= BANDWIDTH_CALC_SHIFT;
> > do_div(pos_ratio, limit - setpoint + 1);
>
> Why do you use do_div()? from the code those things are unsigned long,
> and you can divide that just fine.
Because pos_ratio was "unsigned long long"..
> Also, there's div64_s64 that can do signed divides for s64 types.
> That'll loose the extra conditionals you used for abs and putting the
> sign back.
Ah ok, good to know that :)
> > x = pos_ratio;
> > pos_ratio = pos_ratio * x >> BANDWIDTH_CALC_SHIFT;
> > pos_ratio = pos_ratio * x >> BANDWIDTH_CALC_SHIFT;
>
> So on 32bit with unsigned long that gets 32=2*(10+b) bits for x, that
> solves to 6, which isn't going to be enough I figure since
> (dirty-setpoint) !< 64.
>
> So you really need to use u64/s64 types here, unsigned long just won't
> do, with u64 you have 64=2(10+b) 22 bits for x, which should fit.
Sure, here is the updated code:
long long pos_ratio; /* for scaling up/down the rate limit */
long x;
if (unlikely(dirty >= limit))
return 0;
/*
* global setpoint
*
* setpoint - dirty 3
* f(dirty) := 1 + (----------------)
* limit - setpoint
*
* it's a 3rd order polynomial that subjects to
*
* (1) f(freerun) = 2.0 => rampup base_rate reasonably fast
* (2) f(setpoint) = 1.0 => the balance point
* (3) f(limit) = 0 => the hard limit
* (4) df/dx < 0 => negative feedback control
* (5) the closer to setpoint, the smaller |df/dx| (and the reverse),
* => fast response on large errors; small oscillation near setpoint
*/
setpoint = (limit + freerun) / 2;
pos_ratio = (setpoint - dirty) << RATELIMIT_CALC_SHIFT;
pos_ratio = div_s64(pos_ratio, limit - setpoint + 1);
x = pos_ratio;
pos_ratio = pos_ratio * x >> RATELIMIT_CALC_SHIFT;
pos_ratio = pos_ratio * x >> RATELIMIT_CALC_SHIFT;
pos_ratio += 1 << RATELIMIT_CALC_SHIFT;
Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
