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

Groups > linux.kernel > #1611752 > unrolled thread

[PATCH 0/2] Fix sysfs recursive removal splats in isci

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] Fix sysfs recursive removal splats in isci Johannes Thumshirn <jthumshirn@suse.de> - 2017-03-29 11:50 +0200
    [PATCH 3/6] aic94xx: remove the SAS host after the SCSI host Johannes Thumshirn <jthumshirn@suse.de> - 2017-03-29 11:50 +0200
      Re: [PATCH 3/6] aic94xx: remove the SAS host after the SCSI host Hannes Reinecke <hare@suse.de> - 2017-03-29 12:20 +0200
    [PATCH 5/6] mvsas: remove the SAS host after the SCSI host Johannes Thumshirn <jthumshirn@suse.de> - 2017-03-29 11:50 +0200
      Re: [PATCH 5/6] mvsas: remove the SAS host after the SCSI host Hannes Reinecke <hare@suse.de> - 2017-03-29 12:20 +0200
    [PATCH 2/2] scsi: isci: remove the SAS host after the SCSI host Johannes Thumshirn <jthumshirn@suse.de> - 2017-03-29 11:50 +0200
      Re: [PATCH 2/2] scsi: isci: remove the SAS host after the SCSI host Hannes Reinecke <hare@suse.de> - 2017-03-29 12:20 +0200
    [PATCH 6/6] scsi: pm8001: remove the SAS host after the SCSI host Johannes Thumshirn <jthumshirn@suse.de> - 2017-03-29 11:50 +0200
      Re: [PATCH 6/6] scsi: pm8001: remove the SAS host after the SCSI host Hannes Reinecke <hare@suse.de> - 2017-03-29 12:20 +0200
      Re: [PATCH 6/6] scsi: pm8001: remove the SAS host after the SCSI host Jinpu Wang <jinpu.wang@profitbricks.com> - 2017-03-29 12:30 +0200
        Re: [PATCH 6/6] scsi: pm8001: remove the SAS host after the SCSI host Hannes Reinecke <hare@suse.de> - 2017-03-29 12:40 +0200
    Re: [PATCH 0/2] Fix sysfs recursive removal splats in isci James Bottomley <jejb@linux.vnet.ibm.com> - 2017-03-29 13:40 +0200
    Re: [PATCH 0/2] Fix sysfs recursive removal splats in isci Tejun Heo <tj@kernel.org> - 2017-03-29 17:30 +0200

#1611752 — [PATCH 0/2] Fix sysfs recursive removal splats in isci

This series fixes a sysfs warning caused by isci not being able to cope with
recursive sysfs path removals which are in place since commit bcdde7e
("sysfs: make __sysfs_remove_dir() recursive").

The mvsas, aic94xx and pm8001 and hisi_sas patches have been compile tested
only hence they have no callstack of the affected path in their changelogs.

I'm not sure whether to mark this patches as stable or not. I tend to say no
here, although we've seen complaints/bug reports on lkml and the scsi list.

Johannes Thumshirn (6):
      scsi: sas: flush destruct workqueue on device unregister
      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   | 13 ++++++++++---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 10 ++++++++--
 drivers/scsi/isci/init.c              |  9 ++++++++-
 drivers/scsi/libsas/sas_discover.c    |  4 ++++
 drivers/scsi/mvsas/mv_init.c          | 13 +++++++++++--
 drivers/scsi/pm8001/pm8001_init.c     | 14 ++++++++++++--
 6 files changed, 53 insertions(+), 10 deletions(-)
-- 
1.8.5.6

[toc] | [next] | [standalone]

#1611753 — [PATCH 3/6] aic94xx: 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>
---
 drivers/scsi/aic94xx/aic94xx_init.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index 662b232..362d65a 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -701,13 +701,20 @@ 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;
+	unsigned long flags;
 	int err;
 
-	scsi_remove_host(asd_ha->sas_ha.core.shost);
+	spin_lock_irqsave(shost->host_lock, flags);
+	if (scsi_host_set_state(shost, SHOST_CANCEL))
+		WARN_ON(scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY));
+	spin_unlock_irqrestore(shost->host_lock, flags);
+
 	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);
-- 
1.8.5.6

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

#1611783 — Re: [PATCH 3/6] aic94xx: remove the SAS host after the SCSI host

On 03/29/2017 11:41 AM, Johannes Thumshirn wrote:
> 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>
> ---
>  drivers/scsi/aic94xx/aic94xx_init.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

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

#1611754 — [PATCH 5/6] mvsas: 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>
---
 drivers/scsi/mvsas/mv_init.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index 8280046..32b86c8 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -634,17 +634,26 @@ 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;
+	unsigned long flags;
 
 	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);
+	spin_lock_irqsave(shost->host_lock, flags);
+	if (scsi_host_set_state(shost, SHOST_CANCEL))
+		WARN_ON(scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY));
+	spin_unlock_irqrestore(shost->host_lock, flags);
+
+
 	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);
-- 
1.8.5.6

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

#1611785 — Re: [PATCH 5/6] mvsas: remove the SAS host after the SCSI host

On 03/29/2017 11:41 AM, Johannes Thumshirn wrote:
> 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>
> ---
>  drivers/scsi/mvsas/mv_init.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)

Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

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

#1611757 — [PATCH 2/2] scsi: isci: 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 like the below when
triggering the the removal of the SAS HBA PCI device like with writing to the
sysfs pci remove file:

echo 1 > /sys/module/isci/drivers/pci:isci/<device>/remove

WARNING: CPU: 2 PID: 5 at fs/sysfs/group.c:241 sysfs_remove_group+0xc3/0xd0
sysfs group 'power' not found for kobject 'end_device-6:0'
CPU: 16 PID: 5884 Comm: echo Not tainted 4.11.0-rc3-libsas+ #504
Call Trace:
 dump_stack+0x85/0xc2
 __warn+0xc6/0xe0
 warn_slowpath_fmt+0x4a/0x50
 sysfs_remove_group+0xc3/0xd0
 dpm_sysfs_remove+0x52/0x60
 device_del+0x13c/0x360
 ? device_remove_file+0x14/0x20
 attribute_container_class_device_del+0x15/0x20
 transport_remove_classdev+0x4c/0x60
 ? transport_add_class_device+0x40/0x40
 attribute_container_device_trigger+0xb3/0xc0
 transport_remove_device+0x10/0x20
 sas_port_delete+0x12d/0x160 [scsi_transport_sas]
 sas_deform_port+0x1bf/0x1d0 [libsas]
 sas_unregister_ports+0x36/0x50 [libsas]
 sas_unregister_ha+0x1b/0x40 [libsas]
 isci_unregister+0x2a/0x40 [isci]
 isci_pci_remove+0x52/0xb0 [isci]
 ? __pm_runtime_resume+0x56/0x80
 pci_device_remove+0x34/0xb0
 device_release_driver_internal+0x158/0x210
 device_release_driver+0xd/0x10
 pci_stop_bus_device+0x85/0x90
 pci_stop_and_remove_bus_device_locked+0x15/0x30
 remove_store+0x59/0x70
 dev_attr_store+0x13/0x20
 sysfs_kf_write+0x40/0x50
 kernfs_fop_write+0x130/0x1b0
 __vfs_write+0x23/0x130
 ? rcu_read_lock_sched_held+0x6d/0x80
 ? rcu_sync_lockdep_assert+0x2a/0x50
 ? __sb_start_write+0xd7/0x1e0
 ? vfs_write+0x1a4/0x1f0
 vfs_write+0xc6/0x1f0
 SyS_write+0x44/0xa0
 entry_SYSCALL_64_fastpath+0x23/0xc6

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/scsi/isci/init.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index 0b5b5db..afa6b25 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -267,15 +267,22 @@ static int isci_register_sas_ha(struct isci_host *isci_host)
 static void isci_unregister(struct isci_host *isci_host)
 {
 	struct Scsi_Host *shost;
+	unsigned long flags;
 
 	if (!isci_host)
 		return;
 
 	shost = to_shost(isci_host);
-	scsi_remove_host(shost);
+
+	spin_lock_irqsave(shost->host_lock, flags);
+	if (scsi_host_set_state(shost, SHOST_CANCEL))
+		WARN_ON(scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY));
+	spin_unlock_irqrestore(shost->host_lock, flags);
+
 	sas_unregister_ha(&isci_host->sas_ha);
 
 	sas_remove_host(shost);
+	scsi_remove_host(shost);
 	scsi_host_put(shost);
 }
 
-- 
1.8.5.6

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

#1611775 — Re: [PATCH 2/2] scsi: isci: remove the SAS host after the SCSI host

On 03/29/2017 11:41 AM, Johannes Thumshirn wrote:
> 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 like the below when
> triggering the the removal of the SAS HBA PCI device like with writing to the
> sysfs pci remove file:
> 
> echo 1 > /sys/module/isci/drivers/pci:isci/<device>/remove
> 
> WARNING: CPU: 2 PID: 5 at fs/sysfs/group.c:241 sysfs_remove_group+0xc3/0xd0
> sysfs group 'power' not found for kobject 'end_device-6:0'
> CPU: 16 PID: 5884 Comm: echo Not tainted 4.11.0-rc3-libsas+ #504
> Call Trace:
>  dump_stack+0x85/0xc2
>  __warn+0xc6/0xe0
>  warn_slowpath_fmt+0x4a/0x50
>  sysfs_remove_group+0xc3/0xd0
>  dpm_sysfs_remove+0x52/0x60
>  device_del+0x13c/0x360
>  ? device_remove_file+0x14/0x20
>  attribute_container_class_device_del+0x15/0x20
>  transport_remove_classdev+0x4c/0x60
>  ? transport_add_class_device+0x40/0x40
>  attribute_container_device_trigger+0xb3/0xc0
>  transport_remove_device+0x10/0x20
>  sas_port_delete+0x12d/0x160 [scsi_transport_sas]
>  sas_deform_port+0x1bf/0x1d0 [libsas]
>  sas_unregister_ports+0x36/0x50 [libsas]
>  sas_unregister_ha+0x1b/0x40 [libsas]
>  isci_unregister+0x2a/0x40 [isci]
>  isci_pci_remove+0x52/0xb0 [isci]
>  ? __pm_runtime_resume+0x56/0x80
>  pci_device_remove+0x34/0xb0
>  device_release_driver_internal+0x158/0x210
>  device_release_driver+0xd/0x10
>  pci_stop_bus_device+0x85/0x90
>  pci_stop_and_remove_bus_device_locked+0x15/0x30
>  remove_store+0x59/0x70
>  dev_attr_store+0x13/0x20
>  sysfs_kf_write+0x40/0x50
>  kernfs_fop_write+0x130/0x1b0
>  __vfs_write+0x23/0x130
>  ? rcu_read_lock_sched_held+0x6d/0x80
>  ? rcu_sync_lockdep_assert+0x2a/0x50
>  ? __sb_start_write+0xd7/0x1e0
>  ? vfs_write+0x1a4/0x1f0
>  vfs_write+0xc6/0x1f0
>  SyS_write+0x44/0xa0
>  entry_SYSCALL_64_fastpath+0x23/0xc6
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  drivers/scsi/isci/init.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

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

#1611760 — [PATCH 6/6] 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>
---
 drivers/scsi/pm8001/pm8001_init.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 417368c..9116e9c 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -1086,11 +1086,21 @@ static void pm8001_pci_remove(struct pci_dev *pdev)
 {
 	struct sas_ha_struct *sha = pci_get_drvdata(pdev);
 	struct pm8001_hba_info *pm8001_ha;
+	unsigned long flags;
+	struct Scsi_Host *shost;
 	int i, j;
 	pm8001_ha = sha->lldd_ha;
-	scsi_remove_host(pm8001_ha->shost);
+	shost = pm8001_ha->shost;
+
+	spin_lock_irqsave(shost->host_lock, flags);
+	if (scsi_host_set_state(shost, SHOST_CANCEL))
+		WARN_ON(scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY));
+	spin_unlock_irqrestore(shost->host_lock, flags);
+
 	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);
-- 
1.8.5.6

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

#1611776 — Re: [PATCH 6/6] scsi: pm8001: remove the SAS host after the SCSI host

On 03/29/2017 11:41 AM, Johannes Thumshirn wrote:
> 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>
> ---
>  drivers/scsi/pm8001/pm8001_init.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

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

#1611789 — Re: [PATCH 6/6] scsi: pm8001: remove the SAS host after the SCSI host

