drop abort sequence, kill BLK_EH_HANDLED v2

---
 drivers/ata/libata-eh.c | 51 -----------------------------------------
 include/linux/libata.h  |  2 --
 2 files changed, 53 deletions(-)

drivers/ata/libata-eh.c +0 ~~-51~~

index c016829a38fd..e87785dec151 100644
@@ -500,57 +500,6 @@ void ata_eh_release(struct ata_port *ap)
 	mutex_unlock(&ap->host->eh_mutex);
 }
 
-/**
- *	ata_scsi_timed_out - SCSI layer time out callback
- *	@cmd: timed out SCSI command
- *
- *	Handles SCSI layer timeout.  We race with normal completion of
- *	the qc for @cmd.  If the qc is already gone, we lose and let
- *	the scsi command finish (EH_HANDLED).  Otherwise, the qc has
- *	timed out and EH should be invoked.  Prevent ata_qc_complete()
- *	from finishing it by setting EH_SCHEDULED and return
- *	EH_NOT_HANDLED.
- *
- *	TODO: kill this function once old EH is gone.
- *
- *	LOCKING:
- *	Called from timer context
- *
- *	RETURNS:
- *	EH_HANDLED or EH_NOT_HANDLED
- */
-enum blk_eh_timer_return ata_scsi_timed_out(struct scsi_cmnd *cmd)
-{
-	struct Scsi_Host *host = cmd->device->host;
-	struct ata_port *ap = ata_shost_to_port(host);
-	unsigned long flags;
-	struct ata_queued_cmd *qc;
-	enum blk_eh_timer_return ret;
-
-	DPRINTK("ENTER\n");
-
-	if (ap->ops->error_handler) {
-		ret = BLK_EH_NOT_HANDLED;
-		goto out;
-	}
-
-	ret = BLK_EH_HANDLED;
-	spin_lock_irqsave(ap->lock, flags);
-	qc = ata_qc_from_tag(ap, ap->link.active_tag);
-	if (qc) {
-		WARN_ON(qc->scsicmd != cmd);
-		qc->flags |= ATA_QCFLAG_EH_SCHEDULED;
-		qc->err_mask |= AC_ERR_TIMEOUT;
-		ret = BLK_EH_NOT_HANDLED;
-	}
-	spin_unlock_irqrestore(ap->lock, flags);
-
- out:
-	DPRINTK("EXIT, ret=%d\n", ret);
-	return ret;
-}
-EXPORT_SYMBOL(ata_scsi_timed_out);
-
 static void ata_eh_unload(struct ata_port *ap)
 {
 	struct ata_link *link;

include/linux/libata.h +0 -2

index 1795fecdea17..1c113134c98f 100644
@@ -1133,7 +1133,6 @@ extern int ata_sas_port_start(struct ata_port *ap);
 extern void ata_sas_port_stop(struct ata_port *ap);
 extern int ata_sas_slave_configure(struct scsi_device *, struct ata_port *);
 extern int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap);
-extern enum blk_eh_timer_return ata_scsi_timed_out(struct scsi_cmnd *cmd);
 extern int sata_scr_valid(struct ata_link *link);
 extern int sata_scr_read(struct ata_link *link, int reg, u32 *val);
 extern int sata_scr_write(struct ata_link *link, int reg, u32 val);

@@ -1359,7 +1358,6 @@ extern struct device_attribute *ata_common_sdev_attrs[];
 	.proc_name		= drv_name,			\
 	.slave_configure	= ata_scsi_slave_config,	\
 	.slave_destroy		= ata_scsi_slave_destroy,	\
-	.eh_timed_out		= ata_scsi_timed_out,		\
 	.bios_param		= ata_std_bios_param,		\
 	.unlock_native_capacity	= ata_scsi_unlock_native_capacity, \
 	.sdev_attrs		= ata_common_sdev_attrs

Signature · 1 line

2.17.0

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #2 lore raw

PATCH 2/14 blk-mq: Fix timeout and state order

From: Keith Busch <[email protected]>

The block layer had been setting the state to in-flight prior to updating the timer. This is the wrong order since the timeout handler could observe the in-flight state with the older timeout, believing the request had expired when in fact it is just getting started.

Signed-off-by Keith Busch <[email protected]>

---
 block/blk-mq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

block/blk-mq.c +1 -1

index df928200b17e..3581a1e5c8a7 100644
@@ -697,8 +697,8 @@ void blk_mq_start_request(struct request *rq)
 	preempt_disable();
 	write_seqcount_begin(&rq->gstate_seq);
 
-	blk_mq_rq_update_state(rq, MQ_RQ_IN_FLIGHT);
 	blk_add_timer(rq);
+	blk_mq_rq_update_state(rq, MQ_RQ_IN_FLIGHT);
 
 	write_seqcount_end(&rq->gstate_seq);
 	preempt_enable();

Signature · 1 line

2.17.0

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #3 lore raw

PATCH 3/14 blk-mq: Remove generation seqeunce

From: Keith Busch <[email protected]>

This patch simplifies the timeout handling by relying on the request reference counting to ensure the iterator is operating on an inflight and truly timed out request. Since the reference counting prevents the tag from being reallocated, the block layer no longer needs to prevent drivers from completing their requests while the timeout handler is operating on it: a driver completing a request is allowed to proceed to the next state without additional syncronization with the block layer.

This also removes any need for generation sequence numbers since the request lifetime is prevented from being reallocated as a new sequence while timeout handling is operating on it.

To enables this a refcount is added to struct request so that request users can be sure they're operating on the same request without it changing while they're processing it. The request's tag won't be released for reuse until both the timeout handler and the completion are done with it.

Signed-off-by Keith Busch <[email protected]>

[hch: slight cleanups, added back submission side hctx lock, use cmpxchg
for completions]

Signed-off-by Christoph Hellwig <[email protected]>

---
 block/blk-core.c       |   6 -
 block/blk-mq-debugfs.c |   1 -
 block/blk-mq.c         | 258 ++++++++++++-----------------------------
 block/blk-mq.h         |  42 +------
 block/blk-timeout.c    |   1 -
 include/linux/blkdev.h |  35 ++----
 6 files changed, 89 insertions(+), 254 deletions(-)

block/blk-core.c +0 -6

index 43370faee935..cee03cad99f2 100644
@@ -198,12 +198,6 @@ void blk_rq_init(struct request_queue *q, struct request *rq)
 	rq->internal_tag = -1;
 	rq->start_time_ns = ktime_get_ns();
 	rq->part = NULL;
-	seqcount_init(&rq->gstate_seq);
-	u64_stats_init(&rq->aborted_gstate_sync);
-	/*
-	 * See comment of blk_mq_init_request
-	 */
-	WRITE_ONCE(rq->gstate, MQ_RQ_GEN_INC);
 }
 EXPORT_SYMBOL(blk_rq_init);

block/blk-mq-debugfs.c +0 -1

index 3080e18cb859..ffa622366922 100644
@@ -344,7 +344,6 @@ static const char *const rqf_name[] = {
 	RQF_NAME(STATS),
 	RQF_NAME(SPECIAL_PAYLOAD),
 	RQF_NAME(ZONE_WRITE_LOCKED),
-	RQF_NAME(MQ_TIMEOUT_EXPIRED),
 	RQF_NAME(MQ_POLL_SLEPT),
 };
 #undef RQF_NAME

block/blk-mq.c +75 ~~-183~~

374 lines not on this page · raw message

block/blk-mq.h +2 ~~-40~~

55 lines not on this page · raw message

block/blk-timeout.c +0 -1

index 652d4d4d3e97..f95d6e6cbc96 100644
@@ -214,7 +214,6 @@ void blk_add_timer(struct request *req)
 		req->timeout = q->rq_timeout;
 
 	blk_rq_set_deadline(req, jiffies + req->timeout);
-	req->rq_flags &= ~RQF_MQ_TIMEOUT_EXPIRED;
 
 	/*
 	 * Only the non-mq case needs to add the request to a protected list.

include/linux/blkdev.h +12 ~~-23~~

index f3999719f828..a1c05e85a443 100644
@@ -125,15 +125,22 @@ typedef __u32 __bitwise req_flags_t;
 #define RQF_SPECIAL_PAYLOAD	((__force req_flags_t)(1 << 18))
 /* The per-zone write lock is held for this request */
 #define RQF_ZONE_WRITE_LOCKED	((__force req_flags_t)(1 << 19))
-/* timeout is expired */
-#define RQF_MQ_TIMEOUT_EXPIRED	((__force req_flags_t)(1 << 20))
 /* already slept for hybrid poll */
-#define RQF_MQ_POLL_SLEPT	((__force req_flags_t)(1 << 21))
+#define RQF_MQ_POLL_SLEPT	((__force req_flags_t)(1 << 20))
 
 /* flags that prevent us from merging requests: */
 #define RQF_NOMERGE_FLAGS \
 	(RQF_STARTED | RQF_SOFTBARRIER | RQF_FLUSH_SEQ | RQF_SPECIAL_PAYLOAD)
 
+/*
+ * Request state for blk-mq.
+ */
+enum mq_rq_state {
+	MQ_RQ_IDLE		= 0,
+	MQ_RQ_IN_FLIGHT		= 1,
+	MQ_RQ_COMPLETE		= 2,
+};
+
 /*
  * Try to put the fields that are referenced together in the same cacheline.
  *

@@ -236,26 +243,8 @@ struct request {
 
 	unsigned int extra_len;	/* length of alignment and padding */
 
-	/*
-	 * On blk-mq, the lower bits of ->gstate (generation number and
-	 * state) carry the MQ_RQ_* state value and the upper bits the
-	 * generation number which is monotonically incremented and used to
-	 * distinguish the reuse instances.
-	 *
-	 * ->gstate_seq allows updates to ->gstate and other fields
-	 * (currently ->deadline) during request start to be read
-	 * atomically from the timeout path, so that it can operate on a
-	 * coherent set of information.
-	 */
-	seqcount_t gstate_seq;
-	u64 gstate;
-
-	/*
-	 * ->aborted_gstate is used by the timeout to claim a specific
-	 * recycle instance of this request.  See blk_mq_timeout_work().
-	 */
-	struct u64_stats_sync aborted_gstate_sync;
-	u64 aborted_gstate;
+	enum mq_rq_state state;
+	refcount_t ref;
 
 	/* access through blk_rq_set_deadline, blk_rq_deadline */
 	unsigned long __deadline;

Signature · 1 line

2.17.0

Bart Van Assche

2018-06-04 06:56 UTC #4 lore raw

Re: [PATCH 03/14] blk-mq: Remove generation seqeunce

T24gVHVlLCAyMDE4LTA1LTI5IGF0IDE1OjUyICswMjAwLCBDaHJpc3RvcGggSGVsbHdpZyB3cm90 ZToNCj4gRnJvbTogS2VpdGggQnVzY2ggPGtlaXRoLmJ1c2NoQGludGVsLmNvbT4NCj4gDQo+IFRo aXMgcGF0Y2ggc2ltcGxpZmllcyB0aGUgdGltZW91dCBoYW5kbGluZyBieSByZWx5aW5nIG9uIHRo ZSByZXF1ZXN0DQo+IHJlZmVyZW5jZSBjb3VudGluZyB0byBlbnN1cmUgdGhlIGl0ZXJhdG9yIGlz IG9wZXJhdGluZyBvbiBhbiBpbmZsaWdodA0KPiBhbmQgdHJ1bHkgdGltZWQgb3V0IHJlcXVlc3Qu IFNpbmNlIHRoZSByZWZlcmVuY2UgY291bnRpbmcgcHJldmVudHMgdGhlDQo+IHRhZyBmcm9tIGJl aW5nIHJlYWxsb2NhdGVkLCB0aGUgYmxvY2sgbGF5ZXIgbm8gbG9uZ2VyIG5lZWRzIHRvIHByZXZl bnQNCj4gZHJpdmVycyBmcm9tIGNvbXBsZXRpbmcgdGhlaXIgcmVxdWVzdHMgd2hpbGUgdGhlIHRp bWVvdXQgaGFuZGxlciBpcw0KPiBvcGVyYXRpbmcgb24gaXQ6IGEgZHJpdmVyIGNvbXBsZXRpbmcg YSByZXF1ZXN0IGlzIGFsbG93ZWQgdG8gcHJvY2VlZCB0bw0KPiB0aGUgbmV4dCBzdGF0ZSB3aXRo b3V0IGFkZGl0aW9uYWwgc3luY3Jvbml6YXRpb24gd2l0aCB0aGUgYmxvY2sgbGF5ZXIuDQo+IA0K PiBUaGlzIGFsc28gcmVtb3ZlcyBhbnkgbmVlZCBmb3IgZ2VuZXJhdGlvbiBzZXF1ZW5jZSBudW1i ZXJzIHNpbmNlIHRoZQ0KPiByZXF1ZXN0IGxpZmV0aW1lIGlzIHByZXZlbnRlZCBmcm9tIGJlaW5n IHJlYWxsb2NhdGVkIGFzIGEgbmV3IHNlcXVlbmNlDQo+IHdoaWxlIHRpbWVvdXQgaGFuZGxpbmcg aXMgb3BlcmF0aW5nIG9uIGl0Lg0KPiANCj4gVG8gZW5hYmxlcyB0aGlzIGEgcmVmY291bnQgaXMg YWRkZWQgdG8gc3RydWN0IHJlcXVlc3Qgc28gdGhhdCByZXF1ZXN0DQo+IHVzZXJzIGNhbiBiZSBz dXJlIHRoZXkncmUgb3BlcmF0aW5nIG9uIHRoZSBzYW1lIHJlcXVlc3Qgd2l0aG91dCBpdA0KPiBj aGFuZ2luZyB3aGlsZSB0aGV5J3JlIHByb2Nlc3NpbmcgaXQuICBUaGUgcmVxdWVzdCdzIHRhZyB3 b24ndCBiZQ0KPiByZWxlYXNlZCBmb3IgcmV1c2UgdW50aWwgYm90aCB0aGUgdGltZW91dCBoYW5k bGVyIGFuZCB0aGUgY29tcGxldGlvbg0KPiBhcmUgZG9uZSB3aXRoIGl0Lg0KPiANCj4gU2lnbmVk LW9mZi1ieTogS2VpdGggQnVzY2ggPGtlaXRoLmJ1c2NoQGludGVsLmNvbT4NCj4gW2hjaDogc2xp Z2h0IGNsZWFudXBzLCBhZGRlZCBiYWNrIHN1Ym1pc3Npb24gc2lkZSBoY3R4IGxvY2ssIHVzZSBj bXB4Y2hnDQo+ICBmb3IgY29tcGxldGlvbnNdDQo+IFNpZ25lZC1vZmYtYnk6IENocmlzdG9waCBI ZWxsd2lnIDxoY2hAbHN0LmRlPg0KDQpIZWxsbyBDaHJpc3RvcGgsDQoNCkNhbiB5b3UgZXhwbGFp biB3aHkgeW91IHByZWZlciB0aGlzIGFwcHJvYWNoPyBJIHRoaW5rIHRoaXMgcGF0Y2ggaW50cm9k dWNlcw0KbW9yZSBuZXcgYXRvbWljIG9wZXJhdGlvbnMgaW4gdGhlIGhvdCBwYXRoIHRoYW4gdGhl IGFwcHJvYWNoIEkgaGFkIHByb3Bvc2VkDQooImJsay1tcTogUmV3b3JrIGJsay1tcSB0aW1lb3V0 IGhhbmRsaW5nIGFnYWluIikuIEluIG90aGVyIHdvcmRzLCBJIHRoaW5rIHRoZQ0KcGF0Y2ggdGhh dCBJIGhhZCBwcm9wb3NlZCB3aWxsIHlpZWxkIGJldHRlciBwZXJmb3JtYW5jZS4NCg0KVGhhbmtz LA0KDQpCYXJ0Lg0KDQoNCg==

[email protected]

author maintainer

2018-06-04 13:42 UTC #5 lore raw

Re: [PATCH 03/14] blk-mq: Remove generation seqeunce

Quoted 4 lines · Bart Van Assche

Can you explain why you prefer this approach? I think this patch introduces more new atomic operations in the hot path than the approach I had proposed ("blk-mq: Rework blk-mq timeout handling again"). In other words, I think the patch that I had proposed will yield better performance.

Primarily because the generation counter is fundamentally the wrong thing to do. If we know the tag has some pending action on it we should not reuse it insted of working around races that stem from the fact that we reuse it.

We can look at the performance numbers and tune as needed, but for now we need to get the high level approach right first. I think once we've done a proper audit of the SCSI EH code and blk_abort_request users we can probably drop the cmpxchg and only have a single atomic_dec_and_test left, which in terms of atomic operations isn't the world.

Bart Van Assche

2018-06-04 14:13 UTC #6 lore raw

Re: [PATCH 03/14] blk-mq: Remove generation seqeunce

T24gTW9uLCAyMDE4LTA2LTA0IGF0IDE1OjQyICswMjAwLCBoY2hAbHN0LmRlIHdyb3RlOg0KPiBP biBNb24sIEp1biAwNCwgMjAxOCBhdCAwNjo1NjoyMkFNICswMDAwLCBCYXJ0IFZhbiBBc3NjaGUg d3JvdGU6DQo+ID4gQ2FuIHlvdSBleHBsYWluIHdoeSB5b3UgcHJlZmVyIHRoaXMgYXBwcm9hY2g/ IEkgdGhpbmsgdGhpcyBwYXRjaCBpbnRyb2R1Y2VzDQo+ID4gbW9yZSBuZXcgYXRvbWljIG9wZXJh dGlvbnMgaW4gdGhlIGhvdCBwYXRoIHRoYW4gdGhlIGFwcHJvYWNoIEkgaGFkIHByb3Bvc2VkDQo+ ID4gKCJibGstbXE6IFJld29yayBibGstbXEgdGltZW91dCBoYW5kbGluZyBhZ2FpbiIpLiBJbiBv dGhlciB3b3JkcywgSSB0aGluayB0aGUNCj4gPiBwYXRjaCB0aGF0IEkgaGFkIHByb3Bvc2VkIHdp bGwgeWllbGQgYmV0dGVyIHBlcmZvcm1hbmNlLg0KPiANCj4gUHJpbWFyaWx5IGJlY2F1c2UgdGhl IGdlbmVyYXRpb24gY291bnRlciBpcyBmdW5kYW1lbnRhbGx5IHRoZSB3cm9uZw0KPiB0aGluZyB0 byBkby4gIElmIHdlIGtub3cgdGhlIHRhZyBoYXMgc29tZSBwZW5kaW5nIGFjdGlvbiBvbiBpdCB3 ZQ0KPiBzaG91bGQgbm90IHJldXNlIGl0IGluc3RlZCBvZiB3b3JraW5nIGFyb3VuZCByYWNlcyB0 aGF0IHN0ZW0gZnJvbSB0aGUNCj4gZmFjdCB0aGF0IHdlIHJldXNlIGl0Lg0KDQpXZSBtYXkgZWFj aCBoYXZlIGEgZGlmZmVyZW50IG9waW5pb24gYWJvdXQgdGhpcy4gQW55d2F5LCBsZXQncyBub3Qg Z2V0DQpkaXN0cmFjdGVkIGJ5IGEgZGlzY3Vzc2lvbiBhYm91dCB0aGVvcmV0aWNhbCBhcmd1bWVu dHMuDQoNCj4gV2UgY2FuIGxvb2sgYXQgdGhlIHBlcmZvcm1hbmNlIG51bWJlcnMgYW5kIHR1bmUg YXMgbmVlZGVkLCBidXQgZm9yIG5vdw0KPiB3ZSBuZWVkIHRvIGdldCB0aGUgaGlnaCBsZXZlbCBh cHByb2FjaCByaWdodCBmaXJzdC4gIEkgdGhpbmsgb25jZQ0KPiB3ZSd2ZSBkb25lIGEgcHJvcGVy IGF1ZGl0IG9mIHRoZSBTQ1NJIEVIIGNvZGUgYW5kIGJsa19hYm9ydF9yZXF1ZXN0DQo+IHVzZXJz IHdlIGNhbiBwcm9iYWJseSBkcm9wIHRoZSBjbXB4Y2hnIGFuZCBvbmx5IGhhdmUgYSBzaW5nbGUN Cj4gYXRvbWljX2RlY19hbmRfdGVzdCBsZWZ0LCB3aGljaCBpbiB0ZXJtcyBvZiBhdG9taWMgb3Bl cmF0aW9ucyBpc24ndA0KPiB0aGUgd29ybGQuDQoNCkkgdGhpbmsgdGhhdCB0aGUgcmFjZSBiZXR3 ZWVuIHJlcXVlc3QgY29tcGxldGlvbiBhbmQgcmVxdWVzdCB0aW1lb3V0DQpoYW5kbGluZyBpcyBm dW5kYW1lbnRhbCBzbyBJJ20gbm90IHN1cmUgdGhhdCBpdCBpcyBwb3NzaWJsZSB0byBlbGltaW5h dGUNCnRoZSBjbXB4Y2hnKCkgY2FsbHMuIEFkZGl0aW9uYWxseSwgZXZlbiBpZiB0aGUgY21weGNo ZygpIGNhbGxzIHdvdWxkIGJlDQplbGltaW5hdGVkLCBzdHJ1Y3QgcmVxdWVzdCB3aWxsIHN0aWxs IGhhdmUgdHdvIHZhcmlhYmxlcyB0aGF0IHJlcHJlc2VudA0KdGhlIHJlcXVlc3Qgc3RhdGUgKHJx LT5zdGF0ZSBhbmQgcnEtPnJlZikgYW5kIHRoYXQgYXJlIHVwZGF0ZWQgc2VwYXJhdGVseS4NCldp dGggbXkgYXBwcm9hY2ggYWxsIHJlcXVlc3Qgc3RhdGUgaW5mb3JtYXRpb24gaXMgc3RvcmVkIGlu IGEgc2luZ2xlDQp2YXJpYWJsZSB3aGljaCBtYWtlcyBpdCBlYXN5IHRvIHVwZGF0ZSB0aGUgaW5k aXZpZHVhbCBmaWVsZHMgb2YgdGhhdA0KdmFyaWFibGUgc2ltdWx0YW5lb3VzbHkuDQoNCkJhcnQu

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #7 lore raw

PATCH 4/14 block: rename BLK_EH_NOT_HANDLED to BLK_EH_DONE

The BLK_EH_NOT_HANDLED implies nothing happen, but very often that is not what is happening - instead the driver already completed the command. Fix the symbolic name to reflect that a little better.

Signed-off-by Christoph Hellwig <[email protected]>

---
 Documentation/scsi/scsi_eh.txt            | 4 ++--
 block/blk-mq.c                            | 2 +-
 block/blk-timeout.c                       | 2 +-
 drivers/block/nbd.c                       | 2 +-
 drivers/message/fusion/mptsas.c           | 2 +-
 drivers/s390/block/dasd.c                 | 6 +++---
 drivers/scsi/gdth.c                       | 2 +-
 drivers/scsi/libiscsi.c                   | 2 +-
 drivers/scsi/megaraid/megaraid_sas_base.c | 2 +-
 drivers/scsi/mvumi.c                      | 2 +-
 drivers/scsi/qla4xxx/ql4_os.c             | 2 +-
 drivers/scsi/scsi_error.c                 | 4 ++--
 drivers/scsi/scsi_transport_fc.c          | 4 ++--
 drivers/scsi/scsi_transport_srp.c         | 4 ++--
 drivers/scsi/ufs/ufshcd.c                 | 6 +++---
 include/linux/blkdev.h                    | 2 +-
 include/scsi/scsi_host.h                  | 2 +-
 17 files changed, 25 insertions(+), 25 deletions(-)

Documentation/scsi/scsi_eh.txt +2 -2

index 11e447bdb3a5..3ae8419e72cf 100644
@@ -97,9 +97,9 @@ function
 	This indicates that more time is required to finish the
 	command.  Timer is restarted.  This action is counted as a
 	retry and only allowed scmd->allowed + 1(!) times.  Once the
-	limit is reached, action for BLK_EH_NOT_HANDLED is taken instead.
+	limit is reached, action for BLK_EH_DONE is taken instead.
 
-    - BLK_EH_NOT_HANDLED
+    - BLK_EH_DONE
         eh_timed_out() callback did not handle the command.
 	Step #2 is taken.

block/blk-mq.c +1 -1

index 270d55cd70b5..a7a601f08bf5 100644
@@ -784,7 +784,7 @@ static void blk_mq_rq_timed_out(struct request *req, bool reserved)
 	case BLK_EH_RESET_TIMER:
 		blk_add_timer(req);
 		break;
-	case BLK_EH_NOT_HANDLED:
+	case BLK_EH_DONE:
 		break;
 	default:
 		printk(KERN_ERR "block: bad eh return: %d\n", ret);

block/blk-timeout.c +1 -1

index f95d6e6cbc96..11879e98c249 100644
@@ -93,7 +93,7 @@ static void blk_rq_timed_out(struct request *req)
 		blk_add_timer(req);
 		blk_clear_rq_complete(req);
 		break;
-	case BLK_EH_NOT_HANDLED:
+	case BLK_EH_DONE:
 		/*
 		 * LLD handles this for now but in the future
 		 * we can send a request msg to abort the command

drivers/block/nbd.c +1 -1

index 800e1ec71f3d..88ae833aabe9 100644
@@ -332,7 +332,7 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req,
 			}
 			blk_mq_requeue_request(req, true);
 			nbd_config_put(nbd);
-			return BLK_EH_NOT_HANDLED;
+			return BLK_EH_DONE;
 		}
 	} else {
 		dev_err_ratelimited(nbd_to_dev(nbd),

drivers/message/fusion/mptsas.c +1 -1

index 86503f60468f..19a5aa70ecda 100644
@@ -1929,7 +1929,7 @@ static enum blk_eh_timer_return mptsas_eh_timed_out(struct scsi_cmnd *sc)
 	MPT_SCSI_HOST *hd;
 	MPT_ADAPTER   *ioc;
 	VirtDevice    *vdevice;
-	enum blk_eh_timer_return rc = BLK_EH_NOT_HANDLED;
+	enum blk_eh_timer_return rc = BLK_EH_DONE;
 
 	hd = shost_priv(sc->device->host);
 	if (hd == NULL) {

drivers/s390/block/dasd.c +3 -3

index 04143c08bd6e..b0e89ca48a3c 100644
@@ -3053,7 +3053,7 @@ static blk_status_t do_dasd_request(struct blk_mq_hw_ctx *hctx,
  *
  * Return values:
  * BLK_EH_RESET_TIMER if the request should be left running
- * BLK_EH_NOT_HANDLED if the request is handled or terminated
+ * BLK_EH_DONE if the request is handled or terminated
  *		      by the driver.
  */
 enum blk_eh_timer_return dasd_times_out(struct request *req, bool reserved)

@@ -3065,7 +3065,7 @@ enum blk_eh_timer_return dasd_times_out(struct request *req, bool reserved)
 	int rc = 0;
 
 	if (!cqr)
-		return BLK_EH_NOT_HANDLED;
+		return BLK_EH_DONE;
 
 	spin_lock_irqsave(&cqr->dq->lock, flags);
 	device = cqr->startdev ? cqr->startdev : block->base;

@@ -3124,7 +3124,7 @@ enum blk_eh_timer_return dasd_times_out(struct request *req, bool reserved)
 	spin_unlock(&block->queue_lock);
 	spin_unlock_irqrestore(&cqr->dq->lock, flags);
 
