Re: [PATCH v3 0/4] Avoid that __scsi_remove_device() hangs

2017-04-18 Thread Israel Rukshin

On 4/17/2017 8:34 PM, Bart Van Assche wrote:

__scsi_remove_device() hangs if it is waiting for the SYNCHRONIZE CACHE
command submitted by the sd driver to finish if the block layer queue is
stopped and does not get restarted. This patch series avoids that that
hang occurs.

Changes compared to v2:
- Moved the "stop_disk" assignment after the sdkp check in the sd driver.
- Added a completion function for asynchronous SYNCHRONIZE CACHE commands.
- Added "disk" and "done" arguments to scsi_execute_async().

Changes compared to v1:
- Reworked the approach of this patch series.

Bart Van Assche (4):
   Introduce scsi_start_queue()
   Introduce scsi_execute_async()
   sd: Make synchronize cache upon shutdown asynchronous
   Avoid that __scsi_remove_device() hangs

  drivers/scsi/scsi_lib.c| 114 ++---
  drivers/scsi/scsi_priv.h   |   1 +
  drivers/scsi/scsi_sysfs.c  |   9 
  drivers/scsi/sd.c  |  45 --
  include/scsi/scsi_device.h |   5 ++
  5 files changed, 142 insertions(+), 32 deletions(-)


Hi Bart,

I tested those patches and I got a NULL dereference at sd_sync_cache_done().
The test is unloading ib_srp while one port is down.
The previous version worked fine.

From the log:
[  190.260240] sd 8:0:0:0: [sdc] Synchronizing SCSI cache
[  190.266412] scsi 8:0:0:0: rejecting I/O to dead device
[  190.272412] BUG: unable to handle kernel NULL pointer dereference at 
02f0

[  190.281102] IP: sd_sync_cache_done+0x1b/0x80 [sd_mod]
[  190.482738] Call Trace:
[  190.486052]  blk_finish_request+0x73/0x130
[  190.491007]  __blk_end_bidi_request+0x2d/0x40
[  190.496215]  __blk_end_request_all+0x1f/0x40
[  190.501338]  blk_peek_request+0x1c5/0x2b0
[  190.506203]  scsi_request_fn+0x3f/0x6c0
[  190.510888]  ? kobject_put+0x1f/0x60
[  190.515305]  __blk_run_queue+0x33/0x40
[  190.519877]  blk_start_queue+0x29/0x40
[  190.524438]  scsi_start_queue+0x40/0x60
[  190.529081]  __scsi_remove_device+0x4d/0xe0
[  190.534079]  scsi_forget_host+0x60/0x70
[  190.538738]  scsi_remove_host+0x77/0x110
[  190.543462]  srp_remove_work+0x90/0x230 [ib_srp]

Regards,
Israel.


Re: [PATCH v2] scsi_sysfs: fix hang when removing scsi device

2017-03-16 Thread Israel Rukshin

On 3/16/2017 5:42 PM, Bart Van Assche wrote:

On Thu, 2017-03-16 at 11:02 +0200, Israel Rukshin wrote:

I tested your patches and the hang disappeared when fast_io_fail_tmo
expired. The warning from patch 1 still exist, so we need an additional
fix for that.

Hello Israel,

Thanks for the testing! I will leave out patch 1 for now since it is not
needed to fix the hang.

I assume that your e-mail counts as a Tested-by?

PS: please do not top-post when replying. This is considered annoying in the
Linux kernel community.

Bart.


Hello Bart,

Yes, you can add me as Tested-by.

Israel.


Re: [PATCH v2] scsi_sysfs: fix hang when removing scsi device

2017-03-16 Thread Israel Rukshin

Hi Bart,

I tested your patches and the hang disappeared when fast_io_fail_tmo 
expired.

The warning from patch 1 still exist, so we need an additional fix for that.

Regards,
Israel

On 3/16/2017 1:27 AM, Bart Van Assche wrote:

On Tue, 2017-03-14 at 16:23 +0200, Israel Rukshin wrote:

Patch number 5 doesn't handle the case when device_for_each_child() is
called. device_for_each_child() calls to target_unblock() that uses also
starget_for_each_device(). After applying also the following change the
hang disappeared but it didn't fix the warning. Those fixes are not enough
because if fast_io_fail_tmo is set to infinity then the hang will remain,
because only __rport_fail_io_fast() calls to scsi_target_unblock() and
terminate_rport_io() that free the sync cache command.

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index e5d4b50..09f9566 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -3068,9 +3068,15 @@ void scsi_device_resume(struct scsi_device *sdev)
   static int
   target_unblock(struct device *dev, void *data)
   {
-   if (scsi_is_target_device(dev))
-   starget_for_each_device(to_scsi_target(dev), data,
-   device_unblock);
+   if (scsi_is_target_device(dev)) {
+   struct scsi_target *starget = to_scsi_target(dev);
+   struct Scsi_Host *shost = dev_to_shost(dev->parent);
+   unsigned long flags;
+
+   spin_lock_irqsave(shost->host_lock, flags);
+   __starget_for_each_device(starget, data, device_unblock);
+   spin_unlock_irqrestore(shost->host_lock, flags);
+   }
  return 0;
   }

Hello Israel,

Regarding setting fast_io_fail_tmo to infinity: that does not prevent
kernel module unloading because srp_timed_out() stops resetting the
timer as soon as the SCSI device is unblocked. The above patch should
realize that but suffers from the same issue as a patch attached to
my previous e-mail, namely lock inversion. For at least the following
call chain the block layer queue lock is the outer lock and the SCSI
host lock is the inner lock:
ata_qc_schedule_eh()
-> blk_abort_request()
   -> blk_mq_rq_timed_out()
 -> scsi_timeout()
   -> scsi_times_out()
 -> scsi_eh_scmd_add()

So I think we should avoid introducing code with the SCSI host lock
as outer lock and the block layer queue lock as inner lock. How about
the attached four patches?

Thanks,

Bart.




Re: [PATCH v2] scsi_sysfs: fix hang when removing scsi device

2017-03-14 Thread Israel Rukshin

Hello Bart,

Patch number 5 doesn't handle the case when device_for_each_child() is 
called.
device_for_each_child() calls to target_unblock() that uses also 
starget_for_each_device().
After applying also the following change the hang disappeared but it 
didn't fix the warning.
Those fixes are not enough because if fast_io_fail_tmo is set to 
infinity then the hang will remain,
because only __rport_fail_io_fast() calls to scsi_target_unblock() and 
terminate_rport_io()

that free the sync cache command.

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index e5d4b50..09f9566 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -3068,9 +3068,15 @@ void scsi_device_resume(struct scsi_device *sdev)
 static int
 target_unblock(struct device *dev, void *data)
 {
-   if (scsi_is_target_device(dev))
-   starget_for_each_device(to_scsi_target(dev), data,
-   device_unblock);
+   if (scsi_is_target_device(dev)) {
+   struct scsi_target *starget = to_scsi_target(dev);
+   struct Scsi_Host *shost = dev_to_shost(dev->parent);
+   unsigned long flags;
+
+   spin_lock_irqsave(shost->host_lock, flags);
+   __starget_for_each_device(starget, data, device_unblock);
+   spin_unlock_irqrestore(shost->host_lock, flags);
+   }
return 0;
 }

--
1.8.4.3


Israel


On 3/14/2017 11:44 AM, Israel Rukshin wrote:

Hello Bart,

I applied and tested patches 3, 4 and 5.
I am sorry to tell you that it didn't work.
I saw the warning you added and the hang at device_del().

[  333.696462] [ cut here ]
[  333.696471] WARNING: CPU: 11 PID: 321 at 
drivers/scsi/scsi_sysfs.c:1280 __scsi_remove_device+0x106/0x110
[  333.696472] Modules linked in: nfsv3 ib_srp(-) dm_service_time 
scsi_transport_srp ib_uverbs ib_umad ib_ipoib ib_cm mlx4_ib ib_core 
rpcsec_gss_krb5 nfsv4 dns_resolver nfs netconsole fscache dm_mirror 
dm_region_hash dm_log sb_edac edac_core x86_pkg_temp_thermal 
intel_powerclamp coretemp kvm_intel kvm irqbypass crct10dif_pclmul 
crc32_pclmul ghash_clmulni_intel iTCO_wdt pcbc iTCO_vendor_support 
aesni_intel mei_me crypto_simd joydev input_leds ipmi_si ioatdma mei 
glue_helper lpc_ich pcspkr cryptd ipmi_devintf shpchp ipmi_msghandler 
sg i2c_i801 mfd_core dm_multipath dm_mod nfsd binfmt_misc auth_rpcgss 
nfs_acl lockd grace sunrpc ip_tables ext4 jbd2 mbcache sd_mod mgag200 
drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops isci igb 
ttm ahci libsas ptp libahci scsi_transport_sas pps_core dca crc32c_intel
[  333.696511]  drm i2c_algo_bit libata mlx4_core fjes [last unloaded: 
ib_srp]
[  333.696516] CPU: 11 PID: 321 Comm: kworker/11:1 Not tainted 
4.11.0-rc2+ #99
[  333.696517] Hardware name: Supermicro X9DRFR/X9DRFR, BIOS 1.0a 
09/11/2012

[  333.696522] Workqueue: srp_remove srp_remove_work [ib_srp]
[  333.696523] Call Trace:
[  333.696530]  dump_stack+0x63/0x90
[  333.696534]  __warn+0xcb/0xf0
[  333.696536]  warn_slowpath_null+0x1d/0x20
[  333.696538]  __scsi_remove_device+0x106/0x110
[  333.696540]  scsi_forget_host+0x60/0x70
[  333.696545]  scsi_remove_host+0x77/0x110
[  333.696547]  srp_remove_work+0x90/0x230 [ib_srp]
[  333.696551]  process_one_work+0x177/0x430
[  333.696553]  worker_thread+0x4e/0x4b0
[  333.696555]  kthread+0x101/0x140
[  333.696556]  ? process_one_work+0x430/0x430
[  333.696558]  ? kthread_create_on_node+0x60/0x60
[  333.696563]  ret_from_fork+0x2c/0x40
[  333.696565] ---[ end trace b1edd584bf21aaba ]---

Israel.


On 3/14/2017 4:35 AM, Bart Van Assche wrote:

On Mon, 2017-03-13 at 14:55 -0700, James Bottomley wrote:

This is true, but I don't see how it can cause the host to be freed
before the sdev.  The memory for struct Scsi_Host is freed in the
shost_gendev release routine, which should be pinned by the parent
traversal from sdev.  So it should not be possible for
  scsi_host_dev_release() to be called before
  scsi_device_dev_release_usercontext() becase the latter has the final
put of the parent device.

Hello James,

I will run a bisect to see whether that provides more information about
what caused the change in the reference counting behavior.

Israel, since you did not hit the reference counting issue in your 
tests,

can you repeat your test with patches 3, 4 and 5 applied?

Thanks,

Bart.






Re: [PATCH v2] scsi_sysfs: fix hang when removing scsi device

2017-03-14 Thread Israel Rukshin

Hello Bart,

I applied and tested patches 3, 4 and 5.
I am sorry to tell you that it didn't work.
I saw the warning you added and the hang at device_del().

[  333.696462] [ cut here ]
[  333.696471] WARNING: CPU: 11 PID: 321 at 
drivers/scsi/scsi_sysfs.c:1280 __scsi_remove_device+0x106/0x110
[  333.696472] Modules linked in: nfsv3 ib_srp(-) dm_service_time 
scsi_transport_srp ib_uverbs ib_umad ib_ipoib ib_cm mlx4_ib ib_core 
rpcsec_gss_krb5 nfsv4 dns_resolver nfs netconsole fscache dm_mirror 
dm_region_hash dm_log sb_edac edac_core x86_pkg_temp_thermal 
intel_powerclamp coretemp kvm_intel kvm irqbypass crct10dif_pclmul 
crc32_pclmul ghash_clmulni_intel iTCO_wdt pcbc iTCO_vendor_support 
aesni_intel mei_me crypto_simd joydev input_leds ipmi_si ioatdma mei 
glue_helper lpc_ich pcspkr cryptd ipmi_devintf shpchp ipmi_msghandler sg 
i2c_i801 mfd_core dm_multipath dm_mod nfsd binfmt_misc auth_rpcgss 
nfs_acl lockd grace sunrpc ip_tables ext4 jbd2 mbcache sd_mod mgag200 
drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops isci igb 
ttm ahci libsas ptp libahci scsi_transport_sas pps_core dca crc32c_intel
[  333.696511]  drm i2c_algo_bit libata mlx4_core fjes [last unloaded: 
ib_srp]
[  333.696516] CPU: 11 PID: 321 Comm: kworker/11:1 Not tainted 
4.11.0-rc2+ #99

[  333.696517] Hardware name: Supermicro X9DRFR/X9DRFR, BIOS 1.0a 09/11/2012
[  333.696522] Workqueue: srp_remove srp_remove_work [ib_srp]
[  333.696523] Call Trace:
[  333.696530]  dump_stack+0x63/0x90
[  333.696534]  __warn+0xcb/0xf0
[  333.696536]  warn_slowpath_null+0x1d/0x20
[  333.696538]  __scsi_remove_device+0x106/0x110
[  333.696540]  scsi_forget_host+0x60/0x70
[  333.696545]  scsi_remove_host+0x77/0x110
[  333.696547]  srp_remove_work+0x90/0x230 [ib_srp]
[  333.696551]  process_one_work+0x177/0x430
[  333.696553]  worker_thread+0x4e/0x4b0
[  333.696555]  kthread+0x101/0x140
[  333.696556]  ? process_one_work+0x430/0x430
[  333.696558]  ? kthread_create_on_node+0x60/0x60
[  333.696563]  ret_from_fork+0x2c/0x40
[  333.696565] ---[ end trace b1edd584bf21aaba ]---

Israel.


On 3/14/2017 4:35 AM, Bart Van Assche wrote:

On Mon, 2017-03-13 at 14:55 -0700, James Bottomley wrote:

This is true, but I don't see how it can cause the host to be freed
before the sdev.  The memory for struct Scsi_Host is freed in the
shost_gendev release routine, which should be pinned by the parent
traversal from sdev.  So it should not be possible for
  scsi_host_dev_release() to be called before
  scsi_device_dev_release_usercontext() becase the latter has the final
put of the parent device.

Hello James,

I will run a bisect to see whether that provides more information about
what caused the change in the reference counting behavior.

Israel, since you did not hit the reference counting issue in your tests,
can you repeat your test with patches 3, 4 and 5 applied?

Thanks,

Bart.




Re: [PATCH v2] scsi_sysfs: fix hang when removing scsi device

2017-03-12 Thread Israel Rukshin

Hi Bart,

scsi_device_get() affect I/O because scsi_target_unblock() use it and calls to 
blk_start_queue().
terminate_rport_io() is called after scsi_target_unblock() and completes all 
the commands
including the SYNCHRONIZE CACHE command.

I applied your patch and you can see that QUEUE_FLAG_STOPPED is on.

[  342.485087] sd 7:0:0:0: Device offlined - not ready after error recovery
[  342.505738] scsi host10: ib_srp: Path record query failed
[  342.512023] sd 10:0:0:0: Device offlined - not ready after error recovery
[  342.589265] sd 7:0:0:0: __scsi_remove_device: device_busy = 0 device_blocked 
= 0
[  342.624110] sd 7:0:0:0: [sdc] Synchronizing SCSI cache
[  342.630263] sd 7:0:0:0: [sdc] Synchronize Cache(10) failed: Result: 
hostbyte=DID_TRANSPORT_FAILFAST driverbyte=DRIVER_OK
[  342.649504] scsi 7:0:0:0: alua: Detached
[  342.769099] [ cut here ]
[  342.769107] WARNING: CPU: 10 PID: 317 at drivers/scsi/scsi_sysfs.c:1293 
__scsi_remove_device+0x131/0x140
[  342.769108] Modules linked in: nfsv3 ib_srp(-) dm_service_time 
scsi_transport_srp ib_uverbs ib_umad ib_ipoib ib_cm mlx4_ib ib_core 
rpcsec_gss_krb5 nfsv4 dns_resolver nfs netconsole fscache dm_mirror 
dm_region_hash dm_log sb_edac edac_core x86_pkg_temp_thermal intel_powerclamp 
coretemp kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul 
ghash_clmulni_intel pcbc aesni_intel crypto_simd joydev input_leds glue_helper 
ipmi_si iTCO_wdt pcspkr cryptd mei_me iTCO_vendor_support ipmi_devintf sg 
lpc_ich ipmi_msghandler mei i2c_i801 shpchp mfd_core ioatdma nfsd auth_rpcgss 
dm_multipath nfs_acl dm_mod lockd grace sunrpc ip_tables ext4 jbd2 mbcache 
sd_mod mgag200 drm_kms_helper syscopyarea isci sysfillrect sysimgblt ahci igb 
libsas fb_sys_fops libahci ttm scsi_transport_sas ptp pps_core crc32c_intel dca 
drm
[  342.769152]  i2c_algo_bit libata mlx4_core fjes [last unloaded: ib_srp]
[  342.769157] CPU: 10 PID: 317 Comm: kworker/10:1 Not tainted 4.11.0-rc1+ #97
[  342.769157] Hardware name: Supermicro X9DRFR/X9DRFR, BIOS 1.0a 09/11/2012
[  342.769163] Workqueue: srp_remove srp_remove_work [ib_srp]
[  342.769165] Call Trace:
[  342.769173]  dump_stack+0x63/0x90
[  342.769176]  __warn+0xcb/0xf0
[  342.769178]  warn_slowpath_null+0x1d/0x20
[  342.769180]  __scsi_remove_device+0x131/0x140
[  342.769182]  scsi_forget_host+0x60/0x70
[  342.769186]  scsi_remove_host+0x77/0x110
[  342.769189]  srp_remove_work+0x90/0x230 [ib_srp]
[  342.769192]  process_one_work+0x177/0x430
[  342.769193]  worker_thread+0x4e/0x4b0
[  342.769195]  kthread+0x101/0x140
[  342.769197]  ? process_one_work+0x430/0x430
[  342.769198]  ? kthread_create_on_node+0x60/0x60
[  342.769201]  ret_from_fork+0x2c/0x40
[  342.769202] ---[ end trace 1eef46ba7887fee3 ]---
[  342.769210] sd 10:0:0:0: __scsi_remove_device: device_busy = 0 
device_blocked = 0
[  343.020039] sd 10:0:0:0: [sde] Synchronizing SCSI cache
[  352.717659] scsi host10: ib_srp: Got failed path rec status -110

Israel.


On 3/9/2017 9:36 PM, Bart Van Assche wrote:

On Thu, 2017-03-09 at 18:37 +0200, Israel Rukshin wrote:

The bug reproduce when unloading srp module with one port down.
sd_shutdown() hangs when __scsi_remove_device() get scsi_device with
state SDEV_OFFLINE or SDEV_TRANSPORT_OFFLINE.
It hangs because sd_shutdown() is trying to send sync cache command
when the device is offline but with SDEV_CANCEL status.
The status was changed to SDEV_CANCEL by __scsi_remove_device()
before it calls to device_del().

