This is a re-send of a patch series Hannes sent last january. Stefan
looked at the patches and our tests went well, so I guess this is ready
for upstream integration.
The changes to block/blk-core.c and block/blk-timeout.c look good
to me, but I would like to request an acked-by from Jens for the
block layer parts in patch #6 and #7 of the series (pretty please :-)
The original patch description from Hannes:
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.
Changes since v1:
- Fixed lock inversion in dasd_times_out()
- Checked for 'device->block' when writing to 'timeout' attribute
- Check against 'UINT_MAX' when verifying the 'timeout' value
Once I got the required acked-by I can carry the patch set in the
linux-s390 tree for the next merge window.
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
block,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.9.5
From: Hannes Reinecke <[email protected]>
dasd_cancel_req will never return 1, only 0.
Signed-off-by: Hannes Reinecke <[email protected]>
Signed-off-by: Stefan Weinhuber <[email protected]>
Signed-off-by: Martin Schwidefsky <[email protected]>
---
drivers/s390/block/dasd.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index d72a9216e..4985489 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2402,8 +2402,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.
@@ -2440,7 +2439,6 @@ int dasd_cancel_req(struct dasd_ccw_req *cqr)
return rc;
}
-
/*
* SECTION: Operations of the dasd_block layer.
*/
--
1.7.9.5
From: Hannes Reinecke <[email protected]>
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 <[email protected]>
Signed-off-by: Stefan Weinhuber <[email protected]>
Signed-off-by: Martin Schwidefsky <[email protected]>
---
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..5812a3b 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 54f4bb8..17150a7 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
*/
@@ -2535,6 +2532,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 2bd03f4..690001a 100644
--- a/drivers/s390/block/dasd_int.h
+++ b/drivers/s390/block/dasd_int.h
@@ -524,7 +524,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 8be1b51..25a0f2f 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 multiple units of the device. In terms of the ECKD
@@ -458,6 +511,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.9.5
From: Hannes Reinecke <[email protected]>
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 <[email protected]>
Signed-off-by: Stefan Weinhuber <[email protected]>
Signed-off-by: Martin Schwidefsky <[email protected]>
---
drivers/s390/block/dasd_erp.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/s390/block/dasd_erp.c b/drivers/s390/block/dasd_erp.c
index 3250cb4..8d11f77 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.9.5
From: Hannes Reinecke <[email protected]>
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 <[email protected]>
Signed-off-by: Hannes Reinecke <[email protected]>
Signed-off-by: Stefan Weinhuber <[email protected]>
Signed-off-by: Martin Schwidefsky <[email protected]>
---
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 33c33bc..ef19bf2 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2315,6 +2315,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 87478be..b97624b 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2183,7 +2183,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 */
@@ -2590,8 +2590,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);
}
@@ -2692,6 +2701,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.9.5
From: Hannes Reinecke <[email protected]>
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 <[email protected]>
Acked-by: Stefan Weinhuber <[email protected]>
Signed-off-by: Stefan Weinhuber <[email protected]>
Signed-off-by: Martin Schwidefsky <[email protected]>
---
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
From: Hannes Reinecke <[email protected]>
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 <[email protected]>
Signed-off-by: Hannes Reinecke <[email protected]>
Signed-off-by: Stefan Weinhuber <[email protected]>
Signed-off-by: Martin Schwidefsky <[email protected]>
---
block/blk-timeout.c | 5 +++--
1 file 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.9.5
From: Hannes Reinecke <[email protected]>
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 <[email protected]>
Signed-off-by: Stefan Weinhuber <[email protected]>
Signed-off-by: Martin Schwidefsky <[email protected]>
---
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(+)
diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index b97624b..54f4bb8 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2894,6 +2894,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 bc3e7af..58bc6eb 100644
--- a/drivers/s390/block/dasd_devmap.c
+++ b/drivers/s390/block/dasd_devmap.c
@@ -1280,6 +1280,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) || !device->block)
+ return -ENODEV;
+
+ if ((strict_strtoul(buf, 10, &val) != 0) ||
+ val > UINT_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)
@@ -1391,6 +1446,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 ad42075..2bd03f4 100644
--- a/drivers/s390/block/dasd_int.h
+++ b/drivers/s390/block/dasd_int.h
@@ -470,6 +470,8 @@ struct dasd_device {
unsigned long default_expires;
unsigned long default_retries;
+ unsigned long blk_timeout;
+
struct dentry *debugfs_dentry;
struct dasd_profile profile;
};
@@ -663,6 +665,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.9.5
From: Hannes Reinecke <[email protected]>
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 <[email protected]>
Signed-off-by: Stefan Weinhuber <[email protected]>
Signed-off-by: Martin Schwidefsky <[email protected]>
---
drivers/s390/block/dasd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 4985489..000e514 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -1787,11 +1787,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.9.5
From: Hannes Reinecke <[email protected]>
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 <[email protected]>
Signed-off-by: Stefan Weinhuber <[email protected]>
Signed-off-by: Martin Schwidefsky <[email protected]>
---
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 a71bb8a..bc3e7af 100644
--- a/drivers/s390/block/dasd_devmap.c
+++ b/drivers/s390/block/dasd_devmap.c
@@ -1240,6 +1240,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)
@@ -1350,6 +1390,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 cc06033..1548422 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_tod_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 6a44b27..f4315dc 100644
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -1682,6 +1682,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++)
@@ -2659,7 +2662,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_tod_clock();
cqr->status = DASD_CQR_FILLED;
return cqr;
@@ -2834,7 +2837,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_tod_clock();
cqr->status = DASD_CQR_FILLED;
return cqr;
@@ -3127,7 +3130,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_tod_clock();
cqr->status = DASD_CQR_FILLED;
return cqr;
@@ -3330,7 +3333,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_tod_clock();
cqr->status = DASD_CQR_FILLED;
diff --git a/drivers/s390/block/dasd_fba.c b/drivers/s390/block/dasd_fba.c
index 4dd0e2f..d232c83 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_tod_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 0785bd9..ad42075 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 */
@@ -466,6 +468,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.9.5
On Mon, 3 Jun 2013 17:03:13 +0200
Martin Schwidefsky <[email protected]> wrote:
> This is a re-send of a patch series Hannes sent last january. Stefan
> looked at the patches and our tests went well, so I guess this is ready
> for upstream integration.
>
> The changes to block/blk-core.c and block/blk-timeout.c look good
> to me, but I would like to request an acked-by from Jens for the
> block layer parts in patch #6 and #7 of the series (pretty please :-)
>
> The original patch description from Hannes:
>
> 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.
>
> Changes since v1:
> - Fixed lock inversion in dasd_times_out()
> - Checked for 'device->block' when writing to 'timeout' attribute
> - Check against 'UINT_MAX' when verifying the 'timeout' value
>
> Once I got the required acked-by I can carry the patch set in the
> linux-s390 tree for the next merge window.
>
> 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
> block,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(-)
>
Ping. Jens could you please look at patch #6 and #7? I don't want
to do a please pull with block layer parts without your consent.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
On Mon, 17 Jun 2013 09:22:15 +0200
Martin Schwidefsky <[email protected]> wrote:
> On Mon, 3 Jun 2013 17:03:13 +0200
> Martin Schwidefsky <[email protected]> wrote:
>
> > This is a re-send of a patch series Hannes sent last january. Stefan
> > looked at the patches and our tests went well, so I guess this is ready
> > for upstream integration.
> >
> > The changes to block/blk-core.c and block/blk-timeout.c look good
> > to me, but I would like to request an acked-by from Jens for the
> > block layer parts in patch #6 and #7 of the series (pretty please :-)
> >
> > The original patch description from Hannes:
> >
> > 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.
> >
> > Changes since v1:
> > - Fixed lock inversion in dasd_times_out()
> > - Checked for 'device->block' when writing to 'timeout' attribute
> > - Check against 'UINT_MAX' when verifying the 'timeout' value
> >
> > Once I got the required acked-by I can carry the patch set in the
> > linux-s390 tree for the next merge window.
> >
> > 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
> > block,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(-)
> >
>
> Ping. Jens could you please look at patch #6 and #7? I don't want
> to do a please pull with block layer parts without your consent.
Ping -f.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
On Mon, Jul 01 2013, Martin Schwidefsky wrote:
> On Mon, 17 Jun 2013 09:22:15 +0200
> Martin Schwidefsky <[email protected]> wrote:
>
> > On Mon, 3 Jun 2013 17:03:13 +0200
> > Martin Schwidefsky <[email protected]> wrote:
> >
> > > This is a re-send of a patch series Hannes sent last january. Stefan
> > > looked at the patches and our tests went well, so I guess this is ready
> > > for upstream integration.
> > >
> > > The changes to block/blk-core.c and block/blk-timeout.c look good
> > > to me, but I would like to request an acked-by from Jens for the
> > > block layer parts in patch #6 and #7 of the series (pretty please :-)
> > >
> > > The original patch description from Hannes:
> > >
> > > 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.
> > >
> > > Changes since v1:
> > > - Fixed lock inversion in dasd_times_out()
> > > - Checked for 'device->block' when writing to 'timeout' attribute
> > > - Check against 'UINT_MAX' when verifying the 'timeout' value
> > >
> > > Once I got the required acked-by I can carry the patch set in the
> > > linux-s390 tree for the next merge window.
> > >
> > > 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
> > > block,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(-)
> > >
> >
> > Ping. Jens could you please look at patch #6 and #7? I don't want
> > to do a please pull with block layer parts without your consent.
Both look fine, you can add my acked-by.
--
Jens Axboe
On Mon, 1 Jul 2013 11:47:52 +0200
Jens Axboe <[email protected]> wrote:
> On Mon, Jul 01 2013, Martin Schwidefsky wrote:
> > On Mon, 17 Jun 2013 09:22:15 +0200
> > Martin Schwidefsky <[email protected]> wrote:
> >
> > > On Mon, 3 Jun 2013 17:03:13 +0200
> > > Martin Schwidefsky <[email protected]> wrote:
> > >
> > > > This is a re-send of a patch series Hannes sent last january. Stefan
> > > > looked at the patches and our tests went well, so I guess this is ready
> > > > for upstream integration.
> > > >
> > > > The changes to block/blk-core.c and block/blk-timeout.c look good
> > > > to me, but I would like to request an acked-by from Jens for the
> > > > block layer parts in patch #6 and #7 of the series (pretty please :-)
> > > >
> > > > The original patch description from Hannes:
> > > >
> > > > 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.
> > > >
> > > > Changes since v1:
> > > > - Fixed lock inversion in dasd_times_out()
> > > > - Checked for 'device->block' when writing to 'timeout' attribute
> > > > - Check against 'UINT_MAX' when verifying the 'timeout' value
> > > >
> > > > Once I got the required acked-by I can carry the patch set in the
> > > > linux-s390 tree for the next merge window.
> > > >
> > > > 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
> > > > block,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(-)
> > > >
> > >
> > > Ping. Jens could you please look at patch #6 and #7? I don't want
> > > to do a please pull with block layer parts without your consent.
>
> Both look fine, you can add my acked-by.
Thank you!
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.