On Thu, Sep 14, 2023 at 08:51:06AM +0900, Damien Le Moal wrote:
> On 9/14/23 07:19, Niklas Cassel wrote:
> > From: Niklas Cassel <niklas.cassel@xxxxxxx>
> >
> > commit 1e641060c4b5 ("libata: clear eh_info on reset completion") added
> > a workaround that broke the retry mechanism in ATA EH.
> >
> > Tejun himself suggested to remove this workaround when it was identified
> > to cause additional problems:
> > https://lore.kernel.org/linux-ide/20110426135027.GI878@xxxxxxxxxxxxxx/
> >
> > He and even said:
> > "Hmm... it seems I wasn't thinking straight when I added that work around."
> > https://lore.kernel.org/linux-ide/20110426155229.GM878@xxxxxxxxxxxxxx/
> >
> > While removing the workaround solved the issue, however, the workaround was
> > kept to avoid "spurious hotplug events during reset", and instead another
> > workaround was added on top of the existing workaround in commit
> > 8c56cacc724c ("libata: fix unexpectedly frozen port after ata_eh_reset()").
> >
> > Because these IRQs happened when the port was frozen, we know that they
> > were actually a side effect of PxIS and IS.IPS(x) not being cleared before
> > the COMRESET. This is now done in commit 94152042eaa9 ("ata: libahci: clear
> > pending interrupt status"), so these workarounds can now be removed.
> >
> > Since commit 1e641060c4b5 ("libata: clear eh_info on reset completion") has
> > now been reverted, the ATA EH retry mechanism is functional again, so there
> > is once again no need to thaw the port more than once in ata_eh_reset().
> >
> > This reverts "the workaround on top of the workaround" introduced in commit
> > 8c56cacc724c ("libata: fix unexpectedly frozen port after ata_eh_reset()").
> >
> > Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
>
> We need a fixes tag. Same for patch 1.
The workaround introduced in commit 1e641060c4b5 ("libata: clear eh_info on
reset completion") broke ATA EH retry logic, so the proper commit that we
fix is that commit.
However, if we put a Fixes tag with that commit, then this patch will get
backported to all possible stable kernels that has that commit, something
that we do _not_ want.
We can only remove this workaround for kernels that has commit 94152042eaa9
("ata: libahci: clear pending interrupt status").
Do we really need a Fixes tag?
The workaround (which broke ATA EH retry logic) has been in the kernel for
14 years, since then, we've only seen two complaints..
the one by Bruce Stenning 12 years ago (see commit log for this patch),
and the complaint from Huawei folks this year..
I guess we could set the Fixes tag to 94152042eaa9 ("ata: libahci: clear
pending interrupt status"), since we depend on that commit.
However, that is basically a lie, since we are not fixing that commit.
Kind regards,
Niklas
