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

[Dario Faggioli]

On Tue, 2016-09-20 at 12:19 +0100, George Dunlap wrote:
> 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 " is removed from
> > xen/arch/x86/acpi/suspend.c to
> > avoid the compiling error after we include "" to
> > xen/include/xen/mm.h.
> > 
> > Signed-off-by: Dongli Zhang 
> 
> Acked-by: George Dunlap 
> 
Reviewed-by: Dario Faggioli 

Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)

signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

[George Dunlap]

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 " is removed from xen/arch/x86/acpi/suspend.c to
> avoid the compiling error after we include "" to
> xen/include/xen/mm.h.
> 
> Signed-off-by: Dongli Zhang 

Acked-by: George Dunlap 

> ---
> 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 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> 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(_tlbflush, [i], _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(_lock);
>  
>  if ( need_tlbflush )
> -{
> -cpumask_t mask = cpu_online_map;
> -tlbflush_filter(mask, tlbflush_timestamp);
> -if ( !cpumask_empty() )
> -{
> -perfc_incr(need_flush_tlb_flush);
> -flush_tlb_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 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  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 
> +
> +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() )
> +{
> +perfc_incr(need_flush_tlb_flush);
> +flush_tlb_mask();
> +}
> +}
> +
>  #endif /* __XEN_MM_H__ */
> 


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

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

[Jan Beulich]

>>> On 20.09.16 at 04:31,  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 " is removed from xen/arch/x86/acpi/suspend.c to
> avoid the compiling error after we include "" to
> xen/include/xen/mm.h.
> 
> Signed-off-by: Dongli Zhang 

Acked-by: Jan Beulich 


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

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

[Dongli Zhang]

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 " is removed from xen/arch/x86/acpi/suspend.c to
avoid the compiling error after we include "" to
xen/include/xen/mm.h.

Signed-off-by: Dongli Zhang 
---
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 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
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(_tlbflush, [i], _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(_lock);
 
 if ( need_tlbflush )
-{
-cpumask_t mask = cpu_online_map;
-tlbflush_filter(mask, tlbflush_timestamp);
-if ( !cpumask_empty() )
-{
-perfc_incr(need_flush_tlb_flush);
-flush_tlb_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 
 #include 
 #include 
+#include 
 #include 
 
 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 
+
+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() )
+{
+perfc_incr(need_flush_tlb_flush);
+flush_tlb_mask();
+}
+}
+
 #endif /* __XEN_MM_H__ */
-- 
1.9.1


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