Re: [PATCH] scsi: sas: move scsi_remove_host call into sas_remove_host

[Johannes Thumshirn]

On Mon, Apr 24, 2017 at 02:38:40PM +0100, John Garry wrote:
> On 24/04/2017 11:09, John Garry wrote:
> >On 21/04/2017 13:11, Johannes Thumshirn wrote:
> >>Move scsi_remove_host call into sas_remove_host and remove it from SAS
> >>HBA
> >>drivers, so we don't mess up the ordering. This solves an issue with
> >>double
> >>deleting sysfs entries that was introduced by the change of sysfs
> >>behaviour
> >>from commit bcdde7e ("sysfs: make __sysfs_remove_dir() recursive").
> >>
> >>Signed-off-by: Johannes Thumshirn 
> >>Suggested-by: Christoph Hellwig 
> >>Cc: Hannes Reinecke 
> >>Cc: James Bottomley 
> >>Cc: Jinpu Wang 
> >>Cc: John Garry 
> >
> >For what it's worth:
> >
> >Tested-by: John Garry  # On hisi_sas
> >
> 
> I have actually tested this a little further and now I see an issue. Maybe
> it is related to the internal development kernel I am using or a
> pre-existing issue with our driver, but I doubt it.
> This time I removed the WARN in sysfs_remove_group() [so the console is not
> bombarded] and ran repeated insmod/rmmod, and I see this sometimes:

[...]


> Ring any bells? I don't see this new WARN without the change.

No haven't seen the WARN in my tests but I didn't do a insmod/rmmod stress
test anyways.

I'll have a look.

-- 
Johannes Thumshirn  Storage
jthumsh...@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] scsi: sas: move scsi_remove_host call into sas_remove_host

[Johannes Thumshirn]

On Mon, Apr 24, 2017 at 02:38:40PM +0100, John Garry wrote:
> On 24/04/2017 11:09, John Garry wrote:
> >On 21/04/2017 13:11, Johannes Thumshirn wrote:
> >>Move scsi_remove_host call into sas_remove_host and remove it from SAS
> >>HBA
> >>drivers, so we don't mess up the ordering. This solves an issue with
> >>double
> >>deleting sysfs entries that was introduced by the change of sysfs
> >>behaviour
> >>from commit bcdde7e ("sysfs: make __sysfs_remove_dir() recursive").
> >>
> >>Signed-off-by: Johannes Thumshirn 
> >>Suggested-by: Christoph Hellwig 
> >>Cc: Hannes Reinecke 
> >>Cc: James Bottomley 
> >>Cc: Jinpu Wang 
> >>Cc: John Garry 
> >
> >For what it's worth:
> >
> >Tested-by: John Garry  # On hisi_sas
> >
> 
> I have actually tested this a little further and now I see an issue. Maybe
> it is related to the internal development kernel I am using or a
> pre-existing issue with our driver, but I doubt it.
> This time I removed the WARN in sysfs_remove_group() [so the console is not
> bombarded] and ran repeated insmod/rmmod, and I see this sometimes:

[...]


> Ring any bells? I don't see this new WARN without the change.

No haven't seen the WARN in my tests but I didn't do a insmod/rmmod stress
test anyways.

I'll have a look.

-- 
Johannes Thumshirn  Storage
jthumsh...@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] scsi: sas: move scsi_remove_host call into sas_remove_host

[John Garry]

On 24/04/2017 11:09, John Garry wrote:

On 21/04/2017 13:11, Johannes Thumshirn wrote:

Move scsi_remove_host call into sas_remove_host and remove it from SAS
HBA
drivers, so we don't mess up the ordering. This solves an issue with
double
deleting sysfs entries that was introduced by the change of sysfs
behaviour
from commit bcdde7e ("sysfs: make __sysfs_remove_dir() recursive").

Signed-off-by: Johannes Thumshirn 
Suggested-by: Christoph Hellwig 
Cc: Hannes Reinecke 
Cc: James Bottomley 
Cc: Jinpu Wang 
Cc: John Garry 


For what it's worth:

Tested-by: John Garry  # On hisi_sas



I have actually tested this a little further and now I see an issue. 
Maybe it is related to the internal development kernel I am using or a 
pre-existing issue with our driver, but I doubt it.
This time I removed the WARN in sysfs_remove_group() [so the console is 
not bombarded] and ran repeated insmod/rmmod, and I see this sometimes:


root@(none)$
root@(none)$ insmod hisi_sas_v2_hw.ko
[ 2150.478485] scsi host6: hisi_sas
[ 2151.629343] scsi host7: hisi_sas
[ 2152.248410] hisi_sas_v2_hw HISI0162:01: phyup: phy0 link_rate=11
[ 2152.254424] hisi_sas_v2_hw HISI0162:01: phyup: phy1 link_rate=11
[ 2152.260422] hisi_sas_v2_hw HISI0162:01: phyup: phy2 link_rate=11
[ 2152.266421] hisi_sas_v2_hw HISI0162:01: phyup: phy3 link_rate=11
[ 2152.272419] hisi_sas_v2_hw HISI0162:01: phyup: phy4 link_rate=11
[ 2152.278416] hisi_sas_v2_hw HISI0162:01: phyup: phy5 link_rate=11
[ 2152.284413] hisi_sas_v2_hw HISI0162:01: phyup: phy6 link_rate=11
[ 2152.290410] hisi_sas_v2_hw HISI0162:01: phyup: phy7 link_rate=11
[ 2152.507431] ata3.00: ATA-8: HGST HUS724040ALA640, MFAOA8B0, max UDMA/133
[ 2152.514127] ata3.00: 7814037168 sectors, multi 0: LBA48 NCQ (depth 31/32)
[ 2152.522726] ata3.00: configured for UDMA/133
[ 2152.574540] scsi 7:0:0:0: Direct-Access SanDisk  LT0200MO 
P404 PQ: 0 ANSI: 6
[ 2152.835694] scsi 7:0:1:0: Direct-Access ATA  HGST HUS724040AL 
A8B0 PQ: 0 ANSI: 5
[ 2152.873713] sd 7:0:1:0: [sdb] 7814037168 512-byte logical blocks: 
(4.00 TB/3.64 TiB)

[ 2152.881468] sd 7:0:1:0: [sdb] Write Protect is off
[ 2152.886274] sd 7:0:1:0: [sdb] Write cache: enabled, read cache: 
enabled, doesn't support DPO or FUA
[ 2152.911092] scsi 7:0:2:0: Direct-Access SanDisk  LT0200MO 
P404 PQ: 0 ANSI: 6
[ 2153.050599] sd 7:0:0:0: [sda] 390721968 512-byte logical blocks: (200 
GB/186 GiB)

[ 2153.092625] sd 7:0:0:0: [sda] Write Protect is off
[ 2153.174641] sd 7:0:0:0: [sda] Write cache: disabled, read cache: 
disabled, supports DPO and FUA
[ 2153.209183] scsi 7:0:3:0: Direct-Access SanDisk  LT0200MO 
P404 PQ: 0 ANSI: 6
[ 2153.387054] sd 7:0:2:0: [sdc] 390721968 512-byte logical blocks: (200 
GB/186 GiB)

[ 2153.429080] sd 7:0:2:0: [sdc] Write Protect is off
[ 2153.507273] scsi 7:0:4:0: Direct-Access SanDisk  LT0200MO 
P404 PQ: 0 ANSI: 6
[ 2153.515382] sd 7:0:2:0: [sdc] Write cache: disabled, read cache: 
disabled, supports DPO and FUA
[ 2153.685254] sd 7:0:3:0: [sdd] 390721968 512-byte logical blocks: (200 
GB/186 GiB)

[ 2153.727267] sd 7:0:3:0: [sdd] Write Protect is off
[ 2153.805266] scsi 7:0:5:0: Direct-Access SanDisk  LT0200MO 
P404 PQ: 0 ANSI: 6
[ 2153.813359] sd 7:0:3:0: [sdd] Write cache: disabled, read cache: 
disabled, supports DPO and FUA
[ 2153.983165] sd 7:0:4:0: [sde] 390721968 512-byte logical blocks: (200 
GB/186 GiB)

[ 2154.025158] sd 7:0:4:0: [sde] Write Protect is off
[ 2154.066761] scsi 7:0:6:0: Enclosure 12G SAS  Expander 
RevB PQ: 0 ANSI: 6
[ 2154.107141] sd 7:0:4:0: [sde] Write cache: disabled, read cache: 
disabled, supports DPO and FUA

[ 2154.124866] scsi host8: hisi_sas
[ 2154.281354] sd 7:0:5:0: [sdf] 390721968 512-byte logical blocks: (200 
GB/186 GiB)

[ 2154.323385] sd 7:0:5:0: [sdf] Write Protect is off
[ 2154.405404] sd 7:0:5:0: [sdf] Write cache: disabled, read cache: 
disabled, supports DPO and FUA

[ 2154.409289] sd 7:0:0:0: [sda] Attached SCSI disk
[ 2154.745722] sd 7:0:2:0: [sdc] Attached SCSI disk
[ 2155.044051] sd 7:0:3:0: [sdd] Attached SCSI disk

root@(none)$
root@(none)$ [ 2155.341986] sd 7:0:4:0: [sde] Attached SCSI disk

root@(none)$
root@(none)$ [ 2155.640217] sd 7:0:5:0: [sdf] Attached SCSI disk

root@(none)$
root@(none)$ rmmod hisi_sas_v2_hw.ko
[ 2158.679637] hisi_sas_v2_hw HISI0162:01: found dev[7:1] is gone
[ 2161.587449]  sdb: sdb1 sdb2
[ 2161.590253] kobject_add_internal failed for sdb1 (error: -2 parent: sdb)
[ 2161.596954] [ cut here ]
[ 2161.601562] WARNING: CPU: 0 PID: 2150 at lib/kobject.c:244 
kobject_add_internal+0xdc/0x298
[ 2161.609811] Modules linked in: hisi_sas_v2_hw(-) hisi_sas_main [last 
unloaded: hisi_sas_v2_hw]

[ 2161.618412]
[ 2161.619893] CPU: 0 PID: 2150 Comm: kworker/u129:6 Tainted: GW 
  4.11.0-rc1-13718-gb5b0d57-dirty #1584
[ 2161.630138] Hardware name: Huawei 

Re: [PATCH] scsi: sas: move scsi_remove_host call into sas_remove_host

[John Garry]

On 24/04/2017 11:09, John Garry wrote:

On 21/04/2017 13:11, Johannes Thumshirn wrote:

Move scsi_remove_host call into sas_remove_host and remove it from SAS
HBA
drivers, so we don't mess up the ordering. This solves an issue with
double
deleting sysfs entries that was introduced by the change of sysfs
behaviour
from commit bcdde7e ("sysfs: make __sysfs_remove_dir() recursive").

Signed-off-by: Johannes Thumshirn 
Suggested-by: Christoph Hellwig 
Cc: Hannes Reinecke 
Cc: James Bottomley 
Cc: Jinpu Wang 
Cc: John Garry 


For what it's worth:

Tested-by: John Garry  # On hisi_sas



I have actually tested this a little further and now I see an issue. 
Maybe it is related to the internal development kernel I am using or a 
pre-existing issue with our driver, but I doubt it.
This time I removed the WARN in sysfs_remove_group() [so the console is 
not bombarded] and ran repeated insmod/rmmod, and I see this sometimes:


