Re: [PATCH 2.6.11-rc2 11/29] ide: add ide_drive_t.sleeping

2005-02-03 Thread Bartlomiej Zolnierkiewicz 
On Thu, 3 Feb 2005 14:32:29 +0100, Jens Axboe [EMAIL PROTECTED] wrote:
 On Thu, Feb 03 2005, Bartlomiej Zolnierkiewicz wrote:
  On Thu, 3 Feb 2005 12:37:10 +0100, Jens Axboe [EMAIL PROTECTED] wrote:
   On Thu, Feb 03 2005, Bartlomiej Zolnierkiewicz wrote:
On Wed, 2 Feb 2005 11:54:48 +0900, Tejun Heo [EMAIL PROTECTED] wrote:
  11_ide_drive_sleeping_fix.patch
 
ide_drive_t.sleeping field added.  0 in sleep field used to
indicate inactive sleeping but because 0 is a valid jiffy
value, though slim, there's a chance that something can go
weird.  And while at it, explicit jiffy comparisons are
converted to use time_{after|before} macros.
   
Same question as for add ide_hwgroup_t.polling patch.
AFAICS drive-sleep is either '0' or 'timeout + jiffies' (always  0)
  
   Hmm, what if jiffies + timeout == 0?
 
  Hm, jiffies is unsigned and timeout is always  0
  but this is still possible if jiffies + timeout wraps, right?
 
 Precisely, if jiffies is exactly 'timeout' away from wrapping to 0 it
 could happen. So I think the fix looks sane.

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

Re: [PATCH 2.6.11-rc2 11/29] ide: add ide_drive_t.sleeping

2005-02-02 Thread Bartlomiej Zolnierkiewicz 
On Wed, 2 Feb 2005 11:54:48 +0900, Tejun Heo [EMAIL PROTECTED] wrote:
  11_ide_drive_sleeping_fix.patch
 
ide_drive_t.sleeping field added.  0 in sleep field used to
indicate inactive sleeping but because 0 is a valid jiffy
value, though slim, there's a chance that something can go
weird.  And while at it, explicit jiffy comparisons are
converted to use time_{after|before} macros.

Same question as for add ide_hwgroup_t.polling patch.
AFAICS drive-sleep is either '0' or 'timeout + jiffies' (always  0)
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2.6.11-rc2 11/29] ide: add ide_drive_t.sleeping

2005-02-01 Thread Tejun Heo 
 11_ide_drive_sleeping_fix.patch
 
   ide_drive_t.sleeping field added.  0 in sleep field used to
   indicate inactive sleeping but because 0 is a valid jiffy
   value, though slim, there's a chance that something can go
   weird.  And while at it, explicit jiffy comparisons are
   converted to use time_{after|before} macros.


Signed-off-by: Tejun Heo [EMAIL PROTECTED]


Index: linux-ide-export/drivers/ide/ide-io.c
===
--- linux-ide-export.orig/drivers/ide/ide-io.c  2005-02-02 10:28:03.340503581 
+0900
+++ linux-ide-export/drivers/ide/ide-io.c   2005-02-02 10:28:04.260354336 
+0900
@@ -933,6 +933,7 @@ void ide_stall_queue (ide_drive_t *drive
if (timeout  WAIT_WORSTCASE)
timeout = WAIT_WORSTCASE;
drive-sleep = timeout + jiffies;
+   drive-sleeping = 1;
 }
 
 EXPORT_SYMBOL(ide_stall_queue);
@@ -972,18 +973,18 @@ repeat:   
}
 
do {
-   if ((!drive-sleep || time_after_eq(jiffies, drive-sleep))
+   if ((!drive-sleeping || time_after_eq(jiffies, drive-sleep))
 !elv_queue_empty(drive-queue)) {
if (!best
-|| (drive-sleep  (!best-sleep || 0  (signed 
long)(best-sleep - drive-sleep)))
-|| (!best-sleep  0  (signed long)(WAKEUP(best) - 
WAKEUP(drive
+|| (drive-sleeping  (!best-sleeping || 
time_before(drive-sleep, best-sleep)))
+|| (!best-sleeping  time_before(WAKEUP(drive), 
WAKEUP(best
{
if (!blk_queue_plugged(drive-queue))
best = drive;
}
}
} while ((drive = drive-next) != hwgroup-drive);
-   if (best  best-nice1  !best-sleep  best != hwgroup-drive  
best-service_time  WAIT_MIN_SLEEP) {
+   if (best  best-nice1  !best-sleeping  best != hwgroup-drive  
best-service_time  WAIT_MIN_SLEEP) {
long t = (signed long)(WAKEUP(best) - jiffies);
if (t = WAIT_MIN_SLEEP) {
/*
@@ -992,10 +993,9 @@ repeat:
 */
drive = best-next;
do {
-   if (!drive-sleep
-   /* FIXME: use time_before */
- 0  (signed long)(WAKEUP(drive) - (jiffies 
- best-service_time))
- 0  (signed long)((jiffies + t) - 
WAKEUP(drive)))
+   if (!drive-sleeping
+ time_before(jiffies - best-service_time, 
WAKEUP(drive))
+ time_before(WAKEUP(drive), jiffies + t))
{
ide_stall_queue(best, min_t(long, t, 10 
* WAIT_MIN_SLEEP));
goto repeat;
@@ -1058,14 +1058,17 @@ static void ide_do_request (ide_hwgroup_
hwgroup-busy = 1;
drive = choose_drive(hwgroup);
if (drive == NULL) {
-   unsigned long sleep = 0;
+   int sleeping = 0;
+   unsigned long sleep = 0; /* shut up, gcc */
hwgroup-rq = NULL;
drive = hwgroup-drive;
do {
-   if (drive-sleep  (!sleep || 0  (signed 
long)(sleep - drive-sleep)))
+   if (drive-sleeping  (!sleeping || 
time_before(drive-sleep, sleep))) {
+   sleeping = 1;
sleep = drive-sleep;
+   }
} while ((drive = drive-next) != hwgroup-drive);
-   if (sleep) {
+   if (sleeping) {
/*
 * Take a short snooze, and then wake up this hwgroup again.
 * This gives other hwgroups on the same a chance to
@@ -1105,7 +1108,7 @@ static void ide_do_request (ide_hwgroup_
}
hwgroup-hwif = hwif;
hwgroup-drive = drive;
-   drive-sleep = 0;
+   drive-sleeping = 0;
drive-service_start = jiffies;
 
if (blk_queue_plugged(drive-queue)) {
Index: linux-ide-export/include/linux/ide.h
===
--- linux-ide-export.orig/include/linux/ide.h   2005-02-02 10:27:15.687234943 
+0900
+++ linux-ide-export/include/linux/ide.h2005-02-02 10:28:04.261354174 
+0900
@@ -721,6 +721,7 @@ typedef struct ide_drive_s {
 *  3=64-bit
 */
unsigned scsi   : 1;/* 0=default, 1=ide-scsi emulation */
+