Re: [Xen-devel] [PATCH 02/02] Handles broken page occurred during migration

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

George Dunlap wrote:
> On 31/10/12 12:21, Liu, Jinsong wrote:
>> Handles broken page occurred during migration
>> 
>> This patch handles broken page which occurred during migration.
>> 
>> It monitors the critical area of live migration (from vMCE point of
>> view, 
>> the copypages stage of migration is the critical area while other
>> areas are not). 
>> 
>> If a vMCE occur at the critical area of live migration, it marks the
>> broken page to dirty map, so that at copypages stage of migration,
>> its pfn_type 
>> and pfn number would transfer to target and then take appropriate
>> action. 
>> 
>> At target, it would set p2m as p2m_ram_broken for broken page, so
>> that if 
>> guest access the broken page again, it would kill itself as expected.
>> 
>> Suggested-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
>> Signed-off-by: Liu, Jinsong <jinsong.liu@xxxxxxxxx>
> 
> [snip]
> 
>> diff -r 3313ee9f6142 xen/arch/x86/cpu/mcheck/mce_intel.c
>> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c    Thu Oct 25 05:49:11 2012
>> +0800 +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c      Tue Oct 30 06:07:05
>>                       2012 +0800 @@ -342,6 +342,22 @@ goto
>>                   vmce_failed; }
>> 
>> +                if ( unlikely(d->arch.vmce_monitor) ) +            
>> { +                    /*
>> +                     * vMCE occur during migration +               
>> * +                     * mark broken page to dirty bitmap, so that
>> at copypages +                     * stage of migration, its
>> pfn_type and pfn number would +                     * transfer to
>> target and then take appropriate action +                     * +   
>> * At target, it would set p2m as p2m_ram_broken for broken +        
>> * page, so that if guest access the broken page again, it +         
>> * would kill itself as expected. +                     */ +         
>> paging_mark_dirty(d, mfn); +                }
>> +
> 
> Given that logdirty is only enabled when we're doing some kind of
> migration (Remus or otherwise), wouldn't it make sense to just
> unconditionally call paging_mark_dirty(), rather than doing all this
> stuff with vmce_monitor?  paging_mark_dirty() is already a no-op if
> live-migration is not enabled; and if it is enabled, you probably want
> whoever is using logdirty to go through and deal with the broken page
> anyway.

That's just what I concerned -- can we guarantee paging log dirty mode only 
occur during live migration/remus?
say, under the case vram_tracking (hap_enable_vram_tracking) it also enable log 
dirty.

> 
> So it seems like you could just add paging_mark_dirty() to the
> previous patch, and get rid of the rest of this patch entirely.
> 

Currently I just find 2 cases which use log dirty mode: live migration/remus 
and vram tracking. So if we can make sure that log dirty would not used more in 
the future, it's fine to me to simplify my code by unconditionally call 
paging_mark_dirty at mce handler.

What do you think?

Thanks,
Jinsong


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel