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

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

[Johannes Thumshirn]

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

[PATCH v2 1/5] scsi: isci: remove the SAS host after the SCSI host

[Johannes Thumshirn]

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/isci/init.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index 0b5b5db0d0f8..e5824758aece 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -272,10 +272,11 @@ static void isci_unregister(struct isci_host *isci_host)
 		return;
 
 	shost = to_shost(isci_host);
-	scsi_remove_host(shost);
+
 	sas_unregister_ha(&isci_host->sas_ha);
 
 	sas_remove_host(shost);
+	scsi_remove_host(shost);
 	scsi_host_put(shost);
 }
 
-- 
2.12.0

[PATCH v2 2/5] aic94xx: remove the SAS host after the SCSI host

[Johannes Thumshirn]

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/aic94xx/aic94xx_init.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index 662b2321d1b0..8b9c6b9ea67d 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -701,13 +701,14 @@ static int asd_register_sas_ha(struct asd_ha_struct *asd_ha)
 
 static int asd_unregister_sas_ha(struct asd_ha_struct *asd_ha)
 {
+	struct Scsi_Host *shost = asd_ha->sas_ha.core.shost;
 	int err;
 
-	scsi_remove_host(asd_ha->sas_ha.core.shost);
 	err = sas_unregister_ha(&asd_ha->sas_ha);
 
-	sas_remove_host(asd_ha->sas_ha.core.shost);
-	scsi_host_put(asd_ha->sas_ha.core.shost);
+	sas_remove_host(shost);
+	scsi_remove_host(shost);
+	scsi_host_put(shost);
 
 	kfree(asd_ha->sas_ha.sas_phy);
 	kfree(asd_ha->sas_ha.sas_port);
-- 
2.12.0

[PATCH v2 3/5] scsi: hisi_sas: remove the SAS host after the SCSI host

[Johannes Thumshirn]

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/hisi_sas/hisi_sas_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 53637a941b94..9ae39226d454 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -1583,9 +1583,9 @@ int hisi_sas_remove(struct platform_device *pdev)
 	struct hisi_hba *hisi_hba = sha->lldd_ha;
 	struct Scsi_Host *shost = sha->core.shost;
 
-	scsi_remove_host(sha->core.shost);
 	sas_unregister_ha(sha);
-	sas_remove_host(sha->core.shost);
+	sas_remove_host(shost);
+	scsi_remove_host(shost);
 
 	hisi_sas_free(hisi_hba);
 	kfree(shost);
-- 
2.12.0

[PATCH v2 4/5] mvsas: remove the SAS host after the SCSI host

[Johannes Thumshirn]

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/mvsas/mv_init.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index 8280046fd1f0..4592a915c2b9 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -634,17 +634,19 @@ static void mvs_pci_remove(struct pci_dev *pdev)
 	unsigned short core_nr, i = 0;
 	struct sas_ha_struct *sha = pci_get_drvdata(pdev);
 	struct mvs_info *mvi = NULL;
+	struct Scsi_Host *shost;
 
 	core_nr = ((struct mvs_prv_info *)sha->lldd_ha)->n_host;
 	mvi = ((struct mvs_prv_info *)sha->lldd_ha)->mvi[0];
+	shost = mvi->shost;
 
 #ifdef CONFIG_SCSI_MVSAS_TASKLET
 	tasklet_kill(&((struct mvs_prv_info *)sha->lldd_ha)->mv_tasklet);
 #endif
 
-	scsi_remove_host(mvi->shost);
 	sas_unregister_ha(sha);
-	sas_remove_host(mvi->shost);
+	sas_remove_host(shost);
+	scsi_remove_host(shost);
 
 	MVS_CHIP_DISP->interrupt_disable(mvi);
 	free_irq(mvi->pdev->irq, sha);
-- 
2.12.0

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

[Johannes Thumshirn]

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

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

[John Garry]

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(-)
>

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

[Johannes Thumshirn]

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

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

[John Garry]

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

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

[Christoph Hellwig]

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.

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

[John Garry]

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

> .
>

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

[wangyijing]

>>
>> 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
> 
>> .
>>
> 
> 
> .
> 

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

[Christoph Hellwig]

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?

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

[Johannes Thumshirn]

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