Re: [Xen-devel] [PATCH 3 of 3] RFC: xenpaging: use waitqueue in ept_get

["Andres Lagar-Cavilla" <andres@xxxxxxxxxxxxxxxx>]

Hi Olaf,

thanks for posting these RFC patches, great work!

I have several comments. Most importantly, I want to hash out the
interactions between these wait queues and the work I've been doing on p2m
synchronization. I've been runnning Xen with synchronized (i.e. locking)
p2m lookups for a few weeks now with little/no trouble. You can refer to a
patch I posted to the list previously, which I can resend. (I'm waiting on
feedback on some previous patches I sent to keep pushing on this work)

- I think the waitqueue should be part of struct arch_domain. It is an x86
construct that applies only to the base level p2m of the domain, not every
possible p2m in a nested setting.

- The same could be said of the p2m.pod struct, by the way, and that's a
patch I want to get upstreamed too.

- The right place to wait is not ept->get_entry, imho, but rather the
implementation-independent caller (get_gfn_type_access). More p2m
implementations could be added in the future (however unlikely that may
be) that can also perform paging.

- With locking p2m lookups, one needs to be careful about in_atomic. I
haven't performed a full audit of all callers, but this is what I can say:
1. there are no code paths that perform recursive p2m lookups, *on
different gfns*, with p2m_query_t != p2m_query
2. there are recursive lookups of different gfns, with p2m_query_t ==
p2m_query, most notably pod sweeps. These do not represent a problem here,
since those paths will skip over gfns that are paged.

Why is this important? Because we need to be in a position where a code
snippet such as

get_gfn_type_access(gfn) {
retry:
   p2m_lock()
   mfn = p2m->get_entry(gfn, &p2mt)
   if (p2mt == paging) {
       p2m_unlock()
       sleep_on_waitqueue()
       goto retry;
   }

works. This will get us a non-racy p2m that sends vcpu's to sleep without
holding locks. Makes sense?

- What is the plan for grant operations for pv-on-hvm drivers? Those will
enter get_gfn* holding the domain lock, and thus in an atomic context.

Andres
> Date: Tue, 22 Nov 2011 22:13:25 +0100
> From: Olaf Hering <olaf@xxxxxxxxx>
> To: xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: [Xen-devel] [PATCH 3 of 3] RFC: xenpaging: use waitqueue in
>       ept_get
> Message-ID: <9d63ecd3969bb7a2e398.1321996405@xxxxxxxxxxxx>
> Content-Type: text/plain; charset="us-ascii"
>
> # HG changeset patch
> # User Olaf Hering <olaf@xxxxxxxxx>
> # Date 1321996199 -3600
> # Node ID 9d63ecd3969bb7a2e39841f6c859b4c23f750642
> # Parent  de6860cb9205b68d1287482288d1b7b9d0255609
> RFC: xenpaging: use waitqueue in ept_get
> %0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel