Re: [Xen-devel] [PATCH v8 3/5] xen/mem_sharing: VM forking

From: Tamas K Lengyel <tamas@tklengyel.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>, "Wei Liu" <wl@xen.org>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Tamas K Lengyel" <tamas.lengyel@intel.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	Xen-devel <xen-devel@lists.xenproject.org>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH v8 3/5] xen/mem_sharing: VM forking
Date: Fri, 21 Feb 2020 07:22:53 -0700	[thread overview]
Message-ID: <CABfawhkwm4Nh6zrb9dmUPizaxMJG7ccsg3Mb8QdOdObGkJ-Hqg@mail.gmail.com> (raw)
In-Reply-To: <2e8d6c12-d839-6662-6dd8-b6976527c252@citrix.com>

On Fri, Feb 21, 2020 at 7:02 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 21/02/2020 13:43, Andrew Cooper wrote:
> > On 10/02/2020 19:21, Tamas K Lengyel wrote:
> >> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> >> index 3835bc928f..ccf338918d 100644
> >> --- a/xen/arch/x86/mm/mem_sharing.c
> >> +++ b/xen/arch/x86/mm/mem_sharing.c
> >> @@ -36,6 +37,9 @@
> >>  #include <asm/altp2m.h>
> >>  #include <asm/atomic.h>
> >>  #include <asm/event.h>
> >> +#include <asm/hap.h>
> >> +#include <asm/hvm/hvm.h>
> >> +#include <asm/hvm/save.h>
> > This include is stale, I think.
> >
> >> +static void fork_tsc(struct domain *cd, struct domain *d)
> >> +{
> >> +    uint32_t tsc_mode;
> >> +    uint32_t gtsc_khz;
> >> +    uint32_t incarnation;
> >> +    uint64_t elapsed_nsec;
> >> +
> >> +    tsc_get_info(d, &tsc_mode, &elapsed_nsec, &gtsc_khz, &incarnation);
> >> +    tsc_set_info(cd, tsc_mode, elapsed_nsec, gtsc_khz, incarnation);
> > Sadly, get and set are asymetric.  For reasons best understood by the
> > original authors, incarnation gets automatically incremented on set,
> > rather than happing as part of migration where it logically lives.
> >
> > As a result, you probably want to set incarnation - 1, and leave a
> > comment saying "Don't bump the incarnation" or similar.
>
> P.S. Can both be fixed on commit if you agree.  Seems pointless sending
> a v9 just for these two.

Great, I have no issue with these changes.

Thanks!
Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel