[PATCH 0/9] blk: scsi: blk abort queue updates

[PATCH 0/9] blk: scsi: blk abort queue updates

[Mike Anderson]

  This patch series has three sections explained below. The patches are
  presented as a series as they hit some common functions and may need to
  be applied in order to avoid conflicts.

  1.) Avoid possibly waking up the error handler on a queue that it stopped.
  blk: Do not abort requests if queue is stopped

  2.) It is possible to have a request on the elevator that is already
  prepped. The prepped resources should be released also. This was seen under
  heavy IO load while a lot of requeue activity as going occurring.

  blk: In elv_abort_queue skip requests with REQ_DONTPREP set
  blk: Add a unprep_rq_fn
  blk: Call unprep_fn from elv_abort_queue is available
  scsi: Add a scsi_unprep_fn

  3.) These four patches add a flag to a request indicating that a block
  abort has been called on the request. It also adds a common
  scsi_requeue_request so that the flag can be checked in only one location
  on a attempt to requeue.
  
  Currently we may abort a request and post error recovery the request can
  be requeued. We may also try to abort a request that is in the process of
  completing that we would like to prevent a requeue on. 
  
  The abort flag is different than a fast fail flag in that it is provides a
  hint that the request should be returned as soon as possible (i.e. do not
  requeue even if disposition indicates that it would be allowed). 
  
  The change allowed failovers that where exceeding (SD_MAX_RETRIES * IO
  TIMEOUT) to be able to failover in less than a single timeout (the less
  than case depended on the type of event that triggered the path failure).

  blk: Add request atomic flag for abort
  blk: Mark requests aborted
  scsi: Add scsi_requeue_request function
  scsi: Add blk_request_aborted check

  I tested the patches on a IBM,2105800 connected through a zfcp adapter,
  a IBM,1815 through a qlogic adapter, and scsi_debug.


 block/blk-settings.c    |   17 +++++++++++
 block/blk-timeout.c     |   19 +++++++++++-
 block/blk.h             |   14 +++++++++
 block/elevator.c        |   25 ++++++++++++++--
 drivers/scsi/scsi_lib.c |   73 +++++++++++++++++++++++++++++++---------------
 include/linux/blkdev.h  |    4 ++
 6 files changed, 123 insertions(+), 29 deletions(-)

[PATCH 1/9] blk: Do not abort requests if queue is stopped

[Mike Anderson]

If the queue is stopped it could be an indication that other recovery is
happening in this case skip the blk_abort_request.

Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
Cc: Jens Axobe <jens.axboe@oracle.com>
---
 block/blk-timeout.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/block/blk-timeout.c b/block/blk-timeout.c
index 1ba7e0a..89fbe0a 100644
--- a/block/blk-timeout.c
+++ b/block/blk-timeout.c
@@ -224,7 +224,8 @@ void blk_abort_queue(struct request_queue *q)
 	list_splice_init(&q->timeout_list, &list);
 
 	list_for_each_entry_safe(rq, tmp, &list, timeout_list)
-		blk_abort_request(rq);
+		if (!blk_queue_stopped(q))
+			blk_abort_request(rq);
 
 	/*
 	 * Occasionally, blk_abort_request() will return without
-- 
1.6.6.1

[PATCH 2/9] blk: In elv_abort_queue skip requests with REQ_DONTPREP set

[Mike Anderson]

Having REQ_DONTPREP set on a request can indicated that resources have been
allocated for this request. In elv_abort_queue skip requests with
REQ_DONTPREP set to avoid leaking resources.

Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
Cc: Jens Axobe <jens.axboe@oracle.com>
---
 block/elevator.c |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/block/elevator.c b/block/elevator.c
index df75676..ac98008 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -807,10 +807,18 @@ int elv_may_queue(struct request_queue *q, int rw)
 
 void elv_abort_queue(struct request_queue *q)
 {
-	struct request *rq;
+	struct request *rq, *tmp;
+	LIST_HEAD(list);
 
-	while (!list_empty(&q->queue_head)) {
-		rq = list_entry_rq(q->queue_head.next);
+	/*
+	 * Splice entries to local list, in case the list contains some
+	 * requests marked REQ_DONTPREP.
+	 */
+	list_splice_init(&q->queue_head, &list);
+
+	list_for_each_entry_safe(rq, tmp, &list, queuelist) {
+		if (rq->cmd_flags & REQ_DONTPREP)
+			continue;
 		rq->cmd_flags |= REQ_QUIET;
 		trace_block_rq_abort(q, rq);
 		/*
@@ -820,6 +828,13 @@ void elv_abort_queue(struct request_queue *q)
 		blk_start_request(rq);
 		__blk_end_request_all(rq, -EIO);
 	}
+
+	/*
+	 * In case requests with REQ_DONTPREP where skipped splice the
+	 * local list back.
+	 */
+	list_splice(&list, &q->queue_head);
+
 }
 EXPORT_SYMBOL(elv_abort_queue);
 
-- 
1.6.6.1

[PATCH 3/9] blk: Add a unprep_rq_fn

[Mike Anderson]

