Re: [PATCH 8/8] x86/mm: re-arrange type check around _get_page_type()'s TLB flush

2022-07-28 Thread Jan Beulich
On 27.07.2022 20:31, Andrew Cooper wrote:
> On 26/07/2022 17:07, Jan Beulich wrote:
>> Checks dependent on only d and x can be pulled out, thus allowing to
>> skip the flush mask calculation.
>>
>> Signed-off-by: Jan Beulich 
> 
> Do I get a Suggested-by seeing as this was on the very long list of
> improvements ?

I can add one, but you weren't the only one to notice.

> Either way, Reviewed-by: Andrew Cooper 

Thanks.

Jan



Re: [PATCH 8/8] x86/mm: re-arrange type check around _get_page_type()'s TLB flush

2022-07-27 Thread Andrew Cooper
On 26/07/2022 17:07, Jan Beulich wrote:
> Checks dependent on only d and x can be pulled out, thus allowing to
> skip the flush mask calculation.
>
> Signed-off-by: Jan Beulich 

Do I get a Suggested-by seeing as this was on the very long list of
improvements ?

Either way, Reviewed-by: Andrew Cooper 


[PATCH 8/8] x86/mm: re-arrange type check around _get_page_type()'s TLB flush

2022-07-26 Thread Jan Beulich
Checks dependent on only d and x can be pulled out, thus allowing to
skip the flush mask calculation.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3020,7 +3020,10 @@ static int _get_page_type(struct page_in
 if ( d && shadow_mode_enabled(d) )
 shadow_prepare_page_type_change(d, page);
 
-if ( (x ^ type) & PGT_type_mask )
+if ( ((x ^ type) & PGT_type_mask) &&
+ /* Shadow mode: track only writable pages. */
+ (!shadow_mode_enabled(d) ||
+  ((x & PGT_type_mask) == PGT_writable_page)) )
 {
 /*
  * On type change we check to flush stale TLB entries. It is
@@ -3035,10 +3038,7 @@ static int _get_page_type(struct page_in
 /* Don't flush if the timestamp is old enough */
 tlbflush_filter(mask, page->tlbflush_timestamp);
 
-if ( unlikely(!cpumask_empty(mask)) &&
- /* Shadow mode: track only writable pages. */
- (!shadow_mode_enabled(d) ||
-  ((x & PGT_type_mask) == PGT_writable_page)) )
+if ( unlikely(!cpumask_empty(mask)) )
 {
 perfc_incr(need_flush_tlb_flush);
 /*