[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] migration/postcopy: use static PostcopyDiscardS
From: |
Wei Yang |
Subject: |
Re: [Qemu-devel] [PATCH] migration/postcopy: use static PostcopyDiscardState instead of allocating it for each block |
Date: |
Mon, 22 Jul 2019 14:39:41 +0800 |
User-agent: |
Mutt/1.9.4 (2018-02-28) |
On Fri, Jul 19, 2019 at 06:41:28PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (address@hidden) 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 <address@hidden>
>> ---
>> 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.
Which way do you prefer?
>If there's only one pds then we don't need to pass the pointer around.
>
>Dave
>
>> - if (res) {
>> - res->ramblock_name = name;
>> - }
>> + res.ramblock_name = name;
>> + res.cur_entry = 0;
>> + res.nsentwords = 0;
>> + res.nsentcmds = 0;
>>
>> - return res;
>> + return &res;
>> }
>>
>> /**
>> @@ -1449,8 +1449,6 @@ void postcopy_discard_send_finish(MigrationState *ms,
>> PostcopyDiscardState *pds)
>>
>> trace_postcopy_discard_send_finish(pds->ramblock_name, pds->nsentwords,
>> pds->nsentcmds);
>> -
>> - g_free(pds);
>> }
>>
>> /*
>> --
>> 2.17.1
>>
>--
>Dr. David Alan Gilbert / address@hidden / Manchester, UK
--
Wei Yang
Help you, Help me