Re: [PATCH v1 02/29] virtio-mem: simplify calculation in virtio_mem_mb_state_prepare_next_mb()

[David Hildenbrand]

On 15.10.20 06:02, Wei Yang wrote:
> On Mon, Oct 12, 2020 at 02:52:56PM +0200, David Hildenbrand wrote:
>> We actually need one byte less (next_mb_id is exclusive, first_mb_id is
>> inclusive). Simplify.
>>
>> Cc: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
>> Cc: Jason Wang <jasowang@xxxxxxxxxx>
>> Cc: Pankaj Gupta <pankaj.gupta.linux@xxxxxxxxx>
>> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
>> ---
>> drivers/virtio/virtio_mem.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>> index a1f5bf7a571a..670b3faf412d 100644
>> --- a/drivers/virtio/virtio_mem.c
>> +++ b/drivers/virtio/virtio_mem.c
>> @@ -257,8 +257,8 @@ static enum virtio_mem_mb_state virtio_mem_mb_get_state(struct virtio_mem *vm,
>>  */
>> static int virtio_mem_mb_state_prepare_next_mb(struct virtio_mem *vm)
>> {
>> -	unsigned long old_bytes = vm->next_mb_id - vm->first_mb_id + 1;
>> -	unsigned long new_bytes = vm->next_mb_id - vm->first_mb_id + 2;
>> +	unsigned long old_bytes = vm->next_mb_id - vm->first_mb_id;
>> +	unsigned long new_bytes = old_bytes + 1;
> 
> This is correct.
> 
> So this looks more like a fix?

We allocate an additional new page "one memory block too early".

So we would allocate the first page for blocks 0..510, and already
allocate the second page with block 511, although we could have fit it
into the first page. Block 512 will then find that the second page is
already there and simply use the second page.

So as we do it consistently, nothing will go wrong - that's why I
avoided using the "fix" terminology.

Thanks!

-- 
Thanks,

David / dhildenb