root@(none)$
root@(none)$ insmod hisi_sas_v2_hw.ko
[ 2150.478485] scsi host6: hisi_sas
[ 2151.629343] scsi host7: hisi_sas
[ 2152.248410] hisi_sas_v2_hw HISI0162:01: phyup: phy0 link_rate=11
[ 2152.254424] hisi_sas_v2_hw HISI0162:01: phyup: phy1 link_rate=11
[ 2152.260422] hisi_sas_v2_hw HISI0162:01: phyup: phy2 link_rate=11
[ 2152.266421] hisi_sas_v2_hw HISI0162:01: phyup: phy3 link_rate=11
[ 2152.272419] hisi_sas_v2_hw HISI0162:01: phyup: phy4 link_rate=11
[ 2152.278416] hisi_sas_v2_hw HISI0162:01: phyup: phy5 link_rate=11
[ 2152.284413] hisi_sas_v2_hw HISI0162:01: phyup: phy6 link_rate=11
[ 2152.290410] hisi_sas_v2_hw HISI0162:01: phyup: phy7 link_rate=11
[ 2152.507431] ata3.00: ATA-8: HGST HUS724040ALA640, MFAOA8B0, max UDMA/133
[ 2152.514127] ata3.00: 7814037168 sectors, multi 0: LBA48 NCQ (depth 31/32)
[ 2152.522726] ata3.00: configured for UDMA/133
[ 2152.574540] scsi 7:0:0:0: Direct-Access SanDisk  LT0200MO 
P404 PQ: 0 ANSI: 6
[ 2152.835694] scsi 7:0:1:0: Direct-Access ATA  HGST HUS724040AL 
A8B0 PQ: 0 ANSI: 5
[ 2152.873713] sd 7:0:1:0: [sdb] 7814037168 512-byte logical blocks: 
(4.00 TB/3.64 TiB)

[ 2152.881468] sd 7:0:1:0: [sdb] Write Protect is off
[ 2152.886274] sd 7:0:1:0: [sdb] Write cache: enabled, read cache: 
enabled, doesn't support DPO or FUA
[ 2152.911092] scsi 7:0:2:0: Direct-Access SanDisk  LT0200MO 
P404 PQ: 0 ANSI: 6
[ 2153.050599] sd 7:0:0:0: [sda] 390721968 512-byte logical blocks: (200 
GB/186 GiB)

[ 2153.092625] sd 7:0:0:0: [sda] Write Protect is off
[ 2153.174641] sd 7:0:0:0: [sda] Write cache: disabled, read cache: 
disabled, supports DPO and FUA
[ 2153.209183] scsi 7:0:3:0: Direct-Access SanDisk  LT0200MO 
P404 PQ: 0 ANSI: 6
[ 2153.387054] sd 7:0:2:0: [sdc] 390721968 512-byte logical blocks: (200 
GB/186 GiB)

[ 2153.429080] sd 7:0:2:0: [sdc] Write Protect is off
[ 2153.507273] scsi 7:0:4:0: Direct-Access SanDisk  LT0200MO 
P404 PQ: 0 ANSI: 6
[ 2153.515382] sd 7:0:2:0: [sdc] Write cache: disabled, read cache: 
disabled, supports DPO and FUA
[ 2153.685254] sd 7:0:3:0: [sdd] 390721968 512-byte logical blocks: (200 
GB/186 GiB)

[ 2153.727267] sd 7:0:3:0: [sdd] Write Protect is off
[ 2153.805266] scsi 7:0:5:0: Direct-Access SanDisk  LT0200MO 
P404 PQ: 0 ANSI: 6
[ 2153.813359] sd 7:0:3:0: [sdd] Write cache: disabled, read cache: 
disabled, supports DPO and FUA
[ 2153.983165] sd 7:0:4:0: [sde] 390721968 512-byte logical blocks: (200 
GB/186 GiB)

[ 2154.025158] sd 7:0:4:0: [sde] Write Protect is off
[ 2154.066761] scsi 7:0:6:0: Enclosure 12G SAS  Expander 
RevB PQ: 0 ANSI: 6
[ 2154.107141] sd 7:0:4:0: [sde] Write cache: disabled, read cache: 
disabled, supports DPO and FUA

[ 2154.124866] scsi host8: hisi_sas
[ 2154.281354] sd 7:0:5:0: [sdf] 390721968 512-byte logical blocks: (200 
GB/186 GiB)

[ 2154.323385] sd 7:0:5:0: [sdf] Write Protect is off
[ 2154.405404] sd 7:0:5:0: [sdf] Write cache: disabled, read cache: 
disabled, supports DPO and FUA

[ 2154.409289] sd 7:0:0:0: [sda] Attached SCSI disk
[ 2154.745722] sd 7:0:2:0: [sdc] Attached SCSI disk
[ 2155.044051] sd 7:0:3:0: [sdd] Attached SCSI disk

root@(none)$
root@(none)$ [ 2155.341986] sd 7:0:4:0: [sde] Attached SCSI disk

root@(none)$
root@(none)$ [ 2155.640217] sd 7:0:5:0: [sdf] Attached SCSI disk

root@(none)$
root@(none)$ rmmod hisi_sas_v2_hw.ko
[ 2158.679637] hisi_sas_v2_hw HISI0162:01: found dev[7:1] is gone
[ 2161.587449]  sdb: sdb1 sdb2
[ 2161.590253] kobject_add_internal failed for sdb1 (error: -2 parent: sdb)
[ 2161.596954] [ cut here ]
[ 2161.601562] WARNING: CPU: 0 PID: 2150 at lib/kobject.c:244 
kobject_add_internal+0xdc/0x298
[ 2161.609811] Modules linked in: hisi_sas_v2_hw(-) hisi_sas_main [last 
unloaded: hisi_sas_v2_hw]

[ 2161.618412]
[ 2161.619893] CPU: 0 PID: 2150 Comm: kworker/u129:6 Tainted: GW 
  4.11.0-rc1-13718-gb5b0d57-dirty #1584
[ 2161.630138] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon 
D05 UEFI 16.12 Release 02/27/2017

[ 2161.639345] Workqueue: events_unbound async_run_entry_fn
[ 2161.644644] 

Re: [PATCH] scsi: sas: move scsi_remove_host call into sas_remove_host

[John Garry]

On 21/04/2017 13:11, Johannes Thumshirn wrote:

Move scsi_remove_host call into sas_remove_host and remove it from SAS HBA
drivers, so we don't mess up the ordering. This solves an issue with double
deleting sysfs entries that was introduced by the change of sysfs behaviour
from commit bcdde7e ("sysfs: make __sysfs_remove_dir() recursive").

Signed-off-by: Johannes Thumshirn 
Suggested-by: Christoph Hellwig 
Cc: Hannes Reinecke 
Cc: James Bottomley 
Cc: Jinpu Wang 
Cc: John Garry 


For what it's worth:

Tested-by: John Garry  # On hisi_sas

Re: [PATCH] scsi: sas: move scsi_remove_host call into sas_remove_host

[John Garry]

On 21/04/2017 13:11, Johannes Thumshirn wrote:

Move scsi_remove_host call into sas_remove_host and remove it from SAS HBA
drivers, so we don't mess up the ordering. This solves an issue with double
deleting sysfs entries that was introduced by the change of sysfs behaviour
from commit bcdde7e ("sysfs: make __sysfs_remove_dir() recursive").

Signed-off-by: Johannes Thumshirn 
Suggested-by: Christoph Hellwig 
Cc: Hannes Reinecke 
Cc: James Bottomley 
Cc: Jinpu Wang 
Cc: John Garry 


For what it's worth:

Tested-by: John Garry  # On hisi_sas

Re: [PATCH] scsi: sas: move scsi_remove_host call into sas_remove_host

[Hannes Reinecke]

On 04/21/2017 02:11 PM, Johannes Thumshirn wrote:
> Move scsi_remove_host call into sas_remove_host and remove it from SAS HBA
> drivers, so we don't mess up the ordering. This solves an issue with double
> deleting sysfs entries that was introduced by the change of sysfs behaviour
> from commit bcdde7e ("sysfs: make __sysfs_remove_dir() recursive").
> 
> Signed-off-by: Johannes Thumshirn 
> Suggested-by: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: James Bottomley 
> Cc: Jinpu Wang 
> Cc: John Garry 
> ---
>  drivers/scsi/aic94xx/aic94xx_init.c   | 1 -
>  drivers/scsi/hisi_sas/hisi_sas_main.c | 1 -
>  drivers/scsi/isci/init.c  | 1 -
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c  | 1 -
>  drivers/scsi/mvsas/mv_init.c  | 1 -
>  drivers/scsi/pm8001/pm8001_init.c | 1 -
>  drivers/scsi/scsi_transport_sas.c | 8 ++--
>  7 files changed, 6 insertions(+), 8 deletions(-)
> 
Sadly, you've left out mptsas; that's still in use for VMWare installations.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@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)

Re: [PATCH] scsi: sas: move scsi_remove_host call into sas_remove_host

[Hannes Reinecke]

On 04/21/2017 02:11 PM, Johannes Thumshirn wrote:
> Move scsi_remove_host call into sas_remove_host and remove it from SAS HBA
> drivers, so we don't mess up the ordering. This solves an issue with double
> deleting sysfs entries that was introduced by the change of sysfs behaviour
> from commit bcdde7e ("sysfs: make __sysfs_remove_dir() recursive").
> 
> Signed-off-by: Johannes Thumshirn 
> Suggested-by: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: James Bottomley 
> Cc: Jinpu Wang 
> Cc: John Garry 
> ---
>  drivers/scsi/aic94xx/aic94xx_init.c   | 1 -
>  drivers/scsi/hisi_sas/hisi_sas_main.c | 1 -
>  drivers/scsi/isci/init.c  | 1 -
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c  | 1 -
>  drivers/scsi/mvsas/mv_init.c  | 1 -
>  drivers/scsi/pm8001/pm8001_init.c | 1 -
>  drivers/scsi/scsi_transport_sas.c | 8 ++--
>  7 files changed, 6 insertions(+), 8 deletions(-)
> 
Sadly, you've left out mptsas; that's still in use for VMWare installations.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@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)

Re: [PATCH] scsi: sas: move scsi_remove_host call into sas_remove_host

[Martin K. Petersen]

Johannes,

> Move scsi_remove_host call into sas_remove_host and remove it from SAS
> HBA drivers, so we don't mess up the ordering. This solves an issue
> with double deleting sysfs entries that was introduced by the change
> of sysfs behaviour from commit bcdde7e ("sysfs: make
> __sysfs_remove_dir() recursive").

Yeah, I prefer this approach.

Applied to 4.12/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: sas: move scsi_remove_host call into sas_remove_host

[Martin K. Petersen]

Johannes,

> Move scsi_remove_host call into sas_remove_host and remove it from SAS
> HBA drivers, so we don't mess up the ordering. This solves an issue
> with double deleting sysfs entries that was introduced by the change
> of sysfs behaviour from commit bcdde7e ("sysfs: make
> __sysfs_remove_dir() recursive").

Yeah, I prefer this approach.

Applied to 4.12/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: sas: move scsi_remove_host call into sas_remove_host

[Jinpu Wang]

On Fri, Apr 21, 2017 at 2:11 PM, Johannes Thumshirn  wrote:
> Move scsi_remove_host call into sas_remove_host and remove it from SAS HBA
> drivers, so we don't mess up the ordering. This solves an issue with double
> deleting sysfs entries that was introduced by the change of sysfs behaviour
> from commit bcdde7e ("sysfs: make __sysfs_remove_dir() recursive").
>
> Signed-off-by: Johannes Thumshirn 
> Suggested-by: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: James Bottomley 
> Cc: Jinpu Wang 
> Cc: John Garry 
> ---
>  drivers/scsi/aic94xx/aic94xx_init.c   | 1 -
>  drivers/scsi/hisi_sas/hisi_sas_main.c | 1 -
>  drivers/scsi/isci/init.c  | 1 -
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c  | 1 -
>  drivers/scsi/mvsas/mv_init.c  | 1 -
>  drivers/scsi/pm8001/pm8001_init.c | 1 -
>  drivers/scsi/scsi_transport_sas.c | 8 ++--
>  7 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/aic94xx/aic94xx_init.c 
> b/drivers/scsi/aic94xx/aic94xx_init.c
> index 662b2321d1b0..a14ba7a6b81e 100644
> --- a/drivers/scsi/aic94xx/aic94xx_init.c
> +++ b/drivers/scsi/aic94xx/aic94xx_init.c
> @@ -703,7 +703,6 @@ static int asd_unregister_sas_ha(struct asd_ha_struct 
> *asd_ha)
>  {
> int err;
>
> -   scsi_remove_host(asd_ha->sas_ha.core.shost);
> err = sas_unregister_ha(_ha->sas_ha);
>
> sas_remove_host(asd_ha->sas_ha.core.shost);
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
> b/drivers/scsi/hisi_sas/hisi_sas_main.c
> index 53637a941b94..843bedae6c09 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> @@ -1583,7 +1583,6 @@ 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);
>
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> index 0b5b5db0d0f8..45371179ab87 100644
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -272,7 +272,6 @@ static void isci_unregister(struct isci_host *isci_host)
> return;
>
> shost = to_shost(isci_host);
> -   scsi_remove_host(shost);
> sas_unregister_ha(_host->sas_ha);
>
> sas_remove_host(shost);
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 919ba2bb15f1..a5d872664257 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -8283,7 +8283,6 @@ static void scsih_remove(struct pci_dev *pdev)
> }
>
> sas_remove_host(shost);
> -   scsi_remove_host(shost);
> mpt3sas_base_detach(ioc);
> spin_lock(_lock);
> list_del(>list);
> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> index 8280046fd1f0..4e047b5001a6 100644
> --- a/drivers/scsi/mvsas/mv_init.c
> +++ b/drivers/scsi/mvsas/mv_init.c
> @@ -642,7 +642,6 @@ static void mvs_pci_remove(struct pci_dev *pdev)
> 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);
>
> diff --git a/drivers/scsi/pm8001/pm8001_init.c 
> b/drivers/scsi/pm8001/pm8001_init.c
> index 417368ccb686..034b2f7d1135 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -1088,7 +1088,6 @@ static void pm8001_pci_remove(struct pci_dev *pdev)
> struct pm8001_hba_info *pm8001_ha;
> int i, j;
> pm8001_ha = sha->lldd_ha;
> -   scsi_remove_host(pm8001_ha->shost);
> sas_unregister_ha(sha);
> sas_remove_host(pm8001_ha->shost);
> list_del(_ha->list);
> diff --git a/drivers/scsi/scsi_transport_sas.c 
> b/drivers/scsi/scsi_transport_sas.c
> index cdbb293aca08..ca0e5a9a17f8 100644
> --- a/drivers/scsi/scsi_transport_sas.c
> +++ b/drivers/scsi/scsi_transport_sas.c
> @@ -370,12 +370,16 @@ EXPORT_SYMBOL(sas_remove_children);
>   * sas_remove_host  -  tear down a Scsi_Host's SAS data structures
>   * @shost: Scsi Host that is torn down
>   *
> - * Removes all SAS PHYs and remote PHYs for a given Scsi_Host.
> - * Must be called just before scsi_remove_host for SAS HBAs.
> + * Removes all SAS PHYs and remote PHYs for a given Scsi_Host and remove the
> + * Scsi_Host as well.
> + *
> + * Note: Do not call scsi_remove_host() on the Scsi_Host any more, as it is
> + * already removed.
>   */
>  void sas_remove_host(struct Scsi_Host *shost)
>  {
> sas_remove_children(>shost_gendev);
> +   scsi_remove_host(shost);
>  }
>  EXPORT_SYMBOL(sas_remove_host);
>
> --
> 2.12.0
>
Thanks, Johannes,

Reviewed-by:: Jack Wang 

Re: [PATCH] scsi: sas: move scsi_remove_host call into sas_remove_host

[Jinpu Wang]