The block layer timeout mechanism doesn't cause the SYNCHRONIZE CACHE
command to fail after the timeout expired because the request timer
wasn't started.
blk_peek_request() that is called from scsi_request_fn() didn't return
this request and therefore the request timer didn't start.

This commit doesn't accept new commands if the original state was offline.

The bug was revealed after commit cff549 ("scsi: proper state checking
and module refcount handling in scsi_device_get").
After this commit scsi_device_get() returns error if the device state
is SDEV_CANCEL.
This eventually leads SRP fast I/O failure timeout handler not to clean
the sync cache command because scsi_target_unblock() skip the canceled device.
If this timeout handler is set to infinity then the hang remains forever
also before commit cff549.

How could blk_peek_request() not return a request that has not yet been
started? How could a patch that changes scsi_device_get() affect I/O since
scsi_device_get() is not called from the I/O path? Anyway, could you try to
reproduce the hang with the patch below applied and see whether the output
produced by this patch helps to determine what is going on?

Thanks,

Bart.

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ba2286652ff6..855548ff4c4d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -3018,8 +3018,10 @@ scsi_internal_device_unblock(struct scsi_device *sdev,
else
sdev->sdev_state =

[PATCH v2] scsi_sysfs: fix hang when removing scsi device

2017-03-09 Thread Israel Rukshin
The bug reproduce when unloading srp module with one port down.
sd_shutdown() hangs when __scsi_remove_device() get scsi_device with
state SDEV_OFFLINE or SDEV_TRANSPORT_OFFLINE.
It hangs because sd_shutdown() is trying to send sync cache command
when the device is offline but with SDEV_CANCEL status.
The status was changed to SDEV_CANCEL by __scsi_remove_device()
before it calls to device_del().

The block layer timeout mechanism doesn't cause the SYNCHRONIZE CACHE
command to fail after the timeout expired because the request timer
wasn't started.
blk_peek_request() that is called from scsi_request_fn() didn't return
this request and therefore the request timer didn't start.

This commit doesn't accept new commands if the original state was offline.

The bug was revealed after commit cff549 ("scsi: proper state checking
and module refcount handling in scsi_device_get").
After this commit scsi_device_get() returns error if the device state
is SDEV_CANCEL.
This eventually leads SRP fast I/O failure timeout handler not to clean
the sync cache command because scsi_target_unblock() skip the canceled device.
If this timeout handler is set to infinity then the hang remains forever
also before commit cff549.

sysrq: SysRq : sysrq: Show Blocked State
task PC stack pid father
kworker/2:0 D 88046fa95c00 0 21178 2 0x
Workqueue: srp_remove srp_remove_work [ib_srp]
Call Trace:
[] schedule+0x35/0x80
[] schedule_timeout+0x237/0x2d0
[] io_schedule_timeout+0xa6/0x110
[] wait_for_completion_io+0xa3/0x110
[] blk_execute_rq+0xdf/0x120
[] scsi_execute+0xce/0x150 [scsi_mod]
[] scsi_execute_req_flags+0x8f/0xf0 [scsi_mod]
[] sd_sync_cache+0xa9/0x190 [sd_mod]
[] sd_shutdown+0x6a/0x100 [sd_mod]
[] sd_remove+0x64/0xc0 [sd_mod]
[] __device_release_driver+0x8d/0x120
[] device_release_driver+0x1e/0x30
[] bus_remove_device+0xf9/0x170
[] device_del+0x127/0x240
[] __scsi_remove_device+0xc1/0xd0 [scsi_mod]
[] scsi_forget_host+0x57/0x60 [scsi_mod]
[] scsi_remove_host+0x72/0x110 [scsi_mod]
[] srp_remove_work+0x8b/0x200 [ib_srp]
...

Signed-off-by: Israel Rukshin <isra...@mellanox.com>
Reviewed-by: Max Gurtovoy <m...@mellanox.com>
---

Changes from v1:
 - add extra description to commit message and to the comment.
 - refer to the commit that originally introduced this hang.

---
 drivers/scsi/scsi_sysfs.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 82dfe07..8a977f5 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1282,6 +1282,8 @@ void __scsi_remove_device(struct scsi_device *sdev)
return;
 
if (sdev->is_visible) {
+   enum scsi_device_state oldstate = sdev->sdev_state;
+
if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
return;
 
@@ -1289,6 +1291,17 @@ void __scsi_remove_device(struct scsi_device *sdev)
device_unregister(>sdev_dev);
transport_remove_device(dev);
scsi_dh_remove_device(sdev);
+
+   /*
+* Fail new requests if the old state was offline.
+* This avoids from sd_shutdown() to hang.
+* The SYNCHRONIZE CACHE request timer will never start
+* in that case.
+*/
+   if (oldstate == SDEV_TRANSPORT_OFFLINE ||
+   oldstate == SDEV_OFFLINE)
+   blk_set_queue_dying(sdev->request_queue);
+
device_del(dev);
} else
put_device(>sdev_dev);
-- 
2.4.3



[PATCH] scsi_sysfs: fix hang when removing scsi device

2017-03-02 Thread Israel Rukshin
The bug reproduce when unloading srp module with one port down.
device_del() hangs when __scsi_remove_device() get scsi_device with
state SDEV_OFFLINE or SDEV_TRANSPORT_OFFLINE.
It hangs because device_del() is trying to send sync cache command
when the device is offline but with SDEV_CANCEL status.
The status was changed to SDEV_CANCEL by __scsi_remove_device()
before it calls to device_del().

This commit doesn't accept new commands if the original state was offline.

sysrq: SysRq : sysrq: Show Blocked State
task PC stack pid father
kworker/2:0 D 88046fa95c00 0 21178 2 0x
Workqueue: srp_remove srp_remove_work [ib_srp]
Call Trace:
[] schedule+0x35/0x80
[] schedule_timeout+0x237/0x2d0
[] io_schedule_timeout+0xa6/0x110
[] wait_for_completion_io+0xa3/0x110
[] blk_execute_rq+0xdf/0x120
[] scsi_execute+0xce/0x150 [scsi_mod]
[] scsi_execute_req_flags+0x8f/0xf0 [scsi_mod]
[] sd_sync_cache+0xa9/0x190 [sd_mod]
[] sd_shutdown+0x6a/0x100 [sd_mod]
[] sd_remove+0x64/0xc0 [sd_mod]
[] __device_release_driver+0x8d/0x120
[] device_release_driver+0x1e/0x30
[] bus_remove_device+0xf9/0x170
[] device_del+0x127/0x240
[] __scsi_remove_device+0xc1/0xd0 [scsi_mod]
[] scsi_forget_host+0x57/0x60 [scsi_mod]
[] scsi_remove_host+0x72/0x110 [scsi_mod]
[] srp_remove_work+0x8b/0x200 [ib_srp]
...

Fixes: e619e6cbe (Revert "scsi: Fix a bdi reregistration race")
Signed-off-by: Israel Rukshin <isra...@mellanox.com>
Reviewed-by: Max Gurtovoy <m...@mellanox.com>
---
 drivers/scsi/scsi_sysfs.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 82dfe07..ed55aac 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1282,6 +1282,8 @@ void __scsi_remove_device(struct scsi_device *sdev)
return;
 
if (sdev->is_visible) {
+   enum scsi_device_state oldstate = sdev->sdev_state;
+
if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
return;
 
@@ -1289,6 +1291,15 @@ void __scsi_remove_device(struct scsi_device *sdev)
device_unregister(>sdev_dev);
transport_remove_device(dev);
scsi_dh_remove_device(sdev);
+
+   /*
+* Fail new requests if the old state was offline.
+* This avoids from device_del() to hang.
+*/
+   if (oldstate == SDEV_TRANSPORT_OFFLINE ||
+   oldstate == SDEV_OFFLINE)
+   blk_set_queue_dying(sdev->request_queue);
+
device_del(dev);
} else
put_device(>sdev_dev);
-- 
2.4.3