Re: ring buffer indices: way too much modulo (division!) fiddling

["Ray Lee" <madrabbit@gmail.com>]

On 3/21/06, Andreas Mohr <andi@rhlx01.fht-esslingen.de> wrote:
> "Just One" shocking example (2.6.16 arch/i386/kernel/apm.c):
>
> static apm_event_t get_queued_event(struct apm_user *as)
> {
>         as->event_tail = (as->event_tail + 1) % APM_MAX_EVENTS;
>         return as->events[as->event_tail];
> }
[...]
>      7d7:       83 f9 14                cmp    $0x14,%ecx

APM_MAX_EVENTS appears to be 20, not a power of 2. GCC does the
obvious transform of modulo to an ANDL when the modulus is a sane
value. Rewrite the above as a four line function and run it through
gcc -S to see to difference bewteen APM_MAX_EVENTS=20 and =32.

> Any problems with such a change that I'm missing here?

<shrug> It's probably not a wide-spread problem. How many places in
the kernel use ringbuffers that aren't sized to a power of 2?

> ringbuf_advance_idx(my_idx_var, MY_BUF_SIZE)
> (any more generic name? It's not always about ring buffers...)

modulo_add? But it's probably unneccessary, unless this really is
widespread. Even then, just correcting those already in place would be
better than adding yet another abstraction of a simple operation to
the kernel.

> This kind of unnecessary modulo operation happens in lots of places (mostly
> network drivers).

Check to see if those are power of 2 sized buffers. gcc really will do
those correctly.

Ray