Re: [Xen-devel] [PATCH v6 1/2] xen: replace tlbflush check and operation with inline functions

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 20/09/16 03:31, Dongli Zhang wrote:
> This patch cleaned up the code by replacing complicated tlbflush check and
> operation with inline functions. We should use those inline functions to
> avoid the complicated tlbflush check and tlbflush operations when
> implementing TODOs left in commit a902c12ee45fc9389eb8fe54eeddaf267a555c58
> (More efficient TLB-flush filtering in alloc_heap_pages()).
> 
> "#include <asm/flushtlb.h>" is removed from xen/arch/x86/acpi/suspend.c to
> avoid the compiling error after we include "<asm/flushtlb.h>" to
> xen/include/xen/mm.h.
> 
> Signed-off-by: Dongli Zhang <dongli.zhang@xxxxxxxxxx>

Acked-by: George Dunlap <george.dunlap@xxxxxxxxxx>

> ---
> Changed since v5:
>   * Move the if() and its body of tlbflush check into inline function.
> 
> Changed since v4:
>   * Wrap the filtered tlbflush mask operation as inline function (suggested
>     by Jan).
>   * Remove asm/flushtlb.h from suspend.c to avoid compiling error.
> 
> Changed since v3:
>   * Wrap the complicated tlbflush condition check as inline function
>     (suggested by Dario).
> 
> ---
>  xen/arch/x86/acpi/suspend.c |  1 -
>  xen/common/page_alloc.c     | 19 ++-----------------
>  xen/include/xen/mm.h        | 29 +++++++++++++++++++++++++++++
>  3 files changed, 31 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/arch/x86/acpi/suspend.c b/xen/arch/x86/acpi/suspend.c
> index 1d8344c..d5c67ee 100644
> --- a/xen/arch/x86/acpi/suspend.c
> +++ b/xen/arch/x86/acpi/suspend.c
> @@ -10,7 +10,6 @@
>  #include <asm/processor.h>
>  #include <asm/msr.h>
>  #include <asm/debugreg.h>
> -#include <asm/flushtlb.h>
>  #include <asm/hvm/hvm.h>
>  #include <asm/hvm/support.h>
>  #include <asm/i387.h>
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 18ff6cf..d7ca3a0 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -827,14 +827,7 @@ static struct page_info *alloc_heap_pages(
>          BUG_ON(pg[i].count_info != PGC_state_free);
>          pg[i].count_info = PGC_state_inuse;
>  
> -        if ( pg[i].u.free.need_tlbflush &&
> -             (pg[i].tlbflush_timestamp <= tlbflush_current_time()) &&
> -             (!need_tlbflush ||
> -              (pg[i].tlbflush_timestamp > tlbflush_timestamp)) )
> -        {
> -            need_tlbflush = 1;
> -            tlbflush_timestamp = pg[i].tlbflush_timestamp;
> -        }
> +        accumulate_tlbflush(&need_tlbflush, &pg[i], &tlbflush_timestamp);
>  
>          /* Initialise fields which have other uses for free pages. */
>          pg[i].u.inuse.type_info = 0;
> @@ -849,15 +842,7 @@ static struct page_info *alloc_heap_pages(
>      spin_unlock(&heap_lock);
>  
>      if ( need_tlbflush )
> -    {
> -        cpumask_t mask = cpu_online_map;
> -        tlbflush_filter(mask, tlbflush_timestamp);
> -        if ( !cpumask_empty(&mask) )
> -        {
> -            perfc_incr(need_flush_tlb_flush);
> -            flush_tlb_mask(&mask);
> -        }
> -    }
> +        filtered_flush_tlb_mask(tlbflush_timestamp);
>  
>      return pg;
>  }
> diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
> index f470e49..50db01d 100644
> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -51,6 +51,7 @@
>  #include <xen/spinlock.h>
>  #include <xen/typesafe.h>
>  #include <xen/kernel.h>
> +#include <xen/perfc.h>
>  #include <public/memory.h>
>  
>  TYPE_SAFE(unsigned long, mfn);
> @@ -567,4 +568,32 @@ int prepare_ring_for_helper(struct domain *d, unsigned 
> long gmfn,
>                              struct page_info **_page, void **_va);
>  void destroy_ring_for_helper(void **_va, struct page_info *page);
>  
> +#include <asm/flushtlb.h>
> +
> +static inline void accumulate_tlbflush(bool *need_tlbflush,
> +                                       const struct page_info *page,
> +                                       uint32_t *tlbflush_timestamp)
> +{
> +    if ( page->u.free.need_tlbflush &&
> +         page->tlbflush_timestamp <= tlbflush_current_time() &&
> +         (!*need_tlbflush ||
> +          page->tlbflush_timestamp > *tlbflush_timestamp) )
> +    {
> +        *need_tlbflush = true;
> +        *tlbflush_timestamp = page->tlbflush_timestamp;
> +    }
> +}
> +
> +static inline void filtered_flush_tlb_mask(uint32_t tlbflush_timestamp)
> +{
> +    cpumask_t mask = cpu_online_map;
> +
> +    tlbflush_filter(mask, tlbflush_timestamp);
> +    if ( !cpumask_empty(&mask) )
> +    {
> +        perfc_incr(need_flush_tlb_flush);
> +        flush_tlb_mask(&mask);
> +    }
> +}
> +
>  #endif /* __XEN_MM_H__ */
> 


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