Re: [Qemu-devel] [PATCH] migration/postcopy: use static PostcopyDiscardState instead of allocating it for each block

[Wei Yang <richardw.yang@linux.intel.com>]

On Tue, Jul 23, 2019 at 04:42:12PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.yang@linux.intel.com) wrote:
>> On Fri, Jul 19, 2019 at 06:41:28PM +0100, Dr. David Alan Gilbert wrote:
>> >* Wei Yang (richardw.yang@linux.intel.com) wrote:
>> >> Even we need to do discard for each RAMBlock, we still can leverage the
>> >> same memory space to store the information.
>> >> 
>> >> By doing so, we avoid memory allocation and deallocation to the system
>> >> and also avoid potential failure of memory allocation which breaks the
>> >> migration.
>> >> 
>> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> >> ---
>> >>  migration/postcopy-ram.c | 16 +++++++---------
>> >>  1 file changed, 7 insertions(+), 9 deletions(-)
>> >> 
>> >> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> >> index 9faacacc9e..2e6b076bb7 100644
>> >> --- a/migration/postcopy-ram.c
>> >> +++ b/migration/postcopy-ram.c
>> >> @@ -1377,8 +1377,7 @@ void postcopy_fault_thread_notify(MigrationIncomingState *mis)
>> >>   *   asking to discard individual ranges.
>> >>   *
>> >>   * @ms: The current migration state.
>> >> - * @offset: the bitmap offset of the named RAMBlock in the migration
>> >> - *   bitmap.
>> >> + * @offset: the bitmap offset of the named RAMBlock in the migration bitmap.
>> >>   * @name: RAMBlock that discards will operate on.
>> >>   *
>> >>   * returns: a new PDS.
>> >> @@ -1386,13 +1385,14 @@ void postcopy_fault_thread_notify(MigrationIncomingState *mis)
>> >>  PostcopyDiscardState *postcopy_discard_send_init(MigrationState *ms,
>> >>                                                   const char *name)
>> >>  {
>> >> -    PostcopyDiscardState *res = g_malloc0(sizeof(PostcopyDiscardState));
>> >> +    static PostcopyDiscardState res = {0};
>> >
>> >Do you think it would be better to make this a static at the top of
>> >migration/postcopy-ram.c and then we could remove the pds parameters
>> >from postcopy_discard_send_range and friends?
>> 
>> Just took a look into this one. One problem is not all its friends are in
>> migration/postcopy-ram.c
>> 
>> For example, postcopy_chunk_hostpages_pass() is in migration/ram.c.
>
>But doesn't that just pass teh pds back to postcopy_discard_send_range
>which is in postcopy-ram ?

You are right, I didn't notice this.

>
>Dave
>