Skip to contentSkip to conversation

dasd: implement block timeout

PATCH Superseded Hannes Reinecke 3 participants 16 messages Jan 29, 2013

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover dasd: implement block timeout

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(-)
Signature · 1 line
1.7.4.2
#1 lore raw

PATCH 1/9 dasd: Clarify comment

dasd_cancel_req will never return 1, only 0.

---
 drivers/s390/block/dasd.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)
drivers/s390/block/dasd.c +1 -3
index 29225e1..09ddf70 100644
@@ -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.
  */
Signature · 1 line
1.7.4.2
#2 lore raw

PATCH 2/9 dasd: make number of retries configurable

In reply to 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.

---
 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(-)
drivers/s390/block/dasd_devmap.c +41 -0
index c196827..d237e31 100644
@@ -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,
drivers/s390/block/dasd_diag.c +2 -1
index 704488d..905b2c0 100644
@@ -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)
drivers/s390/block/dasd_eckd.c +7 -4
index e37bc16..5b5b50b 100644
@@ -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;
 
drivers/s390/block/dasd_fba.c +4 -1
index 4146985..3d8dc16 100644
@@ -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;
drivers/s390/block/dasd_int.h +3 -0
index 899e3f5..375ba1f 100644
@@ -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;
Signature · 1 line
1.7.4.2
#3 lore raw

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

In reply to 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.

---
 drivers/s390/block/dasd.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/s390/block/dasd.c +2 -2
index 09ddf70..20b7517 100644
@@ -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);
 		}
Signature · 1 line
1.7.4.2
#4 lore raw

PATCH 4/9 dasd: Implement block timeout handling

In reply to 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.

---
 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(-)
drivers/s390/block/dasd.c +76 -0
index 20b7517..0406325 100644
@@ -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)
drivers/s390/block/dasd_diag.c +4 -1
index 905b2c0..9e245ce 100644
@@ -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. */
drivers/s390/block/dasd_eckd.c +4 -0
index 5b5b50b..137f18d 100644
@@ -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);
drivers/s390/block/dasd_fba.c +4 -1
index 3d8dc16..9f0bff7 100644
@@ -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
Signature · 1 line
1.7.4.2
#5 lore raw

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

Quoted 10 lines · 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.

[...]

Quoted 7 lines
drivers/s390/block/dasd.c +7 -0
+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;
+

drivers/s390/block/dasd.c static void do_dasd_request(struct request_queue *queue) · line 2783

Quoted 10 lines
drivers/s390/block/dasd.c +10 -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));
        ^^^
Quoted 14 lines
drivers/s390/block/dasd.c +14 -0
+	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) {

drivers/s390/block/dasd.c static void do_dasd_request(struct request_queue *queue) · line 2807

Quoted 10 lines
drivers/s390/block/dasd.c +10 -0
+			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.

#6 lore raw

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

Quoted 60 lines · Heiko Carstens
Quoted 10 lines · 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.

[...]

Quoted 7 lines
drivers/s390/block/dasd.c +7 -0
+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;
+

drivers/s390/block/dasd.c static void do_dasd_request(struct request_queue *queue) · line 2783

Quoted 10 lines
drivers/s390/block/dasd.c +10 -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));
         ^^^
Quoted 14 lines
drivers/s390/block/dasd.c +14 -0
+	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) {

drivers/s390/block/dasd.c static void do_dasd_request(struct request_queue *queue) · line 2807

Quoted 10 lines
drivers/s390/block/dasd.c +10 -0
+			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));
				^^^
Quoted 2 lines

This looks like a potential dead lock.

Right.

Will be resending the patch.

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +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)
#7 lore raw

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

In reply to 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.

---
 drivers/s390/block/dasd_erp.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
drivers/s390/block/dasd_erp.c +8 -0
index d01ef82..908b9a0 100644
@@ -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);
Signature · 1 line
1.7.4.2
#8 lore raw

PATCH 6/9 dasd: detailed I/O errors

In reply to 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.

---
 block/blk-core.c          |    3 +++
 drivers/s390/block/dasd.c |   16 +++++++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)
block/blk-core.c +3 -0
index c973249..d5579c2 100644
@@ -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";
drivers/s390/block/dasd.c +13 -3
index 0406325..d806b98 100644
@@ -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;
 		}
Signature · 1 line
1.7.4.2
#9 lore raw

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

In reply to 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().

---
 block/blk-timeout.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
block/blk-timeout.c +3 -2
index 6e4744c..65f1035 100644
@@ -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);
Signature · 1 line
1.7.4.2
#10 lore raw

PATCH 8/9 dasd: Add 'timeout' attribute

In reply to 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.

---
 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(-)
drivers/s390/block/dasd.c +2 -0
index d806b98..61080df 100644
@@ -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);
drivers/s390/block/dasd_devmap.c +56 -0

68 lines not on this page · raw message

drivers/s390/block/dasd_int.h +4 -0
index 375ba1f..d00d07a 100644
@@ -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 *);
Signature · 1 line
1.7.4.2
#11 lore raw

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

Quoted 6 lines · 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.

[...]

Quoted 4 lines
drivers/s390/block/dasd_devmap.c +4 -0
+static ssize_t
+dasd_timeout_store(struct device *dev, struct device_attribute *attr,
+		   const char *buf, size_t count)
+{

drivers/s390/block/dasd_devmap.c dasd_retries_store(struct device *dev, struct device_attribute *attr, · line 1312

Quoted 10 lines
drivers/s390/block/dasd_devmap.c +10 -0
+	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.

Quoted 22 lines
drivers/s390/block/dasd_devmap.c +22 -0
+		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;
+}
#12 lore raw

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

Quoted 20 lines · Heiko Carstens
Quoted 6 lines · 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.

[...]

Quoted 10 lines
drivers/s390/block/dasd_devmap.c +10 -0
+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))
Quoted 9 lines

drivers/s390/block/dasd_devmap.c dasd_retries_store(struct device *dev, struct device_attribute *attr, · line 1312

Quoted 4 lines
drivers/s390/block/dasd_devmap.c +4 -0
+		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

Signature · 4 lines
Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +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)
#13 lore raw

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

In reply to Hannes Reinecke

Hannes Reinecke <[email protected]> wrote on 2013-01-29 08:12:00:
[..]

Quoted 8 lines
drivers/s390/block/dasd_devmap.c +8 -0
+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;
+

drivers/s390/block/dasd_devmap.c dasd_retries_store(struct device *dev, struct device_attribute *attr, · line 1316

Quoted 10 lines
drivers/s390/block/dasd_devmap.c +10 -0
+   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.

Quoted 9 lines
drivers/s390/block/dasd_devmap.c +9 -0
+   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);

drivers/s390/block/dasd_devmap.c dasd_retries_store(struct device *dev, struct device_attribute *attr, · line 1335

Quoted 10 lines
drivers/s390/block/dasd_devmap.c +10 -0
+
+   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

Signature · 15 lines
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: [email protected]

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
#14 lore raw

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

Quoted 16 lines · Stefan Weinhuber

Hannes Reinecke <[email protected]> wrote on 2013-01-29 08:12:00:
[..]

Quoted 14 lines
drivers/s390/block/dasd_devmap.c +14 -0
+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) {
Quoted 9 lines

drivers/s390/block/dasd_devmap.c dasd_retries_store(struct device *dev, struct device_attribute *attr, · line 1316

Quoted 4 lines
drivers/s390/block/dasd_devmap.c +4 -0
+      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

Signature · 4 lines
Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +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)
#15 lore raw

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

In reply to 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'.

---
 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(-)
arch/s390/include/uapi/asm/dasd.h +4 -0
index 38eca3b..a2ea496 100644
@@ -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 */
drivers/s390/block/dasd.c +10 -3
index 61080df..9f8dbbb 100644
@@ -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)
drivers/s390/block/dasd_int.h +3 -0
index d00d07a..a4f3050 100644
@@ -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 *);
 
drivers/s390/block/dasd_ioctl.c +59 -0

71 lines not on this page · raw message

Signature · 1 line
1.7.4.2