Add a unprep_rq_fn to the block layer allowing a function to be called if
set to release resources if a request is being unprepared at the block
level.

Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
Cc: Jens Axobe <jens.axboe@oracle.com>
---
 block/blk-settings.c   |   17 +++++++++++++++++
 include/linux/blkdev.h |    3 +++
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index d9a9db5..1a33494 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -36,6 +36,23 @@ void blk_queue_prep_rq(struct request_queue *q, prep_rq_fn *pfn)
 EXPORT_SYMBOL(blk_queue_prep_rq);
 
 /**
+ * blk_queue_unprep_rq - set a unprepare_request function for queue
+ * @q:		queue
+ * @pfn:	unprepare_request function
+ *
+ * It's possible for a queue to register a unprepare_request callback which
+ * may be invoked to unprepare a request that is on a queue. The goal of
+ * the function is to unprepare a request for I/O, it can be used to
+ * release a cdb from the request for instance.
+ *
+ */
+void blk_queue_unprep_rq(struct request_queue *q, unprep_rq_fn *unpfn)
+{
+	q->unprep_rq_fn = unpfn;
+}
+EXPORT_SYMBOL(blk_queue_unprep_rq);
+
+/**
  * blk_queue_merge_bvec - set a merge_bvec function for queue
  * @q:		queue
  * @mbfn:	merge_bvec_fn
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ebd22db..2818e80 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -259,6 +259,7 @@ struct request_pm_state
 typedef void (request_fn_proc) (struct request_queue *q);
 typedef int (make_request_fn) (struct request_queue *q, struct bio *bio);
 typedef int (prep_rq_fn) (struct request_queue *, struct request *);
+typedef void (unprep_rq_fn) (struct request_queue *, struct request *);
 typedef void (unplug_fn) (struct request_queue *);
 
 struct bio_vec;
@@ -341,6 +342,7 @@ struct request_queue
 	request_fn_proc		*request_fn;
 	make_request_fn		*make_request_fn;
 	prep_rq_fn		*prep_rq_fn;
+	unprep_rq_fn		*unprep_rq_fn;
 	unplug_fn		*unplug_fn;
 	merge_bvec_fn		*merge_bvec_fn;
 	prepare_flush_fn	*prepare_flush_fn;
@@ -968,6 +970,7 @@ extern int blk_queue_dma_drain(struct request_queue *q,
 extern void blk_queue_lld_busy(struct request_queue *q, lld_busy_fn *fn);
 extern void blk_queue_segment_boundary(struct request_queue *, unsigned long);
 extern void blk_queue_prep_rq(struct request_queue *, prep_rq_fn *pfn);
+extern void blk_queue_unprep_rq(struct request_queue *, unprep_rq_fn *unpfn);
 extern void blk_queue_merge_bvec(struct request_queue *, merge_bvec_fn *);
 extern void blk_queue_dma_alignment(struct request_queue *, int);
 extern void blk_queue_update_dma_alignment(struct request_queue *, int);
-- 
1.6.6.1

[PATCH 4/9] blk: Call unprep_fn from elv_abort_queue is available

[Mike Anderson]

Call the unprep_fn for the queue if it has been set for requests that have
been prepped indicated by REQ_DONTPREP being set.

Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
Cc: Jens Axobe <jens.axboe@oracle.com>
---
 block/elevator.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/elevator.c b/block/elevator.c
index ac98008..1f1e942 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -817,8 +817,12 @@ void elv_abort_queue(struct request_queue *q)
 	list_splice_init(&q->queue_head, &list);
 
 	list_for_each_entry_safe(rq, tmp, &list, queuelist) {
-		if (rq->cmd_flags & REQ_DONTPREP)
-			continue;
+		if (rq->cmd_flags & REQ_DONTPREP) {
+			if (q->unprep_rq_fn)
+				q->unprep_rq_fn(q, rq);
+			else
+				continue;
+		}
 		rq->cmd_flags |= REQ_QUIET;
 		trace_block_rq_abort(q, rq);
 		/*
-- 
1.6.6.1

[PATCH 5/9] scsi: Add a scsi_unprep_fn

[Mike Anderson]

Add scsi_unprep_fn and set this function as the unprep_rq_fn for the scsi
queue. This will allow SCSI resources to be released if the block layer
unpreps a request.

Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
Cc: James Bottomley <James.Bottomley@suse.de>
---
 drivers/scsi/scsi_lib.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1646fe7..1685d35 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -91,6 +91,17 @@ static void scsi_unprep_request(struct request *req)
 	scsi_put_command(cmd);
 }
 
+static void scsi_unprep_fn(struct request_queue *q, struct request *req)
+{
+	struct scsi_cmnd *cmd = req->special;
+
+	if (cmd) {
+		scsi_release_buffers(cmd);
+		scsi_unprep_request(req);
+	}
+}
+
+
 /**
  * __scsi_queue_insert - private queue insertion
  * @cmd: The SCSI command being requeued
@@ -1664,6 +1675,7 @@ struct request_queue *scsi_alloc_queue(struct scsi_device *sdev)
 		return NULL;
 
 	blk_queue_prep_rq(q, scsi_prep_fn);
+	blk_queue_unprep_rq(q, scsi_unprep_fn);
 	blk_queue_softirq_done(q, scsi_softirq_done);
 	blk_queue_rq_timed_out(q, scsi_times_out);
 	blk_queue_lld_busy(q, scsi_lld_busy);
-- 
1.6.6.1

[PATCH 6/9] blk: Add request atomic flag for abort

[Mike Anderson]

Add atomic flag for a request to indicate that it has marked as aborted.
Provide functions to mark and test the aborted flag.

Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
Cc: Jens Axobe <jens.axboe@oracle.com>
---
 block/blk-timeout.c    |    6 ++++++
 block/blk.h            |   14 ++++++++++++++
 include/linux/blkdev.h |    1 +
 3 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/block/blk-timeout.c b/block/blk-timeout.c
index 89fbe0a..ad45b44 100644
--- a/block/blk-timeout.c
+++ b/block/blk-timeout.c
@@ -155,6 +155,12 @@ void blk_abort_request(struct request *req)
 }
 EXPORT_SYMBOL_GPL(blk_abort_request);
 
+int blk_request_aborted(struct request *req)
+{
+	return blk_test_rq_aborted(req);
+}
+EXPORT_SYMBOL_GPL(blk_request_aborted);
+
 /**
  * blk_add_timer - Start timeout timer for a single request
  * @req:	request that is about to start running.
diff --git a/block/blk.h b/block/blk.h
index 5ee3d7e..165262a 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -30,6 +30,7 @@ void __generic_unplug_device(struct request_queue *);
  */
 enum rq_atomic_flags {
 	REQ_ATOM_COMPLETE = 0,
+	REQ_ATOM_ABORT = 1,
 };
 
 /*
@@ -47,6 +48,19 @@ static inline void blk_clear_rq_complete(struct request *rq)
 }
 
 /*
+ * Mark and test a request for aborted.
+ */
+static inline int blk_mark_rq_aborted(struct request *rq)
+{
+	return test_and_set_bit(REQ_ATOM_ABORT, &rq->atomic_flags);
+}
+
+static inline int blk_test_rq_aborted(struct request *rq)
+{
+	return test_bit(REQ_ATOM_ABORT, &rq->atomic_flags);
+}
+
+/*
  * Internal elevator interface
  */
 #define ELV_ON_HASH(rq)		(!hlist_unhashed(&(rq)->hash))
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 2818e80..7636e58 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -912,6 +912,7 @@ extern void blk_complete_request(struct request *);
 extern void __blk_complete_request(struct request *);
 extern void blk_abort_request(struct request *);
 extern void blk_abort_queue(struct request_queue *);