-	return rc ? BLK_EH_RESET_TIMER : BLK_EH_NOT_HANDLED;
+	return rc ? BLK_EH_RESET_TIMER : BLK_EH_DONE;
 }
 
 static int dasd_init_hctx(struct blk_mq_hw_ctx *hctx, void *data,

drivers/scsi/gdth.c +1 -1

index c35f05c4c6bb..85604795d8ee 100644
@@ -3882,7 +3882,7 @@ static enum blk_eh_timer_return gdth_timed_out(struct scsi_cmnd *scp)
 	struct gdth_cmndinfo *cmndinfo = gdth_cmnd_priv(scp);
 	u8 b, t;
 	unsigned long flags;
-	enum blk_eh_timer_return retval = BLK_EH_NOT_HANDLED;
+	enum blk_eh_timer_return retval = BLK_EH_DONE;
 
 	TRACE(("%s() cmd 0x%x\n", scp->cmnd[0], __func__));
 	b = scp->device->channel;

drivers/scsi/libiscsi.c +1 -1

index 15a2fef51e38..eee43ba83a60 100644
@@ -1963,7 +1963,7 @@ static int iscsi_has_ping_timed_out(struct iscsi_conn *conn)
 
 enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
 {
-	enum blk_eh_timer_return rc = BLK_EH_NOT_HANDLED;
+	enum blk_eh_timer_return rc = BLK_EH_DONE;
 	struct iscsi_task *task = NULL, *running_task;
 	struct iscsi_cls_session *cls_session;
 	struct iscsi_session *session;

drivers/scsi/megaraid/megaraid_sas_base.c +1 -1

index b89c6e6c0589..ce656c466ca9 100644
@@ -2772,7 +2772,7 @@ blk_eh_timer_return megasas_reset_timer(struct scsi_cmnd *scmd)
 
 	if (time_after(jiffies, scmd->jiffies_at_alloc +
 				(scmd_timeout * 2) * HZ)) {
-		return BLK_EH_NOT_HANDLED;
+		return BLK_EH_DONE;
 	}
 
 	instance = (struct megasas_instance *)scmd->device->host->hostdata;

drivers/scsi/mvumi.c +1 -1

index fe97401ad192..afd27165cd93 100644
@@ -2155,7 +2155,7 @@ static enum blk_eh_timer_return mvumi_timed_out(struct scsi_cmnd *scmd)
 	mvumi_return_cmd(mhba, cmd);
 	spin_unlock_irqrestore(mhba->shost->host_lock, flags);
 
-	return BLK_EH_NOT_HANDLED;
+	return BLK_EH_DONE;
 }
 
 static int

drivers/scsi/qla4xxx/ql4_os.c +1 -1

index 94c14ce94da2..0e13349dce57 100644
@@ -1848,7 +1848,7 @@ static enum blk_eh_timer_return qla4xxx_eh_cmd_timed_out(struct scsi_cmnd *sc)
 	struct iscsi_cls_session *session;
 	struct iscsi_session *sess;
 	unsigned long flags;
-	enum blk_eh_timer_return ret = BLK_EH_NOT_HANDLED;
+	enum blk_eh_timer_return ret = BLK_EH_DONE;
 
 	session = starget_to_session(scsi_target(sc->device));
 	sess = session->dd_data;

drivers/scsi/scsi_error.c +2 -2

index b36e73090018..9c02ba2e7ef3 100644
@@ -282,7 +282,7 @@ void scsi_eh_scmd_add(struct scsi_cmnd *scmd)
 enum blk_eh_timer_return scsi_times_out(struct request *req)
 {
 	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
-	enum blk_eh_timer_return rtn = BLK_EH_NOT_HANDLED;
+	enum blk_eh_timer_return rtn = BLK_EH_DONE;
 	struct Scsi_Host *host = scmd->device->host;
 
 	trace_scsi_dispatch_cmd_timeout(scmd);

@@ -294,7 +294,7 @@ enum blk_eh_timer_return scsi_times_out(struct request *req)
 	if (host->hostt->eh_timed_out)
 		rtn = host->hostt->eh_timed_out(scmd);
 
-	if (rtn == BLK_EH_NOT_HANDLED) {
+	if (rtn == BLK_EH_DONE) {
 		if (scsi_abort_command(scmd) != SUCCESS) {
 			set_host_byte(scmd, DID_TIME_OUT);
 			scsi_eh_scmd_add(scmd);

drivers/scsi/scsi_transport_fc.c +2 -2

index be3be0f9cb2d..90075a0ddcfe 100644
@@ -2087,7 +2087,7 @@ fc_eh_timed_out(struct scsi_cmnd *scmd)
 	if (rport->port_state == FC_PORTSTATE_BLOCKED)
 		return BLK_EH_RESET_TIMER;
 
-	return BLK_EH_NOT_HANDLED;
+	return BLK_EH_DONE;
 }
 EXPORT_SYMBOL(fc_eh_timed_out);

@@ -3592,7 +3592,7 @@ fc_bsg_job_timeout(struct request *req)
 
 	/* the blk_end_sync_io() doesn't check the error */
 	if (!inflight)
-		return BLK_EH_NOT_HANDLED;
+		return BLK_EH_DONE;
 	else
 		return BLK_EH_HANDLED;
 }

drivers/scsi/scsi_transport_srp.c +2 -2

index 36f6190931bc..a9c1c991da35 100644
@@ -587,7 +587,7 @@ EXPORT_SYMBOL(srp_reconnect_rport);
  *
  * If a timeout occurs while an rport is in the blocked state, ask the SCSI
  * EH to continue waiting (BLK_EH_RESET_TIMER). Otherwise let the SCSI core
- * handle the timeout (BLK_EH_NOT_HANDLED).
+ * handle the timeout (BLK_EH_DONE).
  *
  * Note: This function is called from soft-IRQ context and with the request
  * queue lock held.

@@ -602,7 +602,7 @@ enum blk_eh_timer_return srp_timed_out(struct scsi_cmnd *scmd)
 	pr_debug("timeout for sdev %s\n", dev_name(&sdev->sdev_gendev));
 	return rport->fast_io_fail_tmo < 0 && rport->dev_loss_tmo < 0 &&
 		i->f->reset_timer_if_blocked && scsi_device_blocked(sdev) ?
-		BLK_EH_RESET_TIMER : BLK_EH_NOT_HANDLED;
+		BLK_EH_RESET_TIMER : BLK_EH_DONE;
 }
 EXPORT_SYMBOL(srp_timed_out);

drivers/scsi/ufs/ufshcd.c +3 -3

index 00e79057f870..d0a1674915a1 100644
@@ -6497,12 +6497,12 @@ static enum blk_eh_timer_return ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
 	bool found = false;
 
 	if (!scmd || !scmd->device || !scmd->device->host)
-		return BLK_EH_NOT_HANDLED;
+		return BLK_EH_DONE;
 
 	host = scmd->device->host;
 	hba = shost_priv(host);
 	if (!hba)
-		return BLK_EH_NOT_HANDLED;
+		return BLK_EH_DONE;
 
 	spin_lock_irqsave(host->host_lock, flags);

@@ -6520,7 +6520,7 @@ static enum blk_eh_timer_return ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
 	 * SCSI command was not actually dispatched to UFS driver, otherwise
 	 * let SCSI layer handle the error as usual.
 	 */
-	return found ? BLK_EH_NOT_HANDLED : BLK_EH_RESET_TIMER;
+	return found ? BLK_EH_DONE : BLK_EH_RESET_TIMER;
 }
 
 static const struct attribute_group *ufshcd_driver_groups[] = {

include/linux/blkdev.h +1 -1

index a1c05e85a443..f82e05df905b 100644
@@ -326,7 +326,7 @@ typedef int (init_rq_fn)(struct request_queue *, struct request *, gfp_t);
 typedef void (exit_rq_fn)(struct request_queue *, struct request *);
 
 enum blk_eh_timer_return {
-	BLK_EH_NOT_HANDLED,
+	BLK_EH_DONE,
 	BLK_EH_HANDLED,
 	BLK_EH_RESET_TIMER,
 };

include/scsi/scsi_host.h +1 -1

index 12f454cb6f61..53b485fe9b67 100644
@@ -307,7 +307,7 @@ struct scsi_host_template {
 	 * EH_HANDLED:		I fixed the error, please complete the command
 	 * EH_RESET_TIMER:	I need more time, reset the timer and
 	 *			begin counting again
-	 * EH_NOT_HANDLED	Begin normal error recovery
+	 * EH_DONE:		Begin normal error recovery
 	 *
 	 * Status: OPTIONAL
 	 */

Signature · 1 line

2.17.0

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #8 lore raw

PATCH 5/14 nvme: return BLK_EH_DONE from ->timeout

NVMe always completes the request before returning from ->timeout, either by polling for it, or by disabling the controller. Return BLK_EH_DONE so that the block layer doesn't even try to complete it again.

Signed-off-by Christoph Hellwig <[email protected]>

---
 drivers/nvme/host/pci.c    | 14 +++++---------
 drivers/nvme/host/rdma.c   |  2 +-
 drivers/nvme/target/loop.c |  2 +-
 3 files changed, 7 insertions(+), 11 deletions(-)

drivers/nvme/host/pci.c +5 -9

index 917e1714f7d9..31525324b79f 100644
@@ -1205,7 +1205,7 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
 		nvme_warn_reset(dev, csts);
 		nvme_dev_disable(dev, false);
 		nvme_reset_ctrl(&dev->ctrl);
-		return BLK_EH_HANDLED;
+		return BLK_EH_DONE;
 	}
 
 	/*

@@ -1215,14 +1215,14 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
 		dev_warn(dev->ctrl.device,
 			 "I/O %d QID %d timeout, completion polled\n",
 			 req->tag, nvmeq->qid);
-		return BLK_EH_HANDLED;
+		return BLK_EH_DONE;
 	}
 
 	/*
 	 * Shutdown immediately if controller times out while starting. The
 	 * reset work will see the pci device disabled when it gets the forced
 	 * cancellation error. All outstanding requests are completed on
-	 * shutdown, so we return BLK_EH_HANDLED.
+	 * shutdown, so we return BLK_EH_DONE.
 	 */
 	switch (dev->ctrl.state) {
 	case NVME_CTRL_CONNECTING:

@@ -1232,7 +1232,7 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
 			 req->tag, nvmeq->qid);
 		nvme_dev_disable(dev, false);
 		nvme_req(req)->flags |= NVME_REQ_CANCELLED;
-		return BLK_EH_HANDLED;
+		return BLK_EH_DONE;
 	default:
 		break;
 	}

@@ -1249,12 +1249,8 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
 		nvme_dev_disable(dev, false);
 		nvme_reset_ctrl(&dev->ctrl);
 
-		/*
-		 * Mark the request as handled, since the inline shutdown
-		 * forces all outstanding requests to complete.
-		 */
 		nvme_req(req)->flags |= NVME_REQ_CANCELLED;
-		return BLK_EH_HANDLED;
+		return BLK_EH_DONE;
 	}
 
 	if (atomic_dec_return(&dev->ctrl.abort_limit) < 0) {

drivers/nvme/host/rdma.c +1 -1

index 1eb4438a8763..ac7462cd7f0f 100644
@@ -1598,7 +1598,7 @@ nvme_rdma_timeout(struct request *rq, bool reserved)
 	/* fail with DNR on cmd timeout */
 	nvme_req(rq)->status = NVME_SC_ABORT_REQ | NVME_SC_DNR;
 
-	return BLK_EH_HANDLED;
+	return BLK_EH_DONE;
 }
 
 static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,

drivers/nvme/target/loop.c +1 -1

index 27a8561c0cb9..22e3627bf16b 100644
@@ -146,7 +146,7 @@ nvme_loop_timeout(struct request *rq, bool reserved)
 	/* fail with DNR on admin cmd timeout */
 	nvme_req(rq)->status = NVME_SC_ABORT_REQ | NVME_SC_DNR;
 
-	return BLK_EH_HANDLED;
+	return BLK_EH_DONE;
 }
 
 static blk_status_t nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx,

Signature · 1 line

2.17.0

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #9 lore raw

PATCH 6/14 nbd: complete requests from ->timeout

By completing the request entirely in the driver we can remove the BLK_EH_HANDLED return value and thus the split responsibility between the driver and the block layer that has been causing trouble.

Signed-off-by Christoph Hellwig <[email protected]>

---
 drivers/block/nbd.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

drivers/block/nbd.c +4 -3

index 88ae833aabe9..8860b24098bc 100644
@@ -304,7 +304,7 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req,
 
 	if (!refcount_inc_not_zero(&nbd->config_refs)) {
 		cmd->status = BLK_STS_TIMEOUT;
-		return BLK_EH_HANDLED;
+		goto done;
 	}
 	config = nbd->config;

@@ -342,8 +342,9 @@ static enum blk_eh_timer_return nbd_xmit_timeout(struct request *req,
 	cmd->status = BLK_STS_IOERR;
 	sock_shutdown(nbd);
 	nbd_config_put(nbd);
-
-	return BLK_EH_HANDLED;
+done:
+	blk_mq_complete_request(req);
+	return BLK_EH_DONE;
 }
 
 /*

Signature · 1 line

2.17.0

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #10 lore raw

PATCH 7/14 mtip32xx: complete requests from ->timeout

Signed-off-by Christoph Hellwig <[email protected]>

---
 drivers/block/mtip32xx/mtip32xx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/block/mtip32xx/mtip32xx.c +2 -1

index e873daca9d13..95657b814543 100644
@@ -3719,7 +3719,8 @@ static enum blk_eh_timer_return mtip_cmd_timeout(struct request *req,
 		struct mtip_cmd *cmd = blk_mq_rq_to_pdu(req);
 
 		cmd->status = BLK_STS_TIMEOUT;
-		return BLK_EH_HANDLED;
+		blk_mq_complete_request(req);
+		return BLK_EH_DONE;
 	}
 
 	if (test_bit(req->tag, dd->port->cmds_to_issue))

Signature · 1 line

2.17.0

Johannes Thumshirn

Reviewed-by on v1 7/14

2018-05-29 14:17 UTC #11 lore raw

Re: [PATCH 07/14] mtip32xx: complete requests from ->timeout

Looks good,

Signature · 6 lines

Johannes Thumshirn                                          Storage
[email protected]                                +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

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #12 lore raw

PATCH 8/14 null_blk: complete requests from ->timeout

Signed-off-by Christoph Hellwig <[email protected]>

---
 drivers/block/null_blk.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/block/null_blk.c +4 -2

index b4d368e3ddcd..2bdadd7f1454 100644
@@ -1365,7 +1365,8 @@ static blk_qc_t null_queue_bio(struct request_queue *q, struct bio *bio)
 static enum blk_eh_timer_return null_rq_timed_out_fn(struct request *rq)
 {
 	pr_info("null: rq %p timed out\n", rq);
-	return BLK_EH_HANDLED;
+	blk_mq_complete_request(rq);
+	return BLK_EH_DONE;
 }
 
 static int null_rq_prep_fn(struct request_queue *q, struct request *req)

@@ -1427,7 +1428,8 @@ static void null_request_fn(struct request_queue *q)
 static enum blk_eh_timer_return null_timeout_rq(struct request *rq, bool res)
 {
 	pr_info("null: rq %p timed out\n", rq);
-	return BLK_EH_HANDLED;
+	blk_mq_complete_request(rq);
+	return BLK_EH_DONE;
 }
 
 static blk_status_t null_queue_rq(struct blk_mq_hw_ctx *hctx,

Signature · 1 line

2.17.0

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #13 lore raw

PATCH 9/14 scsi_transport_fc: complete requests from ->timeout

Signed-off-by Christoph Hellwig <[email protected]>

---
 drivers/scsi/scsi_transport_fc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

drivers/scsi/scsi_transport_fc.c +3 -4

index 90075a0ddcfe..7a9a65588a1b 100644
@@ -3591,10 +3591,9 @@ fc_bsg_job_timeout(struct request *req)
 	}
 
 	/* the blk_end_sync_io() doesn't check the error */
-	if (!inflight)
-		return BLK_EH_DONE;
-	else
-		return BLK_EH_HANDLED;
+	if (inflight)
+		blk_mq_complete_request(req);
+	return BLK_EH_DONE;
 }
 
 /**

Signature · 1 line

2.17.0

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #14 lore raw

PATCH 10/14 mmc: complete requests from ->timeout

[While this keeps existing behavior it seems to mismatch the comment,
maintainers please chime in!]

Signed-off-by Christoph Hellwig <[email protected]>

---
 drivers/mmc/core/queue.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/mmc/core/queue.c +3 -2

index 56e9a803db21..648eb6743ed5 100644
@@ -111,8 +111,9 @@ static enum blk_eh_timer_return mmc_cqe_timed_out(struct request *req)
 				__mmc_cqe_recovery_notifier(mq);
 			return BLK_EH_RESET_TIMER;
 		}
-		/* No timeout */
-		return BLK_EH_HANDLED;
+		/* No timeout (XXX: huh? comment doesn't make much sense) */
+		blk_mq_complete_request(req);
+		return BLK_EH_DONE;
 	default:
 		/* Timeout is handled by mmc core */
 		return BLK_EH_RESET_TIMER;

Signature · 1 line

2.17.0

Shawn Lin

Reviewed-by on v1 10/14

2018-05-29 14:55 UTC #15 lore raw

Re: [PATCH 10/14] mmc: complete requests from ->timeout

[+ Ulf, Adrian, linux-mmc, as linux-mmc and folks are missing in the recipient list.]

Quoted 6 lines · Christoph Hellwig

By completing the request entirely in the driver we can remove the BLK_EH_HANDLED return value and thus the split responsibility between the driver and the block layer that has been causing trouble.
[While this keeps existing behavior it seems to mismatch the comment,
maintainers please chime in!]

The comment means the timeout doesn't happen for the current in-flight request which command queue is doing, and the request is finished previously(correct me if I'm wrong). So mmc defers the complete stuff to blk_timeout_work to help. Maybe the comment should gone as well.

Reviewed-by Shawn Lin <[email protected]>

Quoted 24 lines

Signed-off-by Christoph Hellwig <[email protected]>

---
  drivers/mmc/core/queue.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

drivers/mmc/core/queue.c +3 -2

index 56e9a803db21..648eb6743ed5 100644
@@ -111,8 +111,9 @@ static enum blk_eh_timer_return mmc_cqe_timed_out(struct request *req)
  				__mmc_cqe_recovery_notifier(mq);
  			return BLK_EH_RESET_TIMER;
  		}
-		/* No timeout */
-		return BLK_EH_HANDLED;
+		/* No timeout (XXX: huh? comment doesn't make much sense) */
+		blk_mq_complete_request(req);
+		return BLK_EH_DONE;
  	default:
  		/* Timeout is handled by mmc core */
  		return BLK_EH_RESET_TIMER;

Signature · 2 lines

Best Regards
Shawn Lin

Adrian Hunter

2018-05-30 08:09 UTC #16 lore raw

Re: [PATCH 10/14] mmc: complete requests from ->timeout

Quoted 25 lines · Shawn Lin

[+ Ulf, Adrian, linux-mmc, as linux-mmc and folks are missing in the recipient list.]
Quoted 6 lines · Christoph Hellwig
By completing the request entirely in the driver we can remove the BLK_EH_HANDLED return value and thus the split responsibility between the driver and the block layer that has been causing trouble.
[While this keeps existing behavior it seems to mismatch the comment, maintainers please chime in!]
The comment means the timeout doesn't happen for the current in-flight request which command queue is doing, and the request is finished previously(correct me if I'm wrong). So mmc defers the complete stuff to blk_timeout_work to help. Maybe the comment should gone as well.
Reviewed-by Shawn Lin <[email protected]>
Quoted 7 lines
Signed-off-by Christoph Hellwig <[email protected]>
Reviewed-by Hannes Reinecke <[email protected]>
---
drivers/mmc/core/queue.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

Quoted 13 lines

drivers/mmc/core/queue.c static enum blk_eh_timer_return · line 116

Quoted 13 lines

drivers/mmc/core/queue.c +2 -2

index 56e9a803db21..648eb6743ed5 100644
@@ -111,8 +111,9 @@ static enum blk_eh_timer_return
 mmc_cqe_timed_out(struct request *req)
                   __mmc_cqe_recovery_notifier(mq);
               return BLK_EH_RESET_TIMER;
           }
-        /* No timeout */
-        return BLK_EH_HANDLED;
+        /* No timeout (XXX: huh? comment doesn't make much sense) */
+        blk_mq_complete_request(req);

It seems the block layer timeout handler no longer castrates the completion path, in which case blk_mq_complete_request() is not needed here.

The "No timeout" comment can be removed.

Quoted 7 lines

Quoted 5 lines
drivers/mmc/core/queue.c +1 -0
+        return BLK_EH_DONE;
      default:
          /* Timeout is handled by mmc core */
          return BLK_EH_RESET_TIMER;

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #17 lore raw

PATCH 11/14 libiscsi: don't try to bypass SCSI EH

libiscsi is the only SCSI code that return BLK_EH_HANDLED, thus trying to bypass the normal SCSI EH code. We are going to remove this return value at the block layer, and at least from a quick look it doesn't look too harmful to try to send an abort for these cases, especially as the first one should not actually be possible. If this doesn't work out iscsi will probably need its own eh_strategy_handler instead to just do the right thing.

Signed-off-by Christoph Hellwig <[email protected]>

---
 drivers/scsi/libiscsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/scsi/libiscsi.c +2 -2

index eee43ba83a60..71bdc0b52cf9 100644
@@ -1982,7 +1982,7 @@ enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
 		 * Raced with completion. Blk layer has taken ownership
 		 * so let timeout code complete it now.
 		 */
-		rc = BLK_EH_HANDLED;
+		rc = BLK_EH_DONE;
 		goto done;
 	}

@@ -1997,7 +1997,7 @@ enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
 		if (unlikely(system_state != SYSTEM_RUNNING)) {
 			sc->result = DID_NO_CONNECT << 16;
 			ISCSI_DBG_EH(session, "sc on shutdown, handled\n");
-			rc = BLK_EH_HANDLED;
+			rc = BLK_EH_DONE;
 			goto done;
 		}
 		/*

Signature · 1 line

2.17.0

Lee Duncan

maintainer

2018-05-30 01:04 UTC #18 lore raw

Re: [PATCH 11/14] libiscsi: don't try to bypass SCSI EH

Quoted 29 lines · Christoph Hellwig

libiscsi is the only SCSI code that return BLK_EH_HANDLED, thus trying to bypass the normal SCSI EH code. We are going to remove this return value at the block layer, and at least from a quick look it doesn't look too harmful to try to send an abort for these cases, especially as the first one should not actually be possible. If this doesn't work out iscsi will probably need its own eh_strategy_handler instead to just do the right thing.
Signed-off-by Christoph Hellwig <[email protected]>
Reviewed-by Hannes Reinecke <[email protected]>
---
 drivers/scsi/libiscsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/libiscsi.c +1 -1
index eee43ba83a60..71bdc0b52cf9 100644
@@ -1982,7 +1982,7 @@ enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
 		 * Raced with completion. Blk layer has taken ownership
 		 * so let timeout code complete it now.
 		 */
-		rc = BLK_EH_HANDLED;
+		rc = BLK_EH_DONE;
 		goto done;
 	}
 

drivers/scsi/libiscsi.c enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc) · line 2003

Quoted 8 lines

drivers/scsi/libiscsi.c +1 -1

@@ -1997,7 +1997,7 @@ enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
 		if (unlikely(system_state != SYSTEM_RUNNING)) {
 			sc->result = DID_NO_CONNECT << 16;
 			ISCSI_DBG_EH(session, "sc on shutdown, handled\n");
-			rc = BLK_EH_HANDLED;
+			rc = BLK_EH_DONE;
 			goto done;
 		}
 		/*

looks good to me.

Signed-off-by Lee Duncan <[email protected]>

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #19 lore raw

PATCH 12/14 block: remove BLK_EH_HANDLED

Signed-off-by Christoph Hellwig <[email protected]>

---
 Documentation/scsi/scsi_eh.txt | 11 -----------
 block/blk-mq.c                 |  4 ----
 block/blk-timeout.c            |  3 ---
 include/linux/blkdev.h         |  1 -
 4 files changed, 19 deletions(-)

Documentation/scsi/scsi_eh.txt +0 ~~-11~~

index 3ae8419e72cf..1b7436932a2b 100644
@@ -82,17 +82,6 @@ function
  1. invokes optional hostt->eh_timed_out() callback.  Return value can
     be one of
 
-    - BLK_EH_HANDLED
-	This indicates that eh_timed_out() dealt with the timeout.
-	The command is passed back to the block layer and completed
-	via __blk_complete_requests().
-
-	*NOTE* After returning BLK_EH_HANDLED the SCSI layer is
-	assumed to be finished with the command, and no other
-	functions from the SCSI layer will be called. So this
-	should typically only be returned if the eh_timed_out()
-	handler raced with normal completion.
-
     - BLK_EH_RESET_TIMER
 	This indicates that more time is required to finish the
 	command.  Timer is restarted.  This action is counted as a

block/blk-mq.c +0 -4

index a7a601f08bf5..d898de8841a0 100644
@@ -777,10 +777,6 @@ static void blk_mq_rq_timed_out(struct request *req, bool reserved)
 		ret = ops->timeout(req, reserved);
 
 	switch (ret) {
-	case BLK_EH_HANDLED:
-		if (blk_mq_rq_state(req) == MQ_RQ_IN_FLIGHT)
-			__blk_mq_complete_request(req);
-		break;
 	case BLK_EH_RESET_TIMER:
 		blk_add_timer(req);
 		break;

block/blk-timeout.c +0 -3

index 11879e98c249..4b8a48d48ba1 100644
@@ -86,9 +86,6 @@ static void blk_rq_timed_out(struct request *req)
 	if (q->rq_timed_out_fn)
 		ret = q->rq_timed_out_fn(req);
 	switch (ret) {
-	case BLK_EH_HANDLED:
-		__blk_complete_request(req);
-		break;
 	case BLK_EH_RESET_TIMER:
 		blk_add_timer(req);
 		blk_clear_rq_complete(req);

include/linux/blkdev.h +0 -1

index f82e05df905b..d838a89639f2 100644
@@ -327,7 +327,6 @@ typedef void (exit_rq_fn)(struct request_queue *, struct request *);
 
 enum blk_eh_timer_return {
 	BLK_EH_DONE,
-	BLK_EH_HANDLED,
 	BLK_EH_RESET_TIMER,
 };

Signature · 1 line

2.17.0

Johannes Thumshirn

Reviewed-by on v1 12/14

2018-05-29 14:17 UTC #20 lore raw

Re: [PATCH 12/14] block: remove BLK_EH_HANDLED

Looks good,

Signature · 6 lines

Johannes Thumshirn                                          Storage
[email protected]                                +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

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #21 lore raw

PATCH 13/14 block: document the blk_eh_timer_return values

Signed-off-by Christoph Hellwig <[email protected]>

---
 include/linux/blkdev.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

include/linux/blkdev.h +2 -2

index d838a89639f2..9f06b29adaa4 100644
@@ -326,8 +326,8 @@ typedef int (init_rq_fn)(struct request_queue *, struct request *, gfp_t);
 typedef void (exit_rq_fn)(struct request_queue *, struct request *);
 
 enum blk_eh_timer_return {
-	BLK_EH_DONE,
-	BLK_EH_RESET_TIMER,
+	BLK_EH_DONE,		/* drivers has completed the command */
+	BLK_EH_RESET_TIMER,	/* reset timer and try again */
 };
 
 typedef enum blk_eh_timer_return (rq_timed_out_fn)(struct request *);

Signature · 1 line

2.17.0

Johannes Thumshirn

Reviewed-by on v1 13/14

2018-05-29 14:17 UTC #22 lore raw

Re: [PATCH 13/14] block: document the blk_eh_timer_return values

Looks good,

Signature · 6 lines

Johannes Thumshirn                                          Storage
[email protected]                                +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

Christoph Hellwig

author maintainer

2018-05-29 13:52 UTC #23 lore raw

PATCH 14/14 blk-mq: simplify blk_mq_rq_timed_out

Signed-off-by Christoph Hellwig <[email protected]>

---
 block/blk-mq.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

block/blk-mq.c +8 ~~-14~~

index d898de8841a0..ba6f30a67027 100644
@@ -770,22 +770,16 @@ EXPORT_SYMBOL(blk_mq_tag_to_rq);
 
 static void blk_mq_rq_timed_out(struct request *req, bool reserved)
 {
-	const struct blk_mq_ops *ops = req->q->mq_ops;
-	enum blk_eh_timer_return ret = BLK_EH_RESET_TIMER;
+	if (req->q->mq_ops->timeout) {
+		enum blk_eh_timer_return ret;
 
-	if (ops->timeout)
-		ret = ops->timeout(req, reserved);
-
-	switch (ret) {
-	case BLK_EH_RESET_TIMER:
-		blk_add_timer(req);
-		break;
-	case BLK_EH_DONE:
-		break;
-	default:
-		printk(KERN_ERR "block: bad eh return: %d\n", ret);
-		break;
+		ret = req->q->mq_ops->timeout(req, reserved);
+		if (ret == BLK_EH_DONE)
+			return;
+		WARN_ON_ONCE(ret != BLK_EH_RESET_TIMER);
 	}
+
+	blk_add_timer(req);
 }
 
 static bool blk_mq_req_expired(struct request *rq, unsigned long *next)

Signature · 1 line

2.17.0

Johannes Thumshirn

Reviewed-by on v1 14/14

2018-05-29 14:19 UTC #24 lore raw

Re: [PATCH 14/14] blk-mq: simplify blk_mq_rq_timed_out

Looks good,

Signature · 6 lines

Johannes Thumshirn                                          Storage
[email protected]                                +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

Jens Axboe

maintainer

2018-05-29 15:12 UTC #25 lore raw