public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* PATCH for ide_floppy
@ 2005-07-01 15:29 Manfred.Scherer.Mhm
  2005-07-01 16:15 ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: Manfred.Scherer.Mhm @ 2005-07-01 15:29 UTC (permalink / raw)
  To: paul; +Cc: linux-kernel, manfred.scherer

Hi Paul

The runtime for copy of two 50MB files to a IOMEGA-100-Zip-IDE-device
seems to be not
deterministic since Linux Kernel 2.6.x. 

A lot of runtime will be spent to waiting for IO. 'top -i' will show
this:

top - 03:54:23 up 37 min,  9 users,  load average: 1.28, 1.52, 0.99
Tasks:  90 total,   1 running,  89 sleeping,   0 stopped,   0 zombie
Cpu(s):  3.6% us,  1.0% sy,  0.0% ni,  0.0% id, 95.0% wa,  0.3% hi, 
0.0% si
Mem:    190836k total,   189416k used,     1420k free,    10796k buffers
Swap:   529912k total,     4288k used,   525624k free,    99364k cached


  PID USER      PR  NI  VIRT  RES  SHR S %CPU %MEM    TIME+  COMMAND
 5649 root      17   0  1968  944 1764 R  0.3  0.5   0:01.47 top
 5672 root      24   0  1648  524 1492 D  0.0  0.3   0:00.18 umount

... umount is pending with "95.0% wa"


The tests will be done with a small shell script cpzip.sh similar to
this:

mount /dev/hdd4 /dzip 
rm /dzip/*.cgz
cp -p $* /dzip
umount /dzip 
eject /dzip

__________________________________
files:
-rw-r--r--  1 root root 50167808 Jun 19 19:17 0_LW_1_ux.cgz
-rw-r--r--  1 root root 50167808 Jun 19 19:18 0_LW_2_ux.cgz


measured runtimes:

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    9m18.486s
user    0m0.042s
sys     0m2.450s
pc1:/scm/save_a/wcb #      

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    4m24.953s
user    0m0.036s
sys     0m2.584s
pc1:/scm/save_a/wcb #        

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    22m50.351s
user    0m0.047s
sys     0m2.789s
pc1:/scm/save_a/wcb #  

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    17m44.786s
user    0m0.048s
sys     0m2.758s
pc1:/scm/save_a/wcb #  

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    5m11.958s
user    0m0.051s
sys     0m2.558s
pc1:/scm/save_a/wcb #      

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    19m37.694s
user    0m0.043s
sys     0m2.821s
pc1:/scm/save_a/wcb #  

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    32m9.964s
user    0m0.047s
sys     0m2.766s
pc1:/scm/save_a/wcb #   

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    6m26.194s
user    0m0.041s
sys     0m2.517s
pc1:/scm/save_a/wcb #            

______________________
runtimes after applying my patch:

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    6m15.780s
user    0m0.057s
sys     0m2.212s
pc1:/scm/save_a/wcb # 

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    5m54.664s
user    0m0.035s
sys     0m2.287s
pc1:/scm/save_a/wcb # 

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    5m55.325s
user    0m0.041s
sys     0m2.330s
pc1:/scm/save_a/wcb #    

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    6m9.513s
user    0m0.040s
sys     0m2.352s
pc1:/scm/save_a/wcb #  

pc1:/scm/save_a/wcb # time ./cpzip.sh 0_LW_1_ux.cgz 0_LW_2_ux.cgz
real    6m24.069s
user    0m0.052s
sys     0m2.322s
pc1:/scm/save_a/wcb # 

_________________________________________________________________

Patch:

--- linux-2.6.12/drivers/ide/ide.c.ORIG 2005-06-17 21:48:29.000000000
+0200
+++ linux-2.6.12/drivers/ide/ide.c      2005-06-25 03:55:28.000000000
+0200
@@ -240,6 +240,7 @@
                drive->name[0]                  = 'h';
                drive->name[1]                  = 'd';
                drive->name[2]                  = 'a' + (index *
MAX_DRIVES) + unit;
+               drive->failures                 = 0;    /* --ms
2004/12/29 */
                drive->max_failures             =
IDE_DEFAULT_MAX_FAILURES;
                drive->using_dma                = 0;
                drive->is_flash                 = 0;


--- linux-2.6.12/drivers/ide/ide-floppy.c.ORIG  2005-06-17
21:48:29.000000000 +0200
+++ linux-2.6.12/drivers/ide/ide-floppy.c       2005-06-25
03:29:45.000000000 +0200
@@ -125,7 +125,14 @@
 /*
  *     Some drives require a longer irq timeout.
  */
+#if 0
 #define IDEFLOPPY_WAIT_CMD             (5 * WAIT_CMD)
+#endif
+#if 0
+#define IDEFLOPPY_WAIT_CMD             200     /* --ms  */
+#endif
+#define IDEFLOPPY_WAIT_CMD             WAIT_CMD        /* 2004/12/31
--ms */
+

 /*
  *     After each failed packet command we issue a request sense
command
@@ -317,7 +324,13 @@
        unsigned long flags;
 } idefloppy_floppy_t;

+#if 0
 #define IDEFLOPPY_TICKS_DELAY  3       /* default delay for ZIP 100 */
+#define IDEFLOPPY_TICKS_DELAY  6       /* default delay for ZIP 100
--ms 2005/01/01 */
+#define IDEFLOPPY_TICKS_DELAY  12      /* default delay for ZIP 100
--ms 2005/01/01 */
+#endif
+#define IDEFLOPPY_TICKS_DELAY  60      /* default delay for ZIP 100
--ms 2005/01/07 */
+

 /*
  *     Floppy flag bits values.
@@ -774,6 +787,35 @@
        pc->callback = &idefloppy_pc_callback;
 }

+/*
+ *     Similar to ide-cd.c     2004/12/31 --ms
+ */
+static int idefloppy_timer_expiry(ide_drive_t *drive)
+{
+       struct request *rq = HWGROUP(drive)->rq;
+       unsigned long wait = 0;
+
+       /*
+        * Some commands are *slow* and normally take a long time to
+        * complete. Usually we can use the ATAPI "disconnect" to bypass
+        * this, but not all commands/drives support that. Let
+        * ide_timer_expiry keep polling us for these.
+        */
+       switch (rq->cmd[0]) {
+               case GPCMD_BLANK:
+               case GPCMD_FORMAT_UNIT:
+               case GPCMD_FLUSH_CACHE:
+                       wait = IDEFLOPPY_WAIT_CMD;
+                       break;
+               default:
+                       if (!(rq->flags & REQ_QUIET))
+                               printk(KERN_INFO "ide-floppy: cmd 0x%x
timed out\n", rq->cmd[0]);
+                       wait = 0;
+                       break;
+       }
+       return wait;
+}
+
 static void idefloppy_create_request_sense_cmd (idefloppy_pc_t *pc)
 {
        idefloppy_init_pc(pc);
@@ -900,7 +942,7 @@
                                ide_set_handler(drive,
                                                &idefloppy_pc_intr,
                                                IDEFLOPPY_WAIT_CMD,
-                                               NULL);
+                                               idefloppy_timer_expiry
/* NULL --ms */);
                                return ide_started;
                        }
                        debug_log(KERN_NOTICE "ide-floppy: The floppy
wants to "
@@ -931,7 +973,10 @@

        if (HWGROUP(drive)->handler != NULL)
                BUG();
-       ide_set_handler(drive, &idefloppy_pc_intr, IDEFLOPPY_WAIT_CMD,
NULL);           /* And set the interrupt handler again */
+       /* And set the interrupt handler again */
+       ide_set_handler(drive, &idefloppy_pc_intr,
+                               IDEFLOPPY_WAIT_CMD,
+                               idefloppy_timer_expiry /* NULL --ms */);
        return ide_started;
 }

@@ -960,7 +1005,9 @@
        if (HWGROUP(drive)->handler != NULL)
                BUG();
        /* Set the interrupt routine */
-       ide_set_handler(drive, &idefloppy_pc_intr, IDEFLOPPY_WAIT_CMD,
NULL);
+       ide_set_handler(drive, &idefloppy_pc_intr,
+                               IDEFLOPPY_WAIT_CMD,
+                               idefloppy_timer_expiry /* NULL --ms */);
        /* Send the actual packet */
        HWIF(drive)->atapi_output_bytes(drive, floppy->pc->c, 12);
        return ide_started;
@@ -1116,6 +1163,7 @@
        }

        /* Can we transfer the packet when we get the interrupt or wait?
*/
+/* 2005/01/01 --ms, this is really necessary! */
        if (test_bit(IDEFLOPPY_ZIP_DRIVE, &floppy->flags)) {
                /* wait */
                pkt_xfer_routine = &idefloppy_transfer_pc1;
@@ -1129,7 +1177,7 @@
                ide_execute_command(drive, WIN_PACKETCMD,
                                pkt_xfer_routine,
                                IDEFLOPPY_WAIT_CMD,
-                               NULL);
+                               idefloppy_timer_expiry /* NULL --ms */);
                return ide_started;
        } else {
                /* Issue the packet command */



Signed-off-by: Manfred Scherer <manfred.scherer.mhm@t-online.de>
___________________________________________________________________
My system is:

pc1:~ # uname -a
Linux pc1 2.6.12 #2 Sat Jun 25 04:02:24 CEST 2005 i686 athlon i386
GNU/Linux
pc1:~ #

pc1:~ # grep -i iomega /var/log/boot.msg
<4>hdd: IOMEGA ZIP 100 ATAPI, ATAPI FLOPPY drive


pc1:~ # cat /proc/cpuinfo
processor       : 0
vendor_id       : AuthenticAMD
cpu family      : 6
model           : 4
model name      : AMD Athlon(tm) processor
stepping        : 2
cpu MHz         : 1000.250


	

EOF




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

* Re: PATCH for ide_floppy
  2005-07-01 15:29 PATCH for ide_floppy Manfred.Scherer.Mhm
@ 2005-07-01 16:15 ` Jens Axboe
  2005-07-01 16:58   ` Manfred.Scherer.Mhm
  0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2005-07-01 16:15 UTC (permalink / raw)
  To: Manfred.Scherer.Mhm@t-online.de; +Cc: paul, linux-kernel, manfred.scherer

On Fri, Jul 01 2005, Manfred.Scherer.Mhm@t-online.de wrote:
> --- linux-2.6.12/drivers/ide/ide-floppy.c.ORIG  2005-06-17
> 21:48:29.000000000 +0200
> +++ linux-2.6.12/drivers/ide/ide-floppy.c       2005-06-25
> 03:29:45.000000000 +0200
> @@ -125,7 +125,14 @@
>  /*
>   *     Some drives require a longer irq timeout.
>   */
> +#if 0
>  #define IDEFLOPPY_WAIT_CMD             (5 * WAIT_CMD)
> +#endif
> +#if 0
> +#define IDEFLOPPY_WAIT_CMD             200     /* --ms  */
> +#endif
> +#define IDEFLOPPY_WAIT_CMD             WAIT_CMD        /* 2004/12/31
> --ms */
> +
> 
>  /*
>   *     After each failed packet command we issue a request sense
> command
> @@ -317,7 +324,13 @@
>         unsigned long flags;
>  } idefloppy_floppy_t;
> 
> +#if 0
>  #define IDEFLOPPY_TICKS_DELAY  3       /* default delay for ZIP 100 */
> +#define IDEFLOPPY_TICKS_DELAY  6       /* default delay for ZIP 100
> --ms 2005/01/01 */
> +#define IDEFLOPPY_TICKS_DELAY  12      /* default delay for ZIP 100
> --ms 2005/01/01 */
> +#endif
> +#define IDEFLOPPY_TICKS_DELAY  60      /* default delay for ZIP 100
> --ms 2005/01/07 */
> +

This seems to be basically what your patch changes, along with the
expiry addition. Neither of which should make any change in performance,
since you should normally not time commands out. Do you normally get
lots of errors or timeouts running that workload?

I don't see how your patch changes anything for a normally running
ide-floppy.

-- 
Jens Axboe


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

* Re: Re: PATCH for ide_floppy
  2005-07-01 16:15 ` Jens Axboe
@ 2005-07-01 16:58   ` Manfred.Scherer.Mhm
  2005-07-01 17:08     ` Bartlomiej Zolnierkiewicz
  2005-07-01 17:41     ` Jens Axboe
  0 siblings, 2 replies; 6+ messages in thread
From: Manfred.Scherer.Mhm @ 2005-07-01 16:58 UTC (permalink / raw)
  To: Jens Axboe; +Cc: paul, linux-kernel, manfred.scherer

it's not really a performance issue but more a timeout issue.
'IDEFLOPPY_TICKS_DELAY  60' avoids error messages in /var/log/messages
like 'reset ide ...'.
Without the idefloppy_timer_expiry the data transfer to the ide-floppy
is pending a long time between some transfer of data. The floppy LED
indicated this too.
With kernel 2.4.x I've never had this problem.

Manfred Scherer




-----Original Message-----
Date: Fri,  1 Jul 2005 18:15:34 +0200
Subject: Re: PATCH for ide_floppy
From: Jens Axboe <axboe@suse.de>
To: "Manfred.Scherer.Mhm@t-online.de" <Manfred.Scherer.Mhm@t-online.de>

On Fri, Jul 01 2005, Manfred.Scherer.Mhm@t-online.de wrote:
> --- linux-2.6.12/drivers/ide/ide-floppy.c.ORIG  2005-06-17
> 21:48:29.000000000 +0200
> +++ linux-2.6.12/drivers/ide/ide-floppy.c       2005-06-25
> 03:29:45.000000000 +0200
> @@ -125,7 +125,14 @@
>  /*
>   *     Some drives require a longer irq timeout.
>   */
> +#if 0
>  #define IDEFLOPPY_WAIT_CMD             (5 * WAIT_CMD)
> +#endif
> +#if 0
> +#define IDEFLOPPY_WAIT_CMD             200     /* --ms  */
> +#endif
> +#define IDEFLOPPY_WAIT_CMD             WAIT_CMD        /* 2004/12/31
> --ms */
> +
>
>  /*
>   *     After each failed packet command we issue a request sense
> command
> @@ -317,7 +324,13 @@
>         unsigned long flags;
>  } idefloppy_floppy_t;
>
> +#if 0
>  #define IDEFLOPPY_TICKS_DELAY  3       /* default delay for ZIP 100
*/
> +#define IDEFLOPPY_TICKS_DELAY  6       /* default delay for ZIP 100
> --ms 2005/01/01 */
> +#define IDEFLOPPY_TICKS_DELAY  12      /* default delay for ZIP 100
> --ms 2005/01/01 */
> +#endif
> +#define IDEFLOPPY_TICKS_DELAY  60      /* default delay for ZIP 100
> --ms 2005/01/07 */
> +

This seems to be basically what your patch changes, along with the
expiry addition. Neither of which should make any change in performance,
since you should normally not time commands out. Do you normally get
lots of errors or timeouts running that workload?

I don't see how your patch changes anything for a normally running
ide-floppy.

--
Jens Axboe




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

* Re: Re: PATCH for ide_floppy
  2005-07-01 16:58   ` Manfred.Scherer.Mhm
@ 2005-07-01 17:08     ` Bartlomiej Zolnierkiewicz
  2005-07-04  9:41       ` Manfred.Scherer.Mhm
  2005-07-01 17:41     ` Jens Axboe
  1 sibling, 1 reply; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2005-07-01 17:08 UTC (permalink / raw)
  To: Manfred.Scherer.Mhm@t-online.de
  Cc: Jens Axboe, paul, linux-kernel, manfred.scherer

On 7/1/05, Manfred.Scherer.Mhm@t-online.de
<Manfred.Scherer.Mhm@t-online.de> wrote:
> it's not really a performance issue but more a timeout issue.
> 'IDEFLOPPY_TICKS_DELAY  60' avoids error messages in /var/log/messages
> like 'reset ide ...'.
> Without the idefloppy_timer_expiry the data transfer to the ide-floppy
> is pending a long time between some transfer of data. The floppy LED
> indicated this too.
> With kernel 2.4.x I've never had this problem.

This seems related to 2.4 -> 2.6 HZ change.

> > @@ -317,7 +324,13 @@
> >         unsigned long flags;
> >  } idefloppy_floppy_t;
> >
> > +#if 0
> >  #define IDEFLOPPY_TICKS_DELAY  3       /* default delay for ZIP 100
> */
> > +#define IDEFLOPPY_TICKS_DELAY  6       /* default delay for ZIP 100
> > --ms 2005/01/01 */
> > +#define IDEFLOPPY_TICKS_DELAY  12      /* default delay for ZIP 100
> > --ms 2005/01/01 */
> > +#endif
> > +#define IDEFLOPPY_TICKS_DELAY  60      /* default delay for ZIP 100
> > --ms 2005/01/07 */
> > +

"ticks" delay should be expressed using HZ

#define IDEFLOPPY_TICKS_DELAY  HZ/20

for 50msec delay (N.B. the comment in the code about default delay
being 50msec also seems wrong - it was more like ~33msec in 2.4)

Could you please test if this fixes your problems?

Bartlomiej

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

* Re: Re: PATCH for ide_floppy
  2005-07-01 16:58   ` Manfred.Scherer.Mhm
  2005-07-01 17:08     ` Bartlomiej Zolnierkiewicz
@ 2005-07-01 17:41     ` Jens Axboe
  1 sibling, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2005-07-01 17:41 UTC (permalink / raw)
  To: Manfred.Scherer.Mhm@t-online.de; +Cc: paul, linux-kernel, manfred.scherer

On Fri, Jul 01 2005, Manfred.Scherer.Mhm@t-online.de wrote:
> it's not really a performance issue but more a timeout issue.
> 'IDEFLOPPY_TICKS_DELAY  60' avoids error messages in /var/log/messages
> like 'reset ide ...'.
> Without the idefloppy_timer_expiry the data transfer to the ide-floppy
> is pending a long time between some transfer of data. The floppy LED
> indicated this too.
> With kernel 2.4.x I've never had this problem.

Ah, so you did see lots of timeouts. The solution is, I suspect, just
to adjust the ticks to be based off HZ like Bart suggests.

-- 
Jens Axboe


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

* Re: Re: Re: PATCH for ide_floppy
  2005-07-01 17:08     ` Bartlomiej Zolnierkiewicz