+extern int blk_request_aborted(struct request *);
 
 /*
  * Access functions for manipulating queue properties
-- 
1.6.6.1

[PATCH 7/9] blk: Mark requests aborted

[Mike Anderson]

Mark requests aborted using blk_mark_rq_aborted to assist lower levels in
identifying aborted requests.

Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
Cc: Jens Axobe <jens.axboe@oracle.com>
---
 block/blk-timeout.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/block/blk-timeout.c b/block/blk-timeout.c
index ad45b44..d8610c1 100644
--- a/block/blk-timeout.c
+++ b/block/blk-timeout.c
@@ -229,9 +229,17 @@ void blk_abort_queue(struct request_queue *q)
 	 */
 	list_splice_init(&q->timeout_list, &list);
 
-	list_for_each_entry_safe(rq, tmp, &list, timeout_list)
+	list_for_each_entry_safe(rq, tmp, &list, timeout_list) {
+                /*
+                 * Mark all requests even if we are unable to abort. The
+                 * aborted flag can used by lower levels to indicate that
+                 * the request should finished as soon as possible.
+                 */
+		blk_mark_rq_aborted(rq);
+
 		if (!blk_queue_stopped(q))
 			blk_abort_request(rq);
+	}
 
 	/*
 	 * Occasionally, blk_abort_request() will return without
-- 
1.6.6.1

[PATCH 8/9] scsi: Add scsi_requeue_request function

[Mike Anderson]

Add a common scsi_requeue_request function and move previous callers of
separate requeue calls to a this common one. This is a preparation step to
allow a follow on patch to be added to a common function.

Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
Cc: James Bottomley <James.Bottomley@suse.de>
---
 drivers/scsi/scsi_lib.c |   53 +++++++++++++++++++++++++---------------------
 1 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1685d35..73182db 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -102,6 +102,33 @@ static void scsi_unprep_fn(struct request_queue *q, struct request *req)
 }
 
 
+/*
+ * Function:	scsi_requeue_request()
+ *
+ * Purpose:	Requeue a request.
+ *
+ * Arguments:	q	- queue to operate on
+ *		req	- request to be requeued.
+ *		unprep	- indicate if unprep needed.
+ *
+ * Returns:	Nothing
+ *
+ * Notes:	Upon return, req is a stale pointer.
+ */
+static void scsi_requeue_request(struct request_queue *q, struct request *req,
+				 int unprep)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(q->queue_lock, flags);
+	if (unprep)
+		scsi_unprep_request(req);
+	blk_requeue_request(q, req);
+	spin_unlock_irqrestore(q->queue_lock, flags);
+
+	scsi_run_queue(q);
+}
+
 /**
  * __scsi_queue_insert - private queue insertion
  * @cmd: The SCSI command being requeued
@@ -120,7 +147,6 @@ static int __scsi_queue_insert(struct scsi_cmnd *cmd, int reason, int unbusy)
 	struct scsi_device *device = cmd->device;
 	struct scsi_target *starget = scsi_target(device);
 	struct request_queue *q = device->request_queue;
-	unsigned long flags;
 
 	SCSI_LOG_MLQUEUE(1,
 		 printk("Inserting command %p into mlqueue\n", cmd));
@@ -157,22 +183,7 @@ static int __scsi_queue_insert(struct scsi_cmnd *cmd, int reason, int unbusy)
 	if (unbusy)
 		scsi_device_unbusy(device);
 
-	/*
-	 * Requeue this command.  It will go before all other commands
-	 * that are already in the queue.
-	 *
-	 * NOTE: there is magic here about the way the queue is plugged if
-	 * we have no outstanding commands.
-	 * 
-	 * Although we *don't* plug the queue, we call the request
-	 * function.  The SCSI request function detects the blocked condition
-	 * and plugs the queue appropriately.
-         */
-	spin_lock_irqsave(q->queue_lock, flags);
-	blk_requeue_request(q, cmd->request);
-	spin_unlock_irqrestore(q->queue_lock, flags);
-
-	scsi_run_queue(q);
+	scsi_requeue_request(q, cmd->request, 0);
 
 	return 0;
 }
@@ -489,14 +500,8 @@ static void scsi_run_queue(struct request_queue *q)
 static void scsi_requeue_command(struct request_queue *q, struct scsi_cmnd *cmd)
 {
 	struct request *req = cmd->request;
-	unsigned long flags;
-
-	spin_lock_irqsave(q->queue_lock, flags);
-	scsi_unprep_request(req);
-	blk_requeue_request(q, req);
-	spin_unlock_irqrestore(q->queue_lock, flags);
 
-	scsi_run_queue(q);
+	scsi_requeue_request(q, req, 1);
 }
 
 void scsi_next_command(struct scsi_cmnd *cmd)
-- 
1.6.6.1

[PATCH 9/9] scsi: Add blk_request_aborted check

[Mike Anderson]

Add a blk_request_aborted check to scsi_requeue_request. This allows for a
single check in a common requeue function to determine if a request has been
aborted.

Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
Cc: James Bottomley <James.Bottomley@suse.de>
---
 drivers/scsi/scsi_lib.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 73182db..68e4bd7 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -121,11 +121,19 @@ static void scsi_requeue_request(struct request_queue *q, struct request *req,
 	unsigned long flags;
 
 	spin_lock_irqsave(q->queue_lock, flags);
+	if (blk_request_aborted(req)) {
+		spin_unlock_irqrestore(q->queue_lock, flags);
+		scsi_unprep_fn(q, req);
+		blk_end_request_all(req, -EIO);
+		goto out;
+	}
+
 	if (unprep)
 		scsi_unprep_request(req);
 	blk_requeue_request(q, req);
 	spin_unlock_irqrestore(q->queue_lock, flags);
 
+out:
 	scsi_run_queue(q);
 }
 
-- 
1.6.6.1

Re: [PATCH 1/9] blk: Do not abort requests if queue is stopped

[Hannes Reinecke]

Mike Anderson wrote:
> If the queue is stopped it could be an indication that other recovery is
> happening in this case skip the blk_abort_request.
> 
> Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
> Cc: Jens Axobe <jens.axboe@oracle.com>
Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/9] blk: In elv_abort_queue skip requests with REQ_DONTPREP set

[Hannes Reinecke]

