On Fri, Jun 07, 2019 at 08:01:14PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.y...@linux.intel.com) wrote:
>> For cache miss condition not in last_stage, we need to insert data into
>> cache. When this step succeed, current_data should be updated. While no
>> matter these checks pass or not, -1 is returned.
>>
>> Based on this, the logic in cache miss handling could be simplified a
>> little.
>>
>> Signed-off-by: Wei Yang <richardw.y...@linux.intel.com>
>> ---
>> migration/ram.c | 17 ++++++++---------
>> 1 file changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 878cd8de7a..67ba075cc4 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -1572,15 +1572,14 @@ static int save_xbzrle_page(RAMState *rs, uint8_t
>> **current_data,
>> if (!cache_is_cached(XBZRLE.cache, current_addr,
>> ram_counters.dirty_sync_count)) {
>> xbzrle_counters.cache_miss++;
>> - if (!last_stage) {
>> - if (cache_insert(XBZRLE.cache, current_addr, *current_data,
>> - ram_counters.dirty_sync_count) == -1) {
>> - return -1;
>> - } else {
>> - /* update *current_data when the page has been
>> - inserted into cache */
>> - *current_data = get_cached_data(XBZRLE.cache, current_addr);
>> - }
>> + if (!last_stage &&
>> + !cache_insert(XBZRLE.cache, current_addr, *current_data,
>> + ram_counters.dirty_sync_count)) {
>> + /*
>> + * update *current_data when the page has been inserted into
>> + * cache
>> + */
>> + *current_data = get_cached_data(XBZRLE.cache, current_addr);
>
>No this change doesn't gain anything and I find the original easier to
>read.
>
>This function is really subtle, every time I do anything with it I have
>to think really hard about it, so ease of reading is more important.
>
Yep, I agree ease of reading is more important.
Since the original version looks better, I will keep current code. :-)
>Dave
>
>> }
>> return -1;
>> }
>> --
>> 2.19.1
>>
>--
>Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
--
Wei Yang
Help you, Help me
