Re: [PATCH v3 1/1] xen: move TLB-flush filtering out into populate_physmap during vm creation

[Wei Liu <wei.liu2@citrix.com>]

On Thu, Sep 08, 2016 at 01:30:03PM +0800, Dongli Zhang wrote:
> This patch implemented parts of TODO left in commit id
> a902c12ee45fc9389eb8fe54eeddaf267a555c58. It moved TLB-flush filtering out
> into populate_physmap. Because of TLB-flush in alloc_heap_pages, it's very
> slow to create a guest with memory size of more than 100GB on host with
> 100+ cpus.
> 
> This patch introduced a "MEMF_no_tlbflush" bit to memflags to indicate
> whether TLB-flush should be done in alloc_heap_pages or its caller
> populate_physmap. Once this bit is set in memflags, alloc_heap_pages will
> ignore TLB-flush. To use this bit after vm is created might lead to
> security issue, that is, this would make pages accessible to the guest B,
> when guest A may still have a cached mapping to them.
> 
> Therefore, this patch also introduced a "already_scheduled" field to struct
> domain to indicate whether this domain has ever got scheduled by
> hypervisor.  MEMF_no_tlbflush can be set only during vm creation phase when
> already_scheduled is still 0 before this domain gets scheduled for the
> first time.
> 
> TODO: ballooning very huge amount of memory cannot benefit from this patch
> and might still be slow.
> 
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> 
> ---
> Changed since v2:
>   * Limit this optimization to domain creation time.
> 
> ---
>  xen/common/domain.c     |  2 ++
>  xen/common/memory.c     | 33 +++++++++++++++++++++++++++++++++
>  xen/common/page_alloc.c |  3 ++-
>  xen/common/schedule.c   |  5 +++++
>  xen/include/xen/mm.h    |  2 ++
>  xen/include/xen/sched.h |  3 +++
>  6 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index a8804e4..611a471 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -303,6 +303,8 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
>      if ( !zalloc_cpumask_var(&d->domain_dirty_cpumask) )
>          goto fail;
>  
> +    d->already_scheduled = 0;
> +

Use false please -- this is a bool_t.

[...]
> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
> index 32a300f..593541a 100644
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -1376,6 +1376,11 @@ static void schedule(void)
>  
>      next = next_slice.task;
>  
> +    /* Set already_scheduled to 1 when this domain gets scheduled for the
> +     * first time */
> +    if ( next->domain->already_scheduled == 0 )
> +        next->domain->already_scheduled = 1;
> +

Can be simplified by omitting the "if" altogether.  And use "true" here.

Wei.

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