Re: [PATCH 1/6] KVM: Fix coalesced_mmio_has_room()

[Roman Kagan <rkagan@amazon.de>]

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.



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