On Fri, Apr 21, 2017 at 2:11 PM, Johannes Thumshirn  wrote:
> Move scsi_remove_host call into sas_remove_host and remove it from SAS HBA
> drivers, so we don't mess up the ordering. This solves an issue with double
> deleting sysfs entries that was introduced by the change of sysfs behaviour
> from commit bcdde7e ("sysfs: make __sysfs_remove_dir() recursive").
>
> Signed-off-by: Johannes Thumshirn 
> Suggested-by: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: James Bottomley 
> Cc: Jinpu Wang 
> Cc: John Garry 
> ---
>  drivers/scsi/aic94xx/aic94xx_init.c   | 1 -
>  drivers/scsi/hisi_sas/hisi_sas_main.c | 1 -
>  drivers/scsi/isci/init.c  | 1 -
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c  | 1 -
>  drivers/scsi/mvsas/mv_init.c  | 1 -
>  drivers/scsi/pm8001/pm8001_init.c | 1 -
>  drivers/scsi/scsi_transport_sas.c | 8 ++--
>  7 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/aic94xx/aic94xx_init.c 
> b/drivers/scsi/aic94xx/aic94xx_init.c
> index 662b2321d1b0..a14ba7a6b81e 100644
> --- a/drivers/scsi/aic94xx/aic94xx_init.c
> +++ b/drivers/scsi/aic94xx/aic94xx_init.c
> @@ -703,7 +703,6 @@ static int asd_unregister_sas_ha(struct asd_ha_struct 
> *asd_ha)
>  {
> int err;
>
> -   scsi_remove_host(asd_ha->sas_ha.core.shost);
> err = sas_unregister_ha(_ha->sas_ha);
>
> sas_remove_host(asd_ha->sas_ha.core.shost);
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 
> b/drivers/scsi/hisi_sas/hisi_sas_main.c
> index 53637a941b94..843bedae6c09 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> @@ -1583,7 +1583,6 @@ 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);
>
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> index 0b5b5db0d0f8..45371179ab87 100644
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -272,7 +272,6 @@ static void isci_unregister(struct isci_host *isci_host)
> return;
>
> shost = to_shost(isci_host);
> -   scsi_remove_host(shost);
> sas_unregister_ha(_host->sas_ha);
>
> sas_remove_host(shost);
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 919ba2bb15f1..a5d872664257 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -8283,7 +8283,6 @@ static void scsih_remove(struct pci_dev *pdev)
> }
>
> sas_remove_host(shost);
> -   scsi_remove_host(shost);
> mpt3sas_base_detach(ioc);
> spin_lock(_lock);
> list_del(>list);
> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> index 8280046fd1f0..4e047b5001a6 100644
> --- a/drivers/scsi/mvsas/mv_init.c
> +++ b/drivers/scsi/mvsas/mv_init.c
> @@ -642,7 +642,6 @@ static void mvs_pci_remove(struct pci_dev *pdev)
> 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);
>
> diff --git a/drivers/scsi/pm8001/pm8001_init.c 
> b/drivers/scsi/pm8001/pm8001_init.c
> index 417368ccb686..034b2f7d1135 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -1088,7 +1088,6 @@ static void pm8001_pci_remove(struct pci_dev *pdev)
> struct pm8001_hba_info *pm8001_ha;
> int i, j;
> pm8001_ha = sha->lldd_ha;
> -   scsi_remove_host(pm8001_ha->shost);
> sas_unregister_ha(sha);
> sas_remove_host(pm8001_ha->shost);
> list_del(_ha->list);
> diff --git a/drivers/scsi/scsi_transport_sas.c 
> b/drivers/scsi/scsi_transport_sas.c
> index cdbb293aca08..ca0e5a9a17f8 100644
> --- a/drivers/scsi/scsi_transport_sas.c
> +++ b/drivers/scsi/scsi_transport_sas.c
> @@ -370,12 +370,16 @@ EXPORT_SYMBOL(sas_remove_children);
>   * sas_remove_host  -  tear down a Scsi_Host's SAS data structures
>   * @shost: Scsi Host that is torn down
>   *
> - * Removes all SAS PHYs and remote PHYs for a given Scsi_Host.
> - * Must be called just before scsi_remove_host for SAS HBAs.
> + * Removes all SAS PHYs and remote PHYs for a given Scsi_Host and remove the
> + * Scsi_Host as well.
> + *
> + * Note: Do not call scsi_remove_host() on the Scsi_Host any more, as it is
> + * already removed.
>   */
>  void sas_remove_host(struct Scsi_Host *shost)
>  {
> sas_remove_children(>shost_gendev);
> +   scsi_remove_host(shost);
>  }
>  EXPORT_SYMBOL(sas_remove_host);
>
> --
> 2.12.0
>
Thanks, Johannes,

Reviewed-by:: Jack Wang 

-- 
Jack Wang
Linux Kernel Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Re: [PATCH] scsi: sas: move scsi_remove_host call into sas_remove_host

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig 

Re: [PATCH] scsi: sas: move scsi_remove_host call into sas_remove_host

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig