[v2,00/39] bitmap handling optimization

Il 06/11/2013 15:37, Gerd Hoffmann ha scritto:
> 
>>> - vga ram by default is not aligned in a page number multiple of
>>> 64,
>>> 
>>> it could be optimized.  Kraxel?  It syncs the kvm bitmap at least
>>> 1 a second or so? bitmap is only 2048 pages (16MB by default). We
>>> need to change the ram_addr only
> It is created using memory_region_init_ram(), in vga_common_init(). 
> Nothing special is done to avoid/force any alignment. Dunno why it
> ends up on a odd page number.

Because some random option ROM is loaded before the RAM region is
created, and thus the ram_addr_t's become misaligned.  Keeping the
ram_addr_t's aligned in find_ram_offset is easy:


I suspect it doesn't.  But perhaps we still have time before RDMA
becomes non-experimental.

Paolo

On 11/06/2013 11:49 PM, Paolo Bonzini wrote:
> Il 06/11/2013 15:37, Gerd Hoffmann ha scritto:
>>>> - vga ram by default is not aligned in a page number multiple of
>>>> 64,
>>>>
>>>> it could be optimized.  Kraxel?  It syncs the kvm bitmap at least
>>>> 1 a second or so? bitmap is only 2048 pages (16MB by default). We
>>>> need to change the ram_addr only
>> It is created using memory_region_init_ram(), in vga_common_init().
>> Nothing special is done to avoid/force any alignment. Dunno why it
>> ends up on a odd page number.
> Because some random option ROM is loaded before the RAM region is
> created, and thus the ram_addr_t's become misaligned.  Keeping the
> ram_addr_t's aligned in find_ram_offset is easy:
>
> diff --git a/exec.c b/exec.c
> index 79610ce..1b82e81 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1002,7 +1002,7 @@ static ram_addr_t find_ram_offset(ram_addr_t size)
>       QTAILQ_FOREACH(block, &ram_list.blocks, next) {
>           ram_addr_t end, next = RAM_ADDR_MAX;
>
> -        end = block->offset + block->length;
> +        end = ROUND_UP(block->offset + block->length, TARGET_PAGE_SIZE * 64);
>
>           QTAILQ_FOREACH(next_block, &ram_list.blocks, next) {
>               if (next_block->offset >= end) {
>
> but I'm not sure if we're allowed to change the ram_addr_t's.  On one hand they
> are not part of guest ABI, on the other hand RDMA migration uses them
> in the protocol.
>
> Michael, can you check if RDMA migration works from a QEMU *without*
> this patch to one *with*:
>
> diff --git a/exec.c b/exec.c
> index 79610ce..21b8b96 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -997,7 +997,7 @@ static ram_addr_t find_ram_offset(ram_addr_t size)
>       assert(size != 0); /* it would hand out same offset multiple times */
>
>       if (QTAILQ_EMPTY(&ram_list.blocks))
> -        return 0;
> +        return TARGET_PAGE_SIZE * 100;
>
>       QTAILQ_FOREACH(block, &ram_list.blocks, next) {
>           ram_addr_t end, next = RAM_ADDR_MAX;
>
> I suspect it doesn't.  But perhaps we still have time before RDMA
> becomes non-experimental.
>
> Paolo
>

I tested the patch as requested. It doesn't seem to break the migration.
I compiled and looped the migration a couple of times and it runs through.

Sorry for the late reply.

- Michael

Il 25/11/2013 07:39, Michael R. Hines ha scritto:
>> Because some random option ROM is loaded before the RAM region is
>> created, and thus the ram_addr_t's become misaligned.  Keeping the
>> ram_addr_t's aligned in find_ram_offset is easy:
>>
>> diff --git a/exec.c b/exec.c
>> index 79610ce..1b82e81 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1002,7 +1002,7 @@ static ram_addr_t find_ram_offset(ram_addr_t size)
>>       QTAILQ_FOREACH(block, &ram_list.blocks, next) {
>>           ram_addr_t end, next = RAM_ADDR_MAX;
>>
>> -        end = block->offset + block->length;
>> +        end = ROUND_UP(block->offset + block->length,
>> TARGET_PAGE_SIZE * 64);
>>
>>           QTAILQ_FOREACH(next_block, &ram_list.blocks, next) {
>>               if (next_block->offset >= end) {
>>
>> but I'm not sure if we're allowed to change the ram_addr_t's.  On one
>> hand they
>> are not part of guest ABI, on the other hand RDMA migration uses them
>> in the protocol.
>>
>> Michael, can you check if RDMA migration works from a QEMU *without*
>> this patch to one *with*:
> 
> I tested the patch as requested. It doesn't seem to break the migration.
> I compiled and looped the migration a couple of times and it runs through.
> 
> Sorry for the late reply.

Thanks, no problem since Juan's patch is 1.8 material anyway.

Juan, looks like I was wrong about the ram_addr_t being part of the
protocol (more likely, it is part of the protocol but it doesn't depend
on equal ram_addr_t).  You could then add the above one-liner as a
separate patch.

Paolo