Mike Anderson wrote:
> Having REQ_DONTPREP set on a request can indicated that resources have been
> allocated for this request. In elv_abort_queue skip requests with
> REQ_DONTPREP set to avoid leaking resources.
> 
> Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
> Cc: Jens Axobe <jens.axboe@oracle.com>
Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/9] blk: Add a unprep_rq_fn

[Hannes Reinecke]

Mike Anderson wrote:
> Add a unprep_rq_fn to the block layer allowing a function to be called if
> set to release resources if a request is being unprepared at the block
> level.
> 
> Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
> Cc: Jens Axobe <jens.axboe@oracle.com>
Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/9] blk: Call unprep_fn from elv_abort_queue is available

[Hannes Reinecke]

Mike Anderson wrote:
> Call the unprep_fn for the queue if it has been set for requests that have
> been prepped indicated by REQ_DONTPREP being set.
> 
> Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
> Cc: Jens Axobe <jens.axboe@oracle.com>
Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] [PATCH 4/9] blk: Call unprep_fn from elv_abort_queue is available

[Hannes Reinecke]

Mike Anderson wrote:
> Call the unprep_fn for the queue if it has been set for requests that have
> been prepped indicated by REQ_DONTPREP being set.
> 
> Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
> Cc: Jens Axobe <jens.axboe@oracle.com>
Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] [PATCH 5/9] scsi: Add a scsi_unprep_fn

[Hannes Reinecke]

Mike Anderson wrote:
> Add scsi_unprep_fn and set this function as the unprep_rq_fn for the scsi
> queue. This will allow SCSI resources to be released if the block layer
> unpreps a request.
> 
> Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
> Cc: James Bottomley <James.Bottomley@suse.de>
Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] [PATCH 6/9] blk: Add request atomic flag for abort

[Hannes Reinecke]

Mike Anderson wrote:
> Add atomic flag for a request to indicate that it has marked as aborted.
> Provide functions to mark and test the aborted flag.
> 
> Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
> Cc: Jens Axobe <jens.axboe@oracle.com>
Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/9] blk: Mark requests aborted

[Hannes Reinecke]

Mike Anderson wrote:
> Mark requests aborted using blk_mark_rq_aborted to assist lower levels in
> identifying aborted requests.
> 
> Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
> Cc: Jens Axobe <jens.axboe@oracle.com>
Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9] blk: Do not abort requests if queue is stopped

[Jens Axboe]

