Re: [Qemu-devel] [virtio-dev] Re: [PATCH v10 6/7] migration/ram.c: add a function to disable the bulk stage

[Wei Wang <wei.w.wang@intel.com>]

On 12/03/2018 04:20 PM, Wei Wang wrote:
> On 12/03/2018 01:31 PM, Peter Xu wrote:
>> On Mon, Dec 03, 2018 at 10:18:30AM +0800, Wei Wang wrote:
>>> This patch adds a function to enable a precopy notifier callback 
>>> outside
>>> the migration subsystem to disable the bulk stage flag. This is 
>>> needed by
>>> the free page optimization offered by virtio-balloon.
>>>
>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>>> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> CC: Juan Quintela <quintela@redhat.com>
>>> CC: Michael S. Tsirkin <mst@redhat.com>
>>> CC: Peter Xu <peterx@redhat.com>
>>> ---
>>>   include/migration/misc.h | 1 +
>>>   migration/ram.c          | 9 +++++++++
>>>   2 files changed, 10 insertions(+)
>>>
>>> diff --git a/include/migration/misc.h b/include/migration/misc.h
>>> index 15f8d00..47e7ff5 100644
>>> --- a/include/migration/misc.h
>>> +++ b/include/migration/misc.h
>>> @@ -37,6 +37,7 @@ void precopy_infrastructure_init(void);
>>>   void precopy_add_notifier(NotifierWithReturn *n);
>>>   void precopy_remove_notifier(NotifierWithReturn *n);
>>>   int precopy_notify(PrecopyNotifyReason reason, Error **errp);
>>> +void precopy_disable_bulk_stage(void);
>>>     void ram_mig_init(void);
>>>   void qemu_guest_free_page_hint(void *addr, size_t len);
>>> diff --git a/migration/ram.c b/migration/ram.c
>>> index b90a3f2..739dc97 100644
>>> --- a/migration/ram.c
>>> +++ b/migration/ram.c
>>> @@ -354,6 +354,15 @@ int precopy_notify(PrecopyNotifyReason reason, 
>>> Error **errp)
>>>       return 
>>> notifier_with_return_list_notify(&precopy_notifier_list, &pnd);
>>>   }
>>>   +void precopy_disable_bulk_stage(void)
>>> +{
>>> +    if (!ram_state) {
>>> +        return;
>>> +    }
>>> +
>>> +    ram_state->ram_bulk_stage = false;
>>> +}
>>> +
>> This one is a bit tricky.  E.g., I think it'll at least affect XBZRLE
>> and compression somehow.  Since we will have the on-off switch for
>> balloon free page hinting so the user can at least decide what
>> features to use, with that I don't see much issue with it so far. But
>> I'd also like to see how other people see this change.
>>
>
> Yes. I think it would be better that this optimization could co-exist 
> with the
> compression feature.
>
> How about adding a new flag "ram_state->free_page_optimization_enabled"?
>
> We will then need a change in migration_bitmap_find_dirty:
>  - if (rs->ram_bulk_stage && start > 0) {
> + if (!rs->free_page_optimization_enabled && rs->ram_bulk_stage && 
> start > 0) {
>         next = start + 1;
>     } else {
>         next = find_next_bit(bitmap, size, start);
>     }
>

Btw, with the new flag, this patch is not needed, so "ram_bulk_stage" 
will not be changed.

Best,
Wei