All of lore.kernel.org
 help / color / mirror / Atom feed
From: George Dunlap <george.dunlap@eu.citrix.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: "Liu, Jinsong" <jinsong.liu@intel.com>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Ian Campbell <Ian.Campbell@citrix.com>,
	Jan Beulich <JBeulich@suse.com>
Subject: Re: [PATCH V3] X86/vMCE: handle broken page with regard to migration
Date: Wed, 21 Nov 2012 11:34:40 +0000	[thread overview]
Message-ID: <50ACBC50.4040204@eu.citrix.com> (raw)
In-Reply-To: <20651.52995.774236.722303@mariner.uk.xensource.com>

On 20/11/12 18:42, Ian Jackson wrote:
> Liu, Jinsong writes ("RE: [Xen-devel] [PATCH V3] X86/vMCE: handle broken page with regard to migration"):
>> Ian Jackson wrote:
>>> Liu, Jinsong writes ("RE: [Xen-devel] [PATCH V3] X86/vMCE: handle
>>> broken page with regard to migration"):
>>>> No, at last lter, there are 4 points:
>>>> 1. start last iter
>>>> 2. get and transfer pfn_type to target
>>>> 3. copy page to target
>>>> 4. end last iter
> ...
>> It indeed checks mce after point 3 for each page, but what's the
>> advantage of keeping a separate list?
> It avoids yet another loop over all the pages.  Unless I have
> misunderstood.  Which I may have, because: if it checks for mce after
> point 3 then surely that is sufficient ?  We don't need to worry about
> mces after that check.

It's sufficient, but wouldn't each check require a separate hypercall?  
That would surely be slower than just a single hypercall and a loop 
(which is what Jinsong's patch does).

We don't actually need a list -- I think we just need to know, "Have any 
pages broken between reading the p2m table ( xc_get_pfn_type_batch() ); 
if so, we do another full iteration.

Since marking a page broken will also mark it dirty, I suppose what we 
could do is, on the last iteration, clear the dirty bitmap after getting 
the list of pages but before copying them; and then check the bit in the 
bitmap corresponding to the pfn after copying it.

But on the whole, is that really that much *faster* to do it that way?  
Either way you're still adding a single hypercall to the whole thing, 
and one iteration of a loop for each page; but in Jinsong's patch, the 
loop is done all at once, so presumably the compiler / processor will be 
able to do make better use of loop unrolling / registers / the pipeline 
/ branch prediction / caches &c.

The only way to avoid the loop would be to add an extra "have any pages 
been marked broken / dirty" bit somewhere, which is probably more 
trouble than it's worth.

  -George

  parent reply	other threads:[~2012-11-21 11:34 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-17  2:04 [PATCH V3] X86/vMCE: handle broken page with regard to migration Liu Jinsong
2012-11-16 18:19 ` Ian Jackson
2012-11-16 18:31   ` Liu, Jinsong
2012-11-19  9:55     ` Ian Campbell
2012-11-19 15:29       ` George Dunlap
2012-11-19 16:57         ` Ian Campbell
2012-11-20 15:08           ` George Dunlap
2012-11-20 17:08             ` Liu, Jinsong
2012-11-20 17:23               ` George Dunlap
2012-11-20 17:49                 ` Liu, Jinsong
2012-11-20 18:54               ` Liu, Jinsong
2012-11-21 11:07                 ` Ian Campbell
2012-11-21 11:18                   ` George Dunlap
2012-11-21 12:11                     ` Liu, Jinsong
2012-11-20 16:43           ` Liu, Jinsong
2012-11-20 16:29         ` Liu, Jinsong
2012-11-20 16:11       ` Liu, Jinsong
2012-11-20 17:48 ` George Dunlap
2012-11-20 18:13   ` Liu, Jinsong
2012-11-20 18:21     ` Ian Jackson
2012-11-20 18:39       ` Liu, Jinsong
2012-11-20 18:42         ` Ian Jackson
2012-11-20 19:07           ` Liu, Jinsong
2012-11-21 11:34           ` George Dunlap [this message]
2012-11-21 11:55             ` Ian Jackson
2012-11-21 12:11             ` Ian Campbell
2012-11-21 12:15               ` George Dunlap
2012-11-21 13:26               ` Liu, Jinsong
2012-11-21 13:37                 ` Jan Beulich
2012-11-22 11:23                   ` Liu, Jinsong
2012-11-21 13:59                 ` George Dunlap
2012-11-22 11:44                   ` Liu, Jinsong
2012-11-21 12:17 ` George Dunlap
2012-11-21 13:31   ` Liu, Jinsong
2012-11-22 12:37   ` Liu, Jinsong
2012-11-22 13:36     ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50ACBC50.4040204@eu.citrix.com \
    --to=george.dunlap@eu.citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=JBeulich@suse.com \
    --cc=jinsong.liu@intel.com \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.