On Mon, May 03 2010, Mike Anderson wrote:
> If the queue is stopped it could be an indication that other recovery is
> happening in this case skip the blk_abort_request.
> 
> Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
> Cc: Jens Axobe <jens.axboe@oracle.com>
> ---
>  block/blk-timeout.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/block/blk-timeout.c b/block/blk-timeout.c
> index 1ba7e0a..89fbe0a 100644
> --- a/block/blk-timeout.c
> +++ b/block/blk-timeout.c
> @@ -224,7 +224,8 @@ void blk_abort_queue(struct request_queue *q)
>  	list_splice_init(&q->timeout_list, &list);
>  
>  	list_for_each_entry_safe(rq, tmp, &list, timeout_list)
> -		blk_abort_request(rq);
> +		if (!blk_queue_stopped(q))
> +			blk_abort_request(rq);
>  
>  	/*
>  	 * Occasionally, blk_abort_request() will return without

That seems like a bit of a mixup, what ties a stopped queue to recovery?
To take one example, the cciss driver stops the queue when it can't
queue more at the hw level and starts it on completion to queue more. If
recovery triggers when the hw queue has been filled, then timeouts will
fail?

It would be better to mark the queue as already doing abort. That state
could be in the queue, or it could be at the driver level.

-- 
Jens Axboe

Re: [PATCH 9/9] scsi: Add blk_request_aborted check

[Hannes Reinecke]

Mike Anderson wrote:
> Add a blk_request_aborted check to scsi_requeue_request. This allows for a
> single check in a common requeue function to determine if a request has been
> aborted.
> 
> Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
> Cc: James Bottomley <James.Bottomley@suse.de>
Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/9] blk: In elv_abort_queue skip requests with REQ_DONTPREP set

[Jens Axboe]

On Mon, May 03 2010, Mike Anderson wrote:
> Having REQ_DONTPREP set on a request can indicated that resources have been
> allocated for this request. In elv_abort_queue skip requests with
> REQ_DONTPREP set to avoid leaking resources.

This also seems weird, are you coding the API backwards from what SCSI
currently has implemented?

Would seem a lot cleaner to separate dont-prep from already-prepped.

-- 
Jens Axboe

Re: [PATCH 5/9] scsi: Add a scsi_unprep_fn

[Jens Axboe]

On Mon, May 03 2010, Mike Anderson wrote:
> Add scsi_unprep_fn and set this function as the unprep_rq_fn for the scsi
> queue. This will allow SCSI resources to be released if the block layer
> unpreps a request.

Looks fine, I would fold this in with patches 3+4, there's no point in
separating them this much.


-- 
Jens Axboe

Re: [PATCH 2/9] blk: In elv_abort_queue skip requests with REQ_DONTPREP set

[Mike Anderson]

Jens Axboe <jens.axboe@oracle.com> wrote:
> On Mon, May 03 2010, Mike Anderson wrote:
> > Having REQ_DONTPREP set on a request can indicated that resources have been
> > allocated for this request. In elv_abort_queue skip requests with
> > REQ_DONTPREP set to avoid leaking resources.
> 
> This also seems weird, are you coding the API backwards from what SCSI
> currently has implemented?
> 

Yes I was coding to current SCSI implementation.

> Would seem a lot cleaner to separate dont-prep from already-prepped.
> 

I assumed to do this we would need another request flag indicating
??REQ_PREPPED?? that would need to be set in the prep function. Then
this flag would be checked vs REQ_DONTPREP to call the unprep_fn or did you
mean something else?

-andmike
--
Michael Anderson
andmike@linux.vnet.ibm.com

Re: [PATCH 1/9] blk: Do not abort requests if queue is stopped

[Mike Anderson]

Jens Axboe <jens.axboe@oracle.com> wrote:
> On Mon, May 03 2010, Mike Anderson wrote:
> > If the queue is stopped it could be an indication that other recovery is
> > happening in this case skip the blk_abort_request.
> > 
> > Signed-off-by: Mike Anderson <andmike@linux.vnet.ibm.com>
> > Cc: Jens Axobe <jens.axboe@oracle.com>
> > ---
> >  block/blk-timeout.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/block/blk-timeout.c b/block/blk-timeout.c
> > index 1ba7e0a..89fbe0a 100644
> > --- a/block/blk-timeout.c
> > +++ b/block/blk-timeout.c
> > @@ -224,7 +224,8 @@ void blk_abort_queue(struct request_queue *q)
> >  	list_splice_init(&q->timeout_list, &list);
> >  
> >  	list_for_each_entry_safe(rq, tmp, &list, timeout_list)
> > -		blk_abort_request(rq);
> > +		if (!blk_queue_stopped(q))
> > +			blk_abort_request(rq);
> >  
> >  	/*
> >  	 * Occasionally, blk_abort_request() will return without
> 
> That seems like a bit of a mixup, what ties a stopped queue to recovery?

This was coding to SCSI behavior again. I tried to reduce the case of
waking the eh if the transport moved the target into a blocked state. It
might be redundant as FC has eh blocking and timer_reset. iSCSI has
blocking but not eh blocking.

> To take one example, the cciss driver stops the queue when it can't
> queue more at the hw level and starts it on completion to queue more. If
> recovery triggers when the hw queue has been filled, then timeouts will
> fail?
> 
> It would be better to mark the queue as already doing abort. That state
> could be in the queue, or it could be at the driver level.

ok. I will look into this.

-andmike
--
Michael Anderson
andmike@linux.vnet.ibm.com

Re: [PATCH 2/9] blk: In elv_abort_queue skip requests with REQ_DONTPREP set

[Jens Axboe]

On Tue, May 04 2010, Mike Anderson wrote:
> Jens Axboe <jens.axboe@oracle.com> wrote:
> > On Mon, May 03 2010, Mike Anderson wrote:
> > > Having REQ_DONTPREP set on a request can indicated that resources have been
> > > allocated for this request. In elv_abort_queue skip requests with
> > > REQ_DONTPREP set to avoid leaking resources.
> > 
> > This also seems weird, are you coding the API backwards from what SCSI
> > currently has implemented?
> > 
> 
> Yes I was coding to current SCSI implementation.
> 
> > Would seem a lot cleaner to separate dont-prep from already-prepped.
> > 
> 
> I assumed to do this we would need another request flag indicating
> ??REQ_PREPPED?? that would need to be set in the prep function. Then
> this flag would be checked vs REQ_DONTPREP to call the unprep_fn or did you
> mean something else?

Yeah, something like that, cleanly seperating the state of needing
unprep vs never being prepped. As that's a driver state, most drivers
should be able to get that info without needing an extra flag. But it's
probably cleaner to add the REQ_PREPPED flag and only call back into the
driver if the rq really needs an unprep.

-- 
Jens Axboe

Re: [PATCH 1/9] blk: Do not abort requests if queue is stopped

[Mike Christie]

On 05/04/2010 11:52 PM, Mike Anderson wrote:
> Jens Axboe<jens.axboe@oracle.com>  wrote:
>> On Mon, May 03 2010, Mike Anderson wrote:
>>> If the queue is stopped it could be an indication that other recovery is
>>> happening in this case skip the blk_abort_request.
>>>
>>> Signed-off-by: Mike Anderson<andmike@linux.vnet.ibm.com>
>>> Cc: Jens Axobe<jens.axboe@oracle.com>
>>> ---
>>>   block/blk-timeout.c |    3 ++-
>>>   1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/block/blk-timeout.c b/block/blk-timeout.c
>>> index 1ba7e0a..89fbe0a 100644
>>> --- a/block/blk-timeout.c
>>> +++ b/block/blk-timeout.c
>>> @@ -224,7 +224,8 @@ void blk_abort_queue(struct request_queue *q)
>>>   	list_splice_init(&q->timeout_list,&list);
>>>
>>>   	list_for_each_entry_safe(rq, tmp,&list, timeout_list)
>>> -		blk_abort_request(rq);
>>> +		if (!blk_queue_stopped(q))
>>> +			blk_abort_request(rq);
>>>
>>>   	/*
>>>   	 * Occasionally, blk_abort_request() will return without
>>
>> That seems like a bit of a mixup, what ties a stopped queue to recovery?
>
> This was coding to SCSI behavior again. I tried to reduce the case of
> waking the eh if the transport moved the target into a blocked state. It
> might be redundant as FC has eh blocking and timer_reset. iSCSI has
> blocking but not eh blocking.

All iscsi drivers should have blocking and timer reset now, so if a 
transport problem casues a IO error to be sent to dm, then when 
blk_abort_request is called that should prevent the scsi layer from 
sending the whole host into recovery.


I do not remember the problem with the lack of eh blocking though. Did 
we need that too?

Re: [PATCH 1/9] blk: Do not abort requests if queue is stopped

[Mike Anderson]

Mike Christie <michaelc@cs.wisc.edu> wrote:
> All iscsi drivers should have blocking and timer reset now, so if a
> transport problem casues a IO error to be sent to dm, then when
> blk_abort_request is called that should prevent the scsi layer from
> sending the whole host into recovery.
> 
> 
> I do not remember the problem with the lack of eh blocking though.
> Did we need that too?

My comment was not on need I was just comparing the transports.

-andmike
--
Michael Anderson
andmike@linux.vnet.ibm.com