* [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.