[PATCH v5 0/8] block, scsi, md: Improve suspend and resume

[PATCH v5 0/8] block, scsi, md: Improve suspend and resume

[Bart Van Assche]

Hello Jens,

It is known that during the resume following a hibernate, especially when
using an md RAID1 array created on top of SCSI devices, sometimes the
system hangs instead of coming up properly. This patch series fixes this
problem. This patch series is an alternative for Ming Lei's "block/scsi:
safe SCSI quiescing" patch series. The advantages of this patch series are
that a fix for the md driver is included, that no third argument has been
added to blk_get_request() and that there is no race between
blk_get_request() and SCSI quiescing.

These patches have been tested on top of a merge of the block layer
for-next branch and Linus' master tree. Linus' master tree includes
patch "KVM: x86: Fix the NULL pointer parameter in check_cr_write()"
but the block layer for-next branch not yet.

Please consider these changes for kernel v4.15.

Thanks,

Bart.

Changes between v4 and v5:
- Split blk_set_preempt_only() into two functions as requested by Christoph.
- Made blk_get_request() trigger WARN_ONCE() if it is attempted to allocate
  a request while the system is frozen. This is a big help to identify drivers
  that submit I/O while the system is frozen.
- Since kernel thread freezing is on its way out, reworked the approach for
  avoiding that the md driver submits I/O while the system is frozen such that
  the approach no longer depends on the kernel thread freeze mechanism.
- Fixed the (theoretical) deadlock in scsi_device_quiesce() that was identified
  by Ming.

Changes between v3 and v4:
- Made sure that this patch series not only works for scsi-mq but also for
  the legacy SCSI stack.
- Removed an smp_rmb()/smp_wmb() pair from the hot path and added a comment
  that explains why that is safe.
- Reordered the patches in this patch series.

Changes between v2 and v3:
- Made md kernel threads freezable.
- Changed the approach for quiescing SCSI devices again.
- Addressed Ming's review comments.

Changes compared to v1 of this patch series:
- Changed the approach and rewrote the patch series.

Bart Van Assche (7):
  md: Introduce md_stop_all_writes()
  md: Neither resync nor reshape while the system is frozen
  block: Convert RQF_PREEMPT into REQ_PREEMPT
  block: Add the QUEUE_FLAG_PREEMPT_ONLY request queue flag
  scsi: Reduce suspend latency
  scsi: Set QUEUE_FLAG_PREEMPT_ONLY while quiesced
  block: Make SCSI device suspend and resume work reliably

Ming Lei (1):
  block: Make q_usage_counter also track legacy requests

 block/blk-core.c          | 67 ++++++++++++++++++++++++++++++++++++++++++-----
 block/blk-mq-debugfs.c    |  2 +-
 block/blk-mq.c            | 14 +++-------
 block/blk-timeout.c       |  2 +-
 drivers/ide/ide-atapi.c   |  3 +--
 drivers/ide/ide-io.c      |  2 +-
 drivers/ide/ide-pm.c      |  4 +--
 drivers/md/md.c           | 52 +++++++++++++++++++++++++++++++-----
 drivers/scsi/scsi_lib.c   | 43 ++++++++++++++++++++----------
 fs/block_dev.c            |  4 +--
 include/linux/blk_types.h |  6 +++++
 include/linux/blkdev.h    | 11 +++++---
 12 files changed, 160 insertions(+), 50 deletions(-)

-- 
2.14.1

[PATCH v5 1/8] md: Introduce md_stop_all_writes()

[Bart Van Assche]

Introduce md_stop_all_writes() because the next patch will add
a second caller for this function. Rename md_notifier into
md_reboot_notifier to avoid that the name of this notifier will
become confusing due to the next patch. This patch does not
change any functionality.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Shaohua Li <shli@kernel.org>
Cc: linux-raid@vger.kernel.org
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/md/md.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 0ff1bbf6c90e..3f7426120a3b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8935,8 +8935,7 @@ int rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
 }
 EXPORT_SYMBOL_GPL(rdev_clear_badblocks);
 
-static int md_notify_reboot(struct notifier_block *this,
-			    unsigned long code, void *x)
+static void md_stop_all_writes(void)
 {
 	struct list_head *tmp;
 	struct mddev *mddev;
@@ -8960,11 +8959,17 @@ static int md_notify_reboot(struct notifier_block *this,
 	 */
 	if (need_delay)
 		mdelay(1000*1);
+}
+
+static int md_notify_reboot(struct notifier_block *this,
+			    unsigned long code, void *x)
+{
+	md_stop_all_writes();
 
 	return NOTIFY_DONE;
 }
 
-static struct notifier_block md_notifier = {
+static struct notifier_block md_reboot_notifier = {
 	.notifier_call	= md_notify_reboot,
 	.next		= NULL,
 	.priority	= INT_MAX, /* before any real devices */
@@ -9001,7 +9006,7 @@ static int __init md_init(void)
 	blk_register_region(MKDEV(mdp_major, 0), 1UL<<MINORBITS, THIS_MODULE,
 			    md_probe, NULL, NULL);
 
-	register_reboot_notifier(&md_notifier);
+	register_reboot_notifier(&md_reboot_notifier);
 	raid_table_header = register_sysctl_table(raid_root_table);
 
 	md_geninit();
@@ -9241,7 +9246,7 @@ static __exit void md_exit(void)
 
 	unregister_blkdev(MD_MAJOR,"md");
 	unregister_blkdev(mdp_major, "mdp");
-	unregister_reboot_notifier(&md_notifier);
+	unregister_reboot_notifier(&md_reboot_notifier);
 	unregister_sysctl_table(raid_table_header);
 
 	/* We cannot unload the modules while some process is
-- 
2.14.1

Re: [PATCH v5 1/8] md: Introduce md_stop_all_writes()

[Luis R. Rodriguez]

On Mon, Oct 02, 2017 at 03:52:11PM -0700, Bart Van Assche wrote:
> Introduce md_stop_all_writes() because the next patch will add
> a second caller for this function. Rename md_notifier into
> md_reboot_notifier to avoid that the name of this notifier will
> become confusing due to the next patch. This patch does not
> change any functionality.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: Shaohua Li <shli@kernel.org>
> Cc: linux-raid@vger.kernel.org
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Johannes Thumshirn <jthumshirn@suse.de>

Reviewed-by: Luis R. Rodriguez <mcgrof@kernel.org>

  Luis

[PATCH v5 2/8] md: Neither resync nor reshape while the system is frozen

[Bart Van Assche]

Some people use the md driver on laptops and use the suspend and
resume functionality. Since it is essential that submitting of
new I/O requests stops before a hibernation image is created,
interrupt the md resync and reshape actions if the system is
being frozen. Note: the resync and reshape will restart after
the system is resumed and a message similar to the following
will appear in the system log:

md: md0: data-check interrupted.

Additionally, only allow resync or reshape activity to be
started after system freezing is over.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Shaohua Li <shli@kernel.org>
Cc: linux-raid@vger.kernel.org
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/md/md.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 3f7426120a3b..a2cf2a93b0cb 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -66,6 +66,7 @@
 #include <linux/raid/md_u.h>
 #include <linux/slab.h>
 #include <linux/percpu-refcount.h>
+#include <linux/suspend.h>
 
 #include <trace/events/block.h>
 #include "md.h"
@@ -7450,7 +7451,8 @@ static int md_thread(void *arg)
 
 		wait_event_interruptible_timeout
 			(thread->wqueue,
-			 test_bit(THREAD_WAKEUP, &thread->flags)
+			 (test_bit(THREAD_WAKEUP, &thread->flags) &&
+			  atomic_read(&system_freezing_cnt) == 0)
 			 || kthread_should_stop() || kthread_should_park(),
 			 thread->timeout);
 
@@ -8961,6 +8963,37 @@ static void md_stop_all_writes(void)
 		mdelay(1000*1);
 }
 
+/*
+ * Ensure that neither resyncing nor reshaping occurs while the system is
+ * frozen.
+ */
+static int md_notify_pm(struct notifier_block *bl, unsigned long state,
+			void *unused)
+{
+	struct mddev *mddev;
+	struct list_head *tmp;
+
+	pr_debug("%s: state = %ld; system_freezing_cnt = %d\n", __func__, state,
+		 atomic_read(&system_freezing_cnt));
+
+	switch (state) {
+	case PM_HIBERNATION_PREPARE:
+		md_stop_all_writes();
+		break;
+	case PM_POST_HIBERNATION:
+		rcu_read_lock();
+		for_each_mddev(mddev, tmp)
+			wake_up(&mddev->thread->wqueue);
+		rcu_read_unlock();
+		break;
+	}
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block md_pm_notifier = {
+	.notifier_call	= md_notify_pm,
+};
+
 static int md_notify_reboot(struct notifier_block *this,
 			    unsigned long code, void *x)
 {
@@ -9007,6 +9040,7 @@ static int __init md_init(void)
 			    md_probe, NULL, NULL);
 
 	register_reboot_notifier(&md_reboot_notifier);
+	register_pm_notifier(&md_pm_notifier);
 	raid_table_header = register_sysctl_table(raid_root_table);
 
 	md_geninit();
@@ -9246,6 +9280,7 @@ static __exit void md_exit(void)
 
 	unregister_blkdev(MD_MAJOR,"md");
 	unregister_blkdev(mdp_major, "mdp");
+	unregister_pm_notifier(&md_pm_notifier);
 	unregister_reboot_notifier(&md_reboot_notifier);
 	unregister_sysctl_table(raid_table_header);
 
-- 
2.14.1

Re: [PATCH v5 2/8] md: Neither resync nor reshape while the system is frozen

[Luis R. Rodriguez]

On Mon, Oct 02, 2017 at 03:52:12PM -0700, Bart Van Assche wrote:
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 3f7426120a3b..a2cf2a93b0cb 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8961,6 +8963,37 @@ static void md_stop_all_writes(void)
>  		mdelay(1000*1);
>  }
>  
> +/*
> + * Ensure that neither resyncing nor reshaping occurs while the system is
> + * frozen.
> + */
> +static int md_notify_pm(struct notifier_block *bl, unsigned long state,
> +			void *unused)
> +{
> +	struct mddev *mddev;
> +	struct list_head *tmp;
> +
> +	pr_debug("%s: state = %ld; system_freezing_cnt = %d\n", __func__, state,
> +		 atomic_read(&system_freezing_cnt));
> +
> +	switch (state) {
> +	case PM_HIBERNATION_PREPARE:

Hm, why not also include and use this for PM_SUSPEND_PREPARE and/or
a PM_RESTORE_PREPARE.

        case PM_HIBERNATION_PREPARE:
        case PM_SUSPEND_PREPARE:
        case PM_RESTORE_PREPARE:

> +		md_stop_all_writes();
> +		break;
> +	case PM_POST_HIBERNATION:

Likewise here:

        case PM_POST_SUSPEND:
        case PM_POST_HIBERNATION:
        case PM_POST_RESTORE:

I have revised the kernel suspend ordering and indeed things issued
with the pm notifier should suffice to be processed given we actually
call the pm ops (dpm_prepare()) for device drivers *after* the notifier
is called and then after userspace is frozen. That is:

   pm_suspend() --> enter_state() -->
     sys_sync()
     suspend_prepare() -->
             __pm_notifier_call_chain(PM_SUSPEND_PREPARE, ...);
             suspend_freeze_processes() -->
                     freeze_processes() -->
                             __usermodehelper_set_disable_depth(UMH_DISABLED);
                             freeze all tasks ...
                     freeze_kernel_threads()
     suspend_devices_and_enter() -->
             dpm_suspend_start() -->
                             dpm_prepare()
                             dpm_suspend()
             suspend_enter()  -->
                     platform_suspend_prepare()
                     dpm_suspend_late()
                     freeze_enter()
                     syscore_suspend()

On our way back up:

     enter_state() -->
     suspend_devices_and_enter() --> (bail from loop)
             dpm_resume_end() -->
                     dpm_resume()
                     dpm_complete()
     suspend_finish() -->
             suspend_thaw_processes() -->
                     thaw_processes() -->
                             __usermodehelper_set_disable_depth(UMH_FREEZING);
                             thaw_workqueues();
                             thaw all processes ...
                             usermodehelper_enable();
             pm_notifier_call_chain(PM_POST_SUSPEND);

So notifier would indeed be the right tooling to use here.

  Luis

Re: [PATCH v5 2/8] md: Neither resync nor reshape while the system is frozen

[Bart Van Assche]

T24gV2VkLCAyMDE3LTEwLTA0IGF0IDAzOjE2ICswMjAwLCBMdWlzIFIuIFJvZHJpZ3VleiB3cm90
ZToNCj4gT24gTW9uLCBPY3QgMDIsIDIwMTcgYXQgMDM6NTI6MTJQTSAtMDcwMCwgQmFydCBWYW4g
QXNzY2hlIHdyb3RlOg0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL21kL21kLmMgYi9kcml2ZXJz
L21kL21kLmMNCj4gPiBpbmRleCAzZjc0MjYxMjBhM2IuLmEyY2YyYTkzYjBjYiAxMDA2NDQNCj4g
PiAtLS0gYS9kcml2ZXJzL21kL21kLmMNCj4gPiArKysgYi9kcml2ZXJzL21kL21kLmMNCj4gPiBA
QCAtODk2MSw2ICs4OTYzLDM3IEBAIHN0YXRpYyB2b2lkIG1kX3N0b3BfYWxsX3dyaXRlcyh2b2lk
KQ0KPiA+ICAJCW1kZWxheSgxMDAwKjEpOw0KPiA+ICB9DQo+ID4gIA0KPiA+ICsvKg0KPiA+ICsg
KiBFbnN1cmUgdGhhdCBuZWl0aGVyIHJlc3luY2luZyBub3IgcmVzaGFwaW5nIG9jY3VycyB3aGls
ZSB0aGUgc3lzdGVtIGlzDQo+ID4gKyAqIGZyb3plbi4NCj4gPiArICovDQo+ID4gK3N0YXRpYyBp
bnQgbWRfbm90aWZ5X3BtKHN0cnVjdCBub3RpZmllcl9ibG9jayAqYmwsIHVuc2lnbmVkIGxvbmcg
c3RhdGUsDQo+ID4gKwkJCXZvaWQgKnVudXNlZCkNCj4gPiArew0KPiA+ICsJc3RydWN0IG1kZGV2
ICptZGRldjsNCj4gPiArCXN0cnVjdCBsaXN0X2hlYWQgKnRtcDsNCj4gPiArDQo+ID4gKwlwcl9k
ZWJ1ZygiJXM6IHN0YXRlID0gJWxkOyBzeXN0ZW1fZnJlZXppbmdfY250ID0gJWRcbiIsIF9fZnVu
Y19fLCBzdGF0ZSwNCj4gPiArCQkgYXRvbWljX3JlYWQoJnN5c3RlbV9mcmVlemluZ19jbnQpKTsN
Cj4gPiArDQo+ID4gKwlzd2l0Y2ggKHN0YXRlKSB7DQo+ID4gKwljYXNlIFBNX0hJQkVSTkFUSU9O
X1BSRVBBUkU6DQo+IA0KPiBIbSwgd2h5IG5vdCBhbHNvIGluY2x1ZGUgYW5kIHVzZSB0aGlzIGZv
ciBQTV9TVVNQRU5EX1BSRVBBUkUgYW5kL29yDQo+IGEgUE1fUkVTVE9SRV9QUkVQQVJFLg0KPiAN
Cj4gICAgICAgICBjYXNlIFBNX0hJQkVSTkFUSU9OX1BSRVBBUkU6DQo+ICAgICAgICAgY2FzZSBQ
TV9TVVNQRU5EX1BSRVBBUkU6DQo+ICAgICAgICAgY2FzZSBQTV9SRVNUT1JFX1BSRVBBUkU6DQoN
ClRoYW5rcyBmb3IgdGhlIGZlZWRiYWNrIEx1aXMuIEkgd2lsbCBtYWtlIHN1cmUgdGhhdCByZXN5
bmMgYWN0aXZpdHkgaXMgYWxzbw0Kc3RvcHBlZCBmb3IgdGhlIFBNX1NVU1BFTkRfUFJFUEFSRSBh
bmQgUE1fUkVTVE9SRV9QUkVQQVJFIGNhc2VzLg0KDQpCYXJ0Lg==

Re: [PATCH v5 2/8] md: Neither resync nor reshape while the system is frozen

[Ming Lei]

On Mon, Oct 02, 2017 at 03:52:12PM -0700, Bart Van Assche wrote:
> Some people use the md driver on laptops and use the suspend and
> resume functionality. Since it is essential that submitting of
> new I/O requests stops before a hibernation image is created,
> interrupt the md resync and reshape actions if the system is
> being frozen. Note: the resync and reshape will restart after
> the system is resumed and a message similar to the following
> will appear in the system log:
> 
> md: md0: data-check interrupted.
> 
> Additionally, only allow resync or reshape activity to be
> started after system freezing is over.

Last time, I asked which issue is fixed by this patch, but
never get answered, so again, what is issue fixed by
this patch?

Given the following patches for making SCSI quiesce safe
is required in either PM suspend/resume or other path
of transport_spi revalidate. So once theses patches are
in, I don't see any I/O hang in suspend/resume given
the SCSI quiesce issue is very specific one, and not
see this issue on other kind of device.

Thanks,
Ming

Re: [PATCH v5 2/8] md: Neither resync nor reshape while the system is frozen

[Bart Van Assche]

T24gV2VkLCAyMDE3LTEwLTA0IGF0IDE5OjMwICswODAwLCBNaW5nIExlaSB3cm90ZToNCj4gTGFz
dCB0aW1lLCBJIGFza2VkIHdoaWNoIGlzc3VlIGlzIGZpeGVkIGJ5IHRoaXMgcGF0Y2gsIGJ1dA0K
PiBuZXZlciBnZXQgYW5zd2VyZWQsIHNvIGFnYWluLCB3aGF0IGlzIGlzc3VlIGZpeGVkIGJ5DQo+
IHRoaXMgcGF0Y2g/DQoNClRoYXQncyBub3QgY29ycmVjdC4gSSBkaWQgYW5zd2VyIHlvdXIgcXVl
c3Rpb24uIFNlZSBlLmcuDQpodHRwczovL21hcmMuaW5mby8/bD1saW51eC1ibG9jayZtPTE1MDY5
NTg3OTMwOTYwNi4NCg0KPiBHaXZlbiB0aGUgZm9sbG93aW5nIHBhdGNoZXMgZm9yIG1ha2luZyBT
Q1NJIHF1aWVzY2Ugc2FmZQ0KPiBpcyByZXF1aXJlZCBpbiBlaXRoZXIgUE0gc3VzcGVuZC9yZXN1
bWUgb3Igb3RoZXIgcGF0aA0KPiBvZiB0cmFuc3BvcnRfc3BpIHJldmFsaWRhdGUuIFNvIG9uY2Ug
dGhlc2VzIHBhdGNoZXMgYXJlDQo+IGluLCBJIGRvbid0IHNlZSBhbnkgSS9PIGhhbmcgaW4gc3Vz
cGVuZC9yZXN1bWUgZ2l2ZW4NCj4gdGhlIFNDU0kgcXVpZXNjZSBpc3N1ZSBpcyB2ZXJ5IHNwZWNp
ZmljIG9uZSwgYW5kIG5vdA0KPiBzZWUgdGhpcyBpc3N1ZSBvbiBvdGhlciBraW5kIG9mIGRldmlj
ZS4NCg0KVGhlcmUgYXJlIG1hbnkgbW9yZSBzdG9yYWdlIGRyaXZlcnMgYW5kIGZpbGVzeXN0ZW1z
IHRoYW4gdGhlIG1kIGRyaXZlciB0aGF0DQpjYW4gc3VibWl0IEkvTyBhZnRlciB1c2VyIHNwYWNl
IHByb2Nlc3NlcyBoYXZlIGJlZW4gZnJvemVuLCBlLmcuIHRoZSBCVFJGUw0KUkFJRCBhbmQgc2Ny
dWIgZnVuY3Rpb25hbGl0aWVzLiBJdCB3aWxsIHRha2UgdGltZSB0byBmaXggYWxsIHRoZXNlIGRy
aXZlcnMNCmFuZCBhbHNvIHRvIGFkZCBmaWxlc3lzdGVtIGZyZWV6ZS90aGF3IGZ1bmN0aW9uYWxp
dHkgdG8gdGhlIHN1c3BlbmQgYW5kDQpyZXN1bWUgc2VxdWVuY2VzLiBBcyBsb25nIGFzIG5vdCBh
bGwgZHJpdmVycyBoYXZlIGJlZW4gZml4ZWQgd2Ugd2lsbCBuZWVkDQpTQ1NJIGRldmljZSBxdWll
c2NpbmcgYW5kIHJlc3VtaW5nIGFzIGEgd29ya2Fyb3VuZCB0byBhdm9pZCB0aGF0IGFueSBJL08N
Cm9jY3VycyB3aGlsZSBhIGhpYmVybmF0aW9uIGltYWdlIGlzIGJlaW5nIGNyZWF0ZWQuDQoNCkJh
cnQu

[PATCH v5 3/8] block: Make q_usage_counter also track legacy requests

[Bart Van Assche]

From: Ming Lei <ming.lei@redhat.com>

This patch makes it possible to pause request allocation for
the legacy block layer by calling blk_mq_freeze_queue() and
blk_mq_unfreeze_queue().

Signed-off-by: Ming Lei <ming.lei@redhat.com>
[ bvanassche: Combined two patches into one, edited a comment and made sure
  REQ_NOWAIT is handled properly in blk_old_get_request() ]
Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 block/blk-core.c | 12 ++++++++++++
 block/blk-mq.c   | 10 ++--------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 048be4aa6024..0343c7574378 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -610,6 +610,9 @@ void blk_set_queue_dying(struct request_queue *q)
 		}
 		spin_unlock_irq(q->queue_lock);
 	}
+
+	/* Make blk_queue_enter() reexamine the DYING flag. */
+	wake_up_all(&q->mq_freeze_wq);
 }
 EXPORT_SYMBOL_GPL(blk_set_queue_dying);
 
@@ -1395,16 +1398,22 @@ static struct request *blk_old_get_request(struct request_queue *q,
 					   unsigned int op, gfp_t gfp_mask)
 {
 	struct request *rq;
+	int ret = 0;
 
 	WARN_ON_ONCE(q->mq_ops);
 
 	/* create ioc upfront */
 	create_io_context(gfp_mask, q->node);
 
+	ret = blk_queue_enter(q, !(gfp_mask & __GFP_DIRECT_RECLAIM) ||
+			      (op & REQ_NOWAIT));
+	if (ret)
+		return ERR_PTR(ret);
 	spin_lock_irq(q->queue_lock);
 	rq = get_request(q, op, NULL, gfp_mask);
 	if (IS_ERR(rq)) {
 		spin_unlock_irq(q->queue_lock);
+		blk_queue_exit(q);
 		return rq;
 	}
 
@@ -1576,6 +1585,7 @@ void __blk_put_request(struct request_queue *q, struct request *req)
 		blk_free_request(rl, req);
 		freed_request(rl, sync, rq_flags);
 		blk_put_rl(rl);
+		blk_queue_exit(q);
 	}
 }
 EXPORT_SYMBOL_GPL(__blk_put_request);
@@ -1857,8 +1867,10 @@ static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio)
 	 * Grab a free request. This is might sleep but can not fail.
 	 * Returns with the queue unlocked.
 	 */
+	blk_queue_enter_live(q);
 	req = get_request(q, bio->bi_opf, bio, GFP_NOIO);
 	if (IS_ERR(req)) {
+		blk_queue_exit(q);
 		__wbt_done(q->rq_wb, wb_acct);
 		if (PTR_ERR(req) == -ENOMEM)
 			bio->bi_status = BLK_STS_RESOURCE;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 7f01d69879d6..ebf85e270509 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -125,7 +125,8 @@ void blk_freeze_queue_start(struct request_queue *q)
 	freeze_depth = atomic_inc_return(&q->mq_freeze_depth);
 	if (freeze_depth == 1) {
 		percpu_ref_kill(&q->q_usage_counter);
-		blk_mq_run_hw_queues(q, false);
+		if (q->mq_ops)
+			blk_mq_run_hw_queues(q, false);
 	}
 }
 EXPORT_SYMBOL_GPL(blk_freeze_queue_start);
@@ -255,13 +256,6 @@ void blk_mq_wake_waiters(struct request_queue *q)
 	queue_for_each_hw_ctx(q, hctx, i)
 		if (blk_mq_hw_queue_mapped(hctx))
 			blk_mq_tag_wakeup_all(hctx->tags, true);
-
-	/*
-	 * If we are called because the queue has now been marked as
-	 * dying, we need to ensure that processes currently waiting on
-	 * the queue are notified as well.
-	 */
-	wake_up_all(&q->mq_freeze_wq);
 }
 
 bool blk_mq_can_queue(struct blk_mq_hw_ctx *hctx)
-- 
2.14.1

[PATCH v5 4/8] block: Convert RQF_PREEMPT into REQ_PREEMPT

[Bart Van Assche]

This patch does not change any functionality but makes the
REQ_PREEMPT flag available to blk_get_request(). A later patch
will add code to blk_get_request() that checks the REQ_PREEMPT
flag. Note: the IDE sense_rq request is allocated statically so
there is no blk_get_request() call that corresponds to this
request.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 block/blk-mq-debugfs.c    |  2 +-
 drivers/ide/ide-atapi.c   |  3 +--
 drivers/ide/ide-io.c      |  2 +-
 drivers/ide/ide-pm.c      |  4 ++--
 drivers/scsi/scsi_lib.c   | 11 ++++++-----
 include/linux/blk_types.h |  6 ++++++
 include/linux/blkdev.h    |  3 ---
 7 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 980e73095643..62ac248a4984 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -266,6 +266,7 @@ static const char *const cmd_flag_name[] = {
 	CMD_FLAG_NAME(BACKGROUND),
 	CMD_FLAG_NAME(NOUNMAP),
 	CMD_FLAG_NAME(NOWAIT),
+	CMD_FLAG_NAME(PREEMPT),
 };
 #undef CMD_FLAG_NAME
 
@@ -279,7 +280,6 @@ static const char *const rqf_name[] = {
 	RQF_NAME(MIXED_MERGE),
 	RQF_NAME(MQ_INFLIGHT),
 	RQF_NAME(DONTPREP),
-	RQF_NAME(PREEMPT),
 	RQF_NAME(COPY_USER),
 	RQF_NAME(FAILED),
 	RQF_NAME(QUIET),
diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index 14d1e7d9a1d6..1258739d5fa1 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -211,9 +211,8 @@ void ide_prep_sense(ide_drive_t *drive, struct request *rq)
 	}
 
 	sense_rq->rq_disk = rq->rq_disk;
-	sense_rq->cmd_flags = REQ_OP_DRV_IN;
+	sense_rq->cmd_flags = REQ_OP_DRV_IN | REQ_PREEMPT;
 	ide_req(sense_rq)->type = ATA_PRIV_SENSE;
-	sense_rq->rq_flags |= RQF_PREEMPT;
 
 	req->cmd[0] = GPCMD_REQUEST_SENSE;
 	req->cmd[4] = cmd_len;
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 3a234701d92c..06ffccd0fb10 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -539,7 +539,7 @@ void do_ide_request(struct request_queue *q)
 		 */
 		if ((drive->dev_flags & IDE_DFLAG_BLOCKED) &&
 		    ata_pm_request(rq) == 0 &&
-		    (rq->rq_flags & RQF_PREEMPT) == 0) {
+		    (rq->cmd_flags & REQ_PREEMPT) == 0) {
 			/* there should be no pending command at this point */
 			ide_unlock_port(hwif);
 			goto plug_device;
diff --git a/drivers/ide/ide-pm.c b/drivers/ide/ide-pm.c
index 544f02d673ca..f8d2709dcd56 100644
--- a/drivers/ide/ide-pm.c
+++ b/drivers/ide/ide-pm.c
@@ -89,9 +89,9 @@ int generic_ide_resume(struct device *dev)
 	}
 
 	memset(&rqpm, 0, sizeof(rqpm));
-	rq = blk_get_request(drive->queue, REQ_OP_DRV_IN, __GFP_RECLAIM);
+	rq = blk_get_request(drive->queue, REQ_OP_DRV_IN | REQ_PREEMPT,
+			     __GFP_RECLAIM);
 	ide_req(rq)->type = ATA_PRIV_PM_RESUME;
-	rq->rq_flags |= RQF_PREEMPT;
 	rq->special = &rqpm;
 	rqpm.pm_step = IDE_PM_START_RESUME;
 	rqpm.pm_state = PM_EVENT_ON;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 9cf6a80fe297..62f905b22821 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -253,8 +253,9 @@ int scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 	int ret = DRIVER_ERROR << 24;
 
 	req = blk_get_request(sdev->request_queue,
-			data_direction == DMA_TO_DEVICE ?
-			REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, __GFP_RECLAIM);
+			      (data_direction == DMA_TO_DEVICE ?
+			       REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN) | REQ_PREEMPT,
+			      __GFP_RECLAIM);
 	if (IS_ERR(req))
 		return ret;
 	rq = scsi_req(req);
@@ -268,7 +269,7 @@ int scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 	rq->retries = retries;
 	req->timeout = timeout;
 	req->cmd_flags |= flags;
-	req->rq_flags |= rq_flags | RQF_QUIET | RQF_PREEMPT;
+	req->rq_flags |= rq_flags | RQF_QUIET;
 
 	/*
 	 * head injection *required* here otherwise quiesce won't work
@@ -1301,7 +1302,7 @@ scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
 			/*
 			 * If the devices is blocked we defer normal commands.
 			 */
-			if (!(req->rq_flags & RQF_PREEMPT))
+			if (!(req->cmd_flags & REQ_PREEMPT))
 				ret = BLKPREP_DEFER;
 			break;
 		default:
@@ -1310,7 +1311,7 @@ scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
 			 * special commands.  In particular any user initiated
 			 * command is not allowed.
 			 */
-			if (!(req->rq_flags & RQF_PREEMPT))
+			if (!(req->cmd_flags & REQ_PREEMPT))
 				ret = BLKPREP_KILL;
 			break;
 		}
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index a2d2aa709cef..b6b6bf44e462 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -229,6 +229,11 @@ enum req_flag_bits {
 	__REQ_NOUNMAP,		/* do not free blocks when zeroing */
 
 	__REQ_NOWAIT,           /* Don't wait if request will block */
+	/*
+	 * set for "ide_preempt" requests and also for requests for which the
+	 * SCSI "quiesce" state must be ignored.
+	 */
+	__REQ_PREEMPT,
 	__REQ_NR_BITS,		/* stops here */
 };
 
@@ -248,6 +253,7 @@ enum req_flag_bits {
 
 #define REQ_NOUNMAP		(1ULL << __REQ_NOUNMAP)
 #define REQ_NOWAIT		(1ULL << __REQ_NOWAIT)
+#define REQ_PREEMPT		(1ULL << __REQ_PREEMPT)
 
 #define REQ_FAILFAST_MASK \
 	(REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 02fa42d24b52..ab6b6c0392c2 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -96,9 +96,6 @@ typedef __u32 __bitwise req_flags_t;
 #define RQF_MQ_INFLIGHT		((__force req_flags_t)(1 << 6))
 /* don't call prep for this one */
 #define RQF_DONTPREP		((__force req_flags_t)(1 << 7))
-/* set for "ide_preempt" requests and also for requests for which the SCSI
-   "quiesce" state must be ignored. */
-#define RQF_PREEMPT		((__force req_flags_t)(1 << 8))
 /* contains copies of user pages */
 #define RQF_COPY_USER		((__force req_flags_t)(1 << 9))
 /* vaguely specified driver internal error.  Ignored by the block layer */
-- 
2.14.1

Re: [PATCH v5 4/8] block: Convert RQF_PREEMPT into REQ_PREEMPT

[Christoph Hellwig]

So as pointed out in the last run (after changing my mind deeper into
the series) I think we should instead use a BLK_MQ_REQ_PREEMPT flag.

The preempt only makes sense at the request level, not for file system
requests.  For the legacy case we can add blk_get_request_flags that
takes the BLK_MQ_REQ_* flags, which is a much saner interface than the
current blk_get_request anyway.

[PATCH v5 5/8] block: Add the QUEUE_FLAG_PREEMPT_ONLY request queue flag

[Bart Van Assche]

This flag will be used in the next patch to let the block layer
core know whether or not a SCSI request queue has been quiesced.
A quiesced SCSI queue namely only processes RQF_PREEMPT requests.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 block/blk-core.c       | 20 ++++++++++++++++++++
 include/linux/blkdev.h |  6 ++++++
 2 files changed, 26 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index 0343c7574378..c4c7fd4dcc2a 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -346,6 +346,26 @@ void blk_sync_queue(struct request_queue *q)
 }
 EXPORT_SYMBOL(blk_sync_queue);
 
+void blk_set_preempt_only(struct request_queue *q)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(q->queue_lock, flags);
+	queue_flag_set(QUEUE_FLAG_PREEMPT_ONLY, q);
+	spin_unlock_irqrestore(q->queue_lock, flags);
+}
+EXPORT_SYMBOL(blk_set_preempt_only);
+
+void blk_clear_preempt_only(struct request_queue *q)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(q->queue_lock, flags);
+	queue_flag_clear(QUEUE_FLAG_PREEMPT_ONLY, q);
+	spin_unlock_irqrestore(q->queue_lock, flags);
+}
+EXPORT_SYMBOL(blk_clear_preempt_only);
+
 /**
  * __blk_run_queue_uncond - run a queue whether or not it has been stopped
  * @q:	The queue to run
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ab6b6c0392c2..50bf37dacf4c 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -628,6 +628,7 @@ struct request_queue {
 #define QUEUE_FLAG_REGISTERED  26	/* queue has been registered to a disk */
 #define QUEUE_FLAG_SCSI_PASSTHROUGH 27	/* queue supports SCSI commands */
 #define QUEUE_FLAG_QUIESCED    28	/* queue has been quiesced */
+#define QUEUE_FLAG_PREEMPT_ONLY	29	/* only process REQ_PREEMPT requests */
 
 #define QUEUE_FLAG_DEFAULT	((1 << QUEUE_FLAG_IO_STAT) |		\
 				 (1 << QUEUE_FLAG_STACKABLE)	|	\
@@ -732,6 +733,11 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
 	((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \
 			     REQ_FAILFAST_DRIVER))
 #define blk_queue_quiesced(q)	test_bit(QUEUE_FLAG_QUIESCED, &(q)->queue_flags)
+#define blk_queue_preempt_only(q)				\
+	test_bit(QUEUE_FLAG_PREEMPT_ONLY, &(q)->queue_flags)
+
+extern void blk_set_preempt_only(struct request_queue *q);
+extern void blk_clear_preempt_only(struct request_queue *q);
 
 static inline bool blk_account_rq(struct request *rq)
 {
-- 
2.14.1

Re: [PATCH v5 5/8] block: Add the QUEUE_FLAG_PREEMPT_ONLY request queue flag

[Christoph Hellwig]

> +EXPORT_SYMBOL(blk_set_preempt_only);

EXPORT_SYMBOL_GPL please.

Except for that this looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH v5 6/8] scsi: Reduce suspend latency

[Bart Van Assche]

Avoid that it can take 200 ms too long to wait for requests to
finish. Note: blk_mq_freeze_queue() uses a wait queue to wait
for ongoing requests to finish.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/scsi/scsi_lib.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 62f905b22821..e0bef5c9dd14 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2927,21 +2927,25 @@ static void scsi_wait_for_queuecommand(struct scsi_device *sdev)
 int
 scsi_device_quiesce(struct scsi_device *sdev)
 {
+	struct request_queue *q = sdev->request_queue;
+	bool freeze;
 	int err;
 
 	mutex_lock(&sdev->state_mutex);
+	/*
+	 * Do not attempt to freeze the queue of an already quiesced device
+	 * because that could result in a deadlock.
+	 */
+	freeze = sdev->sdev_state == SDEV_RUNNING;
+	if (freeze)
+		blk_mq_freeze_queue(q);
 	err = scsi_device_set_state(sdev, SDEV_QUIESCE);
 	mutex_unlock(&sdev->state_mutex);
 
-	if (err)
-		return err;
+	if (freeze)
+		blk_mq_unfreeze_queue(q);
 
-	scsi_run_queue(sdev->request_queue);
-	while (atomic_read(&sdev->device_busy)) {
-		msleep_interruptible(200);
-		scsi_run_queue(sdev->request_queue);
-	}
-	return 0;
+	return err;
 }
 EXPORT_SYMBOL(scsi_device_quiesce);
 
-- 
2.14.1

Re: [PATCH v5 6/8] scsi: Reduce suspend latency

[Christoph Hellwig]

> +	/*
> +	 * Do not attempt to freeze the queue of an already quiesced device
> +	 * because that could result in a deadlock.
> +	 */
> +	freeze = sdev->sdev_state == SDEV_RUNNING;
> +	if (freeze)
> +		blk_mq_freeze_queue(q);
>  	err = scsi_device_set_state(sdev, SDEV_QUIESCE);

I don't really like this magic with a freeze inside the lock
and the magic dependency on the previous.  But I can't really come up
with a better idea either.

Re: [PATCH v5 6/8] scsi: Reduce suspend latency

[Bart Van Assche]

T24gV2VkLCAyMDE3LTEwLTA0IGF0IDA5OjA0ICswMjAwLCBDaHJpc3RvcGggSGVsbHdpZyB3cm90
ZToNCj4gPiArCS8qDQo+ID4gKwkgKiBEbyBub3QgYXR0ZW1wdCB0byBmcmVlemUgdGhlIHF1ZXVl
IG9mIGFuIGFscmVhZHkgcXVpZXNjZWQgZGV2aWNlDQo+ID4gKwkgKiBiZWNhdXNlIHRoYXQgY291
bGQgcmVzdWx0IGluIGEgZGVhZGxvY2suDQo+ID4gKwkgKi8NCj4gPiArCWZyZWV6ZSA9IHNkZXYt
PnNkZXZfc3RhdGUgPT0gU0RFVl9SVU5OSU5HOw0KPiA+ICsJaWYgKGZyZWV6ZSkNCj4gPiArCQli
bGtfbXFfZnJlZXplX3F1ZXVlKHEpOw0KPiA+ICAJZXJyID0gc2NzaV9kZXZpY2Vfc2V0X3N0YXRl
KHNkZXYsIFNERVZfUVVJRVNDRSk7DQo+IA0KPiBJIGRvbid0IHJlYWxseSBsaWtlIHRoaXMgbWFn
aWMgd2l0aCBhIGZyZWV6ZSBpbnNpZGUgdGhlIGxvY2sNCj4gYW5kIHRoZSBtYWdpYyBkZXBlbmRl
bmN5IG9uIHRoZSBwcmV2aW91cy4gIEJ1dCBJIGNhbid0IHJlYWxseSBjb21lIHVwDQo+IHdpdGgg
YSBiZXR0ZXIgaWRlYSBlaXRoZXIuDQoNClRoZSBkZWFkbG9jayBJIHJlZmVycmVkIHRvIGluIG15
IGNvbW1lbnQgY2FuIG9ubHkgb2NjdXIgaWYgYW55IGNvZGUgd291bGQgY2hhbmdlDQp0aGUgU0NT
SSBkZXZpY2Ugc3RhdGUgaW50byBTREVWX1FVSUVTQ0Ugd2l0aG91dCBmcmVlemluZyB0aGUgcmVx
dWVzdCBxdWV1ZSBmaXJzdC4NCkkgd2lsbCBsb29rIGludG8gZnJlZXppbmcgdGhlIHF1ZXVlIHdp
dGhvdXQgaG9sZGluZyB0aGUgc2RldiBzdGF0ZSBtdXRleC4NCg0KQmFydC4=

[PATCH v5 7/8] scsi: Set QUEUE_FLAG_PREEMPT_ONLY while quiesced

[Bart Van Assche]

Make the quiesce state visible to the block layer for the next
patch in this series.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/scsi/scsi_lib.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index e0bef5c9dd14..534c81de0880 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2940,8 +2940,16 @@ scsi_device_quiesce(struct scsi_device *sdev)
 	if (freeze)
 		blk_mq_freeze_queue(q);
 	err = scsi_device_set_state(sdev, SDEV_QUIESCE);
+	if (err == 0)
+		blk_set_preempt_only(q);
 	mutex_unlock(&sdev->state_mutex);
 
+	/*
+	 * Ensure that the effect of blk_set_preempt_only() is globally
+	 * visible before unfreezing the queue.
+	 */
+	if (err == 0)
+		synchronize_rcu();
 	if (freeze)
 		blk_mq_unfreeze_queue(q);
 
@@ -2966,8 +2974,10 @@ void scsi_device_resume(struct scsi_device *sdev)
 	 */
 	mutex_lock(&sdev->state_mutex);
 	if (sdev->sdev_state == SDEV_QUIESCE &&
-	    scsi_device_set_state(sdev, SDEV_RUNNING) == 0)
+	    scsi_device_set_state(sdev, SDEV_RUNNING) == 0) {
+		blk_clear_preempt_only(sdev->request_queue);
 		scsi_run_queue(sdev->request_queue);
+	}
 	mutex_unlock(&sdev->state_mutex);
 }
 EXPORT_SYMBOL(scsi_device_resume);
-- 
2.14.1

Re: [PATCH v5 7/8] scsi: Set QUEUE_FLAG_PREEMPT_ONLY while quiesced

[Christoph Hellwig]

> +	/*
> +	 * Ensure that the effect of blk_set_preempt_only() is globally
> +	 * visible before unfreezing the queue.
> +	 */
> +	if (err == 0)
> +		synchronize_rcu();

I don't understand why we'd need this.  The flag is set both under
a spinlock and a mutex that are unlocked right after.  That should
gurantee all the visibility we'll need.

Re: [PATCH v5 7/8] scsi: Set QUEUE_FLAG_PREEMPT_ONLY while quiesced

[Bart Van Assche]

T24gV2VkLCAyMDE3LTEwLTA0IGF0IDA5OjA2ICswMjAwLCBDaHJpc3RvcGggSGVsbHdpZyB3cm90
ZToNCj4gPiArCS8qDQo+ID4gKwkgKiBFbnN1cmUgdGhhdCB0aGUgZWZmZWN0IG9mIGJsa19zZXRf
cHJlZW1wdF9vbmx5KCkgaXMgZ2xvYmFsbHkNCj4gPiArCSAqIHZpc2libGUgYmVmb3JlIHVuZnJl
ZXppbmcgdGhlIHF1ZXVlLg0KPiA+ICsJICovDQo+ID4gKwlpZiAoZXJyID09IDApDQo+ID4gKwkJ
c3luY2hyb25pemVfcmN1KCk7DQo+IA0KPiBJIGRvbid0IHVuZGVyc3RhbmQgd2h5IHdlJ2QgbmVl
ZCB0aGlzLiAgVGhlIGZsYWcgaXMgc2V0IGJvdGggdW5kZXINCj4gYSBzcGlubG9jayBhbmQgYSBt
dXRleCB0aGF0IGFyZSB1bmxvY2tlZCByaWdodCBhZnRlci4gIFRoYXQgc2hvdWxkDQo+IGd1cmFu
dGVlIGFsbCB0aGUgdmlzaWJpbGl0eSB3ZSdsbCBuZWVkLg0KDQpIZWxsbyBDaHJpc3RvcGgsDQoN
ClRoYXQgc3luY2hyb25pemVfcmN1KCkgY2FsbCBmb2xsb3dzIHRoZSBleGFtcGxlIGluIHRoZSB0
aGlyZCBmaWd1cmUgaW4NCmh0dHBzOi8vbHduLm5ldC9BcnRpY2xlcy81NzM0OTcvLiBJIHdpbGwg
dmVyaWZ5IHdoZXRoZXIgdGhhdCBzeW5jaHJvbml6ZV9yY3UoKQ0KY2FsbCBjYW4gYmUgbGVmdCBv
dXQuDQoNCkJhcnQu

[PATCH v5 8/8] block: Make SCSI device suspend and resume work reliably

[Bart Van Assche]

It is essential during suspend and resume that neither the filesystem
state nor the filesystem metadata in RAM changes. This is why while
the hibernation image is being written or restored that SCSI devices
are quiesced. The SCSI core quiesces devices through scsi_device_quiesce()
and scsi_device_resume(). In the SDEV_QUIESCE state execution of
non-preempt requests is deferred. This is realized by returning
BLKPREP_DEFER from inside scsi_prep_state_check() for quiesced SCSI
devices. Avoid that a full queue prevents power management requests
to be submitted by deferring allocation of non-preempt requests for
devices in the quiesced state. This patch has been tested by running
the following commands and by verifying that after resume the fio job
is still running:

for d in /sys/class/block/sd*[a-z]; do
  hcil=$(readlink "$d/device")
  hcil=${hcil#../../../}
  echo 4 > "$d/queue/nr_requests"
  echo 1 > "/sys/class/scsi_device/$hcil/device/queue_depth"
done
bdev=$(readlink /dev/disk/by-uuid/5217d83f-213e-4b42-b86e-20013325ba6c)
bdev=${bdev#../../}
hcil=$(readlink "/sys/block/$bdev/device")
hcil=${hcil#../../../}
fio --name="$bdev" --filename="/dev/$bdev" --buffered=0 --bs=512 --rw=randread \
  --ioengine=libaio --numjobs=4 --iodepth=16 --iodepth_batch=1 --thread \
  --loops=$((2**31)) &
pid=$!
sleep 1
systemctl hibernate
sleep 10
kill $pid

Reported-by: Oleksandr Natalenko <oleksandr@natalenko.name>
References: "I/O hangs after resuming from suspend-to-ram" (https://marc.info/?l=linux-block&m=150340235201348).
Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 block/blk-core.c       | 37 +++++++++++++++++++++++++++++--------
 block/blk-mq.c         |  4 ++--
 block/blk-timeout.c    |  2 +-
 fs/block_dev.c         |  4 ++--
 include/linux/blkdev.h |  2 +-
 5 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index c4c7fd4dcc2a..e185ca1b35a1 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -362,6 +362,7 @@ void blk_clear_preempt_only(struct request_queue *q)
 
 	spin_lock_irqsave(q->queue_lock, flags);
 	queue_flag_clear(QUEUE_FLAG_PREEMPT_ONLY, q);
+	wake_up_all(&q->mq_freeze_wq);
 	spin_unlock_irqrestore(q->queue_lock, flags);
 }
 EXPORT_SYMBOL(blk_clear_preempt_only);
@@ -783,13 +784,32 @@ struct request_queue *blk_alloc_queue(gfp_t gfp_mask)
 }
 EXPORT_SYMBOL(blk_alloc_queue);
 
-int blk_queue_enter(struct request_queue *q, bool nowait)
+/**
+ * blk_queue_enter() - try to increase q->q_usage_counter
+ * @q: request queue pointer
+ * @nowait: if the queue is frozen, do not wait until it is unfrozen
+ * @preempt: if QUEUE_FLAG_PREEMPT_ONLY has been set, do not wait until that
+ *	flag has been cleared
+ */
+int blk_queue_enter(struct request_queue *q, bool nowait, bool preempt)
 {
 	while (true) {
 		int ret;
 
-		if (percpu_ref_tryget_live(&q->q_usage_counter))
-			return 0;
+		if (percpu_ref_tryget_live(&q->q_usage_counter)) {
+			/*
+			 * The code that sets the PREEMPT_ONLY flag is
+			 * responsible for ensuring that that flag is globally
+			 * visible before the queue is unfrozen.
+			 */
+			if (preempt || !blk_queue_preempt_only(q)) {
+				return 0;
+			} else {
+				percpu_ref_put(&q->q_usage_counter);
+				WARN_ONCE("%s: Attempt to allocate non-preempt request in preempt-only mode.\n",
+					  kobject_name(q->kobj.parent));
+			}
+		}
 
 		if (nowait)
 			return -EBUSY;
@@ -804,7 +824,8 @@ int blk_queue_enter(struct request_queue *q, bool nowait)
 		smp_rmb();
 
 		ret = wait_event_interruptible(q->mq_freeze_wq,
-				!atomic_read(&q->mq_freeze_depth) ||
+				(atomic_read(&q->mq_freeze_depth) == 0 &&
+				 (preempt || !blk_queue_preempt_only(q))) ||
 				blk_queue_dying(q));
 		if (blk_queue_dying(q))
 			return -ENODEV;
@@ -1426,7 +1447,7 @@ static struct request *blk_old_get_request(struct request_queue *q,
 	create_io_context(gfp_mask, q->node);
 
 	ret = blk_queue_enter(q, !(gfp_mask & __GFP_DIRECT_RECLAIM) ||
-			      (op & REQ_NOWAIT));
+			      (op & REQ_NOWAIT), op & REQ_PREEMPT);
 	if (ret)
 		return ERR_PTR(ret);
 	spin_lock_irq(q->queue_lock);
@@ -2194,6 +2215,7 @@ blk_qc_t generic_make_request(struct bio *bio)
 	 */
 	struct bio_list bio_list_on_stack[2];
 	blk_qc_t ret = BLK_QC_T_NONE;
+	const bool nowait = bio->bi_opf & REQ_NOWAIT;
 
 	if (!generic_make_request_checks(bio))
 		goto out;
@@ -2233,7 +2255,7 @@ blk_qc_t generic_make_request(struct bio *bio)
 	do {
 		struct request_queue *q = bio->bi_disk->queue;
 
-		if (likely(blk_queue_enter(q, bio->bi_opf & REQ_NOWAIT) == 0)) {
+		if (likely(blk_queue_enter(q, nowait, false) == 0)) {
 			struct bio_list lower, same;
 
 			/* Create a fresh bio_list for all subordinate requests */
@@ -2258,8 +2280,7 @@ blk_qc_t generic_make_request(struct bio *bio)
 			bio_list_merge(&bio_list_on_stack[0], &same);
 			bio_list_merge(&bio_list_on_stack[0], &bio_list_on_stack[1]);
 		} else {
-			if (unlikely(!blk_queue_dying(q) &&
-					(bio->bi_opf & REQ_NOWAIT)))
+			if (unlikely(!blk_queue_dying(q) && nowait))
 				bio_wouldblock_error(bio);
 			else
 				bio_io_error(bio);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index ebf85e270509..20a5f61cb248 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -384,7 +384,7 @@ struct request *blk_mq_alloc_request(struct request_queue *q, unsigned int op,
 	struct request *rq;
 	int ret;
 
-	ret = blk_queue_enter(q, flags & BLK_MQ_REQ_NOWAIT);
+	ret = blk_queue_enter(q, flags & BLK_MQ_REQ_NOWAIT, op & REQ_PREEMPT);
 	if (ret)
 		return ERR_PTR(ret);
 
@@ -423,7 +423,7 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q,
 	if (hctx_idx >= q->nr_hw_queues)
 		return ERR_PTR(-EIO);
 
-	ret = blk_queue_enter(q, true);
+	ret = blk_queue_enter(q, true, op & REQ_PREEMPT);
 	if (ret)
 		return ERR_PTR(ret);
 
diff --git a/block/blk-timeout.c b/block/blk-timeout.c
index 17ec83bb0900..0dfdc975473a 100644
--- a/block/blk-timeout.c
+++ b/block/blk-timeout.c
@@ -134,7 +134,7 @@ void blk_timeout_work(struct work_struct *work)
 	struct request *rq, *tmp;
 	int next_set = 0;
 
-	if (blk_queue_enter(q, true))
+	if (blk_queue_enter(q, true, true))
 		return;
 	spin_lock_irqsave(q->queue_lock, flags);
 
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 93d088ffc05c..e9ca45087a40 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -674,7 +674,7 @@ int bdev_read_page(struct block_device *bdev, sector_t sector,
 	if (!ops->rw_page || bdev_get_integrity(bdev))
 		return result;
 
-	result = blk_queue_enter(bdev->bd_queue, false);
+	result = blk_queue_enter(bdev->bd_queue, false, false);
 	if (result)
 		return result;
 	result = ops->rw_page(bdev, sector + get_start_sect(bdev), page, false);
@@ -710,7 +710,7 @@ int bdev_write_page(struct block_device *bdev, sector_t sector,
 
 	if (!ops->rw_page || bdev_get_integrity(bdev))
 		return -EOPNOTSUPP;
-	result = blk_queue_enter(bdev->bd_queue, false);
+	result = blk_queue_enter(bdev->bd_queue, false, false);
 	if (result)
 		return result;
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 50bf37dacf4c..645e9f1df8cc 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -966,7 +966,7 @@ extern int scsi_cmd_ioctl(struct request_queue *, struct gendisk *, fmode_t,
 extern int sg_scsi_ioctl(struct request_queue *, struct gendisk *, fmode_t,
 			 struct scsi_ioctl_command __user *);
 
-extern int blk_queue_enter(struct request_queue *q, bool nowait);
+extern int blk_queue_enter(struct request_queue *q, bool nowait, bool preempt);
 extern void blk_queue_exit(struct request_queue *q);
 extern void blk_start_queue(struct request_queue *q);
 extern void blk_start_queue_async(struct request_queue *q);
-- 
2.14.1

Re: [PATCH v5 8/8] block: Make SCSI device suspend and resume work reliably

[Christoph Hellwig]

As mentioned before - if you pass BLK_MQ_REQ_* to blk_queue_enter
this should become a bit nicer without the need for two magic bool
parameters.

Re: [PATCH v5 8/8] block: Make SCSI device suspend and resume work reliably

[Bart Van Assche]

T24gV2VkLCAyMDE3LTEwLTA0IGF0IDA5OjA2ICswMjAwLCBDaHJpc3RvcGggSGVsbHdpZyB3cm90
ZToNCj4gQXMgbWVudGlvbmVkIGJlZm9yZSAtIGlmIHlvdSBwYXNzIEJMS19NUV9SRVFfKiB0byBi
bGtfcXVldWVfZW50ZXINCj4gdGhpcyBzaG91bGQgYmVjb21lIGEgYml0IG5pY2VyIHdpdGhvdXQg
dGhlIG5lZWQgZm9yIHR3byBtYWdpYyBib29sDQo+IHBhcmFtZXRlcnMuDQoNCkhlbGxvIENocmlz
dG9waCwNCg0KSSB3aWxsIG1ha2UgdGhhdCBjaGFuZ2UuIFNvcnJ5IHRoYXQgSSBtaXNzZWQgaXQg
aW4gdjUgb2YgdGhpcyBwYXRjaCBzZXJpZXMuDQoNCkJhcnQu

Re: [PATCH v5 0/8] block, scsi, md: Improve suspend and resume

[Oleksandr Natalenko]

Hello.

I can confirm that this patchset applied on top of v4.14-rc3 also (like Min=
g's=20
one) fixes the issue with suspend/resume+RAID10, reported by me previously.

Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name>

On =C3=BAter=C3=BD 3. =C5=99=C3=ADjna 2017 0:52:10 CEST Bart Van Assche wro=
te:
> Hello Jens,
>=20
> It is known that during the resume following a hibernate, especially when
> using an md RAID1 array created on top of SCSI devices, sometimes the
> system hangs instead of coming up properly. This patch series fixes this
> problem. This patch series is an alternative for Ming Lei's "block/scsi:
> safe SCSI quiescing" patch series. The advantages of this patch series are
> that a fix for the md driver is included, that no third argument has been
> added to blk_get_request() and that there is no race between
> blk_get_request() and SCSI quiescing.
>=20
> These patches have been tested on top of a merge of the block layer
> for-next branch and Linus' master tree. Linus' master tree includes
> patch "KVM: x86: Fix the NULL pointer parameter in check_cr_write()"
> but the block layer for-next branch not yet.
>=20
> Please consider these changes for kernel v4.15.
>=20
> Thanks,
>=20
> Bart.
>=20
> Changes between v4 and v5:
> - Split blk_set_preempt_only() into two functions as requested by Christo=
ph.
> - Made blk_get_request() trigger WARN_ONCE() if it is attempted to alloca=
te
> a request while the system is frozen. This is a big help to identify
> drivers that submit I/O while the system is frozen.
> - Since kernel thread freezing is on its way out, reworked the approach f=
or
>   avoiding that the md driver submits I/O while the system is frozen such
> that the approach no longer depends on the kernel thread freeze mechanism.
> - Fixed the (theoretical) deadlock in scsi_device_quiesce() that was
> identified by Ming.
>=20
> Changes between v3 and v4:
> - Made sure that this patch series not only works for scsi-mq but also for
>   the legacy SCSI stack.
> - Removed an smp_rmb()/smp_wmb() pair from the hot path and added a comme=
nt
>   that explains why that is safe.
> - Reordered the patches in this patch series.
>=20
> Changes between v2 and v3:
> - Made md kernel threads freezable.
> - Changed the approach for quiescing SCSI devices again.
> - Addressed Ming's review comments.
>=20
> Changes compared to v1 of this patch series:
> - Changed the approach and rewrote the patch series.
>=20
> Bart Van Assche (7):
>   md: Introduce md_stop_all_writes()
>   md: Neither resync nor reshape while the system is frozen
>   block: Convert RQF_PREEMPT into REQ_PREEMPT
>   block: Add the QUEUE_FLAG_PREEMPT_ONLY request queue flag
>   scsi: Reduce suspend latency
>   scsi: Set QUEUE_FLAG_PREEMPT_ONLY while quiesced
>   block: Make SCSI device suspend and resume work reliably
>=20
> Ming Lei (1):
>   block: Make q_usage_counter also track legacy requests
>=20
>  block/blk-core.c          | 67
> ++++++++++++++++++++++++++++++++++++++++++----- block/blk-mq-debugfs.c   =
 |
>  2 +-
>  block/blk-mq.c            | 14 +++-------
>  block/blk-timeout.c       |  2 +-
>  drivers/ide/ide-atapi.c   |  3 +--
>  drivers/ide/ide-io.c      |  2 +-
>  drivers/ide/ide-pm.c      |  4 +--
>  drivers/md/md.c           | 52 +++++++++++++++++++++++++++++++-----
>  drivers/scsi/scsi_lib.c   | 43 ++++++++++++++++++++----------
>  fs/block_dev.c            |  4 +--
>  include/linux/blk_types.h |  6 +++++
>  include/linux/blkdev.h    | 11 +++++---
>  12 files changed, 160 insertions(+), 50 deletions(-)

Re: [PATCH v5 0/8] block, scsi, md: Improve suspend and resume

[Christoph Hellwig]

Bart, Ming:

can you guys please work a little better together?  We've now got two
patchsets that are getting very similar.

Bart, please at least CC Ming when you send out the patches.

Ming - instead of sending a separate series right after Bart a
differential series would be nice.  This also applies the other way
around if Ming is the first after a while.

Re: [PATCH v5 0/8] block, scsi, md: Improve suspend and resume

[Ming Lei]

On Wed, Oct 04, 2017 at 08:56:08AM +0200, Christoph Hellwig wrote:
> Bart, Ming:
> 
> can you guys please work a little better together?  We've now got two
> patchsets that are getting very similar.
> 
> Bart, please at least CC Ming when you send out the patches.
> 
> Ming - instead of sending a separate series right after Bart a
> differential series would be nice.  This also applies the other way
> around if Ming is the first after a while.

Hi Chritoph,

Could you take a look at my yesterday's post V8? Which should
address all previous issues? Except for Tejun's comment
about document __percpu_ref_tryget_live() a bit, and I will
do it in V9.

I have commented on Bart's patchset before, but my comments
never gets addressed:

1) no MD changes required on this issue

2) RCU read lock is missed in fast path

3) bad patch title:

- such as :  'scsi-mq: Reduce suspend latency'
	this is very misleading since it is actually bug fix

- such as : "Make SCSI device suspend and resume work reliably"
	also a bit not accurate

Also holding sdev->state_mutex before freezing queue might
cause deadlock since this lock can be acquired in eh handler(
scsi_eh_offline_sdevs())

-- 
Ming

Re: [PATCH v5 0/8] block, scsi, md: Improve suspend and resume

[Ming Lei]

On Wed, Oct 04, 2017 at 03:53:28PM +0800, Ming Lei wrote:
> On Wed, Oct 04, 2017 at 08:56:08AM +0200, Christoph Hellwig wrote:
> > Bart, Ming:
> > 
> > can you guys please work a little better together?  We've now got two
> > patchsets that are getting very similar.
> > 
> > Bart, please at least CC Ming when you send out the patches.
> > 
> > Ming - instead of sending a separate series right after Bart a
> > differential series would be nice.  This also applies the other way
> > around if Ming is the first after a while.
> 
> Hi Chritoph,
> 
> Could you take a look at my yesterday's post V8? Which should
> address all previous issues? Except for Tejun's comment
> about document __percpu_ref_tryget_live() a bit, and I will
> do it in V9.
> 
> I have commented on Bart's patchset before, but my comments
> never gets addressed:
> 
> 1) no MD changes required on this issue
> 
> 2) RCU read lock is missed in fast path
> 
> 3) bad patch title:
> 
> - such as :  'scsi-mq: Reduce suspend latency'
> 	this is very misleading since it is actually bug fix
> 
> - such as : "Make SCSI device suspend and resume work reliably"
> 	also a bit not accurate

Not mention 'block, scsi, md: Improve suspend and resume' in this cover
letter, which is really misleading, since this patchset is really
bug fix.

-- 
Ming

Re: [PATCH v5 0/8] block, scsi, md: Improve suspend and resume

[Bart Van Assche]

T24gV2VkLCAyMDE3LTEwLTA0IGF0IDE1OjUzICswODAwLCBNaW5nIExlaSB3cm90ZToNCj4gSSBo
YXZlIGNvbW1lbnRlZCBvbiBCYXJ0J3MgcGF0Y2hzZXQgYmVmb3JlLCBidXQgbXkgY29tbWVudHMN
Cj4gbmV2ZXIgZ2V0cyBhZGRyZXNzZWQ6DQo+IA0KPiAxKSBubyBNRCBjaGFuZ2VzIHJlcXVpcmVk
IG9uIHRoaXMgaXNzdWUNCg0KSSBkaWQgbm90IGFkZHJlc3MgdGhhdCBjb21tZW50IGJlY2F1c2Ug
eW91IGhhdmUgYW5vdGhlciB2aWV3IG9uIGhvdyBzdXNwZW5kDQphbmQgcmVzdW1lIHNob3VsZCB3
b3JrIHRoYW4gdGhlIHJlc3Qgb2YgdGhlIGtlcm5lbCBjb21tdW5pdHkuIFNlZSBhbHNvIHRoZQ0K
ZGV0YWlsZWQgZXhwbGFuYXRpb24gaW4gaHR0cHM6Ly9tYXJjLmluZm8vP2w9bGludXgtYmxvY2sm
bT0xNTA2OTU4NzkzMDk2MDYuDQpTZWUgYWxzbyBMdWlzJyBjb3ZlciBsZXR0ZXIgIltSRkMgMC81
XSBmczogcmVwbGFjZSBrdGhyZWFkIGZyZWV6aW5nIHdpdGgNCmZpbGVzeXN0ZW0gZnJlZXplL3Ro
YXciIChodHRwczovL21hcmMuaW5mby8/bD1saW51eC1ibG9jayZtPTE1MDcwNTY5MDMwMjQyMCku
DQoNCj4gMikgUkNVIHJlYWQgbG9jayBpcyBtaXNzZWQgaW4gZmFzdCBwYXRoDQoNCkRpZCB5b3Ug
cmVhbGx5IHBvc3QgdGhpcyBhcyBhIHJldmlldyBjb21tZW50IG9uIG15IHBhdGNoIHNlcmllcz8g
QW55d2F5LCB0aGlzDQppcyBlYXN5IHRvIGFkZHJlc3MuDQoNCj4gMykgYmFkIHBhdGNoIHRpdGxl
Og0KPiANCj4gLSBzdWNoIGFzIDogICdzY3NpLW1xOiBSZWR1Y2Ugc3VzcGVuZCBsYXRlbmN5Jw0K
PiAJdGhpcyBpcyB2ZXJ5IG1pc2xlYWRpbmcgc2luY2UgaXQgaXMgYWN0dWFsbHkgYnVnIGZpeA0K
PiANCj4gLSBzdWNoIGFzIDogIk1ha2UgU0NTSSBkZXZpY2Ugc3VzcGVuZCBhbmQgcmVzdW1lIHdv
cmsgcmVsaWFibHkiDQo+IAlhbHNvIGEgYml0IG5vdCBhY2N1cmF0ZQ0KDQpTYW1lIGhlcmUgLSBo
YWQgeW91IHBvc3RlZCB0aGlzIGFzIGEgcmV2aWV3IGNvbW1lbnQgYmVmb3JlPyBBbnl3YXksIHRo
aXMgaXMNCmFsc28gZWFzeSB0byBhZGRyZXNzLg0KDQo+IEFsc28gaG9sZGluZyBzZGV2LT5zdGF0
ZV9tdXRleCBiZWZvcmUgZnJlZXppbmcgcXVldWUgbWlnaHQNCj4gY2F1c2UgZGVhZGxvY2sgc2lu
Y2UgdGhpcyBsb2NrIGNhbiBiZSBhY3F1aXJlZCBpbiBlaCBoYW5kbGVyKA0KPiBzY3NpX2VoX29m
ZmxpbmVfc2RldnMoKSkNCg0KSGFkIHlvdSBwb3N0ZWQgdGhpcyBhcyBhIGNvbW1lbnQgYmVmb3Jl
PyBBbnl3YXksIEkgd2lsbCBhZGRyZXNzIHRoaXMgY29tbWVudA0KdG9vLg0KDQpCYXJ0Lg==

Re: [PATCH v5 0/8] block, scsi, md: Improve suspend and resume

[Ming Lei]

On Wed, Oct 04, 2017 at 03:18:45PM +0000, Bart Van Assche wrote:
> On Wed, 2017-10-04 at 15:53 +0800, Ming Lei wrote:
> > I have commented on Bart's patchset before, but my comments
> > never gets addressed:
> > 
> > 1) no MD changes required on this issue
> 
> I did not address that comment because you have another view on how suspend
> and resume should work than the rest of the kernel community. See also the
> detailed explanation in https://marc.info/?l=linux-block&m=150695879309606.
> See also Luis' cover letter "[RFC 0/5] fs: replace kthread freezing with
> filesystem freeze/thaw" (https://marc.info/?l=linux-block&m=150705690302420).

That is still in discussion, and once SCSI quiesce is safe, these two
MD change isn't necessary, that is why my patchset focuses on making
SCSI quiesce safe only. That is why I suggest to not include these two
patches in this patchset, because isn't necessary simply.

I also replied Luis already.

> 
> > 2) RCU read lock is missed in fast path
> 
> Did you really post this as a review comment on my patch series? Anyway, this
> is easy to address.

https://marc.info/?l=linux-block&m=150641481732572&w=2


> 
> > 3) bad patch title:
> > 
> > - such as :  'scsi-mq: Reduce suspend latency'
> > 	this is very misleading since it is actually bug fix
> > 
> > - such as : "Make SCSI device suspend and resume work reliably"
> > 	also a bit not accurate
> 
> Same here - had you posted this as a review comment before? Anyway, this is
> also easy to address.

I have mentioned lots of times, the root cause is only in SCSI quiesce,
actually not related with suspend/resume directly. But you never
consider my comment, that is why you post out these patches with
wrong title(or very misleading) again and again.

> 
> > Also holding sdev->state_mutex before freezing queue might
> > cause deadlock since this lock can be acquired in eh handler(
> > scsi_eh_offline_sdevs())
> 
> Had you posted this as a comment before? Anyway, I will address this comment
> too.

This one is what I just posted out, so you see it isn't marked as 4).

-- 
Ming

Re: [PATCH v5 0/8] block, scsi, md: Improve suspend and resume

[Bart Van Assche]

T24gV2VkLCAyMDE3LTEwLTA0IGF0IDIzOjMxICswODAwLCBNaW5nIExlaSB3cm90ZToNCj4gT24g
V2VkLCBPY3QgMDQsIDIwMTcgYXQgMDM6MTg6NDVQTSArMDAwMCwgQmFydCBWYW4gQXNzY2hlIHdy
b3RlOg0KPiA+IE9uIFdlZCwgMjAxNy0xMC0wNCBhdCAxNTo1MyArMDgwMCwgTWluZyBMZWkgd3Jv
dGU6DQo+ID4gPiAyKSBSQ1UgcmVhZCBsb2NrIGlzIG1pc3NlZCBpbiBmYXN0IHBhdGgNCj4gPiAN
Cj4gPiBEaWQgeW91IHJlYWxseSBwb3N0IHRoaXMgYXMgYSByZXZpZXcgY29tbWVudCBvbiBteSBw
YXRjaCBzZXJpZXM/IEFueXdheSwgdGhpcw0KPiA+IGlzIGVhc3kgdG8gYWRkcmVzcy4NCj4gDQo+
IGh0dHBzOi8vbWFyYy5pbmZvLz9sPWxpbnV4LWJsb2NrJm09MTUwNjQxNDgxNzMyNTcyJnc9Mg0K
DQpUaGUgZS1tYWlsIHlvdSByZWZlciB0byBpcyBhIHJlcGx5IHRvIGEgcGF0Y2ggaW4gdjQgb2Yg
dGhpcyBwYXRjaCBzZXJpZXMgaW5zdGVhZA0Kb2YgdjUuDQoNCkJhcnQu

Re: [PATCH v5 0/8] block, scsi, md: Improve suspend and resume

[Bart Van Assche]

T24gV2VkLCAyMDE3LTEwLTA0IGF0IDA4OjU2ICswMjAwLCBDaHJpc3RvcGggSGVsbHdpZyB3cm90
ZToNCj4gY2FuIHlvdSBndXlzIHBsZWFzZSB3b3JrIGEgbGl0dGxlIGJldHRlciB0b2dldGhlcj8g
IFdlJ3ZlIG5vdyBnb3QgdHdvDQo+IHBhdGNoc2V0cyB0aGF0IGFyZSBnZXR0aW5nIHZlcnkgc2lt
aWxhci4NCj4gDQo+IEJhcnQsIHBsZWFzZSBhdCBsZWFzdCBDQyBNaW5nIHdoZW4geW91IHNlbmQg
b3V0IHRoZSBwYXRjaGVzLg0KDQpIZWxsbyBDaHJpc3RvcGgsDQoNClNvcnJ5IGZvciB0aGUgbG93
IGxldmVsIG9mIGNvbGxhYm9yYXRpb24gc28gZmFyLiBBbnl3YXksIEkgd2lsbCBDYyBNaW5nIGlm
IEkgcmVwb3N0DQp0aGlzIHBhdGNoIHNlcmllcy4NCg0KQmFydC4=