Re: [PATCH v4 2/9] EDAC/aspeed: Free the interrupt before the mem_ctl_info on remove

[Borislav Petkov]

On Wed, Sep 30, 2026 at 01:15:00PM +0800, Ryan Chen wrote:
> The ECC interrupt is devm-managed, so it is only released after .remove()
> has returned, and masking the controller does not wait for a handler
> already running on another CPU. edac_mc_free() can therefore free the
> mem_ctl_info the handler uses as its context while it is still running.
> 
> Fix the ordering and synchronise by freeing the interrupt prior to
> releasing related memory.
> 
> Fixes: 9b7e6242ee4e ("EDAC, aspeed: Add an Aspeed AST2500 EDAC driver")
> Signed-off-by: Ryan Chen <ryan_chen@xxxxxxxxxxxxxx>
> 
> ---
> Changes in v4:
> - Drop the Cc: stable tag.
> 
> Changes in v2:
> - New patch.
> ---
>  drivers/edac/aspeed_edac.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
> index 83d60414f89a..e05ebed5c2f2 100644
> --- a/drivers/edac/aspeed_edac.c
> +++ b/drivers/edac/aspeed_edac.c
> @@ -359,11 +359,16 @@ static int aspeed_probe(struct platform_device *pdev)
>  static void aspeed_remove(struct platform_device *pdev)
>  {
>  	struct mem_ctl_info *mci;
> +	int irq;
>  
>  	/* disable interrupts */
>  	regmap_update_bits(aspeed_regmap, ASPEED_MCR_INTR_CTRL,
>  			   ASPEED_MCR_INTR_CTRL_ENABLE, 0);
>  
> +	irq = platform_get_irq(pdev, 0);
> +	WARN_ON(irq < 0);
> +	devm_free_irq(&pdev->dev, irq, platform_get_drvdata(pdev));

A couple of things come to mind when looking at this:

- if you have to do platform_get_irq(), then something in the design of this
  driver is missing. Perhaps a private structure:

  struct mem_ctl_info.pvt_info

  which contains driver-specific stuff like the irq number and so on. Look at
  the other EDAC drivers for examples.

- then, what is the point of this being a devm-managed IRQ if you have to
  explicitly free it?

  By that logic, you might as well use the normal IRQ registration stuff.

  But then looking at the other EDAC drivers, they all do devm_request_irq()
  but only the DMC-520 one does free_irq().

  So that would mean that either all the other EDAC drivers need such a fix.
  And they would all need proper auditing to make sure when they disable IRQs,
  all IRQ handlers running have finished.

  Or, their hw can handle that and IRQs firing at driver remove time is not an
  issue...

  Sounds to me like I need to do a EDAC subsystem TODO for all the driver
  owners to go read and check...

So I'm presuming this is Sashiko reporting an issue and you're addressing it.
Or is it a real issue that someone hits?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette