[Qemu-ppc] pseries on qemu-system-ppc64le crashes in doorbell_core_ipi()

[Peter Zijlstra]

+ fweisbec, who did the remote bits

On Sat, Mar 30, 2019 at 01:10:28PM +1000, Nicholas Piggin wrote:
> Something like this?
> 
> kernel/irq_work: Do not raise an IPI when queueing work on the local CPU
> 
> The QEMU powerpc/pseries machine model was not expecting a self-IPI,
> and it may be a bit surprising thing to do, so have irq_work_queue_on
> do local queueing when target is current CPU.

This seems OK to me.

> Suggested-by: Steven Rostedt <rostedt at goodmis.org>
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>

Acked-by: Peter Zijlstra (Intel) <peterz at infradead.org>

> ---
>  kernel/irq_work.c | 78 ++++++++++++++++++++++++++---------------------
>  1 file changed, 43 insertions(+), 35 deletions(-)
> 
> diff --git a/kernel/irq_work.c b/kernel/irq_work.c
> index 6b7cdf17ccf8..f0e539d0f879 100644
> --- a/kernel/irq_work.c
> +++ b/kernel/irq_work.c
> @@ -56,61 +56,69 @@ void __weak arch_irq_work_raise(void)
>  	 */
>  }
>  
> -/*
> - * Enqueue the irq_work @work on @cpu unless it's already pending
> - * somewhere.
> - *
> - * Can be re-enqueued while the callback is still in progress.
> - */
> -bool irq_work_queue_on(struct irq_work *work, int cpu)
> +/* Enqueue on current CPU, work must already be claimed and preempt disabled */
> +static void __irq_work_queue(struct irq_work *work)
>  {
> -	/* All work should have been flushed before going offline */
> -	WARN_ON_ONCE(cpu_is_offline(cpu));
> -
> -#ifdef CONFIG_SMP
> -
> -	/* Arch remote IPI send/receive backend aren't NMI safe */
> -	WARN_ON_ONCE(in_nmi());
> +	/* If the work is "lazy", handle it from next tick if any */
> +	if (work->flags & IRQ_WORK_LAZY) {
> +		if (llist_add(&work->llnode, this_cpu_ptr(&lazy_list)) &&
> +		    tick_nohz_tick_stopped())
> +			arch_irq_work_raise();
> +	} else {
> +		if (llist_add(&work->llnode, this_cpu_ptr(&raised_list)))
> +			arch_irq_work_raise();
> +	}
> +}
>  
> +/* Enqueue the irq work @work on the current CPU */
> +bool irq_work_queue(struct irq_work *work)
> +{
>  	/* Only queue if not already pending */
>  	if (!irq_work_claim(work))
>  		return false;
>  
> -	if (llist_add(&work->llnode, &per_cpu(raised_list, cpu)))
> -		arch_send_call_function_single_ipi(cpu);
> -
> -#else /* #ifdef CONFIG_SMP */
> -	irq_work_queue(work);
> -#endif /* #else #ifdef CONFIG_SMP */
> +	/* Queue the entry and raise the IPI if needed. */
> +	preempt_disable();
> +	__irq_work_queue(work);
> +	preempt_enable();
>  
>  	return true;
>  }
> +EXPORT_SYMBOL_GPL(irq_work_queue);
>  
> -/* Enqueue the irq work @work on the current CPU */
> -bool irq_work_queue(struct irq_work *work)
> +/*
> + * Enqueue the irq_work @work on @cpu unless it's already pending
> + * somewhere.
> + *
> + * Can be re-enqueued while the callback is still in progress.
> + */
> +bool irq_work_queue_on(struct irq_work *work, int cpu)
>  {
> +#ifndef CONFIG_SMP
> +	return irq_work_queue(work);
> +
> +#else /* #ifndef CONFIG_SMP */
> +	/* All work should have been flushed before going offline */
> +	WARN_ON_ONCE(cpu_is_offline(cpu));
> +
>  	/* Only queue if not already pending */
>  	if (!irq_work_claim(work))
>  		return false;
>  
> -	/* Queue the entry and raise the IPI if needed. */
>  	preempt_disable();
> -
> -	/* If the work is "lazy", handle it from next tick if any */
> -	if (work->flags & IRQ_WORK_LAZY) {
> -		if (llist_add(&work->llnode, this_cpu_ptr(&lazy_list)) &&
> -		    tick_nohz_tick_stopped())
> -			arch_irq_work_raise();
> -	} else {
> -		if (llist_add(&work->llnode, this_cpu_ptr(&raised_list)))
> -			arch_irq_work_raise();
> -	}
> -
> +	if (cpu != smp_processor_id()) {
> +		/* Arch remote IPI send/receive backend aren't NMI safe */
> +		WARN_ON_ONCE(in_nmi());
> +		if (llist_add(&work->llnode, &per_cpu(raised_list, cpu)))
> +			arch_send_call_function_single_ipi(cpu);
> +	} else
> +		__irq_work_queue(work);
>  	preempt_enable();
>  
>  	return true;
> +#endif /* #else #ifndef CONFIG_SMP */
>  }
> -EXPORT_SYMBOL_GPL(irq_work_queue);
> +
>  
>  bool irq_work_needs_cpu(void)
>  {
> -- 
> 2.20.1
>