linux-kernel - Re: [PATCH] libata: Use per port sync for detach

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 for Android: free password hash cracker in your pocket

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

Message-Id: <826B8B0D-29EE-4C4F-BBD3-B6442144CA65@canonical.com>
Date:   Tue, 19 May 2020 14:09:33 +0800
From:   Kai-Heng Feng <kai.heng.feng@...onical.com>
To:     John Garry <john.garry@...wei.com>
Cc:     Jens Axboe <axboe@...nel.dk>,
        "open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers)" 
        <linux-ide@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] libata: Use per port sync for detach

Hi John,

> On May 18, 2020, at 23:21, John Garry <john.garry@...wei.com> wrote:
> 
> On 18/05/2020 10:06, John Garry wrote:
>> On 15/05/2020 18:48, Kai-Heng Feng wrote:
>>>> 841]  ret_from_fork+0x10/0x1c
>>>> [   28.393400] ---[ end trace 9972785c7052048f ]---
>>>> [   28.435826] ahci 0000:b4:03.0: SSS flag set, parallel bus scan disabled
>>> Can you please test the following patch:
>>> 
>>> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
>>> index 474c6c34fe02..51ee0cc4d414 100644
>>> --- a/drivers/ata/libata-eh.c
>>> +++ b/drivers/ata/libata-eh.c
>>> @@ -3583,8 +3583,10 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
>>>          rc = 0;
>>>          /* if UNLOADING, finish immediately */
>>> -       if (ap->pflags & ATA_PFLAG_UNLOADING)
>>> +       if (ap->pflags & ATA_PFLAG_UNLOADING) {
>>> +               ap->pflags |= ATA_PFLAG_UNLOADED;
>>>                  goto out;
>>> +       }
>>> 
>>> It's only compile-tested, many drivers panic with CONFIG_DEBUG_TEST_DRIVER_REMOVE enabled, so the system I have can't even boot properly:(
> 
> According to the comment for async_synchronize_cookie(), we sync upto (but excluding) the cookie:
> 
> /**
> * async_synchronize_cookie - synchronize asynchronous function calls with cookie checkpointing
> *
> * This function waits until all asynchronous function calls prior to @cookie
> * have been done.
> 
> So maybe it should be:
> 
> +	for (i = 0; i < host->n_ports; i++)
> +		async_synchronize_cookie(host->ports[i]->cookie + 1);
> 
> That is how other callsites use this API, and that change resolves the WARN for me.

Thanks for catching this up!
Let me ask the user to test it, and I'll send v2 base on this.

Kai-Heng

> 
> Thanks,
> John