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

Marc Zyngier
2021-03-29T10:08:30Z

On Mon, 29 Mar 2021 09:52:10 +0100,
Jingyi Wang <wangjingyi11@huawei.com> 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@huawei.com>
> ---
> 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.