Re: [PATCH] bpf: Simplify tnum_step()

[Hao Sun]

[...]
> > -             res = w;
> > -     }
> > -     return res;
> > +     d = z - t.value;
>
> A bit of comment explaining would be nice.
>

The commit message is self-contained; anyone interested can git blame.
But I am unsure. If a detailed description is preferred, I can add a
comment to v2.

> > +     carry_mask = (1ULL << fls64(d & ~t.mask)) - 1;
> > +     inc = ((d | carry_mask | ~t.mask) + 1) & t.mask;
>
> Maybe break out the calculation of inc, give it a name (next_submask?),

Maybe we can break this down as:

d = z - t.value;
carry_mask = (1ULL << fls64(d & ~t.mask)) - 1;
filled = d | carry_mask | ~t.mask;
inc = (filled + 1) & t.mask;

`next_submask` is not precise, as we ored `~t.mask` into it.
It's always hard to name the intermediate result :)

> and have it as preceding patch? It seems generic enough that I thought
> would have been implemented already, but bitmap_scatter() was the
> closest I could find.
>
> Perhaps could be submitted to include/linux/bitops.h in the future.
>

The calculation above is direct, adding a bitmap helper intros an indirection.
For me, it means I would have to check out the meaning of the helper
and then get back to understanding the algorithm.