[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZONr0f26IT/QKsSu@x1-carbon>
Date: Mon, 21 Aug 2023 13:51:18 +0000
From: Niklas Cassel <Niklas.Cassel@....com>
To: "linan666@...weicloud.com" <linan666@...weicloud.com>
CC: "dlemoal@...nel.org" <dlemoal@...nel.org>,
"linux-ide@...r.kernel.org" <linux-ide@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linan122@...wei.com" <linan122@...wei.com>,
"yukuai3@...wei.com" <yukuai3@...wei.com>,
"yi.zhang@...wei.com" <yi.zhang@...wei.com>,
"houtao1@...wei.com" <houtao1@...wei.com>,
"yangerkun@...wei.com" <yangerkun@...wei.com>
Subject: Re: [PATCH] scsi: ata: Fix a race condition between scsi error
handler and ahci interrupt
On Thu, Aug 10, 2023 at 09:48:48AM +0800, linan666@...weicloud.com wrote:
> From: Li Nan <linan122@...wei.com>
>
> interrupt scsi_eh
>
> ahci_error_intr
> =>ata_port_freeze
> =>__ata_port_freeze
> =>ahci_freeze (turn IRQ off)
> =>ata_port_abort
> =>ata_port_schedule_eh
> =>shost->host_eh_scheduled++;
> host_eh_scheduled = 1
> scsi_error_handler
> =>ata_scsi_error
> =>ata_scsi_port_error_handler
> =>ahci_error_handler
> . =>sata_pmp_error_handler
> . =>ata_eh_thaw_port
> . =>ahci_thaw (turn IRQ on)
> ahci_error_intr .
> =>ata_port_freeze .
> =>__ata_port_freeze .
> =>ahci_freeze (turn IRQ off) .
> =>ata_port_abort .
> =>ata_port_schedule_eh .
> =>shost->host_eh_scheduled++; .
> host_eh_scheduled = 2 .
> =>ata_std_end_eh
> =>host->host_eh_scheduled = 0;
Hello Li Nan,
I do not understand why the code in:
https://github.com/torvalds/linux/blob/v6.5-rc7/drivers/ata/libata-eh.c#L722-L731
does not kick in, and repeats EH.
EH_PENDING is cleared before ->error_handler() is called:
https://github.com/torvalds/linux/blob/v6.5-rc7/drivers/ata/libata-eh.c#L697
So ahci_error_intr() from the second error interrupt, which is called after
thawing the port, should have called ata_std_sched_eh(), which calls
ata_eh_set_pending(), which should have set EH_PENDING:
https://github.com/torvalds/linux/blob/v6.5-rc7/drivers/ata/libata-eh.c#L884
My only guess is that after thawing the port:
https://github.com/torvalds/linux/blob/v6.5-rc7/drivers/ata/libata-eh.c#L2807
The second error irq comes, and sets EH_PENDING,
but then this silly code might clear it:
https://github.com/torvalds/linux/blob/v6.5-rc7/drivers/ata/libata-eh.c#L2825-L2837
I think the best way would be if we could improve this "spurious error
condition check"... because if this is indeed the code that clears EH_PENDING
for you, then this code basically makes the "goto repeat" code in
ata_scsi_port_error_handler() useless...
An alternative to improving the "spurious error condition check" might be for
you to try something like:
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 35e03679b0bf..82f032934ae1 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -962,7 +962,7 @@ void ata_std_end_eh(struct ata_port *ap)
{
struct Scsi_Host *host = ap->scsi_host;
- host->host_eh_scheduled = 0;
+ host->host_eh_scheduled--;
}
EXPORT_SYMBOL(ata_std_end_eh);
...and see if that improves things for you.
Kind regards,
Niklas
Powered by blists - more mailing lists