All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] block: completion releated functions cleanup
@ 2009-05-11  8:56 FUJITA Tomonori
  2009-05-11  8:56 ` [PATCH 1/3] block: let blk_end_request_all handle bidi requests FUJITA Tomonori
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: FUJITA Tomonori @ 2009-05-11  8:56 UTC (permalink / raw)
  To: jens.axboe
  Cc: tj, James.Bottomley, bharrosh, fujita.tomonori, linux-scsi, linux-kernel

This is a minor cleanup on the completion releated functions. It is on
the top of Tejun's unify request processing model patchset.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] block: let blk_end_request_all handle bidi requests
  2009-05-11  8:56 [PATCH 0/3] block: completion releated functions cleanup FUJITA Tomonori
@ 2009-05-11  8:56 ` FUJITA Tomonori
  2009-05-11  9:06   ` Jens Axboe
  2009-05-11  8:56 ` [PATCH 2/3] scsi: simplify the bidi completion FUJITA Tomonori
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: FUJITA Tomonori @ 2009-05-11  8:56 UTC (permalink / raw)
  To: jens.axboe
  Cc: tj, James.Bottomley, bharrosh, fujita.tomonori, linux-scsi, linux-kernel

blk_end_request_all() and __blk_end_request_all() should finish all
bytes including bidi, by definition. That's what all bidi users need ,
bidi requests must be complete as a whole (partial completion is
impossible).

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 include/linux/blkdev.h |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8919683..3b5c564 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -911,8 +911,12 @@ static inline bool blk_end_request(struct request *rq, int error,
 static inline void blk_end_request_all(struct request *rq, int error)
 {
 	bool pending;
+	unsigned int bidi_bytes = 0;
 
-	pending = blk_end_request(rq, error, blk_rq_bytes(rq));
+	if (unlikely(blk_bidi_rq(rq)))
+		bidi_bytes = blk_rq_bytes(rq->next_rq);
+
+	pending = blk_end_bidi_request(rq, error, blk_rq_bytes(rq), bidi_bytes);
 	BUG_ON(pending);
 }
 
@@ -963,8 +967,12 @@ static inline bool __blk_end_request(struct request *rq, int error,
 static inline void __blk_end_request_all(struct request *rq, int error)
 {
 	bool pending;
+	unsigned int bidi_bytes = 0;
+
+	if (unlikely(blk_bidi_rq(rq)))
+		bidi_bytes = blk_rq_bytes(rq->next_rq);
 
-	pending = __blk_end_request(rq, error, blk_rq_bytes(rq));
+	pending = __blk_end_bidi_request(rq, error, blk_rq_bytes(rq), bidi_bytes);
 	BUG_ON(pending);
 }
 
-- 
1.6.0.6


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/3] scsi: simplify the bidi completion
  2009-05-11  8:56 [PATCH 0/3] block: completion releated functions cleanup FUJITA Tomonori
  2009-05-11  8:56 ` [PATCH 1/3] block: let blk_end_request_all handle bidi requests FUJITA Tomonori
@ 2009-05-11  8:56 ` FUJITA Tomonori
  2009-05-11  8:56 ` [PATCH 3/3] block: move completion related functions back to blk-core.c FUJITA Tomonori
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: FUJITA Tomonori @ 2009-05-11  8:56 UTC (permalink / raw)
  To: jens.axboe
  Cc: tj, James.Bottomley, bharrosh, fujita.tomonori, linux-scsi, linux-kernel

Let's use blk_end_request_all() instead of blk_end_bidi_request().

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 drivers/scsi/scsi_lib.c |   43 +++++++++++++------------------------------
 1 files changed, 13 insertions(+), 30 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b12750f..a54bec9 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -673,33 +673,6 @@ void scsi_release_buffers(struct scsi_cmnd *cmd)
 EXPORT_SYMBOL(scsi_release_buffers);
 
 /*
- * Bidi commands Must be complete as a whole, both sides at once.  If
- * part of the bytes were written and lld returned scsi_in()->resid
- * and/or scsi_out()->resid this information will be left in
- * req->resid_len and req->next_rq->resid_len. The upper-layer driver
- * can decide what to do with this information.
- */
-static void scsi_end_bidi_request(struct scsi_cmnd *cmd)
-{
-	struct request *req = cmd->request;
-
-	req->resid_len = scsi_out(cmd)->resid;
-	req->next_rq->resid_len = scsi_in(cmd)->resid;
-
-	/* The req and req->next_rq have not been completed */
-	BUG_ON(blk_end_bidi_request(req, 0, blk_rq_bytes(req),
-				    blk_rq_bytes(req->next_rq)));
-
-	scsi_release_buffers(cmd);
-
-	/*
-	 * This will goose the queue request function at the end, so we don't
-	 * need to worry about launching another command.
-	 */
-	scsi_next_command(cmd);
-}
-
-/*
  * Function:    scsi_io_completion()
  *
  * Purpose:     Completion processing for block device I/O requests.
@@ -772,12 +745,22 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 			if (!sense_deferred)
 				error = -EIO;
 		}
+
+		req->resid_len = scsi_get_resid(cmd);
+
 		if (scsi_bidi_cmnd(cmd)) {
-			/* will also release_buffers */
-			scsi_end_bidi_request(cmd);
+			/*
+			 * Bidi commands Must be complete as a whole,
+			 * both sides at once.
+			 */
+			req->next_rq->resid_len = scsi_in(cmd)->resid;
+
+			blk_end_request_all(req, 0);
+
+			scsi_release_buffers(cmd);
+			scsi_next_command(cmd);
 			return;
 		}
-		req->resid_len = scsi_get_resid(cmd);
 	}
 
 	BUG_ON(blk_bidi_rq(req)); /* bidi not support for !blk_pc_request yet */
-- 
1.6.0.6


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] block: move completion related functions back to blk-core.c
  2009-05-11  8:56 [PATCH 0/3] block: completion releated functions cleanup FUJITA Tomonori
  2009-05-11  8:56 ` [PATCH 1/3] block: let blk_end_request_all handle bidi requests FUJITA Tomonori
  2009-05-11  8:56 ` [PATCH 2/3] scsi: simplify the bidi completion FUJITA Tomonori
@ 2009-05-11  8:56 ` FUJITA Tomonori
  2009-05-11  9:06 ` [PATCH 0/3] block: completion releated functions cleanup Jens Axboe
  2009-05-14 13:54 ` [PATCH 0/3] block: completion related " Boaz Harrosh
  4 siblings, 0 replies; 13+ messages in thread
From: FUJITA Tomonori @ 2009-05-11  8:56 UTC (permalink / raw)
  To: jens.axboe
  Cc: tj, James.Bottomley, bharrosh, fujita.tomonori, linux-scsi, linux-kernel

Let's put the completion related functions back to block/blk-core.c
where they have lived. We can also unexport blk_end_bidi_request() and
__blk_end_bidi_request(), which nobody uses.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 block/blk-core.c       |  128 +++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/blkdev.h |  128 +++---------------------------------------------
 2 files changed, 130 insertions(+), 126 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 93691d2..a2d97de 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2026,8 +2026,8 @@ static void blk_finish_request(struct request *req, int error)
  *     %false - we are done with this request
  *     %true  - still buffers pending for this request
  **/
-bool blk_end_bidi_request(struct request *rq, int error,
-			  unsigned int nr_bytes, unsigned int bidi_bytes)
+static bool blk_end_bidi_request(struct request *rq, int error,
+				 unsigned int nr_bytes, unsigned int bidi_bytes)
 {
 	struct request_queue *q = rq->q;
 	unsigned long flags;
@@ -2041,7 +2041,6 @@ bool blk_end_bidi_request(struct request *rq, int error,
 
 	return false;
 }
-EXPORT_SYMBOL_GPL(blk_end_bidi_request);
 
 /**
  * __blk_end_bidi_request - Complete a bidi request with queue lock held
@@ -2058,8 +2057,8 @@ EXPORT_SYMBOL_GPL(blk_end_bidi_request);
  *     %false - we are done with this request
  *     %true  - still buffers pending for this request
  **/
-bool __blk_end_bidi_request(struct request *rq, int error,
-			    unsigned int nr_bytes, unsigned int bidi_bytes)
+static bool __blk_end_bidi_request(struct request *rq, int error,
+				   unsigned int nr_bytes, unsigned int bidi_bytes)
 {
 	if (blk_update_bidi_request(rq, error, nr_bytes, bidi_bytes))
 		return true;
@@ -2068,7 +2067,124 @@ bool __blk_end_bidi_request(struct request *rq, int error,
 
 	return false;
 }
-EXPORT_SYMBOL_GPL(__blk_end_bidi_request);
+
+/**
+ * blk_end_request - Helper function for drivers to complete the request.
+ * @rq:       the request being processed
+ * @error:    %0 for success, < %0 for error
+ * @nr_bytes: number of bytes to complete
+ *
+ * Description:
+ *     Ends I/O on a number of bytes attached to @rq.
+ *     If @rq has leftover, sets it up for the next range of segments.
+ *
+ * Return:
+ *     %false - we are done with this request
+ *     %true  - still buffers pending for this request
+ **/
+bool blk_end_request(struct request *rq, int error, unsigned int nr_bytes)
+{
+	return blk_end_bidi_request(rq, error, nr_bytes, 0);
+}
+EXPORT_SYMBOL_GPL(blk_end_request);
+
+/**
+ * blk_end_request_all - Helper function for drives to finish the request.
+ * @rq: the request to finish
+ * @err: %0 for success, < %0 for error
+ *
+ * Description:
+ *     Completely finish @rq.
+ */
+void blk_end_request_all(struct request *rq, int error)
+{
+	bool pending;
+	unsigned int bidi_bytes = 0;
+
+	if (unlikely(blk_bidi_rq(rq)))
+		bidi_bytes = blk_rq_bytes(rq->next_rq);
+
+	pending = blk_end_bidi_request(rq, error, blk_rq_bytes(rq), bidi_bytes);
+	BUG_ON(pending);
+}
+EXPORT_SYMBOL_GPL(blk_end_request_all);
+
+/**
+ * blk_end_request_cur - Helper function to finish the current request chunk.
+ * @rq: the request to finish the current chunk for
+ * @err: %0 for success, < %0 for error
+ *
+ * Description:
+ *     Complete the current consecutively mapped chunk from @rq.
+ *
+ * Return:
+ *     %false - we are done with this request
+ *     %true  - still buffers pending for this request
+ */
+bool blk_end_request_cur(struct request *rq, int error)
+{
+	return blk_end_request(rq, error, blk_rq_cur_bytes(rq));
+}
+EXPORT_SYMBOL_GPL(blk_end_request_cur);
+
+/**
+ * __blk_end_request - Helper function for drivers to complete the request.
+ * @rq:       the request being processed
+ * @error:    %0 for success, < %0 for error
+ * @nr_bytes: number of bytes to complete
+ *
+ * Description:
+ *     Must be called with queue lock held unlike blk_end_request().
+ *
+ * Return:
+ *     %false - we are done with this request
+ *     %true  - still buffers pending for this request
+ **/
+bool __blk_end_request(struct request *rq, int error, unsigned int nr_bytes)
+{
+	return __blk_end_bidi_request(rq, error, nr_bytes, 0);
+}
+EXPORT_SYMBOL_GPL(__blk_end_request);
+
+/**
+ * __blk_end_request_all - Helper function for drives to finish the request.
+ * @rq: the request to finish
+ * @err: %0 for success, < %0 for error
+ *
+ * Description:
+ *     Completely finish @rq.  Must be called with queue lock held.
+ */
+void __blk_end_request_all(struct request *rq, int error)
+{
+	bool pending;
+	unsigned int bidi_bytes = 0;
+
+	if (unlikely(blk_bidi_rq(rq)))
+		bidi_bytes = blk_rq_bytes(rq->next_rq);
+
+	pending = __blk_end_bidi_request(rq, error, blk_rq_bytes(rq), bidi_bytes);
+	BUG_ON(pending);
+}
+EXPORT_SYMBOL_GPL(__blk_end_request_all);
+
+/**
+ * __blk_end_request_cur - Helper function to finish the current request chunk.
+ * @rq: the request to finish the current chunk for
+ * @err: %0 for success, < %0 for error
+ *
+ * Description:
+ *     Complete the current consecutively mapped chunk from @rq.  Must
+ *     be called with queue lock held.
+ *
+ * Return:
+ *     %false - we are done with this request
+ *     %true  - still buffers pending for this request
+ */
+bool __blk_end_request_cur(struct request *rq, int error)
+{
+	return __blk_end_request(rq, error, blk_rq_cur_bytes(rq));
+}
+EXPORT_SYMBOL_GPL(__blk_end_request_cur);
 
 void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
 		     struct bio *bio)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 3b5c564..4dcc0b5 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -873,126 +873,14 @@ extern struct request *blk_fetch_request(struct request_queue *q);
  */
 extern bool blk_update_request(struct request *rq, int error,
 			       unsigned int nr_bytes);
-extern bool blk_end_bidi_request(struct request *rq, int error,
-				 unsigned int nr_bytes,
-				 unsigned int bidi_bytes);
-extern bool __blk_end_bidi_request(struct request *rq, int error,
-				   unsigned int nr_bytes,
-				   unsigned int bidi_bytes);
-
-/**
- * blk_end_request - Helper function for drivers to complete the request.
- * @rq:       the request being processed
- * @error:    %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete
- *
- * Description:
- *     Ends I/O on a number of bytes attached to @rq.
- *     If @rq has leftover, sets it up for the next range of segments.
- *
- * Return:
- *     %false - we are done with this request
- *     %true  - still buffers pending for this request
- **/
-static inline bool blk_end_request(struct request *rq, int error,
-				   unsigned int nr_bytes)
-{
-	return blk_end_bidi_request(rq, error, nr_bytes, 0);
-}
-
-/**
- * blk_end_request_all - Helper function for drives to finish the request.
- * @rq: the request to finish
- * @err: %0 for success, < %0 for error
- *
- * Description:
- *     Completely finish @rq.
- */
-static inline void blk_end_request_all(struct request *rq, int error)
-{
-	bool pending;
-	unsigned int bidi_bytes = 0;
-
-	if (unlikely(blk_bidi_rq(rq)))
-		bidi_bytes = blk_rq_bytes(rq->next_rq);
-
-	pending = blk_end_bidi_request(rq, error, blk_rq_bytes(rq), bidi_bytes);
-	BUG_ON(pending);
-}
-
-/**
- * blk_end_request_cur - Helper function to finish the current request chunk.
- * @rq: the request to finish the current chunk for
- * @err: %0 for success, < %0 for error
- *
- * Description:
- *     Complete the current consecutively mapped chunk from @rq.
- *
- * Return:
- *     %false - we are done with this request
- *     %true  - still buffers pending for this request
- */
-static inline bool blk_end_request_cur(struct request *rq, int error)
-{
-	return blk_end_request(rq, error, blk_rq_cur_bytes(rq));
-}
-
-/**
- * __blk_end_request - Helper function for drivers to complete the request.
- * @rq:       the request being processed
- * @error:    %0 for success, < %0 for error
- * @nr_bytes: number of bytes to complete
- *
- * Description:
- *     Must be called with queue lock held unlike blk_end_request().
- *
- * Return:
- *     %false - we are done with this request
- *     %true  - still buffers pending for this request
- **/
-static inline bool __blk_end_request(struct request *rq, int error,
-				     unsigned int nr_bytes)
-{
-	return __blk_end_bidi_request(rq, error, nr_bytes, 0);
-}
-
-/**
- * __blk_end_request_all - Helper function for drives to finish the request.
- * @rq: the request to finish
- * @err: %0 for success, < %0 for error
- *
- * Description:
- *     Completely finish @rq.  Must be called with queue lock held.
- */
-static inline void __blk_end_request_all(struct request *rq, int error)
-{
-	bool pending;
-	unsigned int bidi_bytes = 0;
-
-	if (unlikely(blk_bidi_rq(rq)))
-		bidi_bytes = blk_rq_bytes(rq->next_rq);
-
-	pending = __blk_end_bidi_request(rq, error, blk_rq_bytes(rq), bidi_bytes);
-	BUG_ON(pending);
-}
-
-/**
- * __blk_end_request_cur - Helper function to finish the current request chunk.
- * @rq: the request to finish the current chunk for
- * @err: %0 for success, < %0 for error
- *
- * Description:
- *     Complete the current consecutively mapped chunk from @rq.  Must
- *     be called with queue lock held.
- *
- * Return:
- *     %false - we are done with this request
- *     %true  - still buffers pending for this request
- */
-static inline bool __blk_end_request_cur(struct request *rq, int error)
-{
-	return __blk_end_request(rq, error, blk_rq_cur_bytes(rq));
-}
+extern bool blk_end_request(struct request *rq, int error,
+			    unsigned int nr_bytes);
+extern void blk_end_request_all(struct request *rq, int error);
+extern bool blk_end_request_cur(struct request *rq, int error);
+extern bool __blk_end_request(struct request *rq, int error,
+			      unsigned int nr_bytes);
+extern void __blk_end_request_all(struct request *rq, int error);
+extern bool __blk_end_request_cur(struct request *rq, int error);
 
 extern void blk_complete_request(struct request *);
 extern void __blk_complete_request(struct request *);
-- 
1.6.0.6


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] block: let blk_end_request_all handle bidi requests
  2009-05-11  8:56 ` [PATCH 1/3] block: let blk_end_request_all handle bidi requests FUJITA Tomonori
@ 2009-05-11  9:06   ` Jens Axboe
  2009-05-11  9:11     ` FUJITA Tomonori
  0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2009-05-11  9:06 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: tj, James.Bottomley, bharrosh, linux-scsi, linux-kernel

On Mon, May 11 2009, FUJITA Tomonori wrote:
> blk_end_request_all() and __blk_end_request_all() should finish all
> bytes including bidi, by definition. That's what all bidi users need ,
> bidi requests must be complete as a whole (partial completion is
> impossible).
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> ---
>  include/linux/blkdev.h |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 8919683..3b5c564 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -911,8 +911,12 @@ static inline bool blk_end_request(struct request *rq, int error,
>  static inline void blk_end_request_all(struct request *rq, int error)
>  {
>  	bool pending;
> +	unsigned int bidi_bytes = 0;
>  
> -	pending = blk_end_request(rq, error, blk_rq_bytes(rq));
> +	if (unlikely(blk_bidi_rq(rq)))
> +		bidi_bytes = blk_rq_bytes(rq->next_rq);
> +
> +	pending = blk_end_bidi_request(rq, error, blk_rq_bytes(rq), bidi_bytes);
>  	BUG_ON(pending);
>  }
>  
> @@ -963,8 +967,12 @@ static inline bool __blk_end_request(struct request *rq, int error,
>  static inline void __blk_end_request_all(struct request *rq, int error)
>  {
>  	bool pending;
> +	unsigned int bidi_bytes = 0;
> +
> +	if (unlikely(blk_bidi_rq(rq)))
> +		bidi_bytes = blk_rq_bytes(rq->next_rq);
>  
> -	pending = __blk_end_request(rq, error, blk_rq_bytes(rq));
> +	pending = __blk_end_bidi_request(rq, error, blk_rq_bytes(rq), bidi_bytes);
>  	BUG_ON(pending);
>  }

Looks ok, perhaps we can next get rid of the bidi naming? It's all
pretty much folded into one anyway, using __blk_end_bidi_request() from
generic end-request handling looks confusing.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] block: completion releated functions cleanup
  2009-05-11  8:56 [PATCH 0/3] block: completion releated functions cleanup FUJITA Tomonori
                   ` (2 preceding siblings ...)
  2009-05-11  8:56 ` [PATCH 3/3] block: move completion related functions back to blk-core.c FUJITA Tomonori
@ 2009-05-11  9:06 ` Jens Axboe
  2009-05-14 13:54 ` [PATCH 0/3] block: completion related " Boaz Harrosh
  4 siblings, 0 replies; 13+ messages in thread
From: Jens Axboe @ 2009-05-11  9:06 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: tj, James.Bottomley, bharrosh, linux-scsi, linux-kernel

On Mon, May 11 2009, FUJITA Tomonori wrote:
> This is a minor cleanup on the completion releated functions. It is on
> the top of Tejun's unify request processing model patchset.

Looks good to me, queued up.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] block: let blk_end_request_all handle bidi requests
  2009-05-11  9:06   ` Jens Axboe
@ 2009-05-11  9:11     ` FUJITA Tomonori
  2009-05-11  9:16       ` Jens Axboe
  0 siblings, 1 reply; 13+ messages in thread
From: FUJITA Tomonori @ 2009-05-11  9:11 UTC (permalink / raw)
  To: jens.axboe
  Cc: fujita.tomonori, tj, James.Bottomley, bharrosh, linux-scsi, linux-kernel

On Mon, 11 May 2009 11:06:30 +0200
Jens Axboe <jens.axboe@oracle.com> wrote:

> On Mon, May 11 2009, FUJITA Tomonori wrote:
> > blk_end_request_all() and __blk_end_request_all() should finish all
> > bytes including bidi, by definition. That's what all bidi users need ,
> > bidi requests must be complete as a whole (partial completion is
> > impossible).
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > ---
> >  include/linux/blkdev.h |   12 ++++++++++--
> >  1 files changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > index 8919683..3b5c564 100644
> > --- a/include/linux/blkdev.h
> > +++ b/include/linux/blkdev.h
> > @@ -911,8 +911,12 @@ static inline bool blk_end_request(struct request *rq, int error,
> >  static inline void blk_end_request_all(struct request *rq, int error)
> >  {
> >  	bool pending;
> > +	unsigned int bidi_bytes = 0;
> >  
> > -	pending = blk_end_request(rq, error, blk_rq_bytes(rq));
> > +	if (unlikely(blk_bidi_rq(rq)))
> > +		bidi_bytes = blk_rq_bytes(rq->next_rq);
> > +
> > +	pending = blk_end_bidi_request(rq, error, blk_rq_bytes(rq), bidi_bytes);
> >  	BUG_ON(pending);
> >  }
> >  
> > @@ -963,8 +967,12 @@ static inline bool __blk_end_request(struct request *rq, int error,
> >  static inline void __blk_end_request_all(struct request *rq, int error)
> >  {
> >  	bool pending;
> > +	unsigned int bidi_bytes = 0;
> > +
> > +	if (unlikely(blk_bidi_rq(rq)))
> > +		bidi_bytes = blk_rq_bytes(rq->next_rq);
> >  
> > -	pending = __blk_end_request(rq, error, blk_rq_bytes(rq));
> > +	pending = __blk_end_bidi_request(rq, error, blk_rq_bytes(rq), bidi_bytes);
> >  	BUG_ON(pending);
> >  }
> 
> Looks ok, perhaps we can next get rid of the bidi naming? It's all
> pretty much folded into one anyway, using __blk_end_bidi_request() from
> generic end-request handling looks confusing.

Yeah, agreed; the bidi name is confusing. I'll send another patch to
clean up it on the top of this patchset soon.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] block: let blk_end_request_all handle bidi requests
  2009-05-11  9:11     ` FUJITA Tomonori
@ 2009-05-11  9:16       ` Jens Axboe
  0 siblings, 0 replies; 13+ messages in thread
From: Jens Axboe @ 2009-05-11  9:16 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: tj, James.Bottomley, bharrosh, linux-scsi, linux-kernel

On Mon, May 11 2009, FUJITA Tomonori wrote:
> On Mon, 11 May 2009 11:06:30 +0200
> Jens Axboe <jens.axboe@oracle.com> wrote:
> 
> > On Mon, May 11 2009, FUJITA Tomonori wrote:
> > > blk_end_request_all() and __blk_end_request_all() should finish all
> > > bytes including bidi, by definition. That's what all bidi users need ,
> > > bidi requests must be complete as a whole (partial completion is
> > > impossible).
> > > 
> > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > > ---
> > >  include/linux/blkdev.h |   12 ++++++++++--
> > >  1 files changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > > index 8919683..3b5c564 100644
> > > --- a/include/linux/blkdev.h
> > > +++ b/include/linux/blkdev.h
> > > @@ -911,8 +911,12 @@ static inline bool blk_end_request(struct request *rq, int error,
> > >  static inline void blk_end_request_all(struct request *rq, int error)
> > >  {
> > >  	bool pending;
> > > +	unsigned int bidi_bytes = 0;
> > >  
> > > -	pending = blk_end_request(rq, error, blk_rq_bytes(rq));
> > > +	if (unlikely(blk_bidi_rq(rq)))
> > > +		bidi_bytes = blk_rq_bytes(rq->next_rq);
> > > +
> > > +	pending = blk_end_bidi_request(rq, error, blk_rq_bytes(rq), bidi_bytes);
> > >  	BUG_ON(pending);
> > >  }
> > >  
> > > @@ -963,8 +967,12 @@ static inline bool __blk_end_request(struct request *rq, int error,
> > >  static inline void __blk_end_request_all(struct request *rq, int error)
> > >  {
> > >  	bool pending;
> > > +	unsigned int bidi_bytes = 0;
> > > +
> > > +	if (unlikely(blk_bidi_rq(rq)))
> > > +		bidi_bytes = blk_rq_bytes(rq->next_rq);
> > >  
> > > -	pending = __blk_end_request(rq, error, blk_rq_bytes(rq));
> > > +	pending = __blk_end_bidi_request(rq, error, blk_rq_bytes(rq), bidi_bytes);
> > >  	BUG_ON(pending);
> > >  }
> > 
> > Looks ok, perhaps we can next get rid of the bidi naming? It's all
> > pretty much folded into one anyway, using __blk_end_bidi_request() from
> > generic end-request handling looks confusing.
> 
> Yeah, agreed; the bidi name is confusing. I'll send another patch to
> clean up it on the top of this patchset soon.

Thanks!

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] block: completion related functions cleanup
  2009-05-11  8:56 [PATCH 0/3] block: completion releated functions cleanup FUJITA Tomonori
                   ` (3 preceding siblings ...)
  2009-05-11  9:06 ` [PATCH 0/3] block: completion releated functions cleanup Jens Axboe
@ 2009-05-14 13:54 ` Boaz Harrosh
  2009-05-14 14:05   ` Tejun Heo
  4 siblings, 1 reply; 13+ messages in thread
From: Boaz Harrosh @ 2009-05-14 13:54 UTC (permalink / raw)
  To: FUJITA Tomonori, jens.axboe, Tejun Heo
  Cc: James.Bottomley, linux-scsi, linux-kernel

On 05/11/2009 11:56 AM, FUJITA Tomonori wrote:
> This is a minor cleanup on the completion releated functions. It is on
> the top of Tejun's unify request processing model patchset.
> 
> 

Hi Tomo, Jens, Tejun.

A small left-over from this grate cleanup work. I think it was Tejun's
patch that left this unused.

(Should go through block tree)

Here:
---
From: Boaz Harrosh <bharrosh@panasas.com>
Date: Thu, 14 May 2009 16:46:03 +0300
Subject: [PATCH] scsi_lib: remove unused variable assignment

The last request completion cleanup in scsi_lib left an unused
this_count assignment in scsi_io_completion().
(It was used before in a code segment that now uses blk_end_request_all())

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/scsi_lib.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index e410d66..f8600d6 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -789,7 +789,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 	 */
 	if (scsi_end_request(cmd, error, good_bytes, result == 0) == NULL)
 		return;
-	this_count = blk_rq_bytes(req);
 
 	error = -EIO;
 
-- 
1.6.2.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] block: completion related functions cleanup
  2009-05-14 13:54 ` [PATCH 0/3] block: completion related " Boaz Harrosh
@ 2009-05-14 14:05   ` Tejun Heo
  2009-05-14 14:15     ` Boaz Harrosh
  0 siblings, 1 reply; 13+ messages in thread
From: Tejun Heo @ 2009-05-14 14:05 UTC (permalink / raw)
  To: Boaz Harrosh
  Cc: FUJITA Tomonori, jens.axboe, James.Bottomley, linux-scsi, linux-kernel

Boaz Harrosh wrote:
> ---
>  drivers/scsi/scsi_lib.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index e410d66..f8600d6 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -789,7 +789,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
>  	 */
>  	if (scsi_end_request(cmd, error, good_bytes, result == 0) == NULL)
>  		return;
> -	this_count = blk_rq_bytes(req);
>  
>  	error = -EIO;
>  

Nice spotting.  Looks like the variable can be killed completely?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] block: completion related functions cleanup
  2009-05-14 14:05   ` Tejun Heo
@ 2009-05-14 14:15     ` Boaz Harrosh
  2009-05-14 15:46       ` Tejun Heo
  2009-05-15  0:12       ` FUJITA Tomonori
  0 siblings, 2 replies; 13+ messages in thread
From: Boaz Harrosh @ 2009-05-14 14:15 UTC (permalink / raw)
  To: Tejun Heo
  Cc: FUJITA Tomonori, jens.axboe, James.Bottomley, linux-scsi, linux-kernel

On 05/14/2009 05:05 PM, Tejun Heo wrote:
> Nice spotting.  Looks like the variable can be killed completely?
> 
> Thanks.
> 

Sorry yes, here:
---
From: Boaz Harrosh <bharrosh@panasas.com>
Date: Thu, 14 May 2009 16:46:03 +0300
Subject: [PATCH] scsi_lib: remove unused variable

The last request completion cleanup in scsi_lib left an unused
this_count variable in scsi_io_completion().
(It was used before in a code segment that now uses blk_end_request_all())

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/scsi_lib.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index e410d66..d7c6c75 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -706,7 +706,6 @@ EXPORT_SYMBOL(scsi_release_buffers);
 void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 {
 	int result = cmd->result;
-	int this_count;
 	struct request_queue *q = cmd->device->request_queue;
 	struct request *req = cmd->request;
 	int error = 0;
@@ -789,7 +788,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 	 */
 	if (scsi_end_request(cmd, error, good_bytes, result == 0) == NULL)
 		return;
-	this_count = blk_rq_bytes(req);
 
 	error = -EIO;
 
-- 
1.6.2.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] block: completion related functions cleanup
  2009-05-14 14:15     ` Boaz Harrosh
@ 2009-05-14 15:46       ` Tejun Heo
  2009-05-15  0:12       ` FUJITA Tomonori
  1 sibling, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2009-05-14 15:46 UTC (permalink / raw)
  To: Boaz Harrosh
  Cc: FUJITA Tomonori, jens.axboe, James.Bottomley, linux-scsi, linux-kernel

Boaz Harrosh wrote:
> From: Boaz Harrosh <bharrosh@panasas.com>
> Date: Thu, 14 May 2009 16:46:03 +0300
> Subject: [PATCH] scsi_lib: remove unused variable
> 
> The last request completion cleanup in scsi_lib left an unused
> this_count variable in scsi_io_completion().
> (It was used before in a code segment that now uses blk_end_request_all())
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] block: completion related functions cleanup
  2009-05-14 14:15     ` Boaz Harrosh
  2009-05-14 15:46       ` Tejun Heo
@ 2009-05-15  0:12       ` FUJITA Tomonori
  1 sibling, 0 replies; 13+ messages in thread
From: FUJITA Tomonori @ 2009-05-15  0:12 UTC (permalink / raw)
  To: bharrosh
  Cc: tj, fujita.tomonori, jens.axboe, James.Bottomley, linux-scsi,
	linux-kernel

On Thu, 14 May 2009 17:15:05 +0300
Boaz Harrosh <bharrosh@panasas.com> wrote:

> On 05/14/2009 05:05 PM, Tejun Heo wrote:
> > Nice spotting.  Looks like the variable can be killed completely?
> > 
> > Thanks.
> > 
> 
> Sorry yes, here:
> ---
> From: Boaz Harrosh <bharrosh@panasas.com>
> Date: Thu, 14 May 2009 16:46:03 +0300
> Subject: [PATCH] scsi_lib: remove unused variable
> 
> The last request completion cleanup in scsi_lib left an unused
> this_count variable in scsi_io_completion().
> (It was used before in a code segment that now uses blk_end_request_all())
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>

Ah, I overlooked this.

Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

Thanks,

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2009-05-15  0:14 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-11  8:56 [PATCH 0/3] block: completion releated functions cleanup FUJITA Tomonori
2009-05-11  8:56 ` [PATCH 1/3] block: let blk_end_request_all handle bidi requests FUJITA Tomonori
2009-05-11  9:06   ` Jens Axboe
2009-05-11  9:11     ` FUJITA Tomonori
2009-05-11  9:16       ` Jens Axboe
2009-05-11  8:56 ` [PATCH 2/3] scsi: simplify the bidi completion FUJITA Tomonori
2009-05-11  8:56 ` [PATCH 3/3] block: move completion related functions back to blk-core.c FUJITA Tomonori
2009-05-11  9:06 ` [PATCH 0/3] block: completion releated functions cleanup Jens Axboe
2009-05-14 13:54 ` [PATCH 0/3] block: completion related " Boaz Harrosh
2009-05-14 14:05   ` Tejun Heo
2009-05-14 14:15     ` Boaz Harrosh
2009-05-14 15:46       ` Tejun Heo
2009-05-15  0:12       ` FUJITA Tomonori

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.