Re: [PATCH 27/28] blk_end_request: changing scsi mid-layer for bidi (take 3)

2007-12-09 Thread Boaz Harrosh 
On Thu, Dec 06 2007 at 21:44 +0200, Kiyoshi Ueda [EMAIL PROTECTED] wrote:
 Hi Boaz, Jens,
 
 On Thu, 06 Dec 2007 11:24:44 +0200, Boaz Harrosh [EMAIL PROTECTED] wrote:
 Index: 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
snip
 No I don't like it. The only client left for blk_end_request_callback()
 is bidi,
 
 ide-cd (cdrom_newpc_intr) is another client.
 So I can't drop blk_end_request_callback() even if bidi doesn't use it.
 
It looks like all is needed for the ide-cd case is a flag that says
don't complete the request. And the call-back is not actually used.
(Inspecting the last: [PATCH 26/28] blk_end_request: changing ide-cd (take 3))
The same exact flag could also help the bidi case. Perhaps have an API
for that?

snip
 
 Index: 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
 ===
 --- 2.6.24-rc3-mm2.orig/drivers/scsi/scsi_lib.c
 +++ 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
 @@ -629,28 +629,6 @@ void scsi_run_host_queues(struct Scsi_Ho
   scsi_run_queue(sdev-request_queue);
  }
  
 -static void scsi_finalize_request(struct scsi_cmnd *cmd, int uptodate)
 -{
 - struct request_queue *q = cmd-device-request_queue;
 - struct request *req = cmd-request;
 - unsigned long flags;
 -
 - add_disk_randomness(req-rq_disk);
 -
 - spin_lock_irqsave(q-queue_lock, flags);
 - if (blk_rq_tagged(req))
 - blk_queue_end_tag(q, req);
 -
 - end_that_request_last(req, uptodate);
 - spin_unlock_irqrestore(q-queue_lock, flags);
 -
 - /*
 -  * 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_end_request()
   *
 @@ -930,23 +908,25 @@ EXPORT_SYMBOL(scsi_release_buffers);
  void scsi_end_bidi_request(struct scsi_cmnd *cmd)
  {
   struct request *req = cmd-request;
 + unsigned int dlen = req-data_len;
 + unsigned int next_dlen = req-next_rq-data_len;
  
 - end_that_request_chunk(req, 1, req-data_len);
   req-data_len = scsi_out(cmd)-resid;
 -
 - end_that_request_chunk(req-next_rq, 1, req-next_rq-data_len);
   req-next_rq-data_len = scsi_in(cmd)-resid;
  
 - scsi_release_buffers(cmd);
 -
   /*
*FIXME: If ll_rw_blk.c is changed to also put_request(req-next_rq)
 -  *   in end_that_request_last() then this WARN_ON must be removed.
 +  *   in blk_end_bidi_request() then this WARN_ON must be removed.
*   for now, upper-driver must have registered an end_io.
*/
   WARN_ON(!req-end_io);
  
 - scsi_finalize_request(cmd, 1);
 + if (blk_end_bidi_request(req, 1, dlen, next_dlen))
 + /* req has not been completed */
 + BUG();
 +
 + scsi_release_buffers(cmd);
 + scsi_next_command(cmd);
  }
  
  /*
 Index: 2.6.24-rc3-mm2/block/ll_rw_blk.c
 ===
 --- 2.6.24-rc3-mm2.orig/block/ll_rw_blk.c
 +++ 2.6.24-rc3-mm2/block/ll_rw_blk.c
 @@ -3792,24 +3792,17 @@ static void complete_request(struct requ
   if (!list_empty(rq-queuelist))
   blkdev_dequeue_request(rq);
  
 + if (blk_bidi_rq(rq)  !rq-end_io)
 + __blk_put_request(rq-next_rq-q, rq-next_rq);
 +
   end_that_request_last(rq, uptodate);
  }
  
 -/**
 - * blk_end_request - Helper function for drivers to complete the request.
 - * @rq:   the request being processed
 - * @uptodate: 1 for success, 0 for I/O error,  0 for specific 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:
 - * 0 - we are done with this request
 - * 1 - still buffers pending for this request
 - **/
 -int blk_end_request(struct request *rq, int uptodate, int nr_bytes)
 +/*
 + * Internal function
 + */
 +static int blk_end_io(struct request *rq, int uptodate, int nr_bytes,
 +   int bidi_bytes, int (drv_callback)(struct request *))
  {
   struct request_queue *q = rq-q;
   unsigned long flags = 0UL;
 @@ -3817,8 +3810,17 @@ int blk_end_request(struct request *rq, 
   if (blk_fs_request(rq) || blk_pc_request(rq)) {
   if (__end_that_request_first(rq, uptodate, nr_bytes))
   return 1;
 +
 + /* Bidi request must be completed as a whole */
 + if (blk_bidi_rq(rq) 
 + __end_that_request_first(rq-next_rq, uptodate, bidi_bytes))
 + return 1;
   }
  
 + /* Special feature for tricky drivers */
 + if (drv_callback  drv_callback(rq))
 + return 1;
 +
   add_disk_randomness(rq-rq_disk);
  
   spin_lock_irqsave(q-queue_lock, flags);
 @@ -3827,6 +3829,32 @@ int blk_end_request(struct request *rq, 
  
   return 0;
  }
 +
 +int

Re: [PATCH 27/28] blk_end_request: changing scsi mid-layer for bidi (take 3)

2007-12-06 Thread Boaz Harrosh 
 blk_end_request twice and looks trickier.
 So I'd like to leave the patch but descriptions and comments in it.
 Below is the patch, which updated only descriptions and comments.
 If you still have some concerns on this, please let me know.
 
 
 
 Subject: [PATCH 27/28] blk_end_request: changing scsi mid-layer for bidi 
 (take 3)
 
 This patch converts bidi of scsi mid-layer to use blk_end_request
 interfaces.
 
 rq-next_rq represents a pair of bidi requests.
 (There are no other use of 'next_rq' of struct request.)
 For both requests in the pair, end_that_request_chunk() should be
 called before end_that_request_last() is called for one of them.
 Since the calls to end_that_request_first()/chunk() and
 end_that_request_last() are packaged into blk_end_request(),
 the handling of next_rq completion has to be moved into
 blk_end_request(), too.
 
 Since blk_end_request() family don't have any argument for
 the completion size of next_rq, they use next_rq-data_len for it.
 So bidi has to set the resid after blk_end_request() completes
 the data of next_rq.
 To satisfy the requirement, bidi uses blk_end_request_callback(),
 which is added in PATCH 25 only for the tricky drivers.
 
 Signed-off-by: Kiyoshi Ueda [EMAIL PROTECTED]
 Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED]
 ---
  block/ll_rw_blk.c   |   18 +
  drivers/scsi/scsi_lib.c |   62 
 +++-
  2 files changed, 48 insertions(+), 32 deletions(-)
 
 Index: 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
 ===
 --- 2.6.24-rc3-mm2.orig/drivers/scsi/scsi_lib.c
 +++ 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
 @@ -629,28 +629,6 @@ void scsi_run_host_queues(struct Scsi_Ho
   scsi_run_queue(sdev-request_queue);
  }
  
 -static void scsi_finalize_request(struct scsi_cmnd *cmd, int uptodate)
 -{
 - struct request_queue *q = cmd-device-request_queue;
 - struct request *req = cmd-request;
 - unsigned long flags;
 -
 - add_disk_randomness(req-rq_disk);
 -
 - spin_lock_irqsave(q-queue_lock, flags);
 - if (blk_rq_tagged(req))
 - blk_queue_end_tag(q, req);
 -
 - end_that_request_last(req, uptodate);
 - spin_unlock_irqrestore(q-queue_lock, flags);
 -
 - /*
 -  * 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_end_request()
   *
 @@ -921,6 +899,20 @@ void scsi_release_buffers(struct scsi_cm
  EXPORT_SYMBOL(scsi_release_buffers);
  
  /*
 + * Called from blk_end_request_callback() after all DATA in rq and its 
 next_rq
 + * are completed before rq is completed/freed.
 + */
 +static int scsi_end_bidi_request_cb(struct request *rq)
 +{
 + struct scsi_cmnd *cmd = rq-special;
 +
 + rq-data_len = scsi_out(cmd)-resid;
 + rq-next_rq-data_len = scsi_in(cmd)-resid;
 +
 + return 0;
 +}
 +
 +/*
   * 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
 @@ -931,22 +923,28 @@ void scsi_end_bidi_request(struct scsi_c
  {
   struct request *req = cmd-request;
  
 - end_that_request_chunk(req, 1, req-data_len);
 - req-data_len = scsi_out(cmd)-resid;
 -
 - end_that_request_chunk(req-next_rq, 1, req-next_rq-data_len);
 - req-next_rq-data_len = scsi_in(cmd)-resid;
 -
 - scsi_release_buffers(cmd);
 -
   /*
*FIXME: If ll_rw_blk.c is changed to also put_request(req-next_rq)
 -  *   in end_that_request_last() then this WARN_ON must be removed.
 +  *   in blk_end_request() then this WARN_ON must be removed.
*   for now, upper-driver must have registered an end_io.
*/
   WARN_ON(!req-end_io);
  
 - scsi_finalize_request(cmd, 1);
 + /*
 +  * blk_end_request() family take care of data completion of next_rq.
 +  * blk_end_request() family use next_rq-data_len for 
 +  * the completion data size of next_rq.
 +  * So resid can't be set before the data completion of next_rq
 +  * in blk_end_request().
 +  * To resolve that, use the callback feature of blk_end_request().
 +  */
 + if (blk_end_request_callback(req, 1, req-data_len,
 +  scsi_end_bidi_request_cb))
 + /* req has not been completed */
 + BUG();
 +
 + scsi_release_buffers(cmd);
 + scsi_next_command(cmd);
  }
  
  /*
 Index: 2.6.24-rc3-mm2/block/ll_rw_blk.c
 ===
 --- 2.6.24-rc3-mm2.orig/block/ll_rw_blk.c
 +++ 2.6.24-rc3-mm2/block/ll_rw_blk.c
 @@ -3817,6 +3817,12 @@ int blk_end_request(struct request *rq, 
   if (blk_fs_request(rq) || blk_pc_request(rq)) {
   if (__end_that_request_first(rq, uptodate, nr_bytes

Re: [PATCH 27/28] blk_end_request: changing scsi mid-layer for bidi (take 3)

2007-12-06 Thread Kiyoshi Ueda 
Hi Boaz, Jens,

On Thu, 06 Dec 2007 11:24:44 +0200, Boaz Harrosh [EMAIL PROTECTED] wrote:
  Index: 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
  ===
  --- 2.6.24-rc3-mm2.orig/drivers/scsi/scsi_lib.c
  +++ 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
  @@ -629,28 +629,6 @@ void scsi_run_host_queues(struct Scsi_Ho
  scsi_run_queue(sdev-request_queue);
   }
   
  -static void scsi_finalize_request(struct scsi_cmnd *cmd, int uptodate)
  -{
  -   struct request_queue *q = cmd-device-request_queue;
  -   struct request *req = cmd-request;
  -   unsigned long flags;
  -
  -   add_disk_randomness(req-rq_disk);
  -
  -   spin_lock_irqsave(q-queue_lock, flags);
  -   if (blk_rq_tagged(req))
  -   blk_queue_end_tag(q, req);
  -
  -   end_that_request_last(req, uptodate);
  -   spin_unlock_irqrestore(q-queue_lock, flags);
  -
  -   /*
  -* 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_end_request()
*
  @@ -921,6 +899,20 @@ void scsi_release_buffers(struct scsi_cm
   EXPORT_SYMBOL(scsi_release_buffers);
   
   /*
  + * Called from blk_end_request_callback() after all DATA in rq and its 
  next_rq
  + * are completed before rq is completed/freed.
  + */
  +static int scsi_end_bidi_request_cb(struct request *rq)
  +{
  +   struct scsi_cmnd *cmd = rq-special;
  +
  +   rq-data_len = scsi_out(cmd)-resid;
  +   rq-next_rq-data_len = scsi_in(cmd)-resid;
  +
  +   return 0;
  +}
  +
  +/*
* 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
  @@ -931,22 +923,28 @@ void scsi_end_bidi_request(struct scsi_c
   {
  struct request *req = cmd-request;
   
  -   end_that_request_chunk(req, 1, req-data_len);
  -   req-data_len = scsi_out(cmd)-resid;
  -
  -   end_that_request_chunk(req-next_rq, 1, req-next_rq-data_len);
  -   req-next_rq-data_len = scsi_in(cmd)-resid;
  -
  -   scsi_release_buffers(cmd);
  -
  /*
   *FIXME: If ll_rw_blk.c is changed to also put_request(req-next_rq)
  -*   in end_that_request_last() then this WARN_ON must be removed.
  +*   in blk_end_request() then this WARN_ON must be removed.
   *   for now, upper-driver must have registered an end_io.
   */
  WARN_ON(!req-end_io);
   
  -   scsi_finalize_request(cmd, 1);
  +   /*
  +* blk_end_request() family take care of data completion of next_rq.
  +* blk_end_request() family use next_rq-data_len for 
  +* the completion data size of next_rq.
  +* So resid can't be set before the data completion of next_rq
  +* in blk_end_request().
  +* To resolve that, use the callback feature of blk_end_request().
  +*/
  +   if (blk_end_request_callback(req, 1, req-data_len,
  +scsi_end_bidi_request_cb))
  +   /* req has not been completed */
  +   BUG();
  +
  +   scsi_release_buffers(cmd);
  +   scsi_next_command(cmd);
   }
   
   /*
  Index: 2.6.24-rc3-mm2/block/ll_rw_blk.c
  ===
  --- 2.6.24-rc3-mm2.orig/block/ll_rw_blk.c
  +++ 2.6.24-rc3-mm2/block/ll_rw_blk.c
  @@ -3817,6 +3817,12 @@ int blk_end_request(struct request *rq, 
  if (blk_fs_request(rq) || blk_pc_request(rq)) {
  if (__end_that_request_first(rq, uptodate, nr_bytes))
  return 1;
  +
  +   /* Bidi request must be completed as a whole */
  +   if (blk_bidi_rq(rq) 
  +   __end_that_request_first(rq-next_rq, uptodate,
  +blk_rq_bytes(rq-next_rq)))
  +   return 1;
  }
   
  add_disk_randomness(rq-rq_disk);
  @@ -3840,6 +3846,12 @@ int __blk_end_request(struct request *rq
  if (blk_fs_request(rq) || blk_pc_request(rq)) {
  if (__end_that_request_first(rq, uptodate, nr_bytes))
  return 1;
  +
  +   /* Bidi request must be completed as a whole */
  +   if (blk_bidi_rq(rq) 
  +   __end_that_request_first(rq-next_rq, uptodate,
  +blk_rq_bytes(rq-next_rq)))
  +   return 1;
  }
   
  add_disk_randomness(rq-rq_disk);
  @@ -3884,6 +3896,12 @@ int blk_end_request_callback(struct requ
  if (blk_fs_request(rq) || blk_pc_request(rq)) {
  if (__end_that_request_first(rq, uptodate, nr_bytes))
  return 1;
  +
  +   /* Bidi request must be completed as a whole */
  +   if (blk_bidi_rq(rq) 
  +   __end_that_request_first(rq-next_rq, uptodate,
  +blk_rq_bytes(rq-next_rq)))
  +   return 1;
  }

Re: [PATCH 27/28] blk_end_request: changing scsi mid-layer for bidi (take 3)

2007-12-05 Thread Kiyoshi Ueda 
 the patch but descriptions and comments in it.
Below is the patch, which updated only descriptions and comments.
If you still have some concerns on this, please let me know.



Subject: [PATCH 27/28] blk_end_request: changing scsi mid-layer for bidi (take 
3)

This patch converts bidi of scsi mid-layer to use blk_end_request
interfaces.

rq-next_rq represents a pair of bidi requests.
(There are no other use of 'next_rq' of struct request.)
For both requests in the pair, end_that_request_chunk() should be
called before end_that_request_last() is called for one of them.
Since the calls to end_that_request_first()/chunk() and
end_that_request_last() are packaged into blk_end_request(),
the handling of next_rq completion has to be moved into
blk_end_request(), too.

Since blk_end_request() family don't have any argument for
the completion size of next_rq, they use next_rq-data_len for it.
So bidi has to set the resid after blk_end_request() completes
the data of next_rq.
To satisfy the requirement, bidi uses blk_end_request_callback(),
which is added in PATCH 25 only for the tricky drivers.

Signed-off-by: Kiyoshi Ueda [EMAIL PROTECTED]
Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED]
---
 block/ll_rw_blk.c   |   18 +
 drivers/scsi/scsi_lib.c |   62 +++-
 2 files changed, 48 insertions(+), 32 deletions(-)

Index: 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
===
--- 2.6.24-rc3-mm2.orig/drivers/scsi/scsi_lib.c
+++ 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
@@ -629,28 +629,6 @@ void scsi_run_host_queues(struct Scsi_Ho
scsi_run_queue(sdev-request_queue);
 }
 
-static void scsi_finalize_request(struct scsi_cmnd *cmd, int uptodate)
-{
-   struct request_queue *q = cmd-device-request_queue;
-   struct request *req = cmd-request;
-   unsigned long flags;
-
-   add_disk_randomness(req-rq_disk);
-
-   spin_lock_irqsave(q-queue_lock, flags);
-   if (blk_rq_tagged(req))
-   blk_queue_end_tag(q, req);
-
-   end_that_request_last(req, uptodate);
-   spin_unlock_irqrestore(q-queue_lock, flags);
-
-   /*
-* 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_end_request()
  *
@@ -921,6 +899,20 @@ void scsi_release_buffers(struct scsi_cm
 EXPORT_SYMBOL(scsi_release_buffers);
 
 /*
+ * Called from blk_end_request_callback() after all DATA in rq and its next_rq
+ * are completed before rq is completed/freed.
+ */
+static int scsi_end_bidi_request_cb(struct request *rq)
+{
+   struct scsi_cmnd *cmd = rq-special;
+
+   rq-data_len = scsi_out(cmd)-resid;
+   rq-next_rq-data_len = scsi_in(cmd)-resid;
+
+   return 0;
+}
+
+/*
  * 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
@@ -931,22 +923,28 @@ void scsi_end_bidi_request(struct scsi_c
 {
struct request *req = cmd-request;
 
-   end_that_request_chunk(req, 1, req-data_len);
-   req-data_len = scsi_out(cmd)-resid;
-
-   end_that_request_chunk(req-next_rq, 1, req-next_rq-data_len);
-   req-next_rq-data_len = scsi_in(cmd)-resid;
-
-   scsi_release_buffers(cmd);
-
/*
 *FIXME: If ll_rw_blk.c is changed to also put_request(req-next_rq)
-*   in end_that_request_last() then this WARN_ON must be removed.
+*   in blk_end_request() then this WARN_ON must be removed.
 *   for now, upper-driver must have registered an end_io.
 */
WARN_ON(!req-end_io);
 
-   scsi_finalize_request(cmd, 1);
+   /*
+* blk_end_request() family take care of data completion of next_rq.
+* blk_end_request() family use next_rq-data_len for 
+* the completion data size of next_rq.
+* So resid can't be set before the data completion of next_rq
+* in blk_end_request().
+* To resolve that, use the callback feature of blk_end_request().
+*/
+   if (blk_end_request_callback(req, 1, req-data_len,
+scsi_end_bidi_request_cb))
+   /* req has not been completed */
+   BUG();
+
+   scsi_release_buffers(cmd);
+   scsi_next_command(cmd);
 }
 
 /*
Index: 2.6.24-rc3-mm2/block/ll_rw_blk.c
===
--- 2.6.24-rc3-mm2.orig/block/ll_rw_blk.c
+++ 2.6.24-rc3-mm2/block/ll_rw_blk.c
@@ -3817,6 +3817,12 @@ int blk_end_request(struct request *rq, 
if (blk_fs_request(rq) || blk_pc_request(rq)) {
if (__end_that_request_first(rq, uptodate, nr_bytes))
return 1;
+
+   /* Bidi request must be completed as a whole

Re: [PATCH 27/28] blk_end_request: changing scsi mid-layer for bidi (take 3)

2007-12-04 Thread Boaz Harrosh 
On Sat, Dec 01 2007 at 1:35 +0200, Kiyoshi Ueda [EMAIL PROTECTED] wrote:
 This patch converts bidi of scsi mid-layer to use blk_end_request().
 
 rq-next_rq represents a pair of bidi requests.
 (There are no other use of 'next_rq' of struct request.)
 For both requests in the pair, end_that_request_chunk() should be
 called before end_that_request_last() is called for one of them.
 Since the calls to end_that_request_first()/chunk() and
 end_that_request_last() are packaged into blk_end_request(),
 the handling of next_rq completion has to be moved into
 blk_end_request(), too.
 
 Bidi sets its specific value to rq-data_len before the request is
 completed so that upper-layer can read it.
 This setting must be between end_that_request_chunk() and
 end_that_request_last(), because rq-data_len may be used
 in end_that_request_chunk() by blk_trace and so on.
 To satisfy the requirement, use blk_end_request_callback() which
 is added in PATCH 25 only for the tricky drivers.
 
 If bidi didn't reuse rq-data_len and added new members to request
 for the specific value, it could set before end_that_request_chunk()
 and use the standard blk_end_request() like below.
 
 void scsi_end_bidi_request(struct scsi_cmnd *cmd)
 {
   struct request *req = cmd-request;
 
   rq-resid = scsi_out(cmd)-resid;
   rq-next_rq-resid = scsi_in(cmd)-resid;
 
   if (blk_end_request(req, 1, req-data_len))
   BUG();
 
   scsi_release_buffers(cmd);
   scsi_next_command(cmd);
 }
 
 Signed-off-by: Kiyoshi Ueda [EMAIL PROTECTED]
 Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED]
 ---
  block/ll_rw_blk.c   |   18 +
  drivers/scsi/scsi_lib.c |   66 
 
  2 files changed, 52 insertions(+), 32 deletions(-)
 
 Index: 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
 ===
 --- 2.6.24-rc3-mm2.orig/drivers/scsi/scsi_lib.c
 +++ 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
 @@ -629,28 +629,6 @@ void scsi_run_host_queues(struct Scsi_Ho
   scsi_run_queue(sdev-request_queue);
  }
  
 -static void scsi_finalize_request(struct scsi_cmnd *cmd, int uptodate)
 -{
 - struct request_queue *q = cmd-device-request_queue;
 - struct request *req = cmd-request;
 - unsigned long flags;
 -
 - add_disk_randomness(req-rq_disk);
 -
 - spin_lock_irqsave(q-queue_lock, flags);
 - if (blk_rq_tagged(req))
 - blk_queue_end_tag(q, req);
 -
 - end_that_request_last(req, uptodate);
 - spin_unlock_irqrestore(q-queue_lock, flags);
 -
 - /*
 -  * 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_end_request()
   *
 @@ -921,6 +899,20 @@ void scsi_release_buffers(struct scsi_cm
  EXPORT_SYMBOL(scsi_release_buffers);
  
  /*
 + * Called from blk_end_request_callback() after all DATA in rq and its 
 next_rq
 + * are completed before rq is completed/freed.
 + */
 +static int scsi_end_bidi_request_cb(struct request *rq)
 +{
 + struct scsi_cmnd *cmd = rq-special;
 +
 + rq-data_len = scsi_out(cmd)-resid;
 + rq-next_rq-data_len = scsi_in(cmd)-resid;
 +
 + return 0;
 +}
 +
 +/*
   * 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
 @@ -931,22 +923,32 @@ void scsi_end_bidi_request(struct scsi_c
  {
   struct request *req = cmd-request;
  
 - end_that_request_chunk(req, 1, req-data_len);
 - req-data_len = scsi_out(cmd)-resid;
 -
 - end_that_request_chunk(req-next_rq, 1, req-next_rq-data_len);
 - req-next_rq-data_len = scsi_in(cmd)-resid;
 -
 - scsi_release_buffers(cmd);
 -
   /*
*FIXME: If ll_rw_blk.c is changed to also put_request(req-next_rq)
 -  *   in end_that_request_last() then this WARN_ON must be removed.
 +  *   in blk_end_request() then this WARN_ON must be removed.
*   for now, upper-driver must have registered an end_io.
*/
   WARN_ON(!req-end_io);
  
 - scsi_finalize_request(cmd, 1);
 + /*
 +  * blk_end_request() family take care of data completion of next_rq.
 +  *
 +  * req-data_len and req-next_rq-data_len must be set after
 +  * all data are completed, since they may be referenced during
 +  * the data completion process.
 +  * So use the callback feature of blk_end_request() here.
 +  *
 +  * NOTE: If bidi doesn't reuse the data_len field for upper-layer's
 +  *   reference (e.g. adds new members for it to struct request),
 +  *   we can use the standard blk_end_request() interface here.
 +  */
 + if (blk_end_request_callback(req, 1, req-data_len,
 +  scsi_end_bidi_request_cb))
 + /*

[PATCH 27/28] blk_end_request: changing scsi mid-layer for bidi (take 3)

2007-11-30 Thread Kiyoshi Ueda 
This patch converts bidi of scsi mid-layer to use blk_end_request().

rq-next_rq represents a pair of bidi requests.
(There are no other use of 'next_rq' of struct request.)
For both requests in the pair, end_that_request_chunk() should be
called before end_that_request_last() is called for one of them.
Since the calls to end_that_request_first()/chunk() and
end_that_request_last() are packaged into blk_end_request(),
the handling of next_rq completion has to be moved into
blk_end_request(), too.

Bidi sets its specific value to rq-data_len before the request is
completed so that upper-layer can read it.
This setting must be between end_that_request_chunk() and
end_that_request_last(), because rq-data_len may be used
in end_that_request_chunk() by blk_trace and so on.
To satisfy the requirement, use blk_end_request_callback() which
is added in PATCH 25 only for the tricky drivers.

If bidi didn't reuse rq-data_len and added new members to request
for the specific value, it could set before end_that_request_chunk()
and use the standard blk_end_request() like below.

void scsi_end_bidi_request(struct scsi_cmnd *cmd)
{
struct request *req = cmd-request;

rq-resid = scsi_out(cmd)-resid;
rq-next_rq-resid = scsi_in(cmd)-resid;

if (blk_end_request(req, 1, req-data_len))
BUG();

scsi_release_buffers(cmd);
scsi_next_command(cmd);
}

Signed-off-by: Kiyoshi Ueda [EMAIL PROTECTED]
Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED]
---
 block/ll_rw_blk.c   |   18 +
 drivers/scsi/scsi_lib.c |   66 
 2 files changed, 52 insertions(+), 32 deletions(-)

Index: 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
===
--- 2.6.24-rc3-mm2.orig/drivers/scsi/scsi_lib.c
+++ 2.6.24-rc3-mm2/drivers/scsi/scsi_lib.c
@@ -629,28 +629,6 @@ void scsi_run_host_queues(struct Scsi_Ho
scsi_run_queue(sdev-request_queue);
 }
 
-static void scsi_finalize_request(struct scsi_cmnd *cmd, int uptodate)
-{
-   struct request_queue *q = cmd-device-request_queue;
-   struct request *req = cmd-request;
-   unsigned long flags;
-
-   add_disk_randomness(req-rq_disk);
-
-   spin_lock_irqsave(q-queue_lock, flags);
-   if (blk_rq_tagged(req))
-   blk_queue_end_tag(q, req);
-
-   end_that_request_last(req, uptodate);
-   spin_unlock_irqrestore(q-queue_lock, flags);
-
-   /*
-* 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_end_request()
  *
@@ -921,6 +899,20 @@ void scsi_release_buffers(struct scsi_cm
 EXPORT_SYMBOL(scsi_release_buffers);
 
 /*
+ * Called from blk_end_request_callback() after all DATA in rq and its next_rq
+ * are completed before rq is completed/freed.
+ */
+static int scsi_end_bidi_request_cb(struct request *rq)
+{
+   struct scsi_cmnd *cmd = rq-special;
+
+   rq-data_len = scsi_out(cmd)-resid;
+   rq-next_rq-data_len = scsi_in(cmd)-resid;
+
+   return 0;
+}
+
+/*
  * 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
@@ -931,22 +923,32 @@ void scsi_end_bidi_request(struct scsi_c
 {
struct request *req = cmd-request;
 
-   end_that_request_chunk(req, 1, req-data_len);
-   req-data_len = scsi_out(cmd)-resid;
-
-   end_that_request_chunk(req-next_rq, 1, req-next_rq-data_len);
-   req-next_rq-data_len = scsi_in(cmd)-resid;
-
-   scsi_release_buffers(cmd);
-
/*
 *FIXME: If ll_rw_blk.c is changed to also put_request(req-next_rq)
-*   in end_that_request_last() then this WARN_ON must be removed.
+*   in blk_end_request() then this WARN_ON must be removed.
 *   for now, upper-driver must have registered an end_io.
 */
WARN_ON(!req-end_io);
 
-   scsi_finalize_request(cmd, 1);
+   /*
+* blk_end_request() family take care of data completion of next_rq.
+*
+* req-data_len and req-next_rq-data_len must be set after
+* all data are completed, since they may be referenced during
+* the data completion process.
+* So use the callback feature of blk_end_request() here.
+*
+* NOTE: If bidi doesn't reuse the data_len field for upper-layer's
+*   reference (e.g. adds new members for it to struct request),
+*   we can use the standard blk_end_request() interface here.
+*/
+   if (blk_end_request_callback(req, 1, req-data_len,
+scsi_end_bidi_request_cb))
+   /* req has not been completed */
+   BUG();
+
+   scsi_release_buffers(cmd);
+