RE: [PATCH v5 03/12] x86/mm: consolidate full flush threshold decision

[Michael Kelley <mhklinux@outlook.com>]

From: riel@surriel.com <riel@surriel.com> Sent: Wednesday, January 15, 2025 6:30 PM
> 
> Reduce code duplication by consolidating the decision point
> for whether to do individual invalidations or a full flush
> inside get_flush_tlb_info.
> 
> Signed-off-by: Rik van Riel <riel@surriel.com>
> Suggested-by: Dave Hansen <dave.hansen@intel.com>
> ---
>  arch/x86/mm/tlb.c | 43 ++++++++++++++++++++-----------------------
>  1 file changed, 20 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
> index 6cf881a942bb..2f38cf95dee3 100644
> --- a/arch/x86/mm/tlb.c
> +++ b/arch/x86/mm/tlb.c

[snip]

> @@ -1089,22 +1089,19 @@ static void do_kernel_range_flush(void *info)
> 
>  void flush_tlb_kernel_range(unsigned long start, unsigned long end)
>  {
> -	/* Balance as user space task's flush, a bit conservative */
> -	if (end == TLB_FLUSH_ALL ||
> -	    (end - start) > tlb_single_page_flush_ceiling << PAGE_SHIFT) {
> -		on_each_cpu(do_flush_tlb_all, NULL, 1);
> -	} else {
> -		struct flush_tlb_info *info;
> +	struct flush_tlb_info *info;
> 
> -		preempt_disable();
> -		info = get_flush_tlb_info(NULL, start, end, 0, false,
> -					  TLB_GENERATION_INVALID);
> +	guard(preempt)();
> +
> +	info = get_flush_tlb_info(NULL, start, end, PAGE_SHIFT, false,
> +				  TLB_GENERATION_INVALID);
> 
> +	if (end == TLB_FLUSH_ALL)

This needs to test "info->end", not "end".

In my VM on Hyper-V *without* INVLPGB support, the bug causes
boot to hang as it tries to individually flush each page in the
[0, TLB_FLUSH_ALL] range. :-)

Michael

> +		on_each_cpu(do_flush_tlb_all, NULL, 1);
> +	else
>  		on_each_cpu(do_kernel_range_flush, info, 1);
> 
> -		put_flush_tlb_info();
> -		preempt_enable();
> -	}
> +	put_flush_tlb_info();
>  }
> 
>  /*