Re: [RFC PATCH 3/3] arm/arm64: Use gic_ipi_send_single() to inject single IPI

[Marc Zyngier]

On Mon, 29 Mar 2021 09:52:10 +0100,
Jingyi Wang <wangjingyi11@xxxxxxxxxx> wrote:
> 
> Currently, arm use gic_ipi_send_mask() to inject single IPI, which
> make the procedure a little complex. We use gic_ipi_send_single()
> instead as some other archs.
> 
> Signed-off-by: Jingyi Wang <wangjingyi11@xxxxxxxxxx>
> ---
>  arch/arm/kernel/smp.c   | 16 +++++++++++++---
>  arch/arm64/kernel/smp.c | 16 +++++++++++++---
>  2 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 74679240a9d8..369ce529cdd8 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -534,6 +534,8 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
>  };
>  
>  static void smp_cross_call(const struct cpumask *target, unsigned int ipinr);
> +static void smp_cross_call_single(const struct cpumask *target, int cpu,
> +				  unsigned int ipinr);

Why does this function need to take both a cpumask *and* a cpu, given
that they represent the same thing?

>
>  void show_ipi_list(struct seq_file *p, int prec)
>  {
> @@ -564,14 +566,15 @@ void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
>  
>  void arch_send_call_function_single_ipi(int cpu)
>  {
> -	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC);
> +	smp_cross_call_single(cpumask_of(cpu), cpu, IPI_CALL_FUNC);
>  }
>  
>  #ifdef CONFIG_IRQ_WORK
>  void arch_irq_work_raise(void)
>  {
> +	int cpu = smp_processor_id();
>  	if (arch_irq_work_has_interrupt())
> -		smp_cross_call(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
> +		smp_cross_call(cpumask_of(cpu), cpu, IPI_IRQ_WORK);

Why isn't that a call to smp_cross_call_single()?

>  }
>  #endif
>  
> @@ -707,6 +710,13 @@ static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
>  	__ipi_send_mask(ipi_desc[ipinr], target);
>  }
>  
> +static void smp_cross_call_single(const struct cpumask *target, int cpu,
> +				  unsigned int ipinr)
> +{
> +	trace_ipi_raise_rcuidle(target, ipi_types[ipinr]);

Why don't you compute the cpumask here^^?

> +	__ipi_send_single(ipi_desc[ipinr], cpu);
> +}
> +
>  static void ipi_setup(int cpu)
>  {
>  	int i;
> @@ -744,7 +754,7 @@ void __init set_smp_ipi_range(int ipi_base, int n)
>  
>  void smp_send_reschedule(int cpu)
>  {
> -	smp_cross_call(cpumask_of(cpu), IPI_RESCHEDULE);
> +	smp_cross_call_single(cpumask_of(cpu), cpu, IPI_RESCHEDULE);
>  }
>  
>  void smp_send_stop(void)
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 357590beaabb..d290b6dc5a6e 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c

Similar comments for the arm64 side.

Overall, this needs to be backed by data that indicates that there is
an actual benefit for this extra complexity.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.