Linux-ide Archive on lore.kernel.org
 help / color / Atom feed
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
To: John Garry <john.garry@huawei.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	"open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers)" 
	<linux-ide@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] libata: Use per port sync for detach
Date: Tue, 19 May 2020 14:09:33 +0800
Message-ID: <826B8B0D-29EE-4C4F-BBD3-B6442144CA65@canonical.com> (raw)
In-Reply-To: <1470d3de-7785-9f05-2b12-9272ac0005b8@huawei.com>

Hi John,

> On May 18, 2020, at 23:21, John Garry <john.garry@huawei.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


      reply index

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-15 11:09 Kai-Heng Feng
2020-05-15 12:38 ` John Garry
2020-05-15 17:48   ` Kai-Heng Feng
2020-05-18  9:06     ` John Garry
2020-05-18 15:21       ` John Garry
2020-05-19  6:09         ` Kai-Heng Feng [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=826B8B0D-29EE-4C4F-BBD3-B6442144CA65@canonical.com \
    --to=kai.heng.feng@canonical.com \
    --cc=axboe@kernel.dk \
    --cc=john.garry@huawei.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ide Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ide/0 linux-ide/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ide linux-ide/ https://lore.kernel.org/linux-ide \
		linux-ide@vger.kernel.org
	public-inbox-index linux-ide

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ide


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git