Re: [PATCH 7/8] riscv: improve the local flushing logic in sys_riscv_flush_icache

[Atish Patra <Atish.Patra@wdc.com>]

On Thu, 2019-08-22 at 15:56 +0900, Christoph Hellwig wrote:
> If we have to offload any remote sfence the SBI we might as well let
> it
> handle the local one as well.  This significantly simplifies the
> cpumask
> operations and streamlines the code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/riscv/mm/cacheflush.c | 33 ++++++++++++++-------------------
>  1 file changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index 9180b2e93058..8f1134715fec 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -20,7 +20,6 @@ void flush_icache_all(void)
>  static void flush_icache_mm(bool local)
>  {
>  	unsigned int cpu = get_cpu();

get_cpu should be removed from here as it is called again in below.
> -	cpumask_t others, hmask;
>  
>  	/*
>  	 * Mark the I$ for all harts not concurrently executing as
> needing a
> @@ -29,27 +28,23 @@ static void flush_icache_mm(bool local)
>  	cpumask_andnot(&current->mm->context.icache_stale_mask,
>  		       cpu_possible_mask, mm_cpumask(current->mm));
>  
> -	/* Flush this hart's I$ now, and mark it as flushed. */
> -	local_flush_icache_all();
> -
>  	/*
> -	 * Flush the I$ of other harts concurrently executing.
> +	 * It's assumed that at least one strongly ordered operation is
> +	 * performed on this hart between setting a hart's cpumask bit
> and
> +	 * scheduling this MM context on that hart.  Sending an SBI
> remote
> +	 * message will do this, but in the case where no messages are
> sent we
> +	 * still need to order this hart's writes with
> flush_icache_deferred().
>  	 */
> -	cpumask_andnot(&others, mm_cpumask(current->mm),
> cpumask_of(cpu));
> -	local |= cpumask_empty(&others);
> -	if (!local) {
> -		riscv_cpuid_to_hartid_mask(&others, &hmask);
> -		sbi_remote_fence_i(hmask.bits);
> -	} else {
> -		/*
> -		 * It's assumed that at least one strongly ordered
> operation is
> -		 * performed on this hart between setting a hart's
> cpumask bit
> -		 * and scheduling this MM context on that
> hart.  Sending an SBI
> -		 * remote message will do this, but in the case where
> no
> -		 * messages are sent we still need to order this hart's
> writes
> -		 * with flush_icache_deferred().
> -		 */
> +	cpu = get_cpu();
> +	if (local ||
> +	    cpumask_any_but(mm_cpumask(current->mm), cpu) >=
> nr_cpu_ids) {
> +		local_flush_icache_all();
>  		smp_mb();
> +	} else {
> +		cpumask_t hmask;
> +
> +		riscv_cpuid_to_hartid_mask(mm_cpumask(current->mm),
> &hmask);
> +		sbi_remote_fence_i(cpumask_bits(&hmask));
>  	}
>  
>  	put_cpu();

Otherwise, looks good to me.

Reviewed-by: Atish Patra <atish.patra@wdc.com>

-- 
Regards,
Atish
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv