[PATCH 0/9] dasd: implement block timeout

[PATCH 0/9] dasd: implement block timeout

[Hannes Reinecke]

This patch series implements a block timeout handler for
DASDs. The main impetus was to allow for a fixed upper
timeout value after which a request is aborted.
This is required eg when implementing a host-based
mirroring system where otherwise the entire mirror
would stall under certain circumstances.

Please apply.

Hannes Reinecke (9):
  dasd: Clarify comment
  dasd: make number of retries configurable
  dasd: process all requests in the device tasklet
  dasd: Implement block timeout handling
  dasd: Reduce amount of messages for specific errors
  dasd: detailed I/O errors
  block: check for timeout function in blk_rq_timed_out()
  dasd: Add 'timeout' attribute
  dasd: Fail all requests when DASD_FLAG_ABORTIO is set

 arch/s390/include/uapi/asm/dasd.h |    4 +
 block/blk-core.c                  |    3 +
 block/blk-timeout.c               |    5 +-
 drivers/s390/block/dasd.c         |  115 +++++++++++++++++++++++++++++++++----
 drivers/s390/block/dasd_devmap.c  |   97 +++++++++++++++++++++++++++++++
 drivers/s390/block/dasd_diag.c    |    8 ++-
 drivers/s390/block/dasd_eckd.c    |   15 ++++-
 drivers/s390/block/dasd_erp.c     |    8 +++
 drivers/s390/block/dasd_fba.c     |   10 +++-
 drivers/s390/block/dasd_int.h     |   10 +++
 drivers/s390/block/dasd_ioctl.c   |   59 +++++++++++++++++++
 11 files changed, 313 insertions(+), 21 deletions(-)

-- 
1.7.4.2

[PATCH 1/9] dasd: Clarify comment

[Hannes Reinecke]

dasd_cancel_req will never return 1, only 0.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/s390/block/dasd.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 29225e1..09ddf70 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2313,8 +2313,7 @@ int dasd_sleep_on_immediatly(struct dasd_ccw_req *cqr)
  * Cancels a request that was started with dasd_sleep_on_req.
  * This is useful to timeout requests. The request will be
  * terminated if it is currently in i/o.
- * Returns 1 if the request has been terminated.
- *	   0 if there was no need to terminate the request (not started yet)
+ * Returns 0 if request termination was successful
  *	   negative error code if termination failed
  * Cancellation of a request is an asynchronous operation! The calling
  * function has to wait until the request is properly returned via callback.
@@ -2351,7 +2350,6 @@ int dasd_cancel_req(struct dasd_ccw_req *cqr)
 	return rc;
 }
 
-
 /*
  * SECTION: Operations of the dasd_block layer.
  */
-- 
1.7.4.2

[PATCH 2/9] dasd: make number of retries configurable

[Hannes Reinecke]

Instead of having the number of retries hard-coded in the various
functions we should be using a default retry value, which can
be modified via sysfs.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/s390/block/dasd_devmap.c |   41 ++++++++++++++++++++++++++++++++++++++
 drivers/s390/block/dasd_diag.c   |    3 +-
 drivers/s390/block/dasd_eckd.c   |   11 ++++++---
 drivers/s390/block/dasd_fba.c    |    5 +++-
 drivers/s390/block/dasd_int.h    |    3 ++
 5 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/drivers/s390/block/dasd_devmap.c b/drivers/s390/block/dasd_devmap.c
index c196827..d237e31 100644
--- a/drivers/s390/block/dasd_devmap.c
+++ b/drivers/s390/block/dasd_devmap.c
@@ -1241,6 +1241,46 @@ dasd_expires_store(struct device *dev, struct device_attribute *attr,
 
 static DEVICE_ATTR(expires, 0644, dasd_expires_show, dasd_expires_store);
 
+static ssize_t
+dasd_retries_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct dasd_device *device;
+	int len;
+
+	device = dasd_device_from_cdev(to_ccwdev(dev));
+	if (IS_ERR(device))
+		return -ENODEV;
+	len = snprintf(buf, PAGE_SIZE, "%lu\n", device->default_retries);
+	dasd_put_device(device);
+	return len;
+}
+
+static ssize_t
+dasd_retries_store(struct device *dev, struct device_attribute *attr,
+		   const char *buf, size_t count)
+{
+	struct dasd_device *device;
+	unsigned long val;
+
+	device = dasd_device_from_cdev(to_ccwdev(dev));
+	if (IS_ERR(device))
+		return -ENODEV;
+
+	if ((strict_strtoul(buf, 10, &val) != 0) ||
+	    (val > DASD_RETRIES_MAX)) {
+		dasd_put_device(device);
+		return -EINVAL;
+	}
+
+	if (val)
+		device->default_retries = val;
+
+	dasd_put_device(device);
+	return count;
+}
+
+static DEVICE_ATTR(retries, 0644, dasd_retries_show, dasd_retries_store);
+
 static ssize_t dasd_reservation_policy_show(struct device *dev,
 					    struct device_attribute *attr,
 					    char *buf)
@@ -1351,6 +1391,7 @@ static struct attribute * dasd_attrs[] = {
 	&dev_attr_erplog.attr,
 	&dev_attr_failfast.attr,
 	&dev_attr_expires.attr,
+	&dev_attr_retries.attr,
 	&dev_attr_reservation_policy.attr,
 	&dev_attr_last_known_reservation_state.attr,
 	&dev_attr_safe_offline.attr,
diff --git a/drivers/s390/block/dasd_diag.c b/drivers/s390/block/dasd_diag.c
index 704488d..905b2c0 100644
--- a/drivers/s390/block/dasd_diag.c
+++ b/drivers/s390/block/dasd_diag.c
@@ -359,6 +359,7 @@ dasd_diag_check_device(struct dasd_device *device)
 	}
 
 	device->default_expires = DIAG_TIMEOUT;
+	device->default_retries = DIAG_MAX_RETRIES;
 
 	/* Figure out position of label block */
 	switch (private->rdc_data.vdev_class) {
@@ -555,7 +556,7 @@ static struct dasd_ccw_req *dasd_diag_build_cp(struct dasd_device *memdev,
 			recid++;
 		}
 	}
-	cqr->retries = DIAG_MAX_RETRIES;
+	cqr->retries = memdev->default_retries;
 	cqr->buildclk = get_clock();
 	if (blk_noretry_request(req) ||
 	    block->base->features & DASD_FEATURE_FAILFAST)
diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
index e37bc16..5b5b50b 100644
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -1679,6 +1679,9 @@ dasd_eckd_check_characteristics(struct dasd_device *device)
 
 	/* set default timeout */
 	device->default_expires = DASD_EXPIRES;
+	/* set default retry count */
+	device->default_retries = DASD_RETRIES;
+
 	if (private->gneq) {
 		value = 1;
 		for (i = 0; i < private->gneq->timeout.value; i++)
@@ -2529,7 +2532,7 @@ static struct dasd_ccw_req *dasd_eckd_build_cp_cmd_single(
 	cqr->block = block;
 	cqr->expires = startdev->default_expires * HZ;	/* default 5 minutes */
 	cqr->lpm = startdev->path_data.ppm;
-	cqr->retries = 256;
+	cqr->retries = startdev->default_retries;
 	cqr->buildclk = get_clock();
 	cqr->status = DASD_CQR_FILLED;
 	return cqr;
@@ -2704,7 +2707,7 @@ static struct dasd_ccw_req *dasd_eckd_build_cp_cmd_track(
 	cqr->block = block;
 	cqr->expires = startdev->default_expires * HZ;	/* default 5 minutes */
 	cqr->lpm = startdev->path_data.ppm;
-	cqr->retries = 256;
+	cqr->retries = startdev->default_retries;
 	cqr->buildclk = get_clock();
 	cqr->status = DASD_CQR_FILLED;
 	return cqr;
@@ -2997,7 +3000,7 @@ static struct dasd_ccw_req *dasd_eckd_build_cp_tpm_track(
 	cqr->block = block;
 	cqr->expires = startdev->default_expires * HZ;	/* default 5 minutes */
 	cqr->lpm = startdev->path_data.ppm;
-	cqr->retries = 256;
+	cqr->retries = startdev->default_retries;
 	cqr->buildclk = get_clock();
 	cqr->status = DASD_CQR_FILLED;
 	return cqr;
@@ -3200,7 +3203,7 @@ static struct dasd_ccw_req *dasd_raw_build_cp(struct dasd_device *startdev,
 	cqr->block = block;
 	cqr->expires = startdev->default_expires * HZ;
 	cqr->lpm = startdev->path_data.ppm;
-	cqr->retries = 256;
+	cqr->retries = startdev->default_retries;
 	cqr->buildclk = get_clock();
 	cqr->status = DASD_CQR_FILLED;
 
diff --git a/drivers/s390/block/dasd_fba.c b/drivers/s390/block/dasd_fba.c
index 4146985..3d8dc16 100644
--- a/drivers/s390/block/dasd_fba.c
+++ b/drivers/s390/block/dasd_fba.c
@@ -29,6 +29,8 @@
 #endif				/* PRINTK_HEADER */
 #define PRINTK_HEADER "dasd(fba):"
 
+#define FBA_DEFAULT_RETRIES 32
+
 #define DASD_FBA_CCW_WRITE 0x41
 #define DASD_FBA_CCW_READ 0x42
 #define DASD_FBA_CCW_LOCATE 0x43
@@ -167,6 +169,7 @@ dasd_fba_check_characteristics(struct dasd_device *device)
 	}
 
 	device->default_expires = DASD_EXPIRES;
+	device->default_retries = FBA_DEFAULT_RETRIES;
 	device->path_data.opm = LPM_ANYPATH;
 
 	readonly = dasd_device_is_ro(device);
@@ -369,7 +372,7 @@ static struct dasd_ccw_req *dasd_fba_build_cp(struct dasd_device * memdev,
 	cqr->memdev = memdev;
 	cqr->block = block;
 	cqr->expires = memdev->default_expires * HZ;	/* default 5 minutes */
-	cqr->retries = 32;
+	cqr->retries = memdev->default_retries;
 	cqr->buildclk = get_clock();
 	cqr->status = DASD_CQR_FILLED;
 	return cqr;
diff --git a/drivers/s390/block/dasd_int.h b/drivers/s390/block/dasd_int.h
index 899e3f5..375ba1f 100644
--- a/drivers/s390/block/dasd_int.h
+++ b/drivers/s390/block/dasd_int.h
@@ -224,6 +224,8 @@ struct dasd_ccw_req {
 /* default expiration time*/
 #define DASD_EXPIRES	  300
 #define DASD_EXPIRES_MAX  40000000
+#define DASD_RETRIES	  256
+#define DASD_RETRIES_MAX  32768
 
 /* per dasd_ccw_req flags */
 #define DASD_CQR_FLAGS_USE_ERP   0	/* use ERP for this request */
@@ -465,6 +467,7 @@ struct dasd_device {
 
 	/* default expiration time in s */
 	unsigned long default_expires;
+	unsigned long default_retries;
 
 	struct dentry *debugfs_dentry;
 	struct dasd_profile profile;
-- 
1.7.4.2

[PATCH 3/9] dasd: process all requests in the device tasklet

[Hannes Reinecke]

Originally the DASD device tasklet would process the entries on
the ccw_queue until the first non-final request was found.
Which was okay as long as all requests have the same retries and
expires parameter.
However, as we're now allowing to modify both it is possible to
have requests _after_ the first request which already have expired.
So we need to check all requests in the device tasklet.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/s390/block/dasd.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 09ddf70..20b7517 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -1778,11 +1778,11 @@ static void __dasd_device_process_ccw_queue(struct dasd_device *device,
 	list_for_each_safe(l, n, &device->ccw_queue) {
 		cqr = list_entry(l, struct dasd_ccw_req, devlist);
 
-		/* Stop list processing at the first non-final request. */
+		/* Skip any non-final request. */
 		if (cqr->status == DASD_CQR_QUEUED ||
 		    cqr->status == DASD_CQR_IN_IO ||
 		    cqr->status == DASD_CQR_CLEAR_PENDING)
-			break;
+			continue;
 		if (cqr->status == DASD_CQR_ERROR) {
 			__dasd_device_recovery(device, cqr);
 		}
-- 
1.7.4.2

[PATCH 4/9] dasd: Implement block timeout handling

[Hannes Reinecke]

This patch implements generic block layer timeout handling
callbacks for DASDs. When the timeout expires the respective
cqr is aborted.

With this timeout handler time-critical request abort
is guaranteed as the abort does not depend on the internal
state of the various DASD driver queues.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Stefan Weinhuber <wein@de.ibm.com>
---
 drivers/s390/block/dasd.c      |   76 ++++++++++++++++++++++++++++++++++++++++
 drivers/s390/block/dasd_diag.c |    5 ++-
 drivers/s390/block/dasd_eckd.c |    4 ++
 drivers/s390/block/dasd_fba.c  |    5 ++-
 4 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 20b7517..0406325 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2484,8 +2484,10 @@ static void __dasd_process_request_queue(struct dasd_block *block)
 		 */
 		cqr->callback_data = (void *) req;
 		cqr->status = DASD_CQR_FILLED;
+		req->completion_data = cqr;
 		blk_start_request(req);
 		list_add_tail(&cqr->blocklist, &block->ccw_queue);
+		INIT_LIST_HEAD(&cqr->devlist);
 		dasd_profile_start(block, cqr, req);
 	}
 }
@@ -2753,6 +2755,80 @@ static void do_dasd_request(struct request_queue *queue)
 }
 
 /*
+ * Block timeout callback, called from the block layer
+ *
+ * request_queue lock is held on entry.
+ *
+ * Return values:
+ * BLK_EH_RESET_TIMER if the request should be left running
+ * BLK_EH_NOT_HANDLED if the request is handled or terminated
+ *                    by the driver.
+ */
+enum blk_eh_timer_return dasd_times_out(struct request *req)
+{
+	struct dasd_ccw_req *cqr = req->completion_data;
+	struct dasd_block *block = req->q->queuedata;
+	struct dasd_device *device;
+	int rc = 0;
+
+	if (!cqr)
+		return BLK_EH_NOT_HANDLED;
+
+	device = cqr->startdev ? cqr->startdev : block->base;
+	DBF_DEV_EVENT(DBF_WARNING, device,
+		      " dasd_times_out cqr %p status %x",
+		      cqr, cqr->status);
+
+	spin_lock(&block->queue_lock);
+	spin_lock(get_ccwdev_lock(device->cdev));
+	cqr->retries = -1;
+	cqr->intrc = -ETIMEDOUT;
+	if (cqr->status >= DASD_CQR_QUEUED) {
+		spin_unlock(get_ccwdev_lock(device->cdev));
+		rc = dasd_cancel_req(cqr);
+	} else if (cqr->status == DASD_CQR_FILLED ||
+		   cqr->status == DASD_CQR_NEED_ERP) {
+		cqr->status = DASD_CQR_TERMINATED;
+		spin_unlock(get_ccwdev_lock(device->cdev));
+	} else if (cqr->status == DASD_CQR_IN_ERP) {
+		struct dasd_ccw_req *searchcqr, *nextcqr, *tmpcqr;
+
+		list_for_each_entry_safe(searchcqr, nextcqr,
+					 &block->ccw_queue, blocklist) {
+			tmpcqr = searchcqr;
+			while (tmpcqr->refers)
+				tmpcqr = tmpcqr->refers;
+			if (tmpcqr != cqr)
+				continue;
+			/* searchcqr is an ERP request for cqr */
+			searchcqr->retries = -1;
+			searchcqr->intrc = -ETIMEDOUT;
+			if (searchcqr->status >= DASD_CQR_QUEUED) {
+				spin_lock(get_ccwdev_lock(device->cdev));
+				rc = dasd_cancel_req(searchcqr);
+				spin_unlock(get_ccwdev_lock(device->cdev));
+			} else if ((searchcqr->status == DASD_CQR_FILLED) ||
+				   (searchcqr->status == DASD_CQR_NEED_ERP)) {
+				searchcqr->status = DASD_CQR_TERMINATED;
+				rc = 0;
+			} else if (searchcqr->status == DASD_CQR_IN_ERP) {
+				/*
+				 * Shouldn't happen; most recent ERP
+				 * request is at the front of queue
+				 */
+				continue;
+			}
+			break;
+		}
+		spin_unlock(get_ccwdev_lock(device->cdev));
+	}
+	dasd_schedule_block_bh(block);
+	spin_unlock(&block->queue_lock);
+
+	return rc ? BLK_EH_RESET_TIMER : BLK_EH_NOT_HANDLED;
+}
+
+/*
  * Allocate and initialize request queue and default I/O scheduler.
  */
 static int dasd_alloc_queue(struct dasd_block *block)
diff --git a/drivers/s390/block/dasd_diag.c b/drivers/s390/block/dasd_diag.c
index 905b2c0..9e245ce 100644
--- a/drivers/s390/block/dasd_diag.c
+++ b/drivers/s390/block/dasd_diag.c
@@ -583,7 +583,10 @@ dasd_diag_free_cp(struct dasd_ccw_req *cqr, struct request *req)
 
 static void dasd_diag_handle_terminated_request(struct dasd_ccw_req *cqr)
 {
-	cqr->status = DASD_CQR_FILLED;
+	if (cqr->retries < 0)
+		cqr->status = DASD_CQR_FAILED;
+	else
+		cqr->status = DASD_CQR_FILLED;
 };
 
 /* Fill in IOCTL data for device. */
diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
index 5b5b50b..137f18d 100644
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -2251,6 +2251,10 @@ dasd_eckd_format_device(struct dasd_device * device,
 
 static void dasd_eckd_handle_terminated_request(struct dasd_ccw_req *cqr)
 {
+	if (cqr->retries < 0) {
+		cqr->status = DASD_CQR_FAILED;
+		return;
+	}
 	cqr->status = DASD_CQR_FILLED;
 	if (cqr->block && (cqr->startdev != cqr->block->base)) {
 		dasd_eckd_reset_ccw_to_base_io(cqr);
diff --git a/drivers/s390/block/dasd_fba.c b/drivers/s390/block/dasd_fba.c
index 3d8dc16..9f0bff7 100644
--- a/drivers/s390/block/dasd_fba.c
+++ b/drivers/s390/block/dasd_fba.c
@@ -428,7 +428,10 @@ out:
 
 static void dasd_fba_handle_terminated_request(struct dasd_ccw_req *cqr)
 {
-	cqr->status = DASD_CQR_FILLED;
+	if (cqr->retries < 0)
+		cqr->status = DASD_CQR_FAILED;
+	else
+		cqr->status = DASD_CQR_FILLED;
 };
 
 static int
-- 
1.7.4.2

[PATCH 5/9] dasd: Reduce amount of messages for specific errors

[Hannes Reinecke]

Whenever a request has been aborted internally by the driver
there is no sense data to be had. And printing lots of messages
stalls the system, so better to print out a short one-liner.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/s390/block/dasd_erp.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/block/dasd_erp.c b/drivers/s390/block/dasd_erp.c
index d01ef82..908b9a0 100644
--- a/drivers/s390/block/dasd_erp.c
+++ b/drivers/s390/block/dasd_erp.c
@@ -159,6 +159,14 @@ dasd_log_sense(struct dasd_ccw_req *cqr, struct irb *irb)
 	struct dasd_device *device;
 
 	device = cqr->startdev;
+	if (cqr->intrc == -ETIMEDOUT) {
+		dev_err(&device->cdev->dev, "cqr %p timeout error", cqr);
+		return;
+	}
+	if (cqr->intrc == -ENOLINK) {
+		dev_err(&device->cdev->dev, "cqr %p transport error", cqr);
+		return;
+	}
 	/* dump sense data */
 	if (device->discipline && device->discipline->dump_sense)
 		device->discipline->dump_sense(device, cqr, irb);
-- 
1.7.4.2

[PATCH 6/9] dasd: detailed I/O errors

[Hannes Reinecke]

The DASD driver is using FASTFAIL as an equivalent to the
transport errors in SCSI. And the 'steal lock' function maps
roughly to a reservation error. So we should be returning the
appropriate error codes when completing a request.

Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/blk-core.c          |    3 +++
 drivers/s390/block/dasd.c |   16 +++++++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index c973249..d5579c2 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2291,6 +2291,9 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
 		case -EBADE:
 			error_type = "critical nexus";
 			break;
+		case -ETIMEDOUT:
+			error_type = "timeout";
+			break;
 		case -EIO:
 		default:
 			error_type = "I/O";
diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 0406325..d806b98 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2174,7 +2174,7 @@ static int _dasd_sleep_on(struct dasd_ccw_req *maincqr, int interruptible)
 		    test_bit(DASD_CQR_FLAGS_FAILFAST, &cqr->flags) &&
 		    (!dasd_eer_enabled(device))) {
 			cqr->status = DASD_CQR_FAILED;
-			cqr->intrc = -EAGAIN;
+			cqr->intrc = -ENOLINK;
 			continue;
 		}
 		/* Don't try to start requests if device is stopped */
@@ -2501,8 +2501,17 @@ static void __dasd_cleanup_cqr(struct dasd_ccw_req *cqr)
 	req = (struct request *) cqr->callback_data;
 	dasd_profile_end(cqr->block, cqr, req);
 	status = cqr->block->base->discipline->free_cp(cqr, req);
-	if (status <= 0)
-		error = status ? status : -EIO;
+	if (status < 0)
+		error = status;
+	else if (status == 0) {
+		if (cqr->intrc == -EPERM)
+			error = -EBADE;
+		else if (cqr->intrc == -ENOLINK ||
+			 cqr->intrc == -ETIMEDOUT)
+			error = cqr->intrc;
+		else
+			error = -EIO;
+	}
 	__blk_end_request_all(req, error);
 }
 
@@ -2603,6 +2612,7 @@ static void __dasd_block_start_head(struct dasd_block *block)
 		    test_bit(DASD_CQR_FLAGS_FAILFAST, &cqr->flags) &&
 		    (!dasd_eer_enabled(block->base))) {
 			cqr->status = DASD_CQR_FAILED;
+			cqr->intrc = -ENOLINK;
 			dasd_schedule_block_bh(block);
 			continue;
 		}
-- 
1.7.4.2

[PATCH 7/9] block: check for timeout function in blk_rq_timed_out()

[Hannes Reinecke]

rq_timed_out_fn might have been unset while the request
was in flight, so we need to check for it in blk_rq_timed_out().

Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/blk-timeout.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/blk-timeout.c b/block/blk-timeout.c
index 6e4744c..65f1035 100644
--- a/block/blk-timeout.c
+++ b/block/blk-timeout.c
@@ -82,9 +82,10 @@ void blk_delete_timer(struct request *req)
 static void blk_rq_timed_out(struct request *req)
 {
 	struct request_queue *q = req->q;
-	enum blk_eh_timer_return ret;
+	enum blk_eh_timer_return ret = BLK_EH_RESET_TIMER;
 
-	ret = q->rq_timed_out_fn(req);
+	if (q->rq_timed_out_fn)
+		ret = q->rq_timed_out_fn(req);
 	switch (ret) {
 	case BLK_EH_HANDLED:
 		__blk_complete_request(req);
-- 
1.7.4.2

[PATCH 8/9] dasd: Add 'timeout' attribute

[Hannes Reinecke]

This patch adds a 'timeout' attibute to the DASD driver.
When set to non-zero, the blk_timeout function will
be enabled with the timeout specified in the attribute.
Setting 'timeout' to '0' will disable block timeouts.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/s390/block/dasd.c        |    2 +
 drivers/s390/block/dasd_devmap.c |   56 ++++++++++++++++++++++++++++++++++++++
 drivers/s390/block/dasd_int.h    |    4 +++
 3 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index d806b98..61080df 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2785,6 +2785,8 @@ enum blk_eh_timer_return dasd_times_out(struct request *req)
 		return BLK_EH_NOT_HANDLED;
 
 	device = cqr->startdev ? cqr->startdev : block->base;
+	if (!device->blk_timeout)
+		return BLK_EH_RESET_TIMER;
 	DBF_DEV_EVENT(DBF_WARNING, device,
 		      " dasd_times_out cqr %p status %x",
 		      cqr, cqr->status);
diff --git a/drivers/s390/block/dasd_devmap.c b/drivers/s390/block/dasd_devmap.c
index d237e31..1ca85f6 100644
--- a/drivers/s390/block/dasd_devmap.c
+++ b/drivers/s390/block/dasd_devmap.c
@@ -1281,6 +1281,61 @@ dasd_retries_store(struct device *dev, struct device_attribute *attr,
 
 static DEVICE_ATTR(retries, 0644, dasd_retries_show, dasd_retries_store);
 
+static ssize_t
+dasd_timeout_show(struct device *dev, struct device_attribute *attr,
+		  char *buf)
+{
+	struct dasd_device *device;
+	int len;
+
+	device = dasd_device_from_cdev(to_ccwdev(dev));
+	if (IS_ERR(device))
+		return -ENODEV;
+	len = snprintf(buf, PAGE_SIZE, "%lu\n", device->blk_timeout);
+	dasd_put_device(device);
+	return len;
+}
+
+static ssize_t
+dasd_timeout_store(struct device *dev, struct device_attribute *attr,
+		   const char *buf, size_t count)
+{
+	struct dasd_device *device;
+	struct request_queue *q;
+	unsigned long val, flags;
+
+	device = dasd_device_from_cdev(to_ccwdev(dev));
+	if (IS_ERR(device))
+		return -ENODEV;
+
+	if ((strict_strtoul(buf, 10, &val) != 0) ||
+	    val > ULONG_MAX / HZ) {
+		dasd_put_device(device);
+		return -EINVAL;
+	}
+	q = device->block->request_queue;
+	if (!q) {
+		dasd_put_device(device);
+		return -ENODEV;
+	}
+	spin_lock_irqsave(&device->block->request_queue_lock, flags);
+	if (!val)
+		blk_queue_rq_timed_out(q, NULL);
+	else
+		blk_queue_rq_timed_out(q, dasd_times_out);
+
+	device->blk_timeout = val;
+
+	blk_queue_rq_timeout(q, device->blk_timeout * HZ);
+	spin_unlock_irqrestore(&device->block->request_queue_lock, flags);
+
+	dasd_put_device(device);
+	return count;
+}
+
+static DEVICE_ATTR(timeout, 0644,
+		   dasd_timeout_show, dasd_timeout_store);
+
 static ssize_t dasd_reservation_policy_show(struct device *dev,
 					    struct device_attribute *attr,
 					    char *buf)
@@ -1392,6 +1447,7 @@ static struct attribute * dasd_attrs[] = {
 	&dev_attr_failfast.attr,
 	&dev_attr_expires.attr,
 	&dev_attr_retries.attr,
+	&dev_attr_timeout.attr,
 	&dev_attr_reservation_policy.attr,
 	&dev_attr_last_known_reservation_state.attr,
 	&dev_attr_safe_offline.attr,
diff --git a/drivers/s390/block/dasd_int.h b/drivers/s390/block/dasd_int.h
index 375ba1f..d00d07a 100644
--- a/drivers/s390/block/dasd_int.h
+++ b/drivers/s390/block/dasd_int.h
@@ -469,6 +469,8 @@ struct dasd_device {
 	unsigned long default_expires;
 	unsigned long default_retries;
 
+	unsigned long blk_timeout;
+
 	struct dentry *debugfs_dentry;
 	struct dasd_profile profile;
 };
@@ -662,6 +664,8 @@ void dasd_free_device(struct dasd_device *);
 struct dasd_block *dasd_alloc_block(void);
 void dasd_free_block(struct dasd_block *);
 
+enum blk_eh_timer_return dasd_times_out(struct request *req);
+
 void dasd_enable_device(struct dasd_device *);
 void dasd_set_target_state(struct dasd_device *, int);
 void dasd_kick_device(struct dasd_device *);
-- 
1.7.4.2

[PATCH 9/9] dasd: Fail all requests when DASD_FLAG_ABORTIO is set

[Hannes Reinecke]

Whenever a DASD request encounters a timeout we might
need to abort all outstanding requests on this or
even other devices.

This is especially useful if one wants to fail all
devices on one side of a RAID10 configuration, even
though only one device exhibited an error.

To handle this I've introduced a new device flag
DASD_FLAG_ABORTIO.
This flag is evaluated in __dasd_process_request_queue()
and will invoke blk_abort_request() for all
outstanding requests with DASD_CQR_FLAGS_FAILFAST set.
This will cause any of these requests to be aborted
immediately if the blk_timeout function is activated.

The DASD_FLAG_ABORTIO is also evaluated in
__dasd_process_request_queue to abort all
new request which would have the
DASD_CQR_FLAGS_FAILFAST bit set.

The flag can be set with the new ioctls 'BIODASDABORTIO'
and removed with 'BIODASDALLOWIO'.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 arch/s390/include/uapi/asm/dasd.h |    4 ++
 drivers/s390/block/dasd.c         |   13 ++++++--
 drivers/s390/block/dasd_int.h     |    3 ++
 drivers/s390/block/dasd_ioctl.c   |   59 +++++++++++++++++++++++++++++++++++++
 4 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/arch/s390/include/uapi/asm/dasd.h b/arch/s390/include/uapi/asm/dasd.h
index 38eca3b..a2ea496 100644
--- a/arch/s390/include/uapi/asm/dasd.h
+++ b/arch/s390/include/uapi/asm/dasd.h
@@ -261,6 +261,10 @@ struct dasd_snid_ioctl_data {
 #define BIODASDQUIESCE _IO(DASD_IOCTL_LETTER,6) 
 /* Resume IO on device */
 #define BIODASDRESUME  _IO(DASD_IOCTL_LETTER,7) 
+/* Abort all I/O on a device */
+#define BIODASDABORTIO _IO(DASD_IOCTL_LETTER,240) 
+/* Allow I/O on a device */
+#define BIODASDALLOWIO  _IO(DASD_IOCTL_LETTER,241) 
 
 
 /* retrieve API version number */
diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 61080df..9f8dbbb 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -38,9 +38,6 @@
  */
 #define DASD_CHANQ_MAX_SIZE 4
 
-#define DASD_SLEEPON_START_TAG	(void *) 1
-#define DASD_SLEEPON_END_TAG	(void *) 2
-
 /*
  * SECTION: exported variables of dasd.c
  */
@@ -2446,6 +2443,16 @@ static void __dasd_process_request_queue(struct dasd_block *block)
 			__blk_end_request_all(req, -EIO);
 			continue;
 		}
+		if (test_bit(DASD_FLAG_ABORTALL, &basedev->flags) &&
+		    (basedev->features & DASD_FEATURE_FAILFAST ||
+		     blk_noretry_request(req))) {
+			DBF_DEV_EVENT(DBF_ERR, basedev,
+				      "Rejecting failfast request %p",
+				      req);
+			blk_start_request(req);
+			__blk_end_request_all(req, -ETIMEDOUT);
+			continue;
+		}
 		cqr = basedev->discipline->build_cp(basedev, block, req);
 		if (IS_ERR(cqr)) {
 			if (PTR_ERR(cqr) == -EBUSY)
diff --git a/drivers/s390/block/dasd_int.h b/drivers/s390/block/dasd_int.h
index d00d07a..a4f3050 100644
--- a/drivers/s390/block/dasd_int.h
+++ b/drivers/s390/block/dasd_int.h
@@ -523,7 +523,10 @@ struct dasd_block {
 #define DASD_FLAG_SUSPENDED	9	/* The device was suspended */
 #define DASD_FLAG_SAFE_OFFLINE	10	/* safe offline processing requested*/
 #define DASD_FLAG_SAFE_OFFLINE_RUNNING	11	/* safe offline running */
+#define DASD_FLAG_ABORTALL	12	/* Abort all noretry requests */
 
+#define DASD_SLEEPON_START_TAG	(void *) 1
+#define DASD_SLEEPON_END_TAG	(void *) 2
 
 void dasd_put_device_wake(struct dasd_device *);
 
diff --git a/drivers/s390/block/dasd_ioctl.c b/drivers/s390/block/dasd_ioctl.c
index 03c0e04..d59bdb6 100644
--- a/drivers/s390/block/dasd_ioctl.c
+++ b/drivers/s390/block/dasd_ioctl.c
@@ -141,6 +141,59 @@ static int dasd_ioctl_resume(struct dasd_block *block)
 }
 
 /*
+ * Abort all failfast I/O on a device.
+ */
+static int dasd_ioctl_abortio(struct dasd_block *block)
+{
+	unsigned long flags;
+	struct dasd_device *base;
+	struct dasd_ccw_req *cqr, *n;
+
+	base = block->base;
+	if (!capable (CAP_SYS_ADMIN))
+		return -EACCES;
+
+	if (test_and_set_bit(DASD_FLAG_ABORTALL, &base->flags))
+		return 0;
+	DBF_DEV_EVENT(DBF_NOTICE, base, "%s", "abortall flag set");
+
+	spin_lock_irqsave(&block->request_queue_lock, flags);
+	spin_lock(&block->queue_lock);
+	list_for_each_entry_safe(cqr, n, &block->ccw_queue, blocklist) {
+		if (test_bit(DASD_CQR_FLAGS_FAILFAST, &cqr->flags) &&
+		    cqr->callback_data &&
+		    cqr->callback_data != DASD_SLEEPON_START_TAG &&
+		    cqr->callback_data != DASD_SLEEPON_END_TAG ) {
+			spin_unlock(&block->queue_lock);
+			blk_abort_request(cqr->callback_data);
+			spin_lock(&block->queue_lock);
+		}
+	}
+	spin_unlock(&block->queue_lock);
+	spin_unlock_irqrestore(&block->request_queue_lock, flags);
+
+	dasd_schedule_block_bh(block);
+	return 0;
+}
+
+/*
+ * Allow I/O on a device
+ */
+static int dasd_ioctl_allowio(struct dasd_block *block)
+{
+	struct dasd_device *base;
+
+	base = block->base;
+	if (!capable (CAP_SYS_ADMIN))
+		return -EACCES;
+
+	if (test_and_clear_bit(DASD_FLAG_ABORTALL, &base->flags))
+		DBF_DEV_EVENT(DBF_NOTICE, base, "%s", "abortall flag unset");
+
+	return 0;
+}
+
+/*
  * performs formatting of _device_ according to _fdata_
  * Note: The discipline's format_function is assumed to deliver formatting
  * commands to format a single unit of the device. In terms of the ECKD
@@ -469,6 +522,12 @@ int dasd_ioctl(struct block_device *bdev, fmode_t mode,
 	case BIODASDRESUME:
 		rc = dasd_ioctl_resume(block);
 		break;
+	case BIODASDABORTIO:
+		rc = dasd_ioctl_abortio(block);
+		break;
+	case BIODASDALLOWIO:
+		rc = dasd_ioctl_allowio(block);
+		break;
 	case BIODASDFMT:
 		rc = dasd_ioctl_format(bdev, argp);
 		break;
-- 
1.7.4.2

Re: [PATCH 4/9] dasd: Implement block timeout handling

[Heiko Carstens]

On Tue, Jan 29, 2013 at 08:11:56AM +0100, Hannes Reinecke wrote:
> This patch implements generic block layer timeout handling
> callbacks for DASDs. When the timeout expires the respective
> cqr is aborted.
> 
> With this timeout handler time-critical request abort
> is guaranteed as the abort does not depend on the internal
> state of the various DASD driver queues.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Acked-by: Stefan Weinhuber <wein@de.ibm.com>

[...]

> +enum blk_eh_timer_return dasd_times_out(struct request *req)
> +{
> +	struct dasd_ccw_req *cqr = req->completion_data;
> +	struct dasd_block *block = req->q->queuedata;
> +	struct dasd_device *device;
> +	int rc = 0;
> +
> +	if (!cqr)
> +		return BLK_EH_NOT_HANDLED;
> +
> +	device = cqr->startdev ? cqr->startdev : block->base;
> +	DBF_DEV_EVENT(DBF_WARNING, device,
> +		      " dasd_times_out cqr %p status %x",
> +		      cqr, cqr->status);
> +
> +	spin_lock(&block->queue_lock);
> +	spin_lock(get_ccwdev_lock(device->cdev));

        ^^^

> +	cqr->retries = -1;
> +	cqr->intrc = -ETIMEDOUT;
> +	if (cqr->status >= DASD_CQR_QUEUED) {
> +		spin_unlock(get_ccwdev_lock(device->cdev));
> +		rc = dasd_cancel_req(cqr);
> +	} else if (cqr->status == DASD_CQR_FILLED ||
> +		   cqr->status == DASD_CQR_NEED_ERP) {
> +		cqr->status = DASD_CQR_TERMINATED;
> +		spin_unlock(get_ccwdev_lock(device->cdev));
> +	} else if (cqr->status == DASD_CQR_IN_ERP) {
> +		struct dasd_ccw_req *searchcqr, *nextcqr, *tmpcqr;
> +
> +		list_for_each_entry_safe(searchcqr, nextcqr,
> +					 &block->ccw_queue, blocklist) {
> +			tmpcqr = searchcqr;
> +			while (tmpcqr->refers)
> +				tmpcqr = tmpcqr->refers;
> +			if (tmpcqr != cqr)
> +				continue;
> +			/* searchcqr is an ERP request for cqr */
> +			searchcqr->retries = -1;
> +			searchcqr->intrc = -ETIMEDOUT;
> +			if (searchcqr->status >= DASD_CQR_QUEUED) {
> +				spin_lock(get_ccwdev_lock(device->cdev));

				^^^
This looks like a potential dead lock.

Re: [PATCH 4/9] dasd: Implement block timeout handling

[Hannes Reinecke]

On 01/29/2013 12:32 PM, Heiko Carstens wrote:
> On Tue, Jan 29, 2013 at 08:11:56AM +0100, Hannes Reinecke wrote:
>> This patch implements generic block layer timeout handling
>> callbacks for DASDs. When the timeout expires the respective
>> cqr is aborted.
>>
>> With this timeout handler time-critical request abort
>> is guaranteed as the abort does not depend on the internal
>> state of the various DASD driver queues.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> Acked-by: Stefan Weinhuber <wein@de.ibm.com>
>
> [...]
>
>> +enum blk_eh_timer_return dasd_times_out(struct request *req)
>> +{
>> +	struct dasd_ccw_req *cqr = req->completion_data;
>> +	struct dasd_block *block = req->q->queuedata;
>> +	struct dasd_device *device;
>> +	int rc = 0;
>> +
>> +	if (!cqr)
>> +		return BLK_EH_NOT_HANDLED;
>> +
>> +	device = cqr->startdev ? cqr->startdev : block->base;
>> +	DBF_DEV_EVENT(DBF_WARNING, device,
>> +		      " dasd_times_out cqr %p status %x",
>> +		      cqr, cqr->status);
>> +
>> +	spin_lock(&block->queue_lock);
>> +	spin_lock(get_ccwdev_lock(device->cdev));
>
>          ^^^
>
>> +	cqr->retries = -1;
>> +	cqr->intrc = -ETIMEDOUT;
>> +	if (cqr->status >= DASD_CQR_QUEUED) {
>> +		spin_unlock(get_ccwdev_lock(device->cdev));
>> +		rc = dasd_cancel_req(cqr);
>> +	} else if (cqr->status == DASD_CQR_FILLED ||
>> +		   cqr->status == DASD_CQR_NEED_ERP) {
>> +		cqr->status = DASD_CQR_TERMINATED;
>> +		spin_unlock(get_ccwdev_lock(device->cdev));
>> +	} else if (cqr->status == DASD_CQR_IN_ERP) {
>> +		struct dasd_ccw_req *searchcqr, *nextcqr, *tmpcqr;
>> +
>> +		list_for_each_entry_safe(searchcqr, nextcqr,
>> +					 &block->ccw_queue, blocklist) {
>> +			tmpcqr = searchcqr;
>> +			while (tmpcqr->refers)
>> +				tmpcqr = tmpcqr->refers;
>> +			if (tmpcqr != cqr)
>> +				continue;
>> +			/* searchcqr is an ERP request for cqr */
>> +			searchcqr->retries = -1;
>> +			searchcqr->intrc = -ETIMEDOUT;
>> +			if (searchcqr->status >= DASD_CQR_QUEUED) {
>> +				spin_lock(get_ccwdev_lock(device->cdev));
>
> 				^^^
> This looks like a potential dead lock.
>
Right.

Will be resending the patch.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH 8/9] dasd: Add 'timeout' attribute

[Heiko Carstens]

On Tue, Jan 29, 2013 at 08:12:00AM +0100, Hannes Reinecke wrote:
> This patch adds a 'timeout' attibute to the DASD driver.
> When set to non-zero, the blk_timeout function will
> be enabled with the timeout specified in the attribute.
> Setting 'timeout' to '0' will disable block timeouts.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>

[...]

> +static ssize_t
> +dasd_timeout_store(struct device *dev, struct device_attribute *attr,
> +		   const char *buf, size_t count)
> +{
> +	struct dasd_device *device;
> +	struct request_queue *q;
> +	unsigned long val, flags;
> +
> +	device = dasd_device_from_cdev(to_ccwdev(dev));
> +	if (IS_ERR(device))
> +		return -ENODEV;
> +
> +	if ((strict_strtoul(buf, 10, &val) != 0) ||
> +	    val > ULONG_MAX / HZ) {

Probably this should be UINT_MAX instead of ULONG_MAX, otherwise it
might overflow since blk_queue_rq_timeout(...) expects only an
unsigned int.


> +		dasd_put_device(device);
> +		return -EINVAL;
> +	}
> +	q = device->block->request_queue;
> +	if (!q) {
> +		dasd_put_device(device);
> +		return -ENODEV;
> +	}
> +	spin_lock_irqsave(&device->block->request_queue_lock, flags);
> +	if (!val)
> +		blk_queue_rq_timed_out(q, NULL);
> +	else
> +		blk_queue_rq_timed_out(q, dasd_times_out);
> +
> +	device->blk_timeout = val;
> +
> +	blk_queue_rq_timeout(q, device->blk_timeout * HZ);
> +	spin_unlock_irqrestore(&device->block->request_queue_lock, flags);
> +
> +	dasd_put_device(device);
> +	return count;
> +}

Re: [PATCH 8/9] dasd: Add 'timeout' attribute

[Hannes Reinecke]

On 01/29/2013 12:36 PM, Heiko Carstens wrote:
> On Tue, Jan 29, 2013 at 08:12:00AM +0100, Hannes Reinecke wrote:
>> This patch adds a 'timeout' attibute to the DASD driver.
>> When set to non-zero, the blk_timeout function will
>> be enabled with the timeout specified in the attribute.
>> Setting 'timeout' to '0' will disable block timeouts.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>
> [...]
>
>> +static ssize_t
>> +dasd_timeout_store(struct device *dev, struct device_attribute *attr,
>> +		   const char *buf, size_t count)
>> +{
>> +	struct dasd_device *device;
>> +	struct request_queue *q;
>> +	unsigned long val, flags;
>> +
>> +	device = dasd_device_from_cdev(to_ccwdev(dev));
>> +	if (IS_ERR(device))
>> +		return -ENODEV;
>> +
>> +	if ((strict_strtoul(buf, 10, &val) != 0) ||
>> +	    val > ULONG_MAX / HZ) {
>
> Probably this should be UINT_MAX instead of ULONG_MAX, otherwise it
> might overflow since blk_queue_rq_timeout(...) expects only an
> unsigned int.
>
Ok, will be fixing it up.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH 8/9] dasd: Add 'timeout' attribute

[Stefan Weinhuber]

Hannes Reinecke <hare@suse.de> wrote on 2013-01-29 08:12:00:
[..]
> +static ssize_t
> +dasd_timeout_store(struct device *dev, struct device_attribute *attr,
> +         const char *buf, size_t count)
> +{
> +   struct dasd_device *device;
> +   struct request_queue *q;
> +   unsigned long val, flags;
> +
> +   device = dasd_device_from_cdev(to_ccwdev(dev));
> +   if (IS_ERR(device))
> +      return -ENODEV;
> +
> +   if ((strict_strtoul(buf, 10, &val) != 0) ||
> +       val > ULONG_MAX / HZ) {
> +      dasd_put_device(device);
> +      return -EINVAL;
> +   }
> +   q = device->block->request_queue;

You need to check device->block before using it.
For alias devices the device->block pointer is NULL, as an alias device
has no fixed association with a block device.

> +   if (!q) {
> +      dasd_put_device(device);
> +      return -ENODEV;
> +   }
> +   spin_lock_irqsave(&device->block->request_queue_lock, flags);
> +   if (!val)
> +      blk_queue_rq_timed_out(q, NULL);
> +   else
> +      blk_queue_rq_timed_out(q, dasd_times_out);
> +
> +   device->blk_timeout = val;
> +
> +   blk_queue_rq_timeout(q, device->blk_timeout * HZ);
> +   spin_unlock_irqrestore(&device->block->request_queue_lock, flags);
> +
> +   dasd_put_device(device);
> +   return count;
> +}
> +
[..]

Mit freundlichen Grüßen / Kind regards
 
Stefan Weinhuber

-- 
Linux for zSeries kernel development
IBM Systems &Technology Group, Systems Software Development / SW Linux für 
zSeries Entwicklung

IBM Deutschland
Schoenaicher Str. 220
71032 Boeblingen
Phone: +49-7031-16-4018
E-Mail: wein@de.ibm.com

IBM Deutschland Research & Development GmbH / Vorsitzender des 
Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, 
HRB 243294

Re: [PATCH 8/9] dasd: Add 'timeout' attribute

[Hannes Reinecke]

On 01/29/2013 04:17 PM, Stefan Weinhuber wrote:
> Hannes Reinecke <hare@suse.de> wrote on 2013-01-29 08:12:00:
> [..]
>> +static ssize_t
>> +dasd_timeout_store(struct device *dev, struct device_attribute *attr,
>> +         const char *buf, size_t count)
>> +{
>> +   struct dasd_device *device;
>> +   struct request_queue *q;
>> +   unsigned long val, flags;
>> +
>> +   device = dasd_device_from_cdev(to_ccwdev(dev));
>> +   if (IS_ERR(device))
>> +      return -ENODEV;
>> +
>> +   if ((strict_strtoul(buf, 10, &val) != 0) ||
>> +       val > ULONG_MAX / HZ) {
>> +      dasd_put_device(device);
>> +      return -EINVAL;
>> +   }
>> +   q = device->block->request_queue;
>
> You need to check device->block before using it.
> For alias devices the device->block pointer is NULL, as an alias device
> has no fixed association with a block device.
>
Ah. You live and learn.

Okay, will be fixing it up.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

[PATCH 4/9] dasd: Implement block timeout handling

[Martin Schwidefsky]

From: Hannes Reinecke <hare@suse.de>

This patch implements generic block layer timeout handling
callbacks for DASDs. When the timeout expires the respective
cqr is aborted.

With this timeout handler time-critical request abort
is guaranteed as the abort does not depend on the internal
state of the various DASD driver queues.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Stefan Weinhuber <wein@de.ibm.com>
Signed-off-by: Stefan Weinhuber <wein@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
 drivers/s390/block/dasd.c      |   76 ++++++++++++++++++++++++++++++++++++++++
 drivers/s390/block/dasd_diag.c |    5 ++-
 drivers/s390/block/dasd_eckd.c |    4 +++
 drivers/s390/block/dasd_fba.c  |    5 ++-
 4 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 000e514..87478be 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2573,8 +2573,10 @@ static void __dasd_process_request_queue(struct dasd_block *block)
 		 */
 		cqr->callback_data = (void *) req;
 		cqr->status = DASD_CQR_FILLED;
+		req->completion_data = cqr;
 		blk_start_request(req);
 		list_add_tail(&cqr->blocklist, &block->ccw_queue);
+		INIT_LIST_HEAD(&cqr->devlist);
 		dasd_profile_start(block, cqr, req);
 	}
 }
@@ -2862,6 +2864,80 @@ static void do_dasd_request(struct request_queue *queue)
 }
 
 /*
+ * Block timeout callback, called from the block layer
+ *
+ * request_queue lock is held on entry.
+ *
+ * Return values:
+ * BLK_EH_RESET_TIMER if the request should be left running
+ * BLK_EH_NOT_HANDLED if the request is handled or terminated
+ *		      by the driver.
+ */
+enum blk_eh_timer_return dasd_times_out(struct request *req)
+{
+	struct dasd_ccw_req *cqr = req->completion_data;
+	struct dasd_block *block = req->q->queuedata;
+	struct dasd_device *device;
+	int rc = 0;
+
+	if (!cqr)
+		return BLK_EH_NOT_HANDLED;
+
+	device = cqr->startdev ? cqr->startdev : block->base;
+	DBF_DEV_EVENT(DBF_WARNING, device,
+		      " dasd_times_out cqr %p status %x",
+		      cqr, cqr->status);
+
+	spin_lock(&block->queue_lock);
+	spin_lock(get_ccwdev_lock(device->cdev));
+	cqr->retries = -1;
+	cqr->intrc = -ETIMEDOUT;
+	if (cqr->status >= DASD_CQR_QUEUED) {
+		spin_unlock(get_ccwdev_lock(device->cdev));
+		rc = dasd_cancel_req(cqr);
+	} else if (cqr->status == DASD_CQR_FILLED ||
+		   cqr->status == DASD_CQR_NEED_ERP) {
+		cqr->status = DASD_CQR_TERMINATED;
+		spin_unlock(get_ccwdev_lock(device->cdev));
+	} else if (cqr->status == DASD_CQR_IN_ERP) {
+		struct dasd_ccw_req *searchcqr, *nextcqr, *tmpcqr;
+
+		list_for_each_entry_safe(searchcqr, nextcqr,
+					 &block->ccw_queue, blocklist) {
+			tmpcqr = searchcqr;
+			while (tmpcqr->refers)
+				tmpcqr = tmpcqr->refers;
+			if (tmpcqr != cqr)
+				continue;
+			/* searchcqr is an ERP request for cqr */
+			searchcqr->retries = -1;
+			searchcqr->intrc = -ETIMEDOUT;
+			if (searchcqr->status >= DASD_CQR_QUEUED) {
+				spin_unlock(get_ccwdev_lock(device->cdev));
+				rc = dasd_cancel_req(searchcqr);
+				spin_lock(get_ccwdev_lock(device->cdev));
+			} else if ((searchcqr->status == DASD_CQR_FILLED) ||
+				   (searchcqr->status == DASD_CQR_NEED_ERP)) {
+				searchcqr->status = DASD_CQR_TERMINATED;
+				rc = 0;
+			} else if (searchcqr->status == DASD_CQR_IN_ERP) {
+				/*
+				 * Shouldn't happen; most recent ERP
+				 * request is at the front of queue
+				 */
+				continue;
+			}
+			break;
+		}
+		spin_unlock(get_ccwdev_lock(device->cdev));
+	}
+	dasd_schedule_block_bh(block);
+	spin_unlock(&block->queue_lock);
+
+	return rc ? BLK_EH_RESET_TIMER : BLK_EH_NOT_HANDLED;
+}
+
+/*
  * Allocate and initialize request queue and default I/O scheduler.
  */
 static int dasd_alloc_queue(struct dasd_block *block)
diff --git a/drivers/s390/block/dasd_diag.c b/drivers/s390/block/dasd_diag.c
index 1548422..feca317 100644
--- a/drivers/s390/block/dasd_diag.c
+++ b/drivers/s390/block/dasd_diag.c
@@ -583,7 +583,10 @@ dasd_diag_free_cp(struct dasd_ccw_req *cqr, struct request *req)
 
 static void dasd_diag_handle_terminated_request(struct dasd_ccw_req *cqr)
 {
-	cqr->status = DASD_CQR_FILLED;
+	if (cqr->retries < 0)
+		cqr->status = DASD_CQR_FAILED;
+	else
+		cqr->status = DASD_CQR_FILLED;
 };
 
 /* Fill in IOCTL data for device. */
diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
index f4315dc..e25a817 100644
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -2381,6 +2381,10 @@ sleep:
 
 static void dasd_eckd_handle_terminated_request(struct dasd_ccw_req *cqr)
 {
+	if (cqr->retries < 0) {
+		cqr->status = DASD_CQR_FAILED;
+		return;
+	}
 	cqr->status = DASD_CQR_FILLED;
 	if (cqr->block && (cqr->startdev != cqr->block->base)) {
 		dasd_eckd_reset_ccw_to_base_io(cqr);
diff --git a/drivers/s390/block/dasd_fba.c b/drivers/s390/block/dasd_fba.c
index d232c83..9cbc8c3 100644
--- a/drivers/s390/block/dasd_fba.c
+++ b/drivers/s390/block/dasd_fba.c
@@ -428,7 +428,10 @@ out:
 
 static void dasd_fba_handle_terminated_request(struct dasd_ccw_req *cqr)
 {
-	cqr->status = DASD_CQR_FILLED;
+	if (cqr->retries < 0)
+		cqr->status = DASD_CQR_FAILED;
+	else
+		cqr->status = DASD_CQR_FILLED;
 };
 
 static int
-- 
1.7.9.5

[PATCH 4/9] dasd: Implement block timeout handling

[Hannes Reinecke]

This patch implements generic block layer timeout handling
callbacks for DASDs. When the timeout expires the respective
cqr is aborted.

With this timeout handler time-critical request abort
is guaranteed as the abort does not depend on the internal
state of the various DASD driver queues.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Stefan Weinhuber <wein@de.ibm.com>
---
 drivers/s390/block/dasd.c      |   76 ++++++++++++++++++++++++++++++++++++++++
 drivers/s390/block/dasd_diag.c |    5 ++-
 drivers/s390/block/dasd_eckd.c |    4 ++
 drivers/s390/block/dasd_fba.c  |    5 ++-
 4 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 20b7517..98f62e9 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2484,8 +2484,10 @@ static void __dasd_process_request_queue(struct dasd_block *block)
 		 */
 		cqr->callback_data = (void *) req;
 		cqr->status = DASD_CQR_FILLED;
+		req->completion_data = cqr;
 		blk_start_request(req);
 		list_add_tail(&cqr->blocklist, &block->ccw_queue);
+		INIT_LIST_HEAD(&cqr->devlist);
 		dasd_profile_start(block, cqr, req);
 	}
 }
@@ -2753,6 +2755,80 @@ static void do_dasd_request(struct request_queue *queue)
 }
 
 /*
+ * Block timeout callback, called from the block layer
+ *
+ * request_queue lock is held on entry.
+ *
+ * Return values:
+ * BLK_EH_RESET_TIMER if the request should be left running
+ * BLK_EH_NOT_HANDLED if the request is handled or terminated
+ *                    by the driver.
+ */
+enum blk_eh_timer_return dasd_times_out(struct request *req)
+{
+	struct dasd_ccw_req *cqr = req->completion_data;
+	struct dasd_block *block = req->q->queuedata;
+	struct dasd_device *device;
+	int rc = 0;
+
+	if (!cqr)
+		return BLK_EH_NOT_HANDLED;
+
+	device = cqr->startdev ? cqr->startdev : block->base;
+	DBF_DEV_EVENT(DBF_WARNING, device,
+		      " dasd_times_out cqr %p status %x",
+		      cqr, cqr->status);
+
+	spin_lock(&block->queue_lock);
+	spin_lock(get_ccwdev_lock(device->cdev));
+	cqr->retries = -1;
+	cqr->intrc = -ETIMEDOUT;
+	if (cqr->status >= DASD_CQR_QUEUED) {
+		spin_unlock(get_ccwdev_lock(device->cdev));
+		rc = dasd_cancel_req(cqr);
+	} else if (cqr->status == DASD_CQR_FILLED ||
+		   cqr->status == DASD_CQR_NEED_ERP) {
+		cqr->status = DASD_CQR_TERMINATED;
+		spin_unlock(get_ccwdev_lock(device->cdev));
+	} else if (cqr->status == DASD_CQR_IN_ERP) {
+		struct dasd_ccw_req *searchcqr, *nextcqr, *tmpcqr;
+
+		list_for_each_entry_safe(searchcqr, nextcqr,
+					 &block->ccw_queue, blocklist) {
+			tmpcqr = searchcqr;
+			while (tmpcqr->refers)
+				tmpcqr = tmpcqr->refers;
+			if (tmpcqr != cqr)
+				continue;
+			/* searchcqr is an ERP request for cqr */
+			searchcqr->retries = -1;
+			searchcqr->intrc = -ETIMEDOUT;
+			if (searchcqr->status >= DASD_CQR_QUEUED) {
+				spin_unlock(get_ccwdev_lock(device->cdev));
+				rc = dasd_cancel_req(searchcqr);
+				spin_lock(get_ccwdev_lock(device->cdev));
+			} else if ((searchcqr->status == DASD_CQR_FILLED) ||
+				   (searchcqr->status == DASD_CQR_NEED_ERP)) {
+				searchcqr->status = DASD_CQR_TERMINATED;
+				rc = 0;
+			} else if (searchcqr->status == DASD_CQR_IN_ERP) {
+				/*
+				 * Shouldn't happen; most recent ERP
+				 * request is at the front of queue
+				 */
+				continue;
+			}
+			break;
+		}
+		spin_unlock(get_ccwdev_lock(device->cdev));
+	}
+	dasd_schedule_block_bh(block);
+	spin_unlock(&block->queue_lock);
+
+	return rc ? BLK_EH_RESET_TIMER : BLK_EH_NOT_HANDLED;
+}
+
+/*
  * Allocate and initialize request queue and default I/O scheduler.
  */
 static int dasd_alloc_queue(struct dasd_block *block)
diff --git a/drivers/s390/block/dasd_diag.c b/drivers/s390/block/dasd_diag.c
index 905b2c0..9e245ce 100644
--- a/drivers/s390/block/dasd_diag.c
+++ b/drivers/s390/block/dasd_diag.c
@@ -583,7 +583,10 @@ dasd_diag_free_cp(struct dasd_ccw_req *cqr, struct request *req)
 
 static void dasd_diag_handle_terminated_request(struct dasd_ccw_req *cqr)
 {
-	cqr->status = DASD_CQR_FILLED;
+	if (cqr->retries < 0)
+		cqr->status = DASD_CQR_FAILED;
+	else
+		cqr->status = DASD_CQR_FILLED;
 };
 
 /* Fill in IOCTL data for device. */
diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
index 5b5b50b..137f18d 100644
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -2251,6 +2251,10 @@ dasd_eckd_format_device(struct dasd_device * device,
 
 static void dasd_eckd_handle_terminated_request(struct dasd_ccw_req *cqr)
 {
+	if (cqr->retries < 0) {
+		cqr->status = DASD_CQR_FAILED;
+		return;
+	}
 	cqr->status = DASD_CQR_FILLED;
 	if (cqr->block && (cqr->startdev != cqr->block->base)) {
 		dasd_eckd_reset_ccw_to_base_io(cqr);
diff --git a/drivers/s390/block/dasd_fba.c b/drivers/s390/block/dasd_fba.c
index 3d8dc16..9f0bff7 100644
--- a/drivers/s390/block/dasd_fba.c
+++ b/drivers/s390/block/dasd_fba.c
@@ -428,7 +428,10 @@ out:
 
 static void dasd_fba_handle_terminated_request(struct dasd_ccw_req *cqr)
 {
-	cqr->status = DASD_CQR_FILLED;
+	if (cqr->retries < 0)
+		cqr->status = DASD_CQR_FAILED;
+	else
+		cqr->status = DASD_CQR_FILLED;
 };
 
 static int
-- 
1.7.4.2