dasd: implement block timeout
Subsystems (3)
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
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
@@ -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
PATCH 2/9 dasd: make number of retries configurable
In reply to Hannes ReineckeInstead 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
PATCH 3/9 dasd: process all requests in the device tasklet
In reply to Hannes ReineckeOriginally 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
@@ -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
PATCH 4/9 dasd: Implement block timeout handling
In reply to Hannes ReineckeThis 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
@@ -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
@@ -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
@@ -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
@@ -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
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.
Signed-off-by Hannes Reinecke <[email protected]>Acked-by Stefan Weinhuber <[email protected]>
[...]
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.
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.
Signed-off-by Hannes Reinecke <[email protected]>Acked-by Stefan Weinhuber <[email protected]>[...]
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)
PATCH 5/9 dasd: Reduce amount of messages for specific errors
In reply to Hannes ReineckeWhenever 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
@@ -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
PATCH 6/9 dasd: detailed I/O errors
In reply to Hannes ReineckeThe 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
@@ -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
@@ -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
PATCH 7/9 block: check for timeout function in blk_rq_timed_out()
In reply to Hannes Reineckerq_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
@@ -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
PATCH 8/9 dasd: Add 'timeout' attribute
In reply to Hannes ReineckeThis 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
@@ -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
@@ -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
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_timeoutfunction 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]>
[...]
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; +}
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_timeoutfunction 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]>[...]
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_MAXinstead ofULONG_MAX, otherwise it might overflow sinceblk_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)
Re: [PATCH 8/9] dasd: Add 'timeout' attribute
In reply to Hannes ReineckeHannes 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
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->blockbefore using it.
For alias devices thedevice->blockpointer 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)
PATCH 9/9 dasd: Fail all requests when DASD_FLAG_ABORTIO is set
In reply to Hannes ReineckeWhenever 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 theDASD_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
@@ -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
@@ -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
@@ -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 *) 2void 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