LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jason Yan <yanaijie@huawei.com>
To: John Garry <john.garry@huawei.com>, <martin.petersen@oracle.com>,
	<jejb@linux.vnet.ibm.com>
Cc: <linux-scsi@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<zhaohongjiang@huawei.com>, <hare@suse.com>,
	<dan.j.williams@intel.com>, <jthumshirn@suse.de>, <hch@lst.de>,
	<huangdaode@hisilicon.com>, <chenxiang66@hisilicon.com>,
	<xiexiuqi@huawei.com>, <tj@kernel.org>, <miaoxie@huawei.com>,
	Ewan Milne <emilne@redhat.com>, Tomas Henzl <thenzl@redhat.com>
Subject: Re: [PATCH 4/8] scsi: libsas: trigger a new revalidation to discover the device
Date: Fri, 1 Jun 2018 08:59:37 +0800	[thread overview]
Message-ID: <5B109A79.6060907@huawei.com> (raw)
In-Reply-To: <013c5c91-9792-7951-95ed-22daae2e2dbe@huawei.com>



On 2018/5/31 23:42, John Garry wrote:
> On 29/05/2018 03:23, Jason Yan wrote:
>> Now if a new device replaced a old device, the sas address will change.
>> We unregister the old device and discover the new device in one
>> revalidation process. But after we deferred the sas_port_delete(), the
>> sas port is not deleted when we registering the new port and device.
>> This will make the sysfs complain of creating duplicate filename.
>>
>> Fix this by doing the replacement in two steps. The first revalidation
>> only delete the old device and trigger a new revalidation. The second
>> revalidation discover the new device.
>>
>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>> CC: chenxiang <chenxiang66@hisilicon.com>
>> CC: John Garry <john.garry@huawei.com>
>> CC: Johannes Thumshirn <jthumshirn@suse.de>
>> CC: Ewan Milne <emilne@redhat.com>
>> CC: Christoph Hellwig <hch@lst.de>
>> CC: Tomas Henzl <thenzl@redhat.com>
>> CC: Dan Williams <dan.j.williams@intel.com>
>> CC: Hannes Reinecke <hare@suse.com>
>> ---
>>  drivers/scsi/libsas/sas_expander.c | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_expander.c
>> b/drivers/scsi/libsas/sas_expander.c
>> index 629c580d906b..25ad9ef54e6c 100644
>> --- a/drivers/scsi/libsas/sas_expander.c
>> +++ b/drivers/scsi/libsas/sas_expander.c
>> @@ -2013,6 +2013,8 @@ static int sas_rediscover_dev(struct
>> domain_device *dev, int phy_id, bool last)
>>  {
>>      struct expander_device *ex = &dev->ex_dev;
>>      struct ex_phy *phy = &ex->ex_phy[phy_id];
>> +    struct asd_sas_port *port = dev->port;
>> +    struct asd_sas_phy *sas_phy;
>>      enum sas_device_type type = SAS_PHY_UNUSED;
>>      u8 sas_addr[8];
>>      int res;
>> @@ -2060,7 +2062,14 @@ static int sas_rediscover_dev(struct
>> domain_device *dev, int phy_id, bool last)
>>              SAS_ADDR(phy->attached_sas_addr));
>>      sas_unregister_devs_sas_addr(dev, phy_id, last);
>>
>> -    return sas_discover_new(dev, phy_id);
>> +    /* force the next revalidation find this phy and bring it up */
>> +    phy->phy_change_count = -1;
>> +    ex->ex_change_count = -1;
>> +    sas_phy = container_of(port->phy_list.next, struct asd_sas_phy,
>> +            port_phy_el);
>> +    port->ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
>> +
>
> This is less than ideal: that is, restarting another discovery with this
> artifical broadcast event. We do something similar when re-enabling
> revalidation.
>

That will back to what we have discussed before. The sas port
adding/removing is delayed outside the disco_mutex. we can only do the
adding or removing once inside the disco_mutex.

> Can we do all the event processing synchronised to the original event?
>

Actually bcast is a very special event, and what we do in revalidation
at one time is scanning all phy changes, which may include many bcast
events(especially before our first patchset), and the next
revalidations may have nothing to do.

So "do all the event processing synchronised to the original event" is
impossible actually. Maybe if the bcast can indicate which device
originated it, we will achieve this goal.

But if you mean we shall do this device removing and rediscovering in
one revalidation if it is not a "flutter", I think we can wrap a new
function for sas_revalidate_domain(), such as:


while (need_to_revalidate_again)
	need_to_revalidate_again = sas_revalidate_domain()

In this way the sas_port adding/removing is packed in one loop, we won't 
have the annoyance of "duplicate filename" warning. What do you
think?

>> +    return 0;
>>  }
>>
>>  /**
>>
>
>
>
> .
>

  reply	other threads:[~2018-06-01  1:00 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-29  2:23 [PATCH 0/8] libsas: Support swapping disks and SATA phy link rate matching the pathway Jason Yan
2018-05-29  2:23 ` [PATCH 1/8] scsi: libsas: delete dead code in scsi_transport_sas.c Jason Yan
2018-05-29  7:33   ` Johannes Thumshirn
2018-05-31 14:26   ` John Garry
2018-05-29  2:23 ` [PATCH 2/8] scsi: libsas: check the lldd callback correctly Jason Yan
2018-05-29  7:34   ` Johannes Thumshirn
2018-05-31 14:09   ` John Garry
2018-06-01  0:15     ` Jason Yan
2018-05-29  2:23 ` [PATCH 3/8] scsi: libsas: always unregister the old device if going to discover new Jason Yan
2018-05-29  7:37   ` Johannes Thumshirn
2018-05-31 15:09   ` John Garry
2018-06-01  0:28     ` Jason Yan
2018-05-29  2:23 ` [PATCH 4/8] scsi: libsas: trigger a new revalidation to discover the device Jason Yan
2018-05-29  7:43   ` Johannes Thumshirn
2018-05-31 15:42   ` John Garry
2018-06-01  0:59     ` Jason Yan [this message]
2018-06-01 10:02       ` John Garry
2018-06-04  1:01         ` Jason Yan
2018-05-29  2:23 ` [PATCH 5/8] scsi: libsas: check if the same sata device when flutter Jason Yan
2018-05-29  2:23 ` [PATCH 6/8] scsi: libsas: reset the phy state and address if discover failed Jason Yan
2018-05-29  2:23 ` [PATCH 7/8] scsi: libsas: fix issue of swapping two sas disks Jason Yan
2018-05-29  2:23 ` [PATCH 8/8] scsi: libsas: support SATA phy link rate unmatch the pathway Jason Yan
2018-05-31 16:05   ` John Garry
2018-06-01  1:21     ` Jason Yan
2018-06-01 10:13       ` John Garry

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=5B109A79.6060907@huawei.com \
    --to=yanaijie@huawei.com \
    --cc=chenxiang66@hisilicon.com \
    --cc=dan.j.williams@intel.com \
    --cc=emilne@redhat.com \
    --cc=hare@suse.com \
    --cc=hch@lst.de \
    --cc=huangdaode@hisilicon.com \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=john.garry@huawei.com \
    --cc=jthumshirn@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=miaoxie@huawei.com \
    --cc=thenzl@redhat.com \
    --cc=tj@kernel.org \
    --cc=xiexiuqi@huawei.com \
    --cc=zhaohongjiang@huawei.com \
    --subject='Re: [PATCH 4/8] scsi: libsas: trigger a new revalidation to discover the device' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).