Re: [PATCH 2/4] x86/PoD: Identify when a domain has already been killed from PoD exhaustion

[George Dunlap <george.dunlap@citrix.com>]

On 30/10/15 18:33, Andrew Cooper wrote:
> p2m_pod_demand_populate() can be entered repeatedly during a single path
> through the hypervisor, e.g. on a toolstack batch map operation.
> 
> The domain might be crashed, but the interface currently lacks a way of
> passing an error back through the generic p2m layer.
> 
> Longterm the p2m layer needs reworking to allow errors to be returned, but in
> the short term, avoid repeatedly re-sweeping the domain after it has already
> been crashed from PoD exhaustion.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: George Dunlap <george.dunlap@eu.citrix.com>
> ---
>  xen/arch/x86/mm/p2m-pod.c | 3 ++-
>  xen/include/asm-x86/p2m.h | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
> index be15cf3..6fb054f 100644
> --- a/xen/arch/x86/mm/p2m-pod.c
> +++ b/xen/arch/x86/mm/p2m-pod.c
> @@ -1048,7 +1048,7 @@ p2m_pod_demand_populate(struct p2m_domain *p2m, unsigned long gfn,
>      /* This check is done with the pod lock held.  This will make sure that
>       * even if d->is_dying changes under our feet, p2m_pod_empty_cache() 
>       * won't start until we're done. */
> -    if ( unlikely(d->is_dying) )
> +    if ( unlikely(d->is_dying) || p2m->pod.dead )

So after getting lost in a maze of twisty passages, it looks like
"d->is_dying" might be the wrong thing to check here.  d->is_dying is
*only* set, AFAICT, in two places:
 - in domain_kill(), which is only called for XEN_DOMCTL_destroydomain
 - in domain_create(), if the creation failed for some reason.

Would it make more sense to check d->is_shutting_down instead?

Having some sort of pod-specific flag seems like the wrong solution.

 -George