On Tue, Jun 26, 2018 at 7:21 AM, Song Liu <liu.song....@gmail.com> wrote:
> On Sun, Jun 24, 2018 at 8:54 PM, Jakub Kicinski
> <jakub.kicin...@netronome.com> wrote:
>> From: Jiong Wang <jiong.w...@netronome.com>
<snip>
>> +
>> +struct reciprocal_value_adv reciprocal_value_adv(u32 d, u8 prec)
>> +{
>> + struct reciprocal_value_adv R;
>> + u32 l, post_shift;
>> + u64 mhigh, mlow;
>> +
>> + l = fls(d - 1);
>> + post_shift = l;
>> + /* NOTE: mlow/mhigh could overflow u64 when l == 32 which means d has
>> + * MSB set. This case needs to be handled before calling
>> + * "reciprocal_value_adv", please see the comment at
>> + * include/linux/reciprocal_div.h.
>> + */
>
> Shall we handle l == 32 case better? I guess the concern here is extra
> handling may
> slow down the fast path?
The implementation of "reciprocal_value_adv" hasn't considered l ==
32 which will make the code more complex.
As described at the pseudo code about how to call
"reciprocal_value_adv" in include/linux/reciprocal_div.h, l == 32
means the MSB of dividend is set, so the result of unsigned
divisor/dividend could only be 0 or 1, so the divide result could be
easily get by a comparison then conditional move 0 or 1 to the result.
> If that's the case, we should at least add a WARNING on the slow path.
OK, I will add a pr_warn inside "reciprocal_value_adv" when l == 32 is
triggered.
Thanks,
Jiong
