Re: [Xen-devel] [PATCH v5 2/4] x86/mem_sharing: copy a page_lock version to be internal to memshr

From: "Jan Beulich" <JBeulich@suse.com>
To: "Tamas K Lengyel" <tamas@tklengyel.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Wei Liu <wei.liu2@citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH v5 2/4] x86/mem_sharing: copy a page_lock version to be internal to memshr
Date: Fri, 17 May 2019 01:21:49 -0600	[thread overview]
Message-ID: <5CDE610D020000780022FF42@prv1-mh.provo.novell.com> (raw)
Message-ID: <20190517072149.zQsdVVMQluoAiwXGv0gZfUiWHNWnwKFZx43Sb5vgca8@z> (raw)
In-Reply-To: <20190516213752.1701-2-tamas@tklengyel.com>

>>> On 16.05.19 at 23:37, <tamas@tklengyel.com> wrote:
> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -356,24 +356,15 @@ struct platform_bad_page {
>  const struct platform_bad_page *get_platform_badpages(unsigned int *array_size);
>  
>  /* Per page locks:
> - * page_lock() is used for two purposes: pte serialization, and memory sharing.
> + * page_lock() is used for pte serialization.
>   *
>   * All users of page lock for pte serialization live in mm.c, use it
>   * to lock a page table page during pte updates, do not take other locks within
>   * the critical section delimited by page_lock/unlock, and perform no
>   * nesting.
>   *
> - * All users of page lock for memory sharing live in mm/mem_sharing.c. Page_lock
> - * is used in memory sharing to protect addition (share) and removal (unshare)
> - * of (gfn,domain) tupples to a list of gfn's that the shared page is currently
> - * backing. Nesting may happen when sharing (and locking) two pages -- deadlock
> - * is avoided by locking pages in increasing order.
> - * All memory sharing code paths take the p2m lock of the affected gfn before
> - * taking the lock for the underlying page. We enforce ordering between page_lock
> - * and p2m_lock using an mm-locks.h construct.
> - *
> - * These two users (pte serialization and memory sharing) do not collide, since
> - * sharing is only supported for hvm guests, which do not perform pv pte updates.
> + * The use of PGT_locked in mem_sharing does not collide, since mem_sharing is
> + * only supported for hvm guests, which do not perform pv pte updates.

Hmm, I thought we had agreed on you also correcting the wording of
the sentence you now retain (as requested). As said before, a HVM
(PVH to be precise) Dom0 can very well perform PV PTE updates, just
not on itself. I had suggested the wording "which do not have PV PTEs
updated" - I'd be fine for this to be folded in while committing, to avoid
another round trip. With this
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

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