linux-kernel - Re: [PATCH v4] ata: libata-eh: Honor all EH scheduling requests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 11 Sep 2023 19:32:45 +0800
From:   Li Nan <linan666@...weicloud.com>
To:     Damien Le Moal <dlemoal@...nel.org>,
        Niklas Cassel <Niklas.Cassel@....com>,
        kernel test robot <oliver.sang@...el.com>
Cc:     "oe-lkp@...ts.linux.dev" <oe-lkp@...ts.linux.dev>,
        "lkp@...el.com" <lkp@...el.com>, luojian <luojian5@...wei.com>,
        "linux-ide@...r.kernel.org" <linux-ide@...r.kernel.org>,
        "htejun@...il.com" <htejun@...il.com>,
        "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 v4] ata: libata-eh: Honor all EH scheduling requests



在 2023/9/11 15:16, Damien Le Moal 写道:
> On 9/8/23 06:02, Niklas Cassel wrote:
>> On Thu, Sep 07, 2023 at 03:43:19PM +0800, kernel test robot wrote:
> 
> I am not confident that playing with host_eh_schedule count is the right

Yeah, If there is some way can wake up EH without inc host_eh_schedule,
dec count becomes risky.

> approach. A better solution may be to change the timing of clearing
> ATA_PFLAG_EH_PENDING. Right now, this is done on entry to
> ata_scsi_port_error_handler(), unconditionally. So ata_eh_reset() should not
> need to clear the flag again. If we remove that, then a new interrupt received
> after ata_eh_thaw() and setting EH_PENDING would be cought by the retry loop in
> ata_scsi_port_error_handler(), which would run again ap->ops->error_handler(ap).
> 
> So let's try this fix instead:
> 
> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index 159ba6ba19eb..d1d081aa0c95 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -2807,7 +2807,6 @@ int ata_eh_reset(struct ata_link *link, int classify,
>          memset(&link->eh_info, 0, sizeof(link->eh_info));
>          if (slave)
>                  memset(&slave->eh_info, 0, sizeof(link->eh_info));
> -       ap->pflags &= ~ATA_PFLAG_EH_PENDING;
>          spin_unlock_irqrestore(link->ap->lock, flags);
> 
>          if (ata_port_is_frozen(ap))
> 
> Li,
> 
> Can you please test this ?
> 

OK, Let us test this patch.

>>
>>
>> It might be worth mentioning that the race window for the bug that the patch
>> in $subject is fixing, should be much smaller after this patch is in:
>> https://lore.kernel.org/linux-ide/20230907081710.4946-1-Chloe_Chen@asmedia.com.tw/
>>
>> Li Nan, perhaps you could see if you can still reproduce your original
>> problem with the patch from the ASMedia guys?
> 
> 
>>
>> However, even with the ASMedia patch, it should still be theoretically
>> possible to get an error irq after ata_eh_reset() has called ahci_thaw(),
>> so I suppose that this patch still makes some sense...
>>
>>
>> Kind regards,
>> Niklas
> 

-- 
Thanks,
Nan