Received-SPF: pass (google.com: domain of linux-kernel+bounces-121446-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Wed, 27 Mar 2024 07:36:45 -0700 (PDT)
From: Paul Walmsley <paul.walmsley@sifive.com>
To: Song Shuai <songshuaishuai@tinylab.org>
cc: palmer@dabbelt.com, aou@eecs.berkeley.edu, guoren@kernel.org, 
    xianting.tian@linux.alibaba.com, takahiro.akashi@linaro.org, 
    linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH -fixes] riscv: kexec: Avoid deadlock in kexec crash
 path
In-Reply-To: <20231208111015.173237-1-songshuaishuai@tinylab.org>
Message-ID: <0e34cd12-66d8-4432-780a-f08d03a2c95e@sifive.com>
References: <20231208111015.173237-1-songshuaishuai@tinylab.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII

Hi Song Shuai, 

On Fri, 8 Dec 2023, Song Shuai wrote:

> If the kexec crash code is called in the interrupt context, the
> machine_kexec_mask_interrupts() function will trigger a deadlock while
> trying to acquire the irqdesc spinlock and then deacitive irqchip.
> 
> To avoid the deadlock, this patch directly EOI the irq regardless of
> the active status of irqchip.

Taking a quick look at the other architectures, looks like no one else is 
doing this.  Is this addressing a RISC-V-only problem?

> diff --git a/arch/riscv/kernel/machine_kexec.c b/arch/riscv/kernel/machine_kexec.c
> index f6c7135b00d7..d7ddf4d2b243 100644
> --- a/arch/riscv/kernel/machine_kexec.c
> +++ b/arch/riscv/kernel/machine_kexec.c
> @@ -149,20 +149,12 @@ static void machine_kexec_mask_interrupts(void)
>  
>  	for_each_irq_desc(i, desc) {
>  		struct irq_chip *chip;
> -		int ret;
>  
>  		chip = irq_desc_get_chip(desc);
>  		if (!chip)
>  			continue;
>  
> -		/*
> -		 * First try to remove the active state. If this
> -		 * fails, try to EOI the interrupt.
> -		 */
> -		ret = irq_set_irqchip_state(i, IRQCHIP_STATE_ACTIVE, false);
> -
> -		if (ret && irqd_irq_inprogress(&desc->irq_data) &&
> -		    chip->irq_eoi)
> +		if (chip->irq_eoi && irqd_irq_inprogress(&desc->irq_data))
>  			chip->irq_eoi(&desc->irq_data);


- Paul