Re: [PATCH 24/28] blk_end_request: changing ide normal caller (take 3)

2007-12-04 Thread Bartlomiej Zolnierkiewicz 
On Tuesday 04 December 2007, Kiyoshi Ueda wrote:
 Hi Bartlomiej,
 
 On Sat, 1 Dec 2007 23:53:05 +0100, Bartlomiej Zolnierkiewicz [EMAIL 
 PROTECTED] wrote:
  On Saturday 01 December 2007, Kiyoshi Ueda wrote:
   This patch converts normal parts of ide to use blk_end_request().
   
   Signed-off-by: Kiyoshi Ueda [EMAIL PROTECTED]
   Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED]
   ---
drivers/ide/ide-cd.c |6 +++---
drivers/ide/ide-io.c |   17 ++---
2 files changed, 9 insertions(+), 14 deletions(-)
  
  [...]
  
   Index: 2.6.24-rc3-mm2/drivers/ide/ide-io.c
   ===
   --- 2.6.24-rc3-mm2.orig/drivers/ide/ide-io.c
   +++ 2.6.24-rc3-mm2/drivers/ide/ide-io.c
   @@ -78,14 +78,9 @@ static int __ide_end_request(ide_drive_t
 ide_dma_on(drive);
 }

   - if (!end_that_request_chunk(rq, uptodate, nr_bytes)) {
   - add_disk_randomness(rq-rq_disk);
   - if (dequeue) {
   - if (!list_empty(rq-queuelist))
   - blkdev_dequeue_request(rq);
   + if (!__blk_end_request(rq, uptodate, nr_bytes)) {
   + if (dequeue)
 HWGROUP(drive)-rq = NULL;
   - }
   - end_that_request_last(rq, uptodate);
 ret = 0;
 }
  
  Hmmm, this seems to change the old behavior (the request should
  be dequeued from the queue only if 'dequeue' variable is set)
  and AFAIR some error handling code (in ide-cd?) depends on the
  old behavior so please revisit this patch.
 
 blk_end_request() takes care of the dequeue like below,
 so I think no problem.  (Please see PATCH 01)
 
  +   /* rq-queuelist of dequeued request should be list_empty() */
  +   if (!list_empty(rq-queuelist))
  +   blkdev_dequeue_request(rq);
 
 In the case of ide-cd,
   o 'dequeue' variable is 1 only when the request is still linked
 to the queue (i.e. rq-queuelist is not empty)
   o 'dequeue' variable is 0 only when the request has already been
 removed from the queue (i.e. rq-queuelist is empty)
 So blk_end_request() can handle it correctly.

It would be helpful to add the above explanation to a patch description.

 If there are any drivers which don't want dequeue the queued request,
 the code above would not work.
 But, as far as I investigated, I have never seen such a requirement
 in device drivers.
 
 Do you think that ide may still gets a problem for the 'dequeue'?

Everything seems to be fine now.

Acked-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 24/28] blk_end_request: changing ide normal caller (take 3)

2007-12-04 Thread Kiyoshi Ueda 
Hi Bartlomiej,

On Tue, 4 Dec 2007 14:47:00 +0100, Bartlomiej Zolnierkiewicz wrote:
   Hmmm, this seems to change the old behavior (the request should
   be dequeued from the queue only if 'dequeue' variable is set)
   and AFAIR some error handling code (in ide-cd?) depends on the
   old behavior so please revisit this patch.
  
  blk_end_request() takes care of the dequeue like below,
  so I think no problem.  (Please see PATCH 01)
  
   + /* rq-queuelist of dequeued request should be list_empty() */
   + if (!list_empty(rq-queuelist))
   + blkdev_dequeue_request(rq);
  
  In the case of ide-cd,
o 'dequeue' variable is 1 only when the request is still linked
  to the queue (i.e. rq-queuelist is not empty)
o 'dequeue' variable is 0 only when the request has already been
  removed from the queue (i.e. rq-queuelist is empty)
  So blk_end_request() can handle it correctly.
 
 It would be helpful to add the above explanation to a patch description.
 
  If there are any drivers which don't want dequeue the queued request,
  the code above would not work.
  But, as far as I investigated, I have never seen such a requirement
  in device drivers.
  
  Do you think that ide may still gets a problem for the 'dequeue'?
 
 Everything seems to be fine now.
 
 Acked-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]

Thank you for the check.
OK, I'll add the explanation about the 'dequeue' to patch description.

Thanks,
Kiyoshi Ueda
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 24/28] blk_end_request: changing ide normal caller (take 3)

2007-12-03 Thread Kiyoshi Ueda 
Hi Bartlomiej,

On Sat, 1 Dec 2007 23:53:05 +0100, Bartlomiej Zolnierkiewicz [EMAIL 
PROTECTED] wrote:
 On Saturday 01 December 2007, Kiyoshi Ueda wrote:
  This patch converts normal parts of ide to use blk_end_request().
  
  Signed-off-by: Kiyoshi Ueda [EMAIL PROTECTED]
  Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED]
  ---
   drivers/ide/ide-cd.c |6 +++---
   drivers/ide/ide-io.c |   17 ++---
   2 files changed, 9 insertions(+), 14 deletions(-)
 
 [...]
 
  Index: 2.6.24-rc3-mm2/drivers/ide/ide-io.c
  ===
  --- 2.6.24-rc3-mm2.orig/drivers/ide/ide-io.c
  +++ 2.6.24-rc3-mm2/drivers/ide/ide-io.c
  @@ -78,14 +78,9 @@ static int __ide_end_request(ide_drive_t
  ide_dma_on(drive);
  }
   
  -   if (!end_that_request_chunk(rq, uptodate, nr_bytes)) {
  -   add_disk_randomness(rq-rq_disk);
  -   if (dequeue) {
  -   if (!list_empty(rq-queuelist))
  -   blkdev_dequeue_request(rq);
  +   if (!__blk_end_request(rq, uptodate, nr_bytes)) {
  +   if (dequeue)
  HWGROUP(drive)-rq = NULL;
  -   }
  -   end_that_request_last(rq, uptodate);
  ret = 0;
  }
 
 Hmmm, this seems to change the old behavior (the request should
 be dequeued from the queue only if 'dequeue' variable is set)
 and AFAIR some error handling code (in ide-cd?) depends on the
 old behavior so please revisit this patch.

blk_end_request() takes care of the dequeue like below,
so I think no problem.  (Please see PATCH 01)

 + /* rq-queuelist of dequeued request should be list_empty() */
 + if (!list_empty(rq-queuelist))
 + blkdev_dequeue_request(rq);

In the case of ide-cd,
  o 'dequeue' variable is 1 only when the request is still linked
to the queue (i.e. rq-queuelist is not empty)
  o 'dequeue' variable is 0 only when the request has already been
removed from the queue (i.e. rq-queuelist is empty)
So blk_end_request() can handle it correctly.


If there are any drivers which don't want dequeue the queued request,
the code above would not work.
But, as far as I investigated, I have never seen such a requirement
in device drivers.

Do you think that ide may still gets a problem for the 'dequeue'?

Thanks,
Kiyoshi Ueda
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 24/28] blk_end_request: changing ide normal caller (take 3)

2007-12-01 Thread Bartlomiej Zolnierkiewicz 
On Saturday 01 December 2007, Kiyoshi Ueda wrote:
 This patch converts normal parts of ide to use blk_end_request().
 
 Signed-off-by: Kiyoshi Ueda [EMAIL PROTECTED]
 Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED]
 ---
  drivers/ide/ide-cd.c |6 +++---
  drivers/ide/ide-io.c |   17 ++---
  2 files changed, 9 insertions(+), 14 deletions(-)

[...]

 Index: 2.6.24-rc3-mm2/drivers/ide/ide-io.c
 ===
 --- 2.6.24-rc3-mm2.orig/drivers/ide/ide-io.c
 +++ 2.6.24-rc3-mm2/drivers/ide/ide-io.c
 @@ -78,14 +78,9 @@ static int __ide_end_request(ide_drive_t
   ide_dma_on(drive);
   }
  
 - if (!end_that_request_chunk(rq, uptodate, nr_bytes)) {
 - add_disk_randomness(rq-rq_disk);
 - if (dequeue) {
 - if (!list_empty(rq-queuelist))
 - blkdev_dequeue_request(rq);
 + if (!__blk_end_request(rq, uptodate, nr_bytes)) {
 + if (dequeue)
   HWGROUP(drive)-rq = NULL;
 - }
 - end_that_request_last(rq, uptodate);
   ret = 0;
   }

Hmmm, this seems to change the old behavior (the request should
be dequeued from the queue only if 'dequeue' variable is set)
and AFAIR some error handling code (in ide-cd?) depends on the
old behavior so please revisit this patch.

Thanks,
Bart
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 24/28] blk_end_request: changing ide normal caller (take 3)

2007-11-30 Thread Kiyoshi Ueda 
This patch converts normal parts of ide to use blk_end_request().

Signed-off-by: Kiyoshi Ueda [EMAIL PROTECTED]
Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED]
---
 drivers/ide/ide-cd.c |6 +++---
 drivers/ide/ide-io.c |   17 ++---
 2 files changed, 9 insertions(+), 14 deletions(-)

Index: 2.6.24-rc3-mm2/drivers/ide/ide-cd.c
===
--- 2.6.24-rc3-mm2.orig/drivers/ide/ide-cd.c
+++ 2.6.24-rc3-mm2/drivers/ide/ide-cd.c
@@ -655,9 +655,9 @@ static void cdrom_end_request (ide_drive
BUG();
} else {
spin_lock_irqsave(ide_lock, flags);
-   end_that_request_chunk(failed, 0,
-   failed-data_len);
-   end_that_request_last(failed, 0);
+   if (__blk_end_request(failed, 0,
+ failed-data_len))
+   BUG();
spin_unlock_irqrestore(ide_lock, flags);
}
} else
Index: 2.6.24-rc3-mm2/drivers/ide/ide-io.c
===
--- 2.6.24-rc3-mm2.orig/drivers/ide/ide-io.c
+++ 2.6.24-rc3-mm2/drivers/ide/ide-io.c
@@ -78,14 +78,9 @@ static int __ide_end_request(ide_drive_t
ide_dma_on(drive);
}
 
-   if (!end_that_request_chunk(rq, uptodate, nr_bytes)) {
-   add_disk_randomness(rq-rq_disk);
-   if (dequeue) {
-   if (!list_empty(rq-queuelist))
-   blkdev_dequeue_request(rq);
+   if (!__blk_end_request(rq, uptodate, nr_bytes)) {
+   if (dequeue)
HWGROUP(drive)-rq = NULL;
-   }
-   end_that_request_last(rq, uptodate);
ret = 0;
}
 
@@ -290,9 +285,9 @@ static void ide_complete_pm_request (ide
drive-blocked = 0;
blk_start_queue(drive-queue);
}
-   blkdev_dequeue_request(rq);
HWGROUP(drive)-rq = NULL;
-   end_that_request_last(rq, 1);
+   if (__blk_end_request(rq, 1, 0))
+   BUG();
spin_unlock_irqrestore(ide_lock, flags);
 }
 
@@ -402,10 +397,10 @@ void ide_end_drive_cmd (ide_drive_t *dri
}
 
spin_lock_irqsave(ide_lock, flags);
-   blkdev_dequeue_request(rq);
HWGROUP(drive)-rq = NULL;
rq-errors = err;
-   end_that_request_last(rq, !rq-errors);
+   if (__blk_end_request(rq, !rq-errors, 0))
+   BUG();
spin_unlock_irqrestore(ide_lock, flags);
 }
 
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html