@ 2005-07-04  9:41       ` Manfred.Scherer.Mhm
  0 siblings, 0 replies; 6+ messages in thread
From: Manfred.Scherer.Mhm @ 2005-07-04  9:41 UTC (permalink / raw)
  To: bzolnier, axboe; +Cc: paul, linux-kernel, manfred.scherer

yes, 
#define IDEFLOPPY_TICKS_DELAY  HZ/20
seems to be the solution.

when I've tested some values for IDEFLOPPY_TICKS_DELAY in December 2004,
I cannot found the best value for this. The Kernel version was 2.6.8
from the SuSE9.2 distribution.

I take a look in ide-cd.c and found there the function
cdrom_timer_expiry as
a possibility to handle timeout issues smoother. I tried this function
as 
idefloppy_timer_expiry in ide-floppy.c in combination with
IDEFLOPPY_TICKS_DELAY  60
as a best result for all cases. It seems to reach out to change
IDEFLOPPY_TICKS_DELAY
as suggested from you, idefloppy_timer_expiry is not really necessary.  


-----Original Message-----
Date: Fri,  1 Jul 2005 19:08:58 +0200
Subject: Re: Re: PATCH for ide_floppy
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
To: "Manfred.Scherer.Mhm@t-online.de" <Manfred.Scherer.Mhm@t-online.de>

On 7/1/05, Manfred.Scherer.Mhm@t-online.de
<Manfred.Scherer.Mhm@t-online.de> wrote:
> it's not really a performance issue but more a timeout issue.
> 'IDEFLOPPY_TICKS_DELAY  60' avoids error messages in /var/log/messages
> like 'reset ide ...'.
> Without the idefloppy_timer_expiry the data transfer to the ide-floppy
> is pending a long time between some transfer of data. The floppy LED
> indicated this too.
> With kernel 2.4.x I've never had this problem.

This seems related to 2.4 -> 2.6 HZ change.

> > @@ -317,7 +324,13 @@
> >         unsigned long flags;
> >  } idefloppy_floppy_t;
> >
> > +#if 0
> >  #define IDEFLOPPY_TICKS_DELAY  3       /* default delay for ZIP 100
> */
> > +#define IDEFLOPPY_TICKS_DELAY  6       /* default delay for ZIP 100
> > --ms 2005/01/01 */
> > +#define IDEFLOPPY_TICKS_DELAY  12      /* default delay for ZIP 100
> > --ms 2005/01/01 */
> > +#endif
> > +#define IDEFLOPPY_TICKS_DELAY  60      /* default delay for ZIP 100
> > --ms 2005/01/07 */
> > +

"ticks" delay should be expressed using HZ

#define IDEFLOPPY_TICKS_DELAY  HZ/20

for 50msec delay (N.B. the comment in the code about default delay
being 50msec also seems wrong - it was more like ~33msec in 2.4)

Could you please test if this fixes your problems?

Bartlomiej



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

end of thread, other threads:[~2005-07-04  9:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-07-01 15:29 PATCH for ide_floppy Manfred.Scherer.Mhm
2005-07-01 16:15 ` Jens Axboe
2005-07-01 16:58   ` Manfred.Scherer.Mhm
2005-07-01 17:08     ` Bartlomiej Zolnierkiewicz
2005-07-04  9:41       ` Manfred.Scherer.Mhm
2005-07-01 17:41     ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox