Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1628029 > unrolled thread

[PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in SAS HBA LLDDs

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in SAS HBA LLDDs Johannes Thumshirn <jthumshirn@suse.de> - 2017-04-21 10:10 +0200
    [PATCH v2 5/5] scsi: pm8001: remove the SAS host after the SCSI host Johannes Thumshirn <jthumshirn@suse.de> - 2017-04-21 10:10 +0200
    Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in  SAS HBA LLDDs Johannes Thumshirn <jthumshirn@suse.de> - 2017-04-21 10:40 +0200
      Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in  SAS HBA LLDDs John Garry <john.garry@huawei.com> - 2017-04-21 11:00 +0200
        Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host         in SAS HBA LLDDs Christoph Hellwig <hch@lst.de> - 2017-04-21 13:30 +0200
          Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in  SAS HBA LLDDs John Garry <john.garry@huawei.com> - 2017-04-21 14:00 +0200
            Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in  SAS HBA LLDDs wangyijing <wangyijing@huawei.com> - 2017-04-22 02:50 +0200
    Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in  SAS HBA LLDDs John Garry <john.garry@huawei.com> - 2017-04-21 10:40 +0200
    Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in  SAS HBA LLDDs Johannes Thumshirn <jthumshirn@suse.de> - 2017-04-21 13:30 +0200
    Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host         in SAS HBA LLDDs Christoph Hellwig <hch@lst.de> - 2017-04-21 13:30 +0200

#1628029 — [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in SAS HBA LLDDs

This series re-orders the calls to scsi_remove_host() and sas_remove_host() in
all SAS HBA drivers (apart from mpt3sas which is doing it correctly). This is
for two reasons:
	1) After the change to recursive removal of sysfs entries, we're
	   trying to remove already removed kobjects when doing a
	   sas_remove_host() _after_ a scsi_remove_host()
	2) the documentation mandates it even (becuase of 1)

