diff mbox

[v3,3/4] sd: Make synchronize cache upon shutdown asynchronous

Message ID 1492727247.2642.12.camel@sandisk.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Bart Van Assche April 20, 2017, 10:27 p.m. UTC 
On Thu, 2017-04-20 at 15:13 -0700, James Bottomley wrote:
> On Thu, 2017-04-20 at 21:59 +0000, Bart Van Assche wrote:
> > This approach cannot work. A scsi_target_block() call by the 
> > transport layer can happen concurrently with the 
> > __scsi_remove_device() call and hence can occur at any time between 
> > the scsi_start_queue() call by __scsi_remove_device() and the 
> > sd_shutdown() call, resulting in a deadlock.
> 
> How is that possible?  Once the device goes into the CANCEL state, it
> no longer can be found by starget_for_each_device() because
> scsi_device_get() returns NULL ... unless you also have a patch
> altering that?

No changes were made in the SCSI core other than the attached two patches.
I'm not sure about the root cause but every time I simulated a transport
layer failure before I tried to remove the ib_srp kernel module I ran into
a deadlock (see also the call traces below). Inspection of the lsscsi
output and /sys/kernel/debug/block learned me that both queues involved in
the deadlock were stopped.

sysrq: SysRq : Show Blocked State
  task                        PC stack   pid father
kworker/11:3    D    0  2910      2 0x00000000
Workqueue: srp_remove srp_remove_work [ib_srp]
Call Trace:
 __schedule+0x3df/0xc10
 schedule+0x3d/0x90
 schedule_timeout+0x234/0x4b0
 io_schedule_timeout+0x1e/0x50
 wait_for_completion_io_timeout+0x118/0x180
 blk_execute_rq+0x8e/0xc0
 scsi_execute+0xe7/0x200
 sd_sync_cache+0x8a/0x170
 sd_shutdown+0x5f/0xe0
 sd_remove+0x63/0xc0
 device_release_driver_internal+0x13f/0x1e0
 device_release_driver+0x12/0x20
 bus_remove_device+0x114/0x190
 device_del+0x205/0x320
 __scsi_remove_device+0x132/0x140
 scsi_forget_host+0x60/0x70
 scsi_remove_host+0x71/0x110
 srp_remove_work+0x90/0x220 [ib_srp]
 process_one_work+0x20b/0x6a0
 worker_thread+0x4e/0x4a0
 kthread+0x113/0x150
 ret_from_fork+0x2e/0x40
kworker/4:3     D    0  2913      2 0x00000000
Workqueue: srp_remove srp_remove_work [ib_srp]
Call Trace:
 __schedule+0x3df/0xc10
 schedule+0x3d/0x90
 schedule_timeout+0x234/0x4b0
 io_schedule_timeout+0x1e/0x50
 wait_for_completion_io_timeout+0x118/0x180
 blk_execute_rq+0x8e/0xc0
 scsi_execute+0xe7/0x200
 sd_sync_cache+0x8a/0x170
 sd_shutdown+0x5f/0xe0
 sd_remove+0x63/0xc0
 device_release_driver_internal+0x13f/0x1e0
 device_release_driver+0x12/0x20
 bus_remove_device+0x114/0x190
 device_del+0x205/0x320
 __scsi_remove_device+0x132/0x140
 scsi_forget_host+0x60/0x70
 scsi_remove_host+0x71/0x110
 srp_remove_work+0x90/0x220 [ib_srp]
 process_one_work+0x20b/0x6a0
 worker_thread+0x4e/0x4a0
 kthread+0x113/0x150
 ret_from_fork+0x2e/0x40
modprobe        D    0  2916   2218 0x00000000
Call Trace:
 __schedule+0x3df/0xc10
 schedule+0x3d/0x90
 schedule_timeout+0x273/0x4b0
 wait_for_completion+0x108/0x170
 flush_workqueue+0x207/0x720
 srp_remove_one+0xbe/0x110 [ib_srp]
 ib_unregister_client+0x18f/0x200 [ib_core]
 srp_cleanup_module+0x10/0x618 [ib_srp]
 SyS_delete_module+0x198/0x1f0
 entry_SYSCALL_64_fastpath+0x18/0xad

Bart.
diff mbox

Patch

From fa69092d22f4f58ede7d37e68148a4aa4615d2ab Mon Sep 17 00:00:00 2001
From: Bart Van Assche <bart.vanassche@sandisk.com>
Date: Tue, 18 Apr 2017 10:11:02 -0700
Subject: [PATCH 2/2] Make __scsi_remove_device go straight from BLOCKED to DEL

If a device is blocked, make __scsi_remove_device() cause it to
transition to the DEL state. This means that all the commands
issued in .shutdown() will error in the mid-layer, thus making
the removal proceed without being stopped.

This patch is a slightly modified version of a patch from James
Bottomley.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Israel Rukshin <israelr@mellanox.com>
Cc: Max Gurtovoy <maxg@mellanox.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
---
 drivers/scsi/scsi_lib.c   |  2 +-
 drivers/scsi/scsi_sysfs.c | 19 ++++++++++++++++---
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ffa6e61299a9..376cd1da102c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2611,7 +2611,6 @@  scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 		case SDEV_QUIESCE:
 		case SDEV_OFFLINE:
 		case SDEV_TRANSPORT_OFFLINE:
-		case SDEV_BLOCK:
 			break;
 		default:
 			goto illegal;
@@ -2625,6 +2624,7 @@  scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 		case SDEV_OFFLINE:
 		case SDEV_TRANSPORT_OFFLINE:
 		case SDEV_CANCEL:
+		case SDEV_BLOCK:
 		case SDEV_CREATED_BLOCK:
 			break;
 		default:
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 82dfe07b1d47..5b03e58bdd67 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1282,16 +1282,29 @@  void __scsi_remove_device(struct scsi_device *sdev)
 		return;
 
 	if (sdev->is_visible) {
-		if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
-			return;
+		/*
+		 * If blocked, we go straight to DEL and restart the queue so
+		 * any commands issued during driver shutdown (like sync
+		 * cache) are errored immediately.
+		 */
+		if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0) {
+			if (scsi_device_set_state(sdev, SDEV_DEL) != 0)
+				return;
+
+			scsi_start_queue(sdev);
+			sdev_printk(KERN_DEBUG, sdev,
+				    "Changed state from BLOCKED to DEL\n");
+		}
 
 		bsg_unregister_queue(sdev->request_queue);
 		device_unregister(&sdev->sdev_dev);
 		transport_remove_device(dev);
 		scsi_dh_remove_device(sdev);
 		device_del(dev);
-	} else
+	} else {
 		put_device(&sdev->sdev_dev);
+		scsi_start_queue(sdev);
+	}
 
 	/*
 	 * Stop accepting new requests and wait until all queuecommand() and
-- 
2.12.2