On Fri, Jul 12, 2024 at 09:03:32PM +0200, Stamatis, Ilias wrote:
> On Fri, 2024-07-12 at 17:55 +0200, Roman Kagan wrote:
> > On Wed, Jul 10, 2024 at 09:52:54AM +0100, Ilias Stamatis wrote:
> > > The following calculation used in coalesced_mmio_has_room() to check
> > > whether the ring buffer is full is wrong and only allows half the buffer
> > > to be used.
> > >
> > > avail = (ring->first - last - 1) % KVM_COALESCED_MMIO_MAX;
> > > if (avail == 0)
> > > /* full */
> > >
> > > The % operator in C is not the modulo operator but the remainder
> > > operator. Modulo and remainder operators differ with respect to negative
> > > values. But all values are unsigned in this case anyway.
> > >
> > > The above might have worked as expected in python for example:
> > > > > > (-86) % 170
> > > 84
> > >
> > > However it doesn't work the same way in C.
> > >
> > > printf("avail: %d\n", (-86) % 170);
> > > printf("avail: %u\n", (-86) % 170);
> > > printf("avail: %u\n", (-86u) % 170u);
> > >
> > > Using gcc-11 these print:
> > >
> > > avail: -86
> > > avail: 4294967210
> > > avail: 0
> >
> > Where exactly do you see a problem? As you correctly point out, all
> > values are unsigned, so unsigned arithmetics with wraparound applies,
> > and then % operator is applied to the resulting unsigned value. Out
> > your three examples, only the last one is relevant, and it's perfectly
> > the intended behavior.
> >
> > Thanks,
> > Roman.
>
> KVM_COALESCED_MMIO_MAX on x86 is 170, which means the ring buffer has
> 170 entries (169 of which should be usable).
>
> If first = 0 and last = 85 then the calculation gives 0 available
> entries in which case we consider the buffer to be full and we exit to
> userspace. But we shouldn't as there are still 84 unused entries.
You want to add a stride instead
avail = (ring->first + KVM_COALESCED_MMIO_MAX - 1 - last) %
KVM_COALESCED_MMIO_MAX;
As long as the stride is smaller than half the wraparound, you can think
of the values being on an infinite non-negative axis.
Roman.
Amazon Web Services Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597