Unfortunately this does not completely solve issues with recursive sysfs
removals in SAS, as libsas has asynchronous behaviour where strong ordering
would be needed. But I am working on it and I do know other do as well. So if
anyone else (James, Christoph, Bart, I'm looking at you) has an idea, I do
have test setups and I'm willing to take input in form of ideas and patches.

I also dropped the SDEV_CANCEL state change for now. We re-evaluate it once we
have an idea how to tackle the ordering issues and place it into
sas_unregister_ha() as per James' comment.

Thanks,
	Johannes

Johannes Thumshirn (5):
  scsi: isci: remove the SAS host after the SCSI host
  aic94xx: remove the SAS host after the SCSI host
  scsi: hisi_sas: remove the SAS host after the SCSI host
  mvsas: remove the SAS host after the SCSI host
  scsi: pm8001: remove the SAS host after the SCSI host

 drivers/scsi/aic94xx/aic94xx_init.c   | 7 ++++---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 4 ++--
 drivers/scsi/isci/init.c              | 3 ++-
 drivers/scsi/mvsas/mv_init.c          | 6 ++++--
 drivers/scsi/pm8001/pm8001_init.c     | 8 ++++++--
 5 files changed, 18 insertions(+), 10 deletions(-)

-- 
2.12.0

[toc] | [next] | [standalone]

#1628031 — [PATCH v2 5/5] scsi: pm8001: remove the SAS host after the SCSI host

After commit bcdde7e ("sysfs: make __sysfs_remove_dir() recursive") changed the
removal path of kernfs to make it recursive we have to remove the SAS host
before the SCSI host or we will see sysfs warnings on not found sysfs groups for
kobjects.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/pm8001/pm8001_init.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 417368ccb686..e8f3d5ada201 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -1086,11 +1086,15 @@ static void pm8001_pci_remove(struct pci_dev *pdev)
 {
 	struct sas_ha_struct *sha = pci_get_drvdata(pdev);
 	struct pm8001_hba_info *pm8001_ha;
+	struct Scsi_Host *shost;
 	int i, j;
 	pm8001_ha = sha->lldd_ha;
-	scsi_remove_host(pm8001_ha->shost);
+	shost = pm8001_ha->shost;
+
 	sas_unregister_ha(sha);
-	sas_remove_host(pm8001_ha->shost);
+	sas_remove_host(shost);
+	scsi_remove_host(shost);
+
 	list_del(&pm8001_ha->list);
 	PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF);
 	PM8001_CHIP_DISP->chip_soft_rst(pm8001_ha);
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1628048 — Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in SAS HBA LLDDs

On Fri, Apr 21, 2017 at 09:34:18AM +0100, John Garry wrote:
> Thanks Johannes.
> 
> @wangyijing, can you test this patchset please (specifically 3/5)? I know
> that you have the modified version of libsas which you dabbled with
> upstreaming.
> 
> On 21/04/2017 09:04, Johannes Thumshirn wrote:
> >This series re-orders the calls to scsi_remove_host() and sas_remove_host() in
> >all SAS HBA drivers (apart from mpt3sas which is doing it correctly). This is
> >for two reasons:
> >	1) After the change to recursive removal of sysfs entries, we're
> >	   trying to remove already removed kobjects when doing a
> >	   sas_remove_host() _after_ a scsi_remove_host()
> >	2) the documentation mandates it even (becuase of 1)
> >
> >Unfortunately this does not completely solve issues with recursive sysfs
> >removals in SAS, as libsas has asynchronous behaviour where strong ordering
> >would be needed. But I am working on it and I do know other do as well. So if
> >anyone else (James, Christoph, Bart, I'm looking at you) has an idea, I do
> >have test setups and I'm willing to take input in form of ideas and patches.
> >
> 
> wangyijing already sent an RFC for fixing this issue (mentioned above),
> which was a signifiagnt rewrite of some of libsas.
> I am hoping that he would retry, and that community would support/shepherd
> this activity, or at least say it will be accepted so effort is not wasted.

Do you have a link to the series? No problem if not, I'll probably can find it
in my archives. Anyways, if Wangyijing is re-sending his patches please Cc me
on it.

Thanks,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[toc] | [prev] | [next] | [standalone]

#1628068 — Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in SAS HBA LLDDs

On 21/04/2017 09:39, Johannes Thumshirn wrote:
>> wangyijing already sent an RFC for fixing this issue (mentioned above),
>> > which was a signifiagnt rewrite of some of libsas.
>> > I am hoping that he would retry, and that community would support/shepherd
>> > this activity, or at least say it will be accepted so effort is not wasted.
> Do you have a link to the series? No problem if not, I'll probably can find it
> in my archives. Anyways, if Wangyijing is re-sending his patches please Cc me
> on it.
>

https://marc.info/?l=linux-scsi&m=147494612310529&w=2

> Thanks,
> 	Johannes

John

[toc] | [prev] | [next] | [standalone]

#1628155 — Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in SAS HBA LLDDs

On Fri, Apr 21, 2017 at 09:57:37AM +0100, John Garry wrote:
> On 21/04/2017 09:39, Johannes Thumshirn wrote:
>>> wangyijing already sent an RFC for fixing this issue (mentioned above),
>>> > which was a signifiagnt rewrite of some of libsas.
>>> > I am hoping that he would retry, and that community would support/shepherd
>>> > this activity, or at least say it will be accepted so effort is not wasted.
>> Do you have a link to the series? No problem if not, I'll probably can find it
>> in my archives. Anyways, if Wangyijing is re-sending his patches please Cc me
>> on it.
>>
>
> https://marc.info/?l=linux-scsi&m=147494612310529&w=2

Please repost the series against the current tree.  Also a cover
letter with a bit more explanation would be good.

[toc] | [prev] | [next] | [standalone]

#1628178 — Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in SAS HBA LLDDs

On 21/04/2017 12:20, Christoph Hellwig wrote:
> On Fri, Apr 21, 2017 at 09:57:37AM +0100, John Garry wrote:
>> On 21/04/2017 09:39, Johannes Thumshirn wrote:
>>>> wangyijing already sent an RFC for fixing this issue (mentioned above),
>>>>> which was a signifiagnt rewrite of some of libsas.
>>>>> I am hoping that he would retry, and that community would support/shepherd
>>>>> this activity, or at least say it will be accepted so effort is not wasted.
>>> Do you have a link to the series? No problem if not, I'll probably can find it
>>> in my archives. Anyways, if Wangyijing is re-sending his patches please Cc me
>>> on it.
>>>
>>
>> https://marc.info/?l=linux-scsi&m=147494612310529&w=2
>
> Please repost the series against the current tree.  Also a cover
> letter with a bit more explanation would be good.
>

Great, I think that my colleagues can send an updated version next week.

Much appreciated,
John

> .
>

[toc] | [prev] | [next] | [standalone]

#1628708 — Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in SAS HBA LLDDs

>>
>> Please repost the series against the current tree.  Also a cover
>> letter with a bit more explanation would be good.
>>
> 
> Great, I think that my colleagues can send an updated version next week.

I will send a new version with a cover letter these days, before that, we need
to do a detail test on real hardware based the current mainline.

Thannks!
Yijing.

> 
> Much appreciated,
> John
> 
>> .
>>
> 
> 
> .
> 

[toc] | [prev] | [next] | [standalone]

#1628051 — Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in SAS HBA LLDDs

Thanks Johannes.

@wangyijing, can you test this patchset please (specifically 3/5)? I 
know that you have the modified version of libsas which you dabbled with 
upstreaming.

On 21/04/2017 09:04, Johannes Thumshirn wrote:
> This series re-orders the calls to scsi_remove_host() and sas_remove_host() in
> all SAS HBA drivers (apart from mpt3sas which is doing it correctly). This is
> for two reasons:
> 	1) After the change to recursive removal of sysfs entries, we're
> 	   trying to remove already removed kobjects when doing a
> 	   sas_remove_host() _after_ a scsi_remove_host()
> 	2) the documentation mandates it even (becuase of 1)
>
> Unfortunately this does not completely solve issues with recursive sysfs
> removals in SAS, as libsas has asynchronous behaviour where strong ordering
> would be needed. But I am working on it and I do know other do as well. So if
> anyone else (James, Christoph, Bart, I'm looking at you) has an idea, I do
> have test setups and I'm willing to take input in form of ideas and patches.
>

wangyijing already sent an RFC for fixing this issue (mentioned above), 
which was a signifiagnt rewrite of some of libsas.
I am hoping that he would retry, and that community would 
support/shepherd this activity, or at least say it will be accepted so 
effort is not wasted.

> I also dropped the SDEV_CANCEL state change for now. We re-evaluate it once we
> have an idea how to tackle the ordering issues and place it into
> sas_unregister_ha() as per James' comment.
>
> Thanks,
> 	Johannes

Thanks

>
> Johannes Thumshirn (5):
>   scsi: isci: remove the SAS host after the SCSI host
>   aic94xx: remove the SAS host after the SCSI host
>   scsi: hisi_sas: remove the SAS host after the SCSI host
>   mvsas: remove the SAS host after the SCSI host
>   scsi: pm8001: remove the SAS host after the SCSI host
>
>  drivers/scsi/aic94xx/aic94xx_init.c   | 7 ++++---
>  drivers/scsi/hisi_sas/hisi_sas_main.c | 4 ++--
>  drivers/scsi/isci/init.c              | 3 ++-
>  drivers/scsi/mvsas/mv_init.c          | 6 ++++--
>  drivers/scsi/pm8001/pm8001_init.c     | 8 ++++++--
>  5 files changed, 18 insertions(+), 10 deletions(-)
>

[toc] | [prev] | [next] | [standalone]

#1628153 — Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in SAS HBA LLDDs

On Fri, Apr 21, 2017 at 01:19:41PM +0200, Christoph Hellwig wrote:
> 
> Any reason to not just make sas_remove_host call scsi_remove_host
> to ensure we get the ordering right?

No other than "haven't thought of it". But let me double check the LLDDs fist.

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[toc] | [prev] | [next] | [standalone]

#1628160 — Re: [PATCH v2 0/5] Re-order scsi_remove_host and sas_remove_host in SAS HBA LLDDs

On Fri, Apr 21, 2017 at 10:04:45AM +0200, Johannes Thumshirn wrote:
> This series re-orders the calls to scsi_remove_host() and sas_remove_host() in
> all SAS HBA drivers (apart from mpt3sas which is doing it correctly). This is
> for two reasons:
> 	1) After the change to recursive removal of sysfs entries, we're
> 	   trying to remove already removed kobjects when doing a
> 	   sas_remove_host() _after_ a scsi_remove_host()
> 	2) the documentation mandates it even (becuase of 1)
> 
> Unfortunately this does not completely solve issues with recursive sysfs
> removals in SAS, as libsas has asynchronous behaviour where strong ordering
> would be needed. But I am working on it and I do know other do as well. So if
> anyone else (James, Christoph, Bart, I'm looking at you) has an idea, I do
> have test setups and I'm willing to take input in form of ideas and patches.
> 
> I also dropped the SDEV_CANCEL state change for now. We re-evaluate it once we
> have an idea how to tackle the ordering issues and place it into
> sas_unregister_ha() as per James' comment.

Any reason to not just make sas_remove_host call scsi_remove_host
to ensure we get the ordering right?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web