On Wed, Mar 29, 2017 at 11:41 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> 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>
> ---
>  drivers/scsi/pm8001/pm8001_init.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index 417368c..9116e9c 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -1086,11 +1086,21 @@ static void pm8001_pci_remove(struct pci_dev *pdev)
>  {
>         struct sas_ha_struct *sha = pci_get_drvdata(pdev);
>         struct pm8001_hba_info *pm8001_ha;
> +       unsigned long flags;
> +       struct Scsi_Host *shost;
>         int i, j;
>         pm8001_ha = sha->lldd_ha;
> -       scsi_remove_host(pm8001_ha->shost);
> +       shost = pm8001_ha->shost;
> +
> +       spin_lock_irqsave(shost->host_lock, flags);
> +       if (scsi_host_set_state(shost, SHOST_CANCEL))
> +               WARN_ON(scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY));
> +       spin_unlock_irqrestore(shost->host_lock, flags);
> +
>         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);
> --
> 1.8.5.6
>
Thanks Johannes for taking care of this. Looks good to me,

I have a question regarding the scsi_host_set_state change, why do we need that?
Can't we simply change the order of sas_remove_host and scsi_remove_host?

Cheers?
-- 
Jack Wang

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

#1611796 — Re: [PATCH 6/6] scsi: pm8001: remove the SAS host after the SCSI host

On 03/29/2017 12:27 PM, Jinpu Wang wrote:
> On Wed, Mar 29, 2017 at 11:41 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
>> 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>
>> ---
>>  drivers/scsi/pm8001/pm8001_init.c | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
>> index 417368c..9116e9c 100644
>> --- a/drivers/scsi/pm8001/pm8001_init.c
>> +++ b/drivers/scsi/pm8001/pm8001_init.c
>> @@ -1086,11 +1086,21 @@ static void pm8001_pci_remove(struct pci_dev *pdev)
>>  {
>>         struct sas_ha_struct *sha = pci_get_drvdata(pdev);
>>         struct pm8001_hba_info *pm8001_ha;
>> +       unsigned long flags;
>> +       struct Scsi_Host *shost;
>>         int i, j;
>>         pm8001_ha = sha->lldd_ha;
>> -       scsi_remove_host(pm8001_ha->shost);
>> +       shost = pm8001_ha->shost;
>> +
>> +       spin_lock_irqsave(shost->host_lock, flags);
>> +       if (scsi_host_set_state(shost, SHOST_CANCEL))
>> +               WARN_ON(scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY));
>> +       spin_unlock_irqrestore(shost->host_lock, flags);
>> +
>>         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);
>> --
>> 1.8.5.6
>>
> Thanks Johannes for taking care of this. Looks good to me,
> 
> I have a question regarding the scsi_host_set_state change, why do we need that?
> Can't we simply change the order of sas_remove_host and scsi_remove_host?
> 
If we don't do that I/O might still be coming in for the attached ports
while we're trying to remove the same. Which might cause all sorts of
race conditions.
So it's better to set the host state to CANCEL to stop I/O before
removing the ports.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

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

#1611850

On Wed, 2017-03-29 at 11:41 +0200, Johannes Thumshirn wrote:
> This series fixes a sysfs warning caused by isci not being able to 
> cope with recursive sysfs path removals which are in place since 
> commit bcdde7e ("sysfs: make __sysfs_remove_dir() recursive").
> 
> The mvsas, aic94xx and pm8001 and hisi_sas patches have been compile 
> tested only hence they have no callstack of the affected path in 
> their changelogs.
> 
> I'm not sure whether to mark this patches as stable or not. I tend to 
> say no here, although we've seen complaints/bug reports on lkml and 
> the scsi list.

What happens to the SYNC CACHE for devices with write back caches?  It
looks like you've already torn down most of the sas objects by the time
they're sent, so do they actually reach the device (or worse, do they
hang the system by not making progress)?

Assuming the above is OK, what about putting the state change inside
sas_remove_ha()?  It's better than making every driver do it.

James

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

#1612055

Hello,

On Wed, Mar 29, 2017 at 11:41:07AM +0200, Johannes Thumshirn wrote:
> This series fixes a sysfs warning caused by isci not being able to cope with
> recursive sysfs path removals which are in place since commit bcdde7e
> ("sysfs: make __sysfs_remove_dir() recursive").

Thanks for fixing these.

> The mvsas, aic94xx and pm8001 and hisi_sas patches have been compile tested
> only hence they have no callstack of the affected path in their changelogs.
> 
> I'm not sure whether to mark this patches as stable or not. I tend to say no
> here, although we've seen complaints/bug reports on lkml and the scsi list.

Given that the failures aren't critical or all that common (only
happens on controller removal), I agree that not cc'ing stable is the
right call here.

Thanks.

-- 
tejun

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web