Add i8253 spinlocks where needed.

Hi!

[email protected], 2002-05-26 14:17:40+02:00, [email protected]
This changeset hopefully fixes all the instances (at least on i386 arch and arch-nonspecific drivers) where there the i8253 chip is accessed - to use a spinlock to serialize the multi-byte accesses. This should allow us to understand the timer problems that are happening now and then better.

[email protected], 2002-05-26 10:30:46+02:00, [email protected]
This cset fixes timestamp generation in ftape-calibr.c, which was missing a lock for accessing the i8253 timer. This module running together with i386/kernel/time.c could result in the timer registers losing sync with software and reporting swapped values. There was an workaround for this in ftape-calibr.c, but could never completely fix the problem. Also, there still remains a race (now commented in ftape-calibr.c), with jiffies and the i8253 timer register. This cannot be fixed easily. Last, but not least, this converts ftape timestamps on x86_64 to use TSC timer solely, because it's always present there and doesn't have all the problems with the i8253 timer.

Now there shouldn't be any code accessing the i8253 timer without a spinlock held. Finally!

 arch/i386/kernel/apm.c                     |   15 +---
 arch/i386/kernel/i8259.c                   |    4 +
 arch/i386/kernel/time.c                    |    4 +
 drivers/char/ftape/lowlevel/ftape-calibr.c |   97 +++++++++++++----------------
 drivers/char/vt.c                          |    3
 drivers/ide/hd.c                           |   10 +-
 drivers/input/gameport/gameport.c          |   28 +++++++-
 7 files changed, 91 insertions(+), 70 deletions(-)

diff -Nru a/arch/i386/kernel/apm.c b/arch/i386/kernel/apm.c

arch/i386/kernel/apm.c Sun May 26 14:18:40 2002 +6 -9

@@ -1170,17 +1170,14 @@
 {
 #ifdef INIT_TIMER_AFTER_SUSPEND
        unsigned long   flags;
+       extern spinlock_t i8253_lock;
 
-       save_flags(flags);
-       cli();
+       spin_lock_irqsave(&i8253_lock, flags);
        /* set the clock to 100 Hz */
-       outb_p(0x34,0x43);              /* binary, mode 2, LSB/MSB, ch 0 */
-       udelay(10);
-       outb_p(LATCH & 0xff , 0x40);    /* LSB */
-       udelay(10);
-       outb(LATCH >> 8 , 0x40);        /* MSB */
-       udelay(10);
-       restore_flags(flags);
+       outb_p(0x34, 0x43);             /* binary, mode 2, LSB/MSB, ch 0 */
+       outb_p(LATCH & 0xff, 0x40);     /* LSB */
+       outb_p(LATCH >> 8, 0x40);       /* MSB */
+       spin_unlock_irqrestore(&i8253_lock, flags);
 #endif
 }
 

diff -Nru a/arch/i386/kernel/i8259.c b/arch/i386/kernel/i8259.c

arch/i386/kernel/i8259.c Sun May 26 14:18:40 2002 +4 -0

@@ -360,6 +360,8 @@
 void __init init_IRQ(void)
 {
        int i;
+       extern spinlock_t i8253_lock;
+       unsigned long flags;
 
 #ifndef CONFIG_X86_VISWS_APIC
        init_ISA_irqs();

@@ -415,9 +417,11 @@
         * Set the clock to HZ Hz, we already have a valid
         * vector now:
         */
+       spin_lock_irqsave(&i8253_lock, flags);
        outb_p(0x34,0x43);              /* binary, mode 2, LSB/MSB, ch 0 */
        outb_p(LATCH & 0xff , 0x40);    /* LSB */
        outb(LATCH >> 8 , 0x40);        /* MSB */
+       spin_unlock_irqrestore(&i8253_lock, flags);
 
 #ifndef CONFIG_VISWS
        setup_irq(2, &irq2);

diff -Nru a/arch/i386/kernel/time.c b/arch/i386/kernel/time.c

arch/i386/kernel/time.c Sun May 26 14:18:40 2002 +4 -0

@@ -574,6 +574,8 @@
 
 static unsigned long __init calibrate_tsc(void)
 {
+       unsigned long flags;
+
        /* Set the Gate high, disable speaker */
        outb((inb(0x61) & ~0x02) | 0x01, 0x61);
 

@@ -584,9 +586,11 @@
         * (interrupt on terminal count mode), binary count,
         * load 5 * LATCH count, (LSB and MSB) to begin countdown.
         */
+       spin_lock_irqsave(&i8253_lock, flags);
        outb(0xb0, 0x43);                       /* binary, mode 0, LSB/MSB, Ch 2 */
        outb(CALIBRATE_LATCH & 0xff, 0x42);     /* LSB of count */
        outb(CALIBRATE_LATCH >> 8, 0x42);       /* MSB of count */
+       spin_unlock_irqrestore(&i8253_lock, flags);
 
        {
                unsigned long startlow, starthigh;

diff -Nru a/drivers/char/ftape/lowlevel/ftape-calibr.c b/drivers/char/ftape/lowlevel/ftape-calibr.c

drivers/char/ftape/lowlevel/ftape-calibr.c Sun May 26 14:18:40 2002 +44 -53

@@ -31,7 +31,10 @@
 #include <asm/io.h>
 #if defined(__alpha__)
 # include <asm/hwrpb.h>
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__x86_64__)
+# include <asm/msr.h>
+# include <asm/timex.h>
+#elif defined(__i386__)
 # include <linux/timex.h>
 #endif
 #include <linux/ftape.h>

@@ -45,10 +48,14 @@
 # error Ftape is not implemented for this architecture!
 #endif
 
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
 static unsigned long ps_per_cycle = 0;
 #endif
 
+#if defined(__i386__)
+extern spinlock_t i8253_lock;
+#endif
+
 /*
  * Note: On Intel PCs, the clock ticks at 100 Hz (HZ==100) which is
  * too slow for certain timeouts (and that clock doesn't even tick

@@ -67,48 +74,58 @@
 {
 #if defined(__alpha__)
        unsigned long r;
-
        asm volatile ("rpcc %0" : "=r" (r));
        return r;
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__x86_64__)
+       unsigned long r;
+       rdtscl(r);
+       return r;
+#elif defined(__i386__)
+
+/*
+ * Note that there is some time between counter underflowing and jiffies
+ * increasing, so the code below won't always give correct output.
+ * -Vojtech
+ */
+
        unsigned long flags;
        __u16 lo;
        __u16 hi;
 
-       save_flags(flags);
-       cli();
+       spin_lock_irqsave(&i8253_lock, flags);
        outb_p(0x00, 0x43);     /* latch the count ASAP */
        lo = inb_p(0x40);       /* read the latched count */
        lo |= inb(0x40) << 8;
        hi = jiffies;
-       restore_flags(flags);
+       spin_unlock_irqrestore(&i8253_lock, flags);
+
        return ((hi + 1) * (unsigned int) LATCH) - lo;  /* downcounter ! */
 #endif
 }
 
 static unsigned int short_ftape_timestamp(void)
 {
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
        return ftape_timestamp();
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__i386__)
        unsigned int count;
        unsigned long flags;
-
-       save_flags(flags);
-       cli();
+
+       spin_lock_irqsave(&i8253_lock, flags);
        outb_p(0x00, 0x43);     /* latch the count ASAP */
        count = inb_p(0x40);    /* read the latched count */
        count |= inb(0x40) << 8;
-       restore_flags(flags);
+       spin_unlock_irqrestore(&i8253_lock, flags);
+
        return (LATCH - count); /* normal: downcounter */
 #endif
 }
 
 static unsigned int diff(unsigned int t0, unsigned int t1)
 {
-#if defined(__alpha__)
-       return (t1 <= t0) ? t1 + (1UL << 32) - t0 : t1 - t0;
-#elif defined(__i386__) || defined(__x86_64__)
+#if defined(__alpha__) || defined(__x86_64__)
+       return (t1 - t0);
+#elif defined(__i386__)
        /*
         * This is tricky: to work for both short and full ftape_timestamps
         * we'll have to discriminate between these.

@@ -122,9 +139,9 @@
 
 static unsigned int usecs(unsigned int count)
 {
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
        return (ps_per_cycle * count) / 1000000UL;
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__i386__)
        return (10000 * count) / ((CLOCK_TICK_RATE + 50) / 100);
 #endif
 }

@@ -153,49 +170,24 @@
        save_flags(flags);
        cli();
        t0 = short_ftape_timestamp();
-       for (i = 0; i < 1000; ++i) {
+       for (i = 0; i < 1000; ++i)
                status = inb(fdc.msr);
-       }
        t1 = short_ftape_timestamp();
        restore_flags(flags);
+
        TRACE(ft_t_info, "inb() duration: %d nsec", ftape_timediff(t0, t1));
        TRACE_EXIT;
 }
 
 static void init_clock(void)
 {
-#if defined(__i386__) || defined(__x86_64__)
-       unsigned int t;
-       int i;
        TRACE_FUN(ft_t_any);
 
-       /*  Haven't studied on why, but there sometimes is a problem
-        *  with the tick timer readout. The two bytes get swapped.
-        *  This hack solves that problem by doing one extra input.
-        */
-       for (i = 0; i < 1000; ++i) {
-               t = short_ftape_timestamp();
-               if (t > LATCH) {
-                       inb_p(0x40);    /* get in sync again */
-                       TRACE(ft_t_warn, "clock counter fixed");
-                       break;
-               }
-       }
+#if defined(__x86_64__)
+       ps_per_cycle = 1000000000UL / cpu_khz;
 #elif defined(__alpha__)
-#if CONFIG_FT_ALPHA_CLOCK == 0
-#error You must define and set CONFIG_FT_ALPHA_CLOCK in 'make config' !
-#endif
        extern struct hwrpb_struct *hwrpb;
-       TRACE_FUN(ft_t_any);
-
-       if (hwrpb->cycle_freq != 0) {
-               ps_per_cycle = (1000*1000*1000*1000UL) / hwrpb->cycle_freq;
-       } else {
-               /*
-                * HELP:  Linux 2.0.x doesn't set cycle_freq on my noname !
-                */
-               ps_per_cycle = (1000*1000*1000*1000UL) / CONFIG_FT_ALPHA_CLOCK;
-       }
+       ps_per_cycle = (1000*1000*1000*1000UL) / hwrpb->cycle_freq;
 #endif
        TRACE_EXIT;
 }

@@ -214,7 +206,7 @@
        unsigned int tc = 0;
        unsigned int count;
        unsigned int time;
-#if defined(__i386__) || defined(__x86_64__)
+#if defined(__i386__)
        unsigned int old_tc = 0;
        unsigned int old_count = 1;
        unsigned int old_time = 1;

@@ -257,15 +249,14 @@
                tc = (1000 * time) / (count - 1);
                TRACE(ft_t_any, "once:%3d us,%6d times:%6d us, TC:%5d ns",
                        usecs(once), count - 1, usecs(multiple), tc);
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
                /*
                 * Increase the calibration count exponentially until the
                 * calibration time exceeds 100 ms.
                 */
-               if (time >= 100*1000) {
+               if (time >= 100*1000)
                        break;
-               }
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__i386__)
                /*
                 * increase the count until the resulting time nears 2/HZ,
                 * then the tc will drop sharply because we lose LATCH counts.

diff -Nru a/drivers/char/vt.c b/drivers/char/vt.c

drivers/char/vt.c Sun May 26 14:18:40 2002 +3 -0

@@ -107,6 +107,7 @@
 _kd_mksound(unsigned int hz, unsigned int ticks)
 {
        static struct timer_list sound_timer = { function: kd_nosound };
+        extern spinlock_t i8253_lock;
        unsigned int count = 0;
        unsigned long flags;
 

@@ -120,10 +121,12 @@
                /* enable counter 2 */
                outb_p(inb_p(0x61)|3, 0x61);
                /* set command for counter 2, 2 byte write */
+               spin_lock(&i8253_lock);
                outb_p(0xB6, 0x43);
                /* select desired HZ */
                outb_p(count & 0xff, 0x42);
                outb((count >> 8) & 0xff, 0x42);
+               spin_unlock(&i8253_lock);
 
                if (ticks) {
                        sound_timer.expires = jiffies+ticks;

diff -Nru a/drivers/ide/hd.c b/drivers/ide/hd.c

drivers/ide/hd.c Sun May 26 14:18:40 2002 +6 -4

@@ -110,6 +110,8 @@
 
 static struct timer_list device_timer;
 
+#define TIMEOUT_VALUE  (6*HZ)
+
 #define SET_TIMER                                                      \
        do {                                                            \
                mod_timer(&device_timer, jiffies + TIMEOUT_VALUE);      \

@@ -131,16 +133,16 @@
 
 unsigned long read_timer(void)
 {
+        extern spinlock_t i8253_lock;
        unsigned long t, flags;
        int i;
 
-       save_flags(flags);
-       cli();
+       spin_lock_irqsave(&i8253_lock, flags);
        t = jiffies * 11932;
        outb_p(0, 0x43);
        i = inb_p(0x40);
        i |= inb(0x40) << 8;
-       restore_flags(flags);
+       spin_unlock_irqrestore(&i8253_lock, flags);
        return(t - i);
 }
 #endif

@@ -831,7 +833,7 @@
                printk("hd: unable to get major %d for hard disk\n",MAJOR_NR);
                return -1;
        }
-       blk_init_queue(BLK_DEFAULT_QUEUE(MAJOR_NR), DEVICE_REQUEST, &hd_lock);
+       blk_init_queue(BLK_DEFAULT_QUEUE(MAJOR_NR), do_hd_request, &hd_lock);
        blk_queue_max_sectors(BLK_DEFAULT_QUEUE(MAJOR_NR), 255);
        add_gendisk(&hd_gendisk);
        init_timer(&device_timer);

diff -Nru a/drivers/input/gameport/gameport.c b/drivers/input/gameport/gameport.c

drivers/input/gameport/gameport.c Sun May 26 14:18:40 2002 +24 -4

@@ -54,16 +54,36 @@
 static struct gameport *gameport_list;
 static struct gameport_dev *gameport_dev;
 
+
+#ifdef __i386__
+
+#define DELTA(x,y)      ((y)-(x)+((y)<(x)?1193180/HZ:0))
+#define GET_TIME(x)     do { x = get_time_pit(); } while (0)
+
+static unsigned int get_time_pit(void)
+{
+       extern spinlock_t i8253_lock;
+       unsigned long flags;
+       unsigned int count;
+
+       spin_lock_irqsave(&i8253_lock, flags);
+       outb_p(0x00, 0x43);
+       count = inb_p(0x40);
+       count |= inb_p(0x40) << 8;
+       spin_unlock_irqrestore(&i8253_lock, flags);
+
+       return count;
+}
+
+#endif
+
 /*
  * gameport_measure_speed() measures the gameport i/o speed.
  */
 
 static int gameport_measure_speed(struct gameport *gameport)
 {
-#if defined(__i386__) || defined(__x86_64__)
-
-#define GET_TIME(x)     do { outb(0, 0x43); x = inb(0x40); x |= inb(0x40) << 8; } while (0)
-#define DELTA(x,y)      ((y)-(x)+((y)<(x)?1193180L/HZ:0))
+#ifdef __i386__
 
        unsigned int i, t, t1, t2, t3, tx;
        unsigned long flags;

===================================================================

This BitKeeper patch contains the following changesets:
1.582..1.584
## Wrapped with gzip_uu ##

begin 664 bkpatch17031 M'XL(`*#2\#P``\U:ZW/;-A+_+/X5N,E,*R66!(!OYW%Q(K?QU6ESB=T/G<YH M(!(2&5.DRH=EM>K_?KL)<NR99MJW3G9)F426"QV][<O\ADY+V1^V+K,OI8R MB(QGY$-6E(>M<AXG\20J>U4P[P6_P_7/60;7^U$VE?UZ=']TT4_BM"JZO&<; M,.:3*(.(7,J\.&RQGKF^4BYF\K#U^?C[\].CSX;Q^C5Y'XET(K_(DKQ^;919 M?BF2L'@KRBC)TEZ9B[28RE+T@FRZ7]=<DHY_-C,-:GM+)E#+7<9L)`Q83$9 M4FYYCG5-#5F]GY;-;<JXR?VE:5F>:PP(Z]D>)Y3WJ=WG%F'>H64>VGZ7NH>4 MDGM(DQ>,=*GQCOR]FWEO!.1\%HI2D@N9IS)1THVSU/B!F);K<^/3M2R-;L./ M85!!C3</\/Q17,AQG,A-EGW36U+JN<XR=$3H!TQXH>W;PKY/1EN4E/0MTW*6 MW'9=;C36G&7:J#F4PL>[-&<=4NLQFJ-/H;E-O7PAEX"0'O.,@DS@+X&_1IME MU*:F[8+(X;.'FO^*:#4HS)5H'<+HH4D/+><%Y2C:VAF\W7(93PB(LR@N2%" M\QC'5Q)$&4]E48KIC$QD*G-1CY(G))Q*6:R&X@D'N6]X(#,HQB<T5P40&,:
M%T6<3H@@219<D'&6$Q$$4E\L(TEBC]NF(IWW](K3+*P22?(J36$0T"BSB82A M.9G'941BTW/Z&J1]G-8+2)!524AR651)B0PA7441KDWBH@0HYDD4XL6BS30 ME(IL7,Y%+HE(<?8LRTLU8"YF,QD2D&8E"V1*PAB]'9&2>99?B#RK8[NID26 M[Q#"J"IKOE()O@2^3V>)+&6R#(@3\7D+,]&B9SVR%%29=X"58JRCA)@)^I MB-,"!)>+0))VFLV1QE2FI0R!Q*TE.P=Z5U_C\3@&;>&FM@2\%H>6-%)1)IF M)1E)I>*02%'$R:)'3D51ZCW@[42J?]56@RR%[92%7AVUL[**@HY7'G.T+% M9:0J)#G[\KY>N<@2V#J0E('.W'Y+;"8S,4"V9B![F!CM0"0\S"31?IM22)Q M"1='AOB*O1&MZT'Z,#OCR"G6HX1BA^)C)#FE@/Y6[K4T2SJD0MDV(6I\I@ M(YF$/?)=G(/BW^I:.[SC\0#<(\QA#4#R*1]Y6L^TDV3\"6DOY-Q6\@VJ+4 M7E)F.G3)\N2PN:!,^+AV'5V^8_&"]G<H9Y)+0O"B<<MP]A%^):_T_.<I1:A M]G?6AK^S#IE[:-&'_)WSM/Y.342G%V4S.:Y[;7[6UDAH+(4*5@1:8L:&VCY MZ)?0=G+P?6C!^*6;9FDQDT$\C@-2"[H#_A'-LUP=:R,,HGA&%"BUB0(:NRL< M;1@D7(%,,@:5_*YG3\'IQ=W1HES9-OBLU68T!)!S0$55*')I"$R4*^^@37\- MK#*"+8%+!(1>D'TP1=3ST^!2R5Z$`0";;CTW\0"7$H^U&X9>_,7IH<+&L9 MFBSDWL@-'#EVY"A\T-YOD@/K9)RYU%HZ0-M[D"G4;G\S%*$2_2WF.%O:ONM[ MRW'@^Z[CP46(^6-F[63N?K+73,(2_&')W:(F9M/;+#K,\<!?V+YCFX('H<U- M$.'C6=P@>LT@A3S*;^;D+LMM7P;*,!D%(3J.8XX%LP4H7)@[?<,N>M=L6=SB M9G.YZ3SCEFYMZHJ@*-1Z'-K#+=<]_%RVR1ZS:');/]A\[NQT:_9G*&X*(_ M$5.5Q=R."8R"]2UMX4CP$4P$KLD\_V&(I+.J7%.]17[#'!V(.:K@7)4>#]>; MC<H=XT+$;\L9Z^55E'>K%!Q>%D35M!?*6]6.Q3GW.575#E-A!BYLUYJ6\\A:
MD_VCM::NT'XBW7RN?L%'?EK+=__.C),T[P\.7\W>GQS\>GY#5(0*GJ;B_ MK+B_XE7NT^/]CL4$:3K@[1UNLUJK]G;N0*U'Y[^DRCT*,1$7X80M3%,JY)' M%PFYC-.X5'[9M:)1\C]6<YS2#)K4*9B$6;T0Y4&<F.E#TH$Q.2M6V2Q0 MPT04,UI(S&%RO6:78"FBJR$,U1EF%YGKS)@+%J(C/&J#MW^MI'=+?T]3.Z$ M,9>#P;7D%20*Z3IS&99ZWT/\YZ4Q@&$6X6JXC<-QG+HWC//?"DC[V]]<CS\@ MXT1,BHZ>YQ(7YT&Q;!DM2-E'PUF;7IG6:%7EMEYV6KUGY,1Y.PY5!Q3S/KY M3G]\J[_\<N[Y`[H>1Y?SWS].CL_0?R#<P=CQ4%"A2``$RX/>S-&^)M#OI8 M#U+<5^F*?ZAGP,9V;.%.\-6!?@_X-<H\&@%P*_DP.3--;BZI1QU?09#QYA"T MGBI]1PAN80^1=QMX.N>-H9#.\VI6KK/EJ5B0NAH6(RA8*ZS6:X0=:"1BRH[) MLAC!G1@K5-446&2I)#(II$Z@==VOL_&-=84J<$D!853Q<!S59MJ$JW@LB* M(4"E=@^ALU;-/O@T'0ZX>P">K2HMXDFJW!D(3YGN2^/$8FX#K)Y8G*Z'_Q5P MU)ZM.38:)&Z-D'$S=UL!@W-F6QH8]/\;&*M*$2M";9QQ2G2=#Z%G6!9!NX.X M.)J(.*T[/QOP*,H\#LH-<*9JY:.+@]5TP[#V(K8VWT8SAS06A:BXU+9B+ MPP)=6,,*XRH-L*D(T^4EU)HF+Q*4=:K\=3U]TTF1:80V$33A7MN+WWG\YA M,I3<,E1Q3B73#V%)*W,?*-FN@U"Z$RN_PFW/:8(6&U+R/=#R^;.PP#ZN[M. M1EX5Y>(M'H,LG^$:/5'MT7/B#(!*G:7E4:X?(%FW.N4/(PP@9IM/@K'OL*.[ M[H\#@3!!>T3CK%NBF):AU2D?KSIG6W;Y>*'L50A8JA*P(&5Z)I-X3$*H*<!F MV\.A9GX[!C/PT$"62BY)4HIOUID?>B-]M7<9M7ZOH6'=P=4"'&P/)P,75\
M=F.(2&:1P#'+Y9T,G-@,&;R+K'%_P'HFTS>^H&+L:<@:LJ'SSR>S:\!=T< ME\>@O=+VGD'O\NR@B7SE[OV"NOUGQOD.?DQ*^O@KY,$;+YE=9Q'ISB7X,\@ M+\"D0_?@QJ!<Y9'!+NJ>/1("4>?8A4\G!T!!^4[5MQY)[.'-,TP5=-^<3,!> MX&:>RZDD*5"L=Y#$MV?ZZ?,F)FB0#S,LUW_\;X(5.@I>_$P16_BD0@NZ#.< MJX[-U#_PE=;4<9?$![Y#3!B#FVHUV!"C6)6<X&F/+3'JXZJ,,3@UV]3*BMHE MPXXN%Z[]\:XK9A4IX:R8]S5<]W[I,=L1XU2IQ;ZIW9,7A/ZDL3D%7A2"M]> MO(C52(5B<'IP0A$X+NQ]H*H[K-N@_.);/&YL>E8,9S(?!HLFT&*L/Z<GY(^ M"6;5\"+Z'<LYSU94,?91H.KCHMNSVSC]^<W#^6D'"$7S?#;JOE$#A^-<_@8D MN4I23_3I;E\RX&I7)_K44-#<L?5<5;>V8');P?R-VJ9B3HUR$7;<\>Y1QV;\
MWMEM:QBV]VT'KA_D%%4A&[0!.7<XT&R);6H9:O@[#9/?SD$9^O)\]\Y/L/8 M_00:8[/J9>Z(S3M%L5?V",FC"<@"\P/+("NKP"O:4LC@^/3LJ'UUL.@0]6FW M%YUN^ZKSK^\@B__9LP'W='^AU\.::>SGOC]\=GP[.3C,0Q1$\.,_$&N$D3 M60YQJ\-97+;!M_V)3^L!8VV*L0P;3W%UD$1:N2;,RZS..P8?^Q7/;9NT%7! M$-/DQ^;'UYT>2E>='J.ER,#&XE3?P\;,ZNKRQF7RZA7Q7C;+KW]=>^^:W3]1 M.^M4P\$.U(GC:!]R0XFW,O/+YDA^_!,1(\JB[.T(=BN*J)<"/'8]"J$N(;; M%-\Z,?5;)Q#3&J/5?,IB]>Y&ZJV:M4>.L4K$I_83]3HOM"`N5(8%V*4R.*Z MQP/)4I*-L'5Z4+]?L;ZEIP0"'VUBX7DE@TJ]!P*5JVJC8E&Y>IVB*C:>FFZ1 M@)H6&SJ;KQCLZ/O4KZJH$>"-=.%P@=W@'XA^0G5?:7"YI[O!%(89I/[<#V#( M(OC&X&N(;L*DH\8YM\9I;&V-O!'IZD>OCX=#LV>_1B@2";;S5E[&^&"E"V8" M();%/32QQ/0XAQ+3-BVO;F_N$<:<IXIB6&$JH6'+))O.T&L+;,_HMPWNC7'X MOM#-**>?<N^*<K5D]NO^8WZ]BD,8@WXZ/QO^?'1Z?MQJ.\\__()QYH29YJ-M M<<!,E4DQTVOTG,!2M80^-7+ZT\7R_K4&B4P)8W+X6^5K&3[W>D/P\'Q=T?G MIV?#_YX?GQ^W/Q[]YZ?/PQ\_=PX@S@ZC<AI:"7QC:EOX)^5_:]>T@TB,,VB 3FK[V>!@X^#KL_P!<D_!-'2P`````
`
end

--
Vojtech Pavlik
SuSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Hi Vojtech,

i8253_lock seems to be defined only if __i386__ is defined but is accessed in drivers/char/vt.c where we could attempt to lock i8253_lock even though __i386__ is not defined.

The preprocessor check there seems to be

#if defined(__i386__) || defined(__alpha__) || defined(__powerpc__) \
    || (defined(__mips__) && defined(CONFIG_ISA)) \
    || (defined(__arm__) && defined(CONFIG_HOST_FOOTBRIDGE)) \
    || defined(__x86_64__)

	Won't this cause a build break on alpha ?

Am i missing something ?

thanks for your time,
regards
Manik

Quoted 509 lines · Vojtech Pavlik

Hi!

[email protected], 2002-05-26 14:17:40+02:00, [email protected]
This changeset hopefully fixes all the instances (at least on i386 arch and arch-nonspecific drivers) where there the i8253 chip is accessed - to use a spinlock to serialize the multi-byte accesses. This should allow us to understand the timer problems that are happening now and then better.

[email protected], 2002-05-26 10:30:46+02:00, [email protected]
This cset fixes timestamp generation in ftape-calibr.c, which was missing a lock for accessing the i8253 timer. This module running together with i386/kernel/time.c could result in the timer registers losing sync with software and reporting swapped values. There was an workaround for this in ftape-calibr.c, but could never completely fix the problem. Also, there still remains a race (now commented in ftape-calibr.c), with jiffies and the i8253 timer register. This cannot be fixed easily. Last, but not least, this converts ftape timestamps on x86_64 to use TSC timer solely, because it's always present there and doesn't have all the problems with the i8253 timer.

Now there shouldn't be any code accessing the i8253 timer without a spinlock held. Finally!

 arch/i386/kernel/apm.c                     |   15 +---
 arch/i386/kernel/i8259.c                   |    4 +
 arch/i386/kernel/time.c                    |    4 +
 drivers/char/ftape/lowlevel/ftape-calibr.c |   97 +++++++++++++----------------
 drivers/char/vt.c                          |    3
 drivers/ide/hd.c                           |   10 +-
 drivers/input/gameport/gameport.c          |   28 +++++++-
 7 files changed, 91 insertions(+), 70 deletions(-)

diff -Nru a/arch/i386/kernel/apm.c b/arch/i386/kernel/apm.c

arch/i386/kernel/apm.c Sun May 26 14:18:40 2002 +6 -9

@@ -1170,17 +1170,14 @@
 {
 #ifdef INIT_TIMER_AFTER_SUSPEND
        unsigned long   flags;
+       extern spinlock_t i8253_lock;
 
-       save_flags(flags);
-       cli();
+       spin_lock_irqsave(&i8253_lock, flags);
        /* set the clock to 100 Hz */
-       outb_p(0x34,0x43);              /* binary, mode 2, LSB/MSB, ch 0 */
-       udelay(10);
-       outb_p(LATCH & 0xff , 0x40);    /* LSB */
-       udelay(10);
-       outb(LATCH >> 8 , 0x40);        /* MSB */
-       udelay(10);
-       restore_flags(flags);
+       outb_p(0x34, 0x43);             /* binary, mode 2, LSB/MSB, ch 0 */
+       outb_p(LATCH & 0xff, 0x40);     /* LSB */
+       outb_p(LATCH >> 8, 0x40);       /* MSB */
+       spin_unlock_irqrestore(&i8253_lock, flags);
 #endif
 }
 

diff -Nru a/arch/i386/kernel/i8259.c b/arch/i386/kernel/i8259.c

arch/i386/kernel/i8259.c Sun May 26 14:18:40 2002 +4 -0

@@ -360,6 +360,8 @@
 void __init init_IRQ(void)
 {
        int i;
+       extern spinlock_t i8253_lock;
+       unsigned long flags;
 
 #ifndef CONFIG_X86_VISWS_APIC
        init_ISA_irqs();

@@ -415,9 +417,11 @@
         * Set the clock to HZ Hz, we already have a valid
         * vector now:
         */
+       spin_lock_irqsave(&i8253_lock, flags);
        outb_p(0x34,0x43);              /* binary, mode 2, LSB/MSB, ch 0 */
        outb_p(LATCH & 0xff , 0x40);    /* LSB */
        outb(LATCH >> 8 , 0x40);        /* MSB */
+       spin_unlock_irqrestore(&i8253_lock, flags);
 
 #ifndef CONFIG_VISWS
        setup_irq(2, &irq2);

diff -Nru a/arch/i386/kernel/time.c b/arch/i386/kernel/time.c

arch/i386/kernel/time.c Sun May 26 14:18:40 2002 +4 -0

@@ -574,6 +574,8 @@
 
 static unsigned long __init calibrate_tsc(void)
 {
+       unsigned long flags;
+
        /* Set the Gate high, disable speaker */
        outb((inb(0x61) & ~0x02) | 0x01, 0x61);
 

@@ -584,9 +586,11 @@
         * (interrupt on terminal count mode), binary count,
         * load 5 * LATCH count, (LSB and MSB) to begin countdown.
         */
+       spin_lock_irqsave(&i8253_lock, flags);
        outb(0xb0, 0x43);                       /* binary, mode 0, LSB/MSB, Ch 2 */
        outb(CALIBRATE_LATCH & 0xff, 0x42);     /* LSB of count */
        outb(CALIBRATE_LATCH >> 8, 0x42);       /* MSB of count */
+       spin_unlock_irqrestore(&i8253_lock, flags);
 
        {
                unsigned long startlow, starthigh;

diff -Nru a/drivers/char/ftape/lowlevel/ftape-calibr.c b/drivers/char/ftape/lowlevel/ftape-calibr.c

drivers/char/ftape/lowlevel/ftape-calibr.c Sun May 26 14:18:40 2002 +44 -53

@@ -31,7 +31,10 @@
 #include <asm/io.h>
 #if defined(__alpha__)
 # include <asm/hwrpb.h>
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__x86_64__)
+# include <asm/msr.h>
+# include <asm/timex.h>
+#elif defined(__i386__)
 # include <linux/timex.h>
 #endif
 #include <linux/ftape.h>

@@ -45,10 +48,14 @@
 # error Ftape is not implemented for this architecture!
 #endif
 
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
 static unsigned long ps_per_cycle = 0;
 #endif
 
+#if defined(__i386__)
+extern spinlock_t i8253_lock;
+#endif
+
 /*
  * Note: On Intel PCs, the clock ticks at 100 Hz (HZ==100) which is
  * too slow for certain timeouts (and that clock doesn't even tick

@@ -67,48 +74,58 @@
 {
 #if defined(__alpha__)
        unsigned long r;
-
        asm volatile ("rpcc %0" : "=r" (r));
        return r;
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__x86_64__)
+       unsigned long r;
+       rdtscl(r);
+       return r;
+#elif defined(__i386__)
+
+/*
+ * Note that there is some time between counter underflowing and jiffies
+ * increasing, so the code below won't always give correct output.
+ * -Vojtech
+ */
+
        unsigned long flags;
        __u16 lo;
        __u16 hi;
 
-       save_flags(flags);
-       cli();
+       spin_lock_irqsave(&i8253_lock, flags);
        outb_p(0x00, 0x43);     /* latch the count ASAP */
        lo = inb_p(0x40);       /* read the latched count */
        lo |= inb(0x40) << 8;
        hi = jiffies;
-       restore_flags(flags);
+       spin_unlock_irqrestore(&i8253_lock, flags);
+
        return ((hi + 1) * (unsigned int) LATCH) - lo;  /* downcounter ! */
 #endif
 }
 
 static unsigned int short_ftape_timestamp(void)
 {
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
        return ftape_timestamp();
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__i386__)
        unsigned int count;
        unsigned long flags;
-
-       save_flags(flags);
-       cli();
+
+       spin_lock_irqsave(&i8253_lock, flags);
        outb_p(0x00, 0x43);     /* latch the count ASAP */
        count = inb_p(0x40);    /* read the latched count */
        count |= inb(0x40) << 8;
-       restore_flags(flags);
+       spin_unlock_irqrestore(&i8253_lock, flags);
+
        return (LATCH - count); /* normal: downcounter */
 #endif
 }
 
 static unsigned int diff(unsigned int t0, unsigned int t1)
 {
-#if defined(__alpha__)
-       return (t1 <= t0) ? t1 + (1UL << 32) - t0 : t1 - t0;
-#elif defined(__i386__) || defined(__x86_64__)
+#if defined(__alpha__) || defined(__x86_64__)
+       return (t1 - t0);
+#elif defined(__i386__)
        /*
         * This is tricky: to work for both short and full ftape_timestamps
         * we'll have to discriminate between these.

@@ -122,9 +139,9 @@
 
 static unsigned int usecs(unsigned int count)
 {
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
        return (ps_per_cycle * count) / 1000000UL;
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__i386__)
        return (10000 * count) / ((CLOCK_TICK_RATE + 50) / 100);
 #endif
 }

@@ -153,49 +170,24 @@
        save_flags(flags);
        cli();
        t0 = short_ftape_timestamp();
-       for (i = 0; i < 1000; ++i) {
+       for (i = 0; i < 1000; ++i)
                status = inb(fdc.msr);
-       }
        t1 = short_ftape_timestamp();
        restore_flags(flags);
+
        TRACE(ft_t_info, "inb() duration: %d nsec", ftape_timediff(t0, t1));
        TRACE_EXIT;
 }
 
 static void init_clock(void)
 {
-#if defined(__i386__) || defined(__x86_64__)
-       unsigned int t;
-       int i;
        TRACE_FUN(ft_t_any);
 
-       /*  Haven't studied on why, but there sometimes is a problem
-        *  with the tick timer readout. The two bytes get swapped.
-        *  This hack solves that problem by doing one extra input.
-        */
-       for (i = 0; i < 1000; ++i) {
-               t = short_ftape_timestamp();
-               if (t > LATCH) {
-                       inb_p(0x40);    /* get in sync again */
-                       TRACE(ft_t_warn, "clock counter fixed");
-                       break;
-               }
-       }
+#if defined(__x86_64__)
+       ps_per_cycle = 1000000000UL / cpu_khz;
 #elif defined(__alpha__)
-#if CONFIG_FT_ALPHA_CLOCK == 0
-#error You must define and set CONFIG_FT_ALPHA_CLOCK in 'make config' !
-#endif
        extern struct hwrpb_struct *hwrpb;
-       TRACE_FUN(ft_t_any);
-
-       if (hwrpb->cycle_freq != 0) {
-               ps_per_cycle = (1000*1000*1000*1000UL) / hwrpb->cycle_freq;
-       } else {
-               /*
-                * HELP:  Linux 2.0.x doesn't set cycle_freq on my noname !
-                */
-               ps_per_cycle = (1000*1000*1000*1000UL) / CONFIG_FT_ALPHA_CLOCK;
-       }
+       ps_per_cycle = (1000*1000*1000*1000UL) / hwrpb->cycle_freq;
 #endif
        TRACE_EXIT;
 }

@@ -214,7 +206,7 @@
        unsigned int tc = 0;
        unsigned int count;
        unsigned int time;
-#if defined(__i386__) || defined(__x86_64__)
+#if defined(__i386__)
        unsigned int old_tc = 0;
        unsigned int old_count = 1;
        unsigned int old_time = 1;

@@ -257,15 +249,14 @@
                tc = (1000 * time) / (count - 1);
                TRACE(ft_t_any, "once:%3d us,%6d times:%6d us, TC:%5d ns",
                        usecs(once), count - 1, usecs(multiple), tc);
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
                /*
                 * Increase the calibration count exponentially until the
                 * calibration time exceeds 100 ms.
                 */
-               if (time >= 100*1000) {
+               if (time >= 100*1000)
                        break;
-               }
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__i386__)
                /*
                 * increase the count until the resulting time nears 2/HZ,
                 * then the tc will drop sharply because we lose LATCH counts.

diff -Nru a/drivers/char/vt.c b/drivers/char/vt.c

drivers/char/vt.c Sun May 26 14:18:40 2002 +3 -0

@@ -107,6 +107,7 @@
 _kd_mksound(unsigned int hz, unsigned int ticks)
 {
        static struct timer_list sound_timer = { function: kd_nosound };
+        extern spinlock_t i8253_lock;
        unsigned int count = 0;
        unsigned long flags;
 

@@ -120,10 +121,12 @@
                /* enable counter 2 */
                outb_p(inb_p(0x61)|3, 0x61);
                /* set command for counter 2, 2 byte write */
+               spin_lock(&i8253_lock);
                outb_p(0xB6, 0x43);
                /* select desired HZ */
                outb_p(count & 0xff, 0x42);
                outb((count >> 8) & 0xff, 0x42);
+               spin_unlock(&i8253_lock);
 
                if (ticks) {
                        sound_timer.expires = jiffies+ticks;

diff -Nru a/drivers/ide/hd.c b/drivers/ide/hd.c

drivers/ide/hd.c Sun May 26 14:18:40 2002 +6 -4

@@ -110,6 +110,8 @@
 
 static struct timer_list device_timer;
 
+#define TIMEOUT_VALUE  (6*HZ)
+
 #define SET_TIMER                                                      \
        do {                                                            \
                mod_timer(&device_timer, jiffies + TIMEOUT_VALUE);      \

@@ -131,16 +133,16 @@
 
 unsigned long read_timer(void)
 {
+        extern spinlock_t i8253_lock;
        unsigned long t, flags;
        int i;
 
-       save_flags(flags);
-       cli();
+       spin_lock_irqsave(&i8253_lock, flags);
        t = jiffies * 11932;
        outb_p(0, 0x43);
        i = inb_p(0x40);
        i |= inb(0x40) << 8;
-       restore_flags(flags);
+       spin_unlock_irqrestore(&i8253_lock, flags);
        return(t - i);
 }
 #endif

@@ -831,7 +833,7 @@
                printk("hd: unable to get major %d for hard disk\n",MAJOR_NR);
                return -1;
        }
-       blk_init_queue(BLK_DEFAULT_QUEUE(MAJOR_NR), DEVICE_REQUEST, &hd_lock);
+       blk_init_queue(BLK_DEFAULT_QUEUE(MAJOR_NR), do_hd_request, &hd_lock);
        blk_queue_max_sectors(BLK_DEFAULT_QUEUE(MAJOR_NR), 255);
        add_gendisk(&hd_gendisk);
        init_timer(&device_timer);

diff -Nru a/drivers/input/gameport/gameport.c b/drivers/input/gameport/gameport.c

drivers/input/gameport/gameport.c Sun May 26 14:18:40 2002 +24 -4

@@ -54,16 +54,36 @@
 static struct gameport *gameport_list;
 static struct gameport_dev *gameport_dev;
 
+
+#ifdef __i386__
+
+#define DELTA(x,y)      ((y)-(x)+((y)<(x)?1193180/HZ:0))
+#define GET_TIME(x)     do { x = get_time_pit(); } while (0)
+
+static unsigned int get_time_pit(void)
+{
+       extern spinlock_t i8253_lock;
+       unsigned long flags;
+       unsigned int count;
+
+       spin_lock_irqsave(&i8253_lock, flags);
+       outb_p(0x00, 0x43);
+       count = inb_p(0x40);
+       count |= inb_p(0x40) << 8;
+       spin_unlock_irqrestore(&i8253_lock, flags);
+
+       return count;
+}
+
+#endif
+
 /*
  * gameport_measure_speed() measures the gameport i/o speed.
  */
 
 static int gameport_measure_speed(struct gameport *gameport)
 {
-#if defined(__i386__) || defined(__x86_64__)
-
-#define GET_TIME(x)     do { outb(0, 0x43); x = inb(0x40); x |= inb(0x40) << 8; } while (0)
-#define DELTA(x,y)      ((y)-(x)+((y)<(x)?1193180L/HZ:0))
+#ifdef __i386__
 
        unsigned int i, t, t1, t2, t3, tx;
        unsigned long flags;

===================================================================

This BitKeeper patch contains the following changesets:
1.582..1.584
## Wrapped with gzip_uu ##

begin 664 bkpatch17031 M'XL(`*#2\#P``\U:ZW/;-A+_+/X5N,E,*R66!(!OYW%Q(K?QU6ESB=T/G<YH M(!(2&5.DRH=EM>K_?KL)<NR99MJW3G9)F426"QV][<O\ADY+V1^V+K,OI8R MB(QGY$-6E(>M<AXG\20J>U4P[P6_P_7/60;7^U$VE?UZ=']TT4_BM"JZO&<; M,.:3*(.(7,J\.&RQGKF^4BYF\K#U^?C[\].CSX;Q^C5Y'XET(K_(DKQ^;919 M?BF2L'@KRBC)TEZ9B[28RE+T@FRZ7]=<DHY_-C,-:GM+)E#+7<9L)`Q83$9 M4FYYCG5-#5F]GY;-;<JXR?VE:5F>:PP(Z]D>)Y3WJ=WG%F'>H64>VGZ7NH>4 MDGM(DQ>,=*GQCOR]FWEO!.1\%HI2D@N9IS)1THVSU/B!F);K<^/3M2R-;L./ M85!!C3</\/Q17,AQG,A-EGW36U+JN<XR=$3H!TQXH>W;PKY/1EN4E/0MTW*6 MW'9=;C36G&7:J#F4PL>[-&<=4NLQFJ-/H;E-O7PAEX"0'O.,@DS@+X&_1IME MU*:F[8+(X;.'FO^*:#4HS)5H'<+HH4D/+><%Y2C:VAF\W7(93PB(LR@N2%" M\QC'5Q)$&4]E48KIC$QD*G-1CY(G))Q*6:R&X@D'N6]X(#,HQB<T5P40&,:
M%T6<3H@@219<D'&6$Q$$4E\L(TEBC]NF(IWW](K3+*P22?(J36$0T"BSB82A M.9G'941BTW/Z&J1]G-8+2)!524AR651)B0PA7441KDWBH@0HYDD4XL6BS30 ME(IL7,Y%+HE(<?8LRTLU8"YF,QD2D&8E"V1*PAB]'9&2>99?B#RK8[NID26 M[Q#"J"IKOE()O@2^3V>)+&6R#(@3\7D+,]&B9SVR%%29=X"58JRCA)@)^I MB-,"!)>+0))VFLV1QE2FI0R!Q*TE.P=Z5U_C\3@&;>&FM@2\%H>6-%)1)IF M)1E)I>*02%'$R:)'3D51ZCW@[42J?]56@RR%[92%7AVUL[**@HY7'G.T+% M9:0J)#G[\KY>N<@2V#J0E('.W'Y+;"8S,4"V9B![F!CM0"0\S"31?IM22)Q M"1='AOB*O1&MZT'Z,#OCR"G6HX1BA^)C)#FE@/Y6[K4T2SJD0MDV(6I\I@ M(YF$/?)=G(/BW^I:.[SC\0#<(\QA#4#R*1]Y6L^TDV3\"6DOY-Q6\@VJ+4 M7E)F.G3)\N2PN:!,^+AV'5V^8_&"]G<H9Y)+0O"B<<MP]A%^):_T_.<I1:A M]G?6AK^S#IE[:-&'_)WSM/Y.342G%V4S.:Y[;7[6UDAH+(4*5@1:8L:&VCY MZ)?0=G+P?6C!^*6;9FDQDT$\C@-2"[H#_A'-LUP=:R,,HGA&%"BUB0(:NRL< M;1@D7(%,,@:5_*YG3\'IQ=W1HES9-OBLU68T!)!S0$55*')I"$R4*^^@37\- MK#*"+8%+!(1>D'TP1=3ST^!2R5Z$`0";;CTW\0"7$H^U&X9>_,7IH<+&L9 MFBSDWL@-'#EVY"A\T-YOD@/K9)RYU%HZ0-M[D"G4;G\S%*$2_2WF.%O:ONM[ MRW'@^Z[CP46(^6-F[63N?K+73,(2_&')W:(F9M/;+#K,\<!?V+YCFX('H<U- M$.'C6=P@>LT@A3S*;^;D+LMM7P;*,!D%(3J.8XX%LP4H7)@[?<,N>M=L6=SB M9G.YZ3SCEFYMZHJ@*-1Z'-K#+=<]_%RVR1ZS:');/]A\[NQT:_9G*&X*(_ M$5.5Q=R."8R"]2UMX4CP$4P$KLD\_V&(I+.J7%.]17[#'!V(.:K@7)4>#]>; MC<H=XT+$;\L9Z^55E'>K%!Q>%D35M!?*6]6.Q3GW.575#E-A!BYLUYJ6\\A:
MD_VCM::NT'XBW7RN?L%'?EK+=__.C),T[P\.7\W>GQS\>GY#5(0*GJ;B_ MK+B_XE7NT^/]CL4$:3K@[1UNLUJK]G;N0*U'Y[^DRCT*,1$7X80M3%,JY)' M%PFYC-.X5'[9M:)1\C]6<YS2#)K4*9B$6;T0Y4&<F.E#TH$Q.2M6V2Q0 MPT04,UI(S&%RO6:78"FBJR$,U1EF%YGKS)@+%J(C/&J#MW^MI'=+?T]3.Z$ M,9>#P;7D%20*Z3IS&99ZWT/\YZ4Q@&$6X6JXC<-QG+HWC//?"DC[V]]<CS\@ MXT1,BHZ>YQ(7YT&Q;!DM2-E'PUF;7IG6:%7EMEYV6KUGY,1Y.PY5!Q3S/KY M3G]\J[_\<N[Y`[H>1Y?SWS].CL_0?R#<P=CQ4%"A2``$RX/>S-&^)M#OI8 M#U+<5^F*?ZAGP,9V;.%.\-6!?@_X-<H\&@%P*_DP.3--;BZI1QU?09#QYA"T MGBI]1PAN80^1=QMX.N>-H9#.\VI6KK/EJ5B0NAH6(RA8*ZS6:X0=:"1BRH[) MLAC!G1@K5-446&2I)#(II$Z@==VOL_&-=84J<$D!853Q<!S59MJ$JW@LB* M(4"E=@^ALU;-/O@T'0ZX>P">K2HMXDFJW!D(3YGN2^/$8FX#K)Y8G*Z'_Q5P MU)ZM.38:)&Z-D'$S=UL!@W-F6QH8]/\;&*M*$2M";9QQ2G2=#Z%G6!9!NX.X M.)J(.*T[/QOP*,H\#LH-<*9JY:.+@]5TP[#V(K8VWT8SAS06A:BXU+9B+ MPP)=6,,*XRH-L*D(T^4EU)HF+Q*4=:K\=3U]TTF1:80V$33A7MN+WWG\YA M,I3<,E1Q3B73#V%)*W,?*-FN@U"Z$RN_PFW/:8(6&U+R/=#R^;.PP#ZN[M. M1EX5Y>(M'H,LG^$:/5'MT7/B#(!*G:7E4:X?(%FW.N4/(PP@9IM/@K'OL*.[ M[H\#@3!!>T3CK%NBF):AU2D?KSIG6W;Y>*'L50A8JA*P(&5Z)I-X3$*H*<!F MV\.A9GX[!C/PT$"62BY)4HIOUID?>B-]M7<9M7ZOH6'=P=4"'&P/)P,75\
M=F.(2&:1P#'+Y9T,G-@,&;R+K'%_P'HFTS>^H&+L:<@:LJ'SSR>S:\!=T< ME\>@O=+VGD'O\NR@B7SE[OV"NOUGQOD.?DQ*^O@KY,$;+YE=9Q'ISB7X,\@ M+\"D0_?@QJ!<Y9'!+NJ>/1("4>?8A4\G!T!!^4[5MQY)[.'-,TP5=-^<3,!> MX&:>RZDD*5"L=Y#$MV?ZZ?,F)FB0#S,LUW_\;X(5.@I>_$P16_BD0@NZ#.< MJX[-U#_PE=;4<9?$![Y#3!B#FVHUV!"C6)6<X&F/+3'JXZJ,,3@UV]3*BMHE MPXXN%Z[]\:XK9A4IX:R8]S5<]W[I,=L1XU2IQ;ZIW9,7A/ZDL3D%7A2"M]> MO(C52(5B<'IP0A$X+NQ]H*H[K-N@_.);/&YL>E8,9S(?!HLFT&*L/Z<GY(^ M"6;5\"+Z'<LYSU94,?91H.KCHMNSVSC]^<W#^6D'"$7S?#;JOE$#A^-<_@8D MN4I23_3I;E\RX&I7)_K44-#<L?5<5;>V8');P?R-VJ9B3HUR$7;<\>Y1QV;\
MWMEM:QBV]VT'KA_D%%4A&[0!.7<XT&R);6H9:O@[#9/?SD$9^O)\]\Y/L/8 M_00:8[/J9>Z(S3M%L5?V",FC"<@"\P/+("NKP"O:4LC@^/3LJ'UUL.@0]6FW M%YUN^ZKSK^\@B__9LP'W='^AU\.::>SGOC]\=GP[.3C,0Q1$\.,_$&N$D3 M60YQJ\-97+;!M_V)3^L!8VV*L0P;3W%UD$1:N2;,RZS..P8?^Q7/;9NT%7! M$-/DQ^;'UYT>2E>='J.ER,#&XE3?P\;,ZNKRQF7RZA7Q7C;+KW]=>^^:W3]1 M.^M4P\$.U(GC:!]R0XFW,O/+YDA^_!,1(\JB[.T(=BN*J)<"/'8]"J$N(;; M%-\Z,?5;)Q#3&J/5?,IB]>Y&ZJV:M4>.L4K$I_83]3HOM"`N5(8%V*4R.*Z MQP/)4I*-L'5Z4+]?L;ZEIP0"'VUBX7DE@TJ]!P*5JVJC8E&Y>IVB*C:>FFZ1 M@)H6&SJ;KQCLZ/O4KZJH$>"-=.%P@=W@'XA^0G5?:7"YI[O!%(89I/[<#V#( M(OC&X&N(;L*DH\8YM\9I;&V-O!'IZD>OCX=#LV>_1B@2";;S5E[&^&"E"V8" M();%/32QQ/0XAQ+3-BVO;F_N$<:<IXIB6&$JH6'+))O.T&L+;,_HMPWNC7'X MOM#-**>?<N^*<K5D]NO^8WZ]BD,8@WXZ/QO^?'1Z?MQJ.\\__()QYH29YJ-M M<<!,E4DQTVOTG,!2M80^-7+ZT\7R_K4&B4P)8W+X6^5K&3[W>D/P\'Q=T?G MIV?#_YX?GQ^W/Q[]YZ?/PQ\_=PX@S@ZC<AI:"7QC:EOX)^5_:]>T@TB,,VB 3FK[V>!@X^#KL_P!<D_!-'2P`````
`
end

--
Vojtech Pavlik
SuSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 7 lines · Manik Raina

Hi Vojtech,

i8253_lock seems to be defined only if __i386__ is defined but is accessed in drivers/char/vt.c where we could attempt to lock i8253_lock even though __i386__ is not defined.

Quoted 8 lines

The preprocessor check there seems to be

#if defined(__i386__) || defined(__alpha__) || defined(__powerpc__) \
    || (defined(__mips__) && defined(CONFIG_ISA)) \
    || (defined(__arm__) && defined(CONFIG_HOST_FOOTBRIDGE)) \
    || defined(__x86_64__)

      Won't this cause a build break on alpha ?

Yes, you're right - it will. Ho hum, this means that Alpha has the bug too, though it isn't a big problem there, because the ports aren't accessed often (not in gettimeofday, not in timer interrupt).

Quoted 1 line

Am i missing something ?

No, I missed that the spinlock isn't defined on alpha probably also other archs that use the i8253.

Possible ways to fix that:

* #ifdef the spinlock code to __i386__ only.
* Add an otherwise unused i8253_lock to other archs.
* Add the spinlock to other archs and make them use it.

Linus: What do you prefer?

Quoted 506 lines

thanks for your time,
regards
Manik

Quoted 501 lines · Vojtech Pavlik

Hi!

[email protected], 2002-05-26 14:17:40+02:00, [email protected]
This changeset hopefully fixes all the instances (at least on i386 arch and arch-nonspecific drivers) where there the i8253 chip is accessed - to use a spinlock to serialize the multi-byte accesses. This should allow us to understand the timer problems that are happening now and then better.

[email protected], 2002-05-26 10:30:46+02:00, [email protected]
This cset fixes timestamp generation in ftape-calibr.c, which was missing a lock for accessing the i8253 timer. This module running together with i386/kernel/time.c could result in the timer registers losing sync with software and reporting swapped values. There was an workaround for this in ftape-calibr.c, but could never completely fix the problem. Also, there still remains a race (now commented in ftape-calibr.c), with jiffies and the i8253 timer register. This cannot be fixed easily. Last, but not least, this converts ftape timestamps on x86_64 to use TSC timer solely, because it's always present there and doesn't have all the problems with the i8253 timer.

Now there shouldn't be any code accessing the i8253 timer without a spinlock held. Finally!

 arch/i386/kernel/apm.c                     |   15 +---
 arch/i386/kernel/i8259.c                   |    4 +
 arch/i386/kernel/time.c                    |    4 +
 drivers/char/ftape/lowlevel/ftape-calibr.c |   97 +++++++++++++----------------
 drivers/char/vt.c                          |    3
 drivers/ide/hd.c                           |   10 +-
 drivers/input/gameport/gameport.c          |   28 +++++++-
 7 files changed, 91 insertions(+), 70 deletions(-)

diff -Nru a/arch/i386/kernel/apm.c b/arch/i386/kernel/apm.c

arch/i386/kernel/apm.c Sun May 26 14:18:40 2002 +6 -9

@@ -1170,17 +1170,14 @@
 {
 #ifdef INIT_TIMER_AFTER_SUSPEND
        unsigned long   flags;
+       extern spinlock_t i8253_lock;
 
-       save_flags(flags);
-       cli();
+       spin_lock_irqsave(&i8253_lock, flags);
        /* set the clock to 100 Hz */
-       outb_p(0x34,0x43);              /* binary, mode 2, LSB/MSB, ch 0 */
-       udelay(10);
-       outb_p(LATCH & 0xff , 0x40);    /* LSB */
-       udelay(10);
-       outb(LATCH >> 8 , 0x40);        /* MSB */
-       udelay(10);
-       restore_flags(flags);
+       outb_p(0x34, 0x43);             /* binary, mode 2, LSB/MSB, ch 0 */
+       outb_p(LATCH & 0xff, 0x40);     /* LSB */
+       outb_p(LATCH >> 8, 0x40);       /* MSB */
+       spin_unlock_irqrestore(&i8253_lock, flags);
 #endif
 }
 

diff -Nru a/arch/i386/kernel/i8259.c b/arch/i386/kernel/i8259.c

arch/i386/kernel/i8259.c Sun May 26 14:18:40 2002 +4 -0

@@ -360,6 +360,8 @@
 void __init init_IRQ(void)
 {
        int i;
+       extern spinlock_t i8253_lock;
+       unsigned long flags;
 
 #ifndef CONFIG_X86_VISWS_APIC
        init_ISA_irqs();

@@ -415,9 +417,11 @@
         * Set the clock to HZ Hz, we already have a valid
         * vector now:
         */
+       spin_lock_irqsave(&i8253_lock, flags);
        outb_p(0x34,0x43);              /* binary, mode 2, LSB/MSB, ch 0 */
        outb_p(LATCH & 0xff , 0x40);    /* LSB */
        outb(LATCH >> 8 , 0x40);        /* MSB */
+       spin_unlock_irqrestore(&i8253_lock, flags);
 
 #ifndef CONFIG_VISWS
        setup_irq(2, &irq2);

diff -Nru a/arch/i386/kernel/time.c b/arch/i386/kernel/time.c

arch/i386/kernel/time.c Sun May 26 14:18:40 2002 +4 -0

@@ -574,6 +574,8 @@
 
 static unsigned long __init calibrate_tsc(void)
 {
+       unsigned long flags;
+
        /* Set the Gate high, disable speaker */
        outb((inb(0x61) & ~0x02) | 0x01, 0x61);
 

@@ -584,9 +586,11 @@
         * (interrupt on terminal count mode), binary count,
         * load 5 * LATCH count, (LSB and MSB) to begin countdown.
         */
+       spin_lock_irqsave(&i8253_lock, flags);
        outb(0xb0, 0x43);                       /* binary, mode 0, LSB/MSB, Ch 2 */
        outb(CALIBRATE_LATCH & 0xff, 0x42);     /* LSB of count */
        outb(CALIBRATE_LATCH >> 8, 0x42);       /* MSB of count */
+       spin_unlock_irqrestore(&i8253_lock, flags);
 
        {
                unsigned long startlow, starthigh;

diff -Nru a/drivers/char/ftape/lowlevel/ftape-calibr.c b/drivers/char/ftape/lowlevel/ftape-calibr.c

drivers/char/ftape/lowlevel/ftape-calibr.c Sun May 26 14:18:40 2002 +44 -53

@@ -31,7 +31,10 @@
 #include <asm/io.h>
 #if defined(__alpha__)
 # include <asm/hwrpb.h>
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__x86_64__)
+# include <asm/msr.h>
+# include <asm/timex.h>
+#elif defined(__i386__)
 # include <linux/timex.h>
 #endif
 #include <linux/ftape.h>

@@ -45,10 +48,14 @@
 # error Ftape is not implemented for this architecture!
 #endif
 
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
 static unsigned long ps_per_cycle = 0;
 #endif
 
+#if defined(__i386__)
+extern spinlock_t i8253_lock;
+#endif
+
 /*
  * Note: On Intel PCs, the clock ticks at 100 Hz (HZ==100) which is
  * too slow for certain timeouts (and that clock doesn't even tick

@@ -67,48 +74,58 @@
 {
 #if defined(__alpha__)
        unsigned long r;
-
        asm volatile ("rpcc %0" : "=r" (r));
        return r;
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__x86_64__)
+       unsigned long r;
+       rdtscl(r);
+       return r;
+#elif defined(__i386__)
+
+/*
+ * Note that there is some time between counter underflowing and jiffies
+ * increasing, so the code below won't always give correct output.
+ * -Vojtech
+ */
+
        unsigned long flags;
        __u16 lo;
        __u16 hi;
 
-       save_flags(flags);
-       cli();
+       spin_lock_irqsave(&i8253_lock, flags);
        outb_p(0x00, 0x43);     /* latch the count ASAP */
        lo = inb_p(0x40);       /* read the latched count */
        lo |= inb(0x40) << 8;
        hi = jiffies;
-       restore_flags(flags);
+       spin_unlock_irqrestore(&i8253_lock, flags);
+
        return ((hi + 1) * (unsigned int) LATCH) - lo;  /* downcounter ! */
 #endif
 }
 
 static unsigned int short_ftape_timestamp(void)
 {
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
        return ftape_timestamp();
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__i386__)
        unsigned int count;
        unsigned long flags;
-
-       save_flags(flags);
-       cli();
+
+       spin_lock_irqsave(&i8253_lock, flags);
        outb_p(0x00, 0x43);     /* latch the count ASAP */
        count = inb_p(0x40);    /* read the latched count */
        count |= inb(0x40) << 8;
-       restore_flags(flags);
+       spin_unlock_irqrestore(&i8253_lock, flags);
+
        return (LATCH - count); /* normal: downcounter */
 #endif
 }
 
 static unsigned int diff(unsigned int t0, unsigned int t1)
 {
-#if defined(__alpha__)
-       return (t1 <= t0) ? t1 + (1UL << 32) - t0 : t1 - t0;
-#elif defined(__i386__) || defined(__x86_64__)
+#if defined(__alpha__) || defined(__x86_64__)
+       return (t1 - t0);
+#elif defined(__i386__)
        /*
         * This is tricky: to work for both short and full ftape_timestamps
         * we'll have to discriminate between these.

@@ -122,9 +139,9 @@
 
 static unsigned int usecs(unsigned int count)
 {
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
        return (ps_per_cycle * count) / 1000000UL;
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__i386__)
        return (10000 * count) / ((CLOCK_TICK_RATE + 50) / 100);
 #endif
 }

@@ -153,49 +170,24 @@
        save_flags(flags);
        cli();
        t0 = short_ftape_timestamp();
-       for (i = 0; i < 1000; ++i) {
+       for (i = 0; i < 1000; ++i)
                status = inb(fdc.msr);
-       }
        t1 = short_ftape_timestamp();
        restore_flags(flags);
+
        TRACE(ft_t_info, "inb() duration: %d nsec", ftape_timediff(t0, t1));
        TRACE_EXIT;
 }
 
 static void init_clock(void)
 {
-#if defined(__i386__) || defined(__x86_64__)
-       unsigned int t;
-       int i;
        TRACE_FUN(ft_t_any);
 
-       /*  Haven't studied on why, but there sometimes is a problem
-        *  with the tick timer readout. The two bytes get swapped.
-        *  This hack solves that problem by doing one extra input.
-        */
-       for (i = 0; i < 1000; ++i) {
-               t = short_ftape_timestamp();
-               if (t > LATCH) {
-                       inb_p(0x40);    /* get in sync again */
-                       TRACE(ft_t_warn, "clock counter fixed");
-                       break;
-               }
-       }
+#if defined(__x86_64__)
+       ps_per_cycle = 1000000000UL / cpu_khz;
 #elif defined(__alpha__)
-#if CONFIG_FT_ALPHA_CLOCK == 0
-#error You must define and set CONFIG_FT_ALPHA_CLOCK in 'make config' !
-#endif
        extern struct hwrpb_struct *hwrpb;
-       TRACE_FUN(ft_t_any);
-
-       if (hwrpb->cycle_freq != 0) {
-               ps_per_cycle = (1000*1000*1000*1000UL) / hwrpb->cycle_freq;
-       } else {
-               /*
-                * HELP:  Linux 2.0.x doesn't set cycle_freq on my noname !
-                */
-               ps_per_cycle = (1000*1000*1000*1000UL) / CONFIG_FT_ALPHA_CLOCK;
-       }
+       ps_per_cycle = (1000*1000*1000*1000UL) / hwrpb->cycle_freq;
 #endif
        TRACE_EXIT;
 }

@@ -214,7 +206,7 @@
        unsigned int tc = 0;
        unsigned int count;
        unsigned int time;
-#if defined(__i386__) || defined(__x86_64__)
+#if defined(__i386__)
        unsigned int old_tc = 0;
        unsigned int old_count = 1;
        unsigned int old_time = 1;

@@ -257,15 +249,14 @@
                tc = (1000 * time) / (count - 1);
                TRACE(ft_t_any, "once:%3d us,%6d times:%6d us, TC:%5d ns",
                        usecs(once), count - 1, usecs(multiple), tc);
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
                /*
                 * Increase the calibration count exponentially until the
                 * calibration time exceeds 100 ms.
                 */
-               if (time >= 100*1000) {
+               if (time >= 100*1000)
                        break;
-               }
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__i386__)
                /*
                 * increase the count until the resulting time nears 2/HZ,
                 * then the tc will drop sharply because we lose LATCH counts.

diff -Nru a/drivers/char/vt.c b/drivers/char/vt.c

drivers/char/vt.c Sun May 26 14:18:40 2002 +3 -0

@@ -107,6 +107,7 @@
 _kd_mksound(unsigned int hz, unsigned int ticks)
 {
        static struct timer_list sound_timer = { function: kd_nosound };
+        extern spinlock_t i8253_lock;
        unsigned int count = 0;
        unsigned long flags;
 

@@ -120,10 +121,12 @@
                /* enable counter 2 */
                outb_p(inb_p(0x61)|3, 0x61);
                /* set command for counter 2, 2 byte write */
+               spin_lock(&i8253_lock);
                outb_p(0xB6, 0x43);
                /* select desired HZ */
                outb_p(count & 0xff, 0x42);
                outb((count >> 8) & 0xff, 0x42);
+               spin_unlock(&i8253_lock);
 
                if (ticks) {
                        sound_timer.expires = jiffies+ticks;

diff -Nru a/drivers/ide/hd.c b/drivers/ide/hd.c

drivers/ide/hd.c Sun May 26 14:18:40 2002 +6 -4

@@ -110,6 +110,8 @@
 
 static struct timer_list device_timer;
 
+#define TIMEOUT_VALUE  (6*HZ)
+
 #define SET_TIMER                                                      \
        do {                                                            \
                mod_timer(&device_timer, jiffies + TIMEOUT_VALUE);      \

@@ -131,16 +133,16 @@
 
 unsigned long read_timer(void)
 {
+        extern spinlock_t i8253_lock;
        unsigned long t, flags;
        int i;
 
-       save_flags(flags);
-       cli();
+       spin_lock_irqsave(&i8253_lock, flags);
        t = jiffies * 11932;
        outb_p(0, 0x43);
        i = inb_p(0x40);
        i |= inb(0x40) << 8;
-       restore_flags(flags);
+       spin_unlock_irqrestore(&i8253_lock, flags);
        return(t - i);
 }
 #endif

@@ -831,7 +833,7 @@
                printk("hd: unable to get major %d for hard disk\n",MAJOR_NR);
                return -1;
        }
-       blk_init_queue(BLK_DEFAULT_QUEUE(MAJOR_NR), DEVICE_REQUEST, &hd_lock);
+       blk_init_queue(BLK_DEFAULT_QUEUE(MAJOR_NR), do_hd_request, &hd_lock);
        blk_queue_max_sectors(BLK_DEFAULT_QUEUE(MAJOR_NR), 255);
        add_gendisk(&hd_gendisk);
        init_timer(&device_timer);

diff -Nru a/drivers/input/gameport/gameport.c b/drivers/input/gameport/gameport.c

drivers/input/gameport/gameport.c Sun May 26 14:18:40 2002 +24 -4

@@ -54,16 +54,36 @@
 static struct gameport *gameport_list;
 static struct gameport_dev *gameport_dev;
 
+
+#ifdef __i386__
+
+#define DELTA(x,y)      ((y)-(x)+((y)<(x)?1193180/HZ:0))
+#define GET_TIME(x)     do { x = get_time_pit(); } while (0)
+
+static unsigned int get_time_pit(void)
+{
+       extern spinlock_t i8253_lock;
+       unsigned long flags;
+       unsigned int count;
+
+       spin_lock_irqsave(&i8253_lock, flags);
+       outb_p(0x00, 0x43);
+       count = inb_p(0x40);
+       count |= inb_p(0x40) << 8;
+       spin_unlock_irqrestore(&i8253_lock, flags);
+
+       return count;
+}
+
+#endif
+
 /*
  * gameport_measure_speed() measures the gameport i/o speed.
  */
 
 static int gameport_measure_speed(struct gameport *gameport)
 {
-#if defined(__i386__) || defined(__x86_64__)
-
-#define GET_TIME(x)     do { outb(0, 0x43); x = inb(0x40); x |= inb(0x40) << 8; } while (0)
-#define DELTA(x,y)      ((y)-(x)+((y)<(x)?1193180L/HZ:0))
+#ifdef __i386__
 
        unsigned int i, t, t1, t2, t3, tx;
        unsigned long flags;

===================================================================

This BitKeeper patch contains the following changesets:
1.582..1.584
## Wrapped with gzip_uu ##

begin 664 bkpatch17031 M'XL(`*#2\#P``\U:ZW/;-A+_+/X5N,E,*R66!(!OYW%Q(K?QU6ESB=T/G<YH M(!(2&5.DRH=EM>K_?KL)<NR99MJW3G9)F426"QV][<O\ADY+V1^V+K,OI8R MB(QGY$-6E(>M<AXG\20J>U4P[P6_P_7/60;7^U$VE?UZ=']TT4_BM"JZO&<; M,.:3*(.(7,J\.&RQGKF^4BYF\K#U^?C[\].CSX;Q^C5Y'XET(K_(DKQ^;919 M?BF2L'@KRBC)TEZ9B[28RE+T@FRZ7]=<DHY_-C,-:GM+)E#+7<9L)`Q83$9 M4FYYCG5-#5F]GY;-;<JXR?VE:5F>:PP(Z]D>)Y3WJ=WG%F'>H64>VGZ7NH>4 MDGM(DQ>,=*GQCOR]FWEO!.1\%HI2D@N9IS)1THVSU/B!F);K<^/3M2R-;L./ M85!!C3</\/Q17,AQG,A-EGW36U+JN<XR=$3H!TQXH>W;PKY/1EN4E/0MTW*6 MW'9=;C36G&7:J#F4PL>[-&<=4NLQFJ-/H;E-O7PAEX"0'O.,@DS@+X&_1IME MU*:F[8+(X;.'FO^*:#4HS)5H'<+HH4D/+><%Y2C:VAF\W7(93PB(LR@N2%" M\QC'5Q)$&4]E48KIC$QD*G-1CY(G))Q*6:R&X@D'N6]X(#,HQB<T5P40&,:
M%T6<3H@@219<D'&6$Q$$4E\L(TEBC]NF(IWW](K3+*P22?(J36$0T"BSB82A M.9G'941BTW/Z&J1]G-8+2)!524AR651)B0PA7441KDWBH@0HYDD4XL6BS30 ME(IL7,Y%+HE(<?8LRTLU8"YF,QD2D&8E"V1*PAB]'9&2>99?B#RK8[NID26 M[Q#"J"IKOE()O@2^3V>)+&6R#(@3\7D+,]&B9SVR%%29=X"58JRCA)@)^I MB-,"!)>+0))VFLV1QE2FI0R!Q*TE.P=Z5U_C\3@&;>&FM@2\%H>6-%)1)IF M)1E)I>*02%'$R:)'3D51ZCW@[42J?]56@RR%[92%7AVUL[**@HY7'G.T+% M9:0J)#G[\KY>N<@2V#J0E('.W'Y+;"8S,4"V9B![F!CM0"0\S"31?IM22)Q M"1='AOB*O1&MZT'Z,#OCR"G6HX1BA^)C)#FE@/Y6[K4T2SJD0MDV(6I\I@ M(YF$/?)=G(/BW^I:.[SC\0#<(\QA#4#R*1]Y6L^TDV3\"6DOY-Q6\@VJ+4 M7E)F.G3)\N2PN:!,^+AV'5V^8_&"]G<H9Y)+0O"B<<MP]A%^):_T_.<I1:A M]G?6AK^S#IE[:-&'_)WSM/Y.342G%V4S.:Y[;7[6UDAH+(4*5@1:8L:&VCY MZ)?0=G+P?6C!^*6;9FDQDT$\C@-2"[H#_A'-LUP=:R,,HGA&%"BUB0(:NRL< M;1@D7(%,,@:5_*YG3\'IQ=W1HES9-OBLU68T!)!S0$55*')I"$R4*^^@37\- MK#*"+8%+!(1>D'TP1=3ST^!2R5Z$`0";;CTW\0"7$H^U&X9>_,7IH<+&L9 MFBSDWL@-'#EVY"A\T-YOD@/K9)RYU%HZ0-M[D"G4;G\S%*$2_2WF.%O:ONM[ MRW'@^Z[CP46(^6-F[63N?K+73,(2_&')W:(F9M/;+#K,\<!?V+YCFX('H<U- M$.'C6=P@>LT@A3S*;^;D+LMM7P;*,!D%(3J.8XX%LP4H7)@[?<,N>M=L6=SB M9G.YZ3SCEFYMZHJ@*-1Z'-K#+=<]_%RVR1ZS:');/]A\[NQT:_9G*&X*(_ M$5.5Q=R."8R"]2UMX4CP$4P$KLD\_V&(I+.J7%.]17[#'!V(.:K@7)4>#]>; MC<H=XT+$;\L9Z^55E'>K%!Q>%D35M!?*6]6.Q3GW.575#E-A!BYLUYJ6\\A:
MD_VCM::NT'XBW7RN?L%'?EK+=__.C),T[P\.7\W>GQS\>GY#5(0*GJ;B_ MK+B_XE7NT^/]CL4$:3K@[1UNLUJK]G;N0*U'Y[^DRCT*,1$7X80M3%,JY)' M%PFYC-.X5'[9M:)1\C]6<YS2#)K4*9B$6;T0Y4&<F.E#TH$Q.2M6V2Q0 MPT04,UI(S&%RO6:78"FBJR$,U1EF%YGKS)@+%J(C/&J#MW^MI'=+?T]3.Z$ M,9>#P;7D%20*Z3IS&99ZWT/\YZ4Q@&$6X6JXC<-QG+HWC//?"DC[V]]<CS\@ MXT1,BHZ>YQ(7YT&Q;!DM2-E'PUF;7IG6:%7EMEYV6KUGY,1Y.PY5!Q3S/KY M3G]\J[_\<N[Y`[H>1Y?SWS].CL_0?R#<P=CQ4%"A2``$RX/>S-&^)M#OI8 M#U+<5^F*?ZAGP,9V;.%.\-6!?@_X-<H\&@%P*_DP.3--;BZI1QU?09#QYA"T MGBI]1PAN80^1=QMX.N>-H9#.\VI6KK/EJ5B0NAH6(RA8*ZS6:X0=:"1BRH[) MLAC!G1@K5-446&2I)#(II$Z@==VOL_&-=84J<$D!853Q<!S59MJ$JW@LB* M(4"E=@^ALU;-/O@T'0ZX>P">K2HMXDFJW!D(3YGN2^/$8FX#K)Y8G*Z'_Q5P MU)ZM.38:)&Z-D'$S=UL!@W-F6QH8]/\;&*M*$2M";9QQ2G2=#Z%G6!9!NX.X M.)J(.*T[/QOP*,H\#LH-<*9JY:.+@]5TP[#V(K8VWT8SAS06A:BXU+9B+ MPP)=6,,*XRH-L*D(T^4EU)HF+Q*4=:K\=3U]TTF1:80V$33A7MN+WWG\YA M,I3<,E1Q3B73#V%)*W,?*-FN@U"Z$RN_PFW/:8(6&U+R/=#R^;.PP#ZN[M. M1EX5Y>(M'H,LG^$:/5'MT7/B#(!*G:7E4:X?(%FW.N4/(PP@9IM/@K'OL*.[ M[H\#@3!!>T3CK%NBF):AU2D?KSIG6W;Y>*'L50A8JA*P(&5Z)I-X3$*H*<!F MV\.A9GX[!C/PT$"62BY)4HIOUID?>B-]M7<9M7ZOH6'=P=4"'&P/)P,75\
M=F.(2&:1P#'+Y9T,G-@,&;R+K'%_P'HFTS>^H&+L:<@:LJ'SSR>S:\!=T< ME\>@O=+VGD'O\NR@B7SE[OV"NOUGQOD.?DQ*^O@KY,$;+YE=9Q'ISB7X,\@ M+\"D0_?@QJ!<Y9'!+NJ>/1("4>?8A4\G!T!!^4[5MQY)[.'-,TP5=-^<3,!> MX&:>RZDD*5"L=Y#$MV?ZZ?,F)FB0#S,LUW_\;X(5.@I>_$P16_BD0@NZ#.< MJX[-U#_PE=;4<9?$![Y#3!B#FVHUV!"C6)6<X&F/+3'JXZJ,,3@UV]3*BMHE MPXXN%Z[]\:XK9A4IX:R8]S5<]W[I,=L1XU2IQ;ZIW9,7A/ZDL3D%7A2"M]> MO(C52(5B<'IP0A$X+NQ]H*H[K-N@_.);/&YL>E8,9S(?!HLFT&*L/Z<GY(^ M"6;5\"+Z'<LYSU94,?91H.KCHMNSVSC]^<W#^6D'"$7S?#;JOE$#A^-<_@8D MN4I23_3I;E\RX&I7)_K44-#<L?5<5;>V8');P?R-VJ9B3HUR$7;<\>Y1QV;\
MWMEM:QBV]VT'KA_D%%4A&[0!.7<XT&R);6H9:O@[#9/?SD$9^O)\]\Y/L/8 M_00:8[/J9>Z(S3M%L5?V",FC"<@"\P/+("NKP"O:4LC@^/3LJ'UUL.@0]6FW M%YUN^ZKSK^\@B__9LP'W='^AU\.::>SGOC]\=GP[.3C,0Q1$\.,_$&N$D3 M60YQJ\-97+;!M_V)3^L!8VV*L0P;3W%UD$1:N2;,RZS..P8?^Q7/;9NT%7! M$-/DQ^;'UYT>2E>='J.ER,#&XE3?P\;,ZNKRQF7RZA7Q7C;+KW]=>^^:W3]1 M.^M4P\$.U(GC:!]R0XFW,O/+YDA^_!,1(\JB[.T(=BN*J)<"/'8]"J$N(;; M%-\Z,?5;)Q#3&J/5?,IB]>Y&ZJV:M4>.L4K$I_83]3HOM"`N5(8%V*4R.*Z MQP/)4I*-L'5Z4+]?L;ZEIP0"'VUBX7DE@TJ]!P*5JVJC8E&Y>IVB*C:>FFZ1 M@)H6&SJ;KQCLZ/O4KZJH$>"-=.%P@=W@'XA^0G5?:7"YI[O!%(89I/[<#V#( M(OC&X&N(;L*DH\8YM\9I;&V-O!'IZD>OCX=#LV>_1B@2";;S5E[&^&"E"V8" M();%/32QQ/0XAQ+3-BVO;F_N$<:<IXIB6&$JH6'+))O.T&L+;,_HMPWNC7'X MOM#-**>?<N^*<K5D]NO^8WZ]BD,8@WXZ/QO^?'1Z?MQJ.\\__()QYH29YJ-M M<<!,E4DQTVOTG,!2M80^-7+ZT\7R_K4&B4P)8W+X6^5K&3[W>D/P\'Q=T?G MIV?#_YX?GQ^W/Q[]YZ?/PQ\_=PX@S@ZC<AI:"7QC:EOX)^5_:]>T@TB,,VB 3FK[V>!@X^#KL_P!<D_!-'2P`````
`
end

Quoted 8 lines

Quoted 8 lines

--
Vojtech Pavlik
SuSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

--
Vojtech Pavlik
SuSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Hi Vojtech,

Thanks for the response. I looked at the changes to hd.c and noticed that the function read_timer() in drivers/ide/hd.c is not platform specific and seems to be present in all architectures so IMHO it'd make sense to have the spinlocks for all architectures.

Does this make sense ?

thanks
Manik

Quoted 558 lines · Vojtech Pavlik

Quoted 7 lines · Manik Raina

Hi Vojtech,

i8253_lock seems to be defined only if __i386__ is defined but is accessed in drivers/char/vt.c where we could attempt to lock i8253_lock even though __i386__ is not defined.

Quoted 8 lines

The preprocessor check there seems to be

#if defined(__i386__) || defined(__alpha__) || defined(__powerpc__) \
    || (defined(__mips__) && defined(CONFIG_ISA)) \
    || (defined(__arm__) && defined(CONFIG_HOST_FOOTBRIDGE)) \
    || defined(__x86_64__)

      Won't this cause a build break on alpha ?

Yes, you're right - it will. Ho hum, this means that Alpha has the bug too, though it isn't a big problem there, because the ports aren't accessed often (not in gettimeofday, not in timer interrupt).

Quoted 1 line

Am i missing something ?

No, I missed that the spinlock isn't defined on alpha probably also other archs that use the i8253.

Possible ways to fix that:

* #ifdef the spinlock code to __i386__ only.
* Add an otherwise unused i8253_lock to other archs.
* Add the spinlock to other archs and make them use it.

Linus: What do you prefer?

Quoted 506 lines

thanks for your time,
regards
Manik

Quoted 501 lines · Vojtech Pavlik

Hi!

[email protected], 2002-05-26 14:17:40+02:00, [email protected]
This changeset hopefully fixes all the instances (at least on i386 arch and arch-nonspecific drivers) where there the i8253 chip is accessed - to use a spinlock to serialize the multi-byte accesses. This should allow us to understand the timer problems that are happening now and then better.

[email protected], 2002-05-26 10:30:46+02:00, [email protected]
This cset fixes timestamp generation in ftape-calibr.c, which was missing a lock for accessing the i8253 timer. This module running together with i386/kernel/time.c could result in the timer registers losing sync with software and reporting swapped values. There was an workaround for this in ftape-calibr.c, but could never completely fix the problem. Also, there still remains a race (now commented in ftape-calibr.c), with jiffies and the i8253 timer register. This cannot be fixed easily. Last, but not least, this converts ftape timestamps on x86_64 to use TSC timer solely, because it's always present there and doesn't have all the problems with the i8253 timer.

Now there shouldn't be any code accessing the i8253 timer without a spinlock held. Finally!

 arch/i386/kernel/apm.c                     |   15 +---
 arch/i386/kernel/i8259.c                   |    4 +
 arch/i386/kernel/time.c                    |    4 +
 drivers/char/ftape/lowlevel/ftape-calibr.c |   97 +++++++++++++----------------
 drivers/char/vt.c                          |    3
 drivers/ide/hd.c                           |   10 +-
 drivers/input/gameport/gameport.c          |   28 +++++++-
 7 files changed, 91 insertions(+), 70 deletions(-)

diff -Nru a/arch/i386/kernel/apm.c b/arch/i386/kernel/apm.c

arch/i386/kernel/apm.c Sun May 26 14:18:40 2002 +6 -9

@@ -1170,17 +1170,14 @@
 {
 #ifdef INIT_TIMER_AFTER_SUSPEND
        unsigned long   flags;
+       extern spinlock_t i8253_lock;
 
-       save_flags(flags);
-       cli();
+       spin_lock_irqsave(&i8253_lock, flags);
        /* set the clock to 100 Hz */
-       outb_p(0x34,0x43);              /* binary, mode 2, LSB/MSB, ch 0 */
-       udelay(10);
-       outb_p(LATCH & 0xff , 0x40);    /* LSB */
-       udelay(10);
-       outb(LATCH >> 8 , 0x40);        /* MSB */
-       udelay(10);
-       restore_flags(flags);
+       outb_p(0x34, 0x43);             /* binary, mode 2, LSB/MSB, ch 0 */
+       outb_p(LATCH & 0xff, 0x40);     /* LSB */
+       outb_p(LATCH >> 8, 0x40);       /* MSB */
+       spin_unlock_irqrestore(&i8253_lock, flags);
 #endif
 }
 

diff -Nru a/arch/i386/kernel/i8259.c b/arch/i386/kernel/i8259.c

arch/i386/kernel/i8259.c Sun May 26 14:18:40 2002 +4 -0

@@ -360,6 +360,8 @@
 void __init init_IRQ(void)
 {
        int i;
+       extern spinlock_t i8253_lock;
+       unsigned long flags;
 
 #ifndef CONFIG_X86_VISWS_APIC
        init_ISA_irqs();

@@ -415,9 +417,11 @@
         * Set the clock to HZ Hz, we already have a valid
         * vector now:
         */
+       spin_lock_irqsave(&i8253_lock, flags);
        outb_p(0x34,0x43);              /* binary, mode 2, LSB/MSB, ch 0 */
        outb_p(LATCH & 0xff , 0x40);    /* LSB */
        outb(LATCH >> 8 , 0x40);        /* MSB */
+       spin_unlock_irqrestore(&i8253_lock, flags);
 
 #ifndef CONFIG_VISWS
        setup_irq(2, &irq2);

diff -Nru a/arch/i386/kernel/time.c b/arch/i386/kernel/time.c

arch/i386/kernel/time.c Sun May 26 14:18:40 2002 +4 -0

@@ -574,6 +574,8 @@
 
 static unsigned long __init calibrate_tsc(void)
 {
+       unsigned long flags;
+
        /* Set the Gate high, disable speaker */
        outb((inb(0x61) & ~0x02) | 0x01, 0x61);
 

@@ -584,9 +586,11 @@
         * (interrupt on terminal count mode), binary count,
         * load 5 * LATCH count, (LSB and MSB) to begin countdown.
         */
+       spin_lock_irqsave(&i8253_lock, flags);
        outb(0xb0, 0x43);                       /* binary, mode 0, LSB/MSB, Ch 2 */
        outb(CALIBRATE_LATCH & 0xff, 0x42);     /* LSB of count */
        outb(CALIBRATE_LATCH >> 8, 0x42);       /* MSB of count */
+       spin_unlock_irqrestore(&i8253_lock, flags);
 
        {
                unsigned long startlow, starthigh;

diff -Nru a/drivers/char/ftape/lowlevel/ftape-calibr.c b/drivers/char/ftape/lowlevel/ftape-calibr.c

drivers/char/ftape/lowlevel/ftape-calibr.c Sun May 26 14:18:40 2002 +44 -53

@@ -31,7 +31,10 @@
 #include <asm/io.h>
 #if defined(__alpha__)
 # include <asm/hwrpb.h>
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__x86_64__)
+# include <asm/msr.h>
+# include <asm/timex.h>
+#elif defined(__i386__)
 # include <linux/timex.h>
 #endif
 #include <linux/ftape.h>

@@ -45,10 +48,14 @@
 # error Ftape is not implemented for this architecture!
 #endif
 
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
 static unsigned long ps_per_cycle = 0;
 #endif
 
+#if defined(__i386__)
+extern spinlock_t i8253_lock;
+#endif
+
 /*
  * Note: On Intel PCs, the clock ticks at 100 Hz (HZ==100) which is
  * too slow for certain timeouts (and that clock doesn't even tick

@@ -67,48 +74,58 @@
 {
 #if defined(__alpha__)
        unsigned long r;
-
        asm volatile ("rpcc %0" : "=r" (r));
        return r;
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__x86_64__)
+       unsigned long r;
+       rdtscl(r);
+       return r;
+#elif defined(__i386__)
+
+/*
+ * Note that there is some time between counter underflowing and jiffies
+ * increasing, so the code below won't always give correct output.
+ * -Vojtech
+ */
+
        unsigned long flags;
        __u16 lo;
        __u16 hi;
 
-       save_flags(flags);
-       cli();
+       spin_lock_irqsave(&i8253_lock, flags);
        outb_p(0x00, 0x43);     /* latch the count ASAP */
        lo = inb_p(0x40);       /* read the latched count */
        lo |= inb(0x40) << 8;
        hi = jiffies;
-       restore_flags(flags);
+       spin_unlock_irqrestore(&i8253_lock, flags);
+
        return ((hi + 1) * (unsigned int) LATCH) - lo;  /* downcounter ! */
 #endif
 }
 
 static unsigned int short_ftape_timestamp(void)
 {
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
        return ftape_timestamp();
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__i386__)
        unsigned int count;
        unsigned long flags;
-
-       save_flags(flags);
-       cli();
+
+       spin_lock_irqsave(&i8253_lock, flags);
        outb_p(0x00, 0x43);     /* latch the count ASAP */
        count = inb_p(0x40);    /* read the latched count */
        count |= inb(0x40) << 8;
-       restore_flags(flags);
+       spin_unlock_irqrestore(&i8253_lock, flags);
+
        return (LATCH - count); /* normal: downcounter */
 #endif
 }
 
 static unsigned int diff(unsigned int t0, unsigned int t1)
 {
-#if defined(__alpha__)
-       return (t1 <= t0) ? t1 + (1UL << 32) - t0 : t1 - t0;
-#elif defined(__i386__) || defined(__x86_64__)
+#if defined(__alpha__) || defined(__x86_64__)
+       return (t1 - t0);
+#elif defined(__i386__)
        /*
         * This is tricky: to work for both short and full ftape_timestamps
         * we'll have to discriminate between these.

@@ -122,9 +139,9 @@
 
 static unsigned int usecs(unsigned int count)
 {
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
        return (ps_per_cycle * count) / 1000000UL;
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__i386__)
        return (10000 * count) / ((CLOCK_TICK_RATE + 50) / 100);
 #endif
 }

@@ -153,49 +170,24 @@
        save_flags(flags);
        cli();
        t0 = short_ftape_timestamp();
-       for (i = 0; i < 1000; ++i) {
+       for (i = 0; i < 1000; ++i)
                status = inb(fdc.msr);
-       }
        t1 = short_ftape_timestamp();
        restore_flags(flags);
+
        TRACE(ft_t_info, "inb() duration: %d nsec", ftape_timediff(t0, t1));
        TRACE_EXIT;
 }
 
 static void init_clock(void)
 {
-#if defined(__i386__) || defined(__x86_64__)
-       unsigned int t;
-       int i;
        TRACE_FUN(ft_t_any);
 
-       /*  Haven't studied on why, but there sometimes is a problem
-        *  with the tick timer readout. The two bytes get swapped.
-        *  This hack solves that problem by doing one extra input.
-        */
-       for (i = 0; i < 1000; ++i) {
-               t = short_ftape_timestamp();
-               if (t > LATCH) {
-                       inb_p(0x40);    /* get in sync again */
-                       TRACE(ft_t_warn, "clock counter fixed");
-                       break;
-               }
-       }
+#if defined(__x86_64__)
+       ps_per_cycle = 1000000000UL / cpu_khz;
 #elif defined(__alpha__)
-#if CONFIG_FT_ALPHA_CLOCK == 0
-#error You must define and set CONFIG_FT_ALPHA_CLOCK in 'make config' !
-#endif
        extern struct hwrpb_struct *hwrpb;
-       TRACE_FUN(ft_t_any);
-
-       if (hwrpb->cycle_freq != 0) {
-               ps_per_cycle = (1000*1000*1000*1000UL) / hwrpb->cycle_freq;
-       } else {
-               /*
-                * HELP:  Linux 2.0.x doesn't set cycle_freq on my noname !
-                */
-               ps_per_cycle = (1000*1000*1000*1000UL) / CONFIG_FT_ALPHA_CLOCK;
-       }
+       ps_per_cycle = (1000*1000*1000*1000UL) / hwrpb->cycle_freq;
 #endif
        TRACE_EXIT;
 }

@@ -214,7 +206,7 @@
        unsigned int tc = 0;
        unsigned int count;
        unsigned int time;
-#if defined(__i386__) || defined(__x86_64__)
+#if defined(__i386__)
        unsigned int old_tc = 0;
        unsigned int old_count = 1;
        unsigned int old_time = 1;

@@ -257,15 +249,14 @@
                tc = (1000 * time) / (count - 1);
                TRACE(ft_t_any, "once:%3d us,%6d times:%6d us, TC:%5d ns",
                        usecs(once), count - 1, usecs(multiple), tc);
-#if defined(__alpha__)
+#if defined(__alpha__) || defined(__x86_64__)
                /*
                 * Increase the calibration count exponentially until the
                 * calibration time exceeds 100 ms.
                 */
-               if (time >= 100*1000) {
+               if (time >= 100*1000)
                        break;
-               }
-#elif defined(__i386__) || defined(__x86_64__)
+#elif defined(__i386__)
                /*
                 * increase the count until the resulting time nears 2/HZ,
                 * then the tc will drop sharply because we lose LATCH counts.

diff -Nru a/drivers/char/vt.c b/drivers/char/vt.c

drivers/char/vt.c Sun May 26 14:18:40 2002 +3 -0

@@ -107,6 +107,7 @@
 _kd_mksound(unsigned int hz, unsigned int ticks)
 {
        static struct timer_list sound_timer = { function: kd_nosound };
+        extern spinlock_t i8253_lock;
        unsigned int count = 0;
        unsigned long flags;
 

@@ -120,10 +121,12 @@
                /* enable counter 2 */
                outb_p(inb_p(0x61)|3, 0x61);
                /* set command for counter 2, 2 byte write */
+               spin_lock(&i8253_lock);
                outb_p(0xB6, 0x43);
                /* select desired HZ */
                outb_p(count & 0xff, 0x42);
                outb((count >> 8) & 0xff, 0x42);
+               spin_unlock(&i8253_lock);
 
                if (ticks) {
                        sound_timer.expires = jiffies+ticks;

diff -Nru a/drivers/ide/hd.c b/drivers/ide/hd.c

drivers/ide/hd.c Sun May 26 14:18:40 2002 +6 -4

@@ -110,6 +110,8 @@
 
 static struct timer_list device_timer;
 
+#define TIMEOUT_VALUE  (6*HZ)
+
 #define SET_TIMER                                                      \
        do {                                                            \
                mod_timer(&device_timer, jiffies + TIMEOUT_VALUE);      \

@@ -131,16 +133,16 @@
 
 unsigned long read_timer(void)
 {
+        extern spinlock_t i8253_lock;
        unsigned long t, flags;
        int i;
 
-       save_flags(flags);
-       cli();
+       spin_lock_irqsave(&i8253_lock, flags);
        t = jiffies * 11932;
        outb_p(0, 0x43);
        i = inb_p(0x40);
        i |= inb(0x40) << 8;
-       restore_flags(flags);
+       spin_unlock_irqrestore(&i8253_lock, flags);
        return(t - i);
 }
 #endif

@@ -831,7 +833,7 @@
                printk("hd: unable to get major %d for hard disk\n",MAJOR_NR);
                return -1;
        }
-       blk_init_queue(BLK_DEFAULT_QUEUE(MAJOR_NR), DEVICE_REQUEST, &hd_lock);
+       blk_init_queue(BLK_DEFAULT_QUEUE(MAJOR_NR), do_hd_request, &hd_lock);
        blk_queue_max_sectors(BLK_DEFAULT_QUEUE(MAJOR_NR), 255);
        add_gendisk(&hd_gendisk);
        init_timer(&device_timer);

diff -Nru a/drivers/input/gameport/gameport.c b/drivers/input/gameport/gameport.c

drivers/input/gameport/gameport.c Sun May 26 14:18:40 2002 +24 -4

@@ -54,16 +54,36 @@
 static struct gameport *gameport_list;
 static struct gameport_dev *gameport_dev;
 
+
+#ifdef __i386__
+
+#define DELTA(x,y)      ((y)-(x)+((y)<(x)?1193180/HZ:0))
+#define GET_TIME(x)     do { x = get_time_pit(); } while (0)
+
+static unsigned int get_time_pit(void)
+{
+       extern spinlock_t i8253_lock;
+       unsigned long flags;
+       unsigned int count;
+
+       spin_lock_irqsave(&i8253_lock, flags);
+       outb_p(0x00, 0x43);
+       count = inb_p(0x40);
+       count |= inb_p(0x40) << 8;
+       spin_unlock_irqrestore(&i8253_lock, flags);
+
+       return count;
+}
+
+#endif
+
 /*
  * gameport_measure_speed() measures the gameport i/o speed.
  */
 
 static int gameport_measure_speed(struct gameport *gameport)
 {
-#if defined(__i386__) || defined(__x86_64__)
-
-#define GET_TIME(x)     do { outb(0, 0x43); x = inb(0x40); x |= inb(0x40) << 8; } while (0)
-#define DELTA(x,y)      ((y)-(x)+((y)<(x)?1193180L/HZ:0))
+#ifdef __i386__
 
        unsigned int i, t, t1, t2, t3, tx;
        unsigned long flags;

===================================================================

This BitKeeper patch contains the following changesets:
1.582..1.584
## Wrapped with gzip_uu ##

begin 664 bkpatch17031 M'XL(`*#2\#P``\U:ZW/;-A+_+/X5N,E,*R66!(!OYW%Q(K?QU6ESB=T/G<YH M(!(2&5.DRH=EM>K_?KL)<NR99MJW3G9)F426"QV][<O\ADY+V1^V+K,OI8R MB(QGY$-6E(>M<AXG\20J>U4P[P6_P_7/60;7^U$VE?UZ=']TT4_BM"JZO&<; M,.:3*(.(7,J\.&RQGKF^4BYF\K#U^?C[\].CSX;Q^C5Y'XET(K_(DKQ^;919 M?BF2L'@KRBC)TEZ9B[28RE+T@FRZ7]=<DHY_-C,-:GM+)E#+7<9L)`Q83$9 M4FYYCG5-#5F]GY;-;<JXR?VE:5F>:PP(Z]D>)Y3WJ=WG%F'>H64>VGZ7NH>4 MDGM(DQ>,=*GQCOR]FWEO!.1\%HI2D@N9IS)1THVSU/B!F);K<^/3M2R-;L./ M85!!C3</\/Q17,AQG,A-EGW36U+JN<XR=$3H!TQXH>W;PKY/1EN4E/0MTW*6 MW'9=;C36G&7:J#F4PL>[-&<=4NLQFJ-/H;E-O7PAEX"0'O.,@DS@+X&_1IME MU*:F[8+(X;.'FO^*:#4HS)5H'<+HH4D/+><%Y2C:VAF\W7(93PB(LR@N2%" M\QC'5Q)$&4]E48KIC$QD*G-1CY(G))Q*6:R&X@D'N6]X(#,HQB<T5P40&,:
M%T6<3H@@219<D'&6$Q$$4E\L(TEBC]NF(IWW](K3+*P22?(J36$0T"BSB82A M.9G'941BTW/Z&J1]G-8+2)!524AR651)B0PA7441KDWBH@0HYDD4XL6BS30 ME(IL7,Y%+HE(<?8LRTLU8"YF,QD2D&8E"V1*PAB]'9&2>99?B#RK8[NID26 M[Q#"J"IKOE()O@2^3V>)+&6R#(@3\7D+,]&B9SVR%%29=X"58JRCA)@)^I MB-,"!)>+0))VFLV1QE2FI0R!Q*TE.P=Z5U_C\3@&;>&FM@2\%H>6-%)1)IF M)1E)I>*02%'$R:)'3D51ZCW@[42J?]56@RR%[92%7AVUL[**@HY7'G.T+% M9:0J)#G[\KY>N<@2V#J0E('.W'Y+;"8S,4"V9B![F!CM0"0\S"31?IM22)Q M"1='AOB*O1&MZT'Z,#OCR"G6HX1BA^)C)#FE@/Y6[K4T2SJD0MDV(6I\I@ M(YF$/?)=G(/BW^I:.[SC\0#<(\QA#4#R*1]Y6L^TDV3\"6DOY-Q6\@VJ+4 M7E)F.G3)\N2PN:!,^+AV'5V^8_&"]G<H9Y)+0O"B<<MP]A%^):_T_.<I1:A M]G?6AK^S#IE[:-&'_)WSM/Y.342G%V4S.:Y[;7[6UDAH+(4*5@1:8L:&VCY MZ)?0=G+P?6C!^*6;9FDQDT$\C@-2"[H#_A'-LUP=:R,,HGA&%"BUB0(:NRL< M;1@D7(%,,@:5_*YG3\'IQ=W1HES9-OBLU68T!)!S0$55*')I"$R4*^^@37\- MK#*"+8%+!(1>D'TP1=3ST^!2R5Z$`0";;CTW\0"7$H^U&X9>_,7IH<+&L9 MFBSDWL@-'#EVY"A\T-YOD@/K9)RYU%HZ0-M[D"G4;G\S%*$2_2WF.%O:ONM[ MRW'@^Z[CP46(^6-F[63N?K+73,(2_&')W:(F9M/;+#K,\<!?V+YCFX('H<U- M$.'C6=P@>LT@A3S*;^;D+LMM7P;*,!D%(3J.8XX%LP4H7)@[?<,N>M=L6=SB M9G.YZ3SCEFYMZHJ@*-1Z'-K#+=<]_%RVR1ZS:');/]A\[NQT:_9G*&X*(_ M$5.5Q=R."8R"]2UMX4CP$4P$KLD\_V&(I+.J7%.]17[#'!V(.:K@7)4>#]>; MC<H=XT+$;\L9Z^55E'>K%!Q>%D35M!?*6]6.Q3GW.575#E-A!BYLUYJ6\\A:
MD_VCM::NT'XBW7RN?L%'?EK+=__.C),T[P\.7\W>GQS\>GY#5(0*GJ;B_ MK+B_XE7NT^/]CL4$:3K@[1UNLUJK]G;N0*U'Y[^DRCT*,1$7X80M3%,JY)' M%PFYC-.X5'[9M:)1\C]6<YS2#)K4*9B$6;T0Y4&<F.E#TH$Q.2M6V2Q0 MPT04,UI(S&%RO6:78"FBJR$,U1EF%YGKS)@+%J(C/&J#MW^MI'=+?T]3.Z$ M,9>#P;7D%20*Z3IS&99ZWT/\YZ4Q@&$6X6JXC<-QG+HWC//?"DC[V]]<CS\@ MXT1,BHZ>YQ(7YT&Q;!DM2-E'PUF;7IG6:%7EMEYV6KUGY,1Y.PY5!Q3S/KY M3G]\J[_\<N[Y`[H>1Y?SWS].CL_0?R#<P=CQ4%"A2``$RX/>S-&^)M#OI8 M#U+<5^F*?ZAGP,9V;.%.\-6!?@_X-<H\&@%P*_DP.3--;BZI1QU?09#QYA"T MGBI]1PAN80^1=QMX.N>-H9#.\VI6KK/EJ5B0NAH6(RA8*ZS6:X0=:"1BRH[) MLAC!G1@K5-446&2I)#(II$Z@==VOL_&-=84J<$D!853Q<!S59MJ$JW@LB* M(4"E=@^ALU;-/O@T'0ZX>P">K2HMXDFJW!D(3YGN2^/$8FX#K)Y8G*Z'_Q5P MU)ZM.38:)&Z-D'$S=UL!@W-F6QH8]/\;&*M*$2M";9QQ2G2=#Z%G6!9!NX.X M.)J(.*T[/QOP*,H\#LH-<*9JY:.+@]5TP[#V(K8VWT8SAS06A:BXU+9B+ MPP)=6,,*XRH-L*D(T^4EU)HF+Q*4=:K\=3U]TTF1:80V$33A7MN+WWG\YA M,I3<,E1Q3B73#V%)*W,?*-FN@U"Z$RN_PFW/:8(6&U+R/=#R^;.PP#ZN[M. M1EX5Y>(M'H,LG^$:/5'MT7/B#(!*G:7E4:X?(%FW.N4/(PP@9IM/@K'OL*.[ M[H\#@3!!>T3CK%NBF):AU2D?KSIG6W;Y>*'L50A8JA*P(&5Z)I-X3$*H*<!F MV\.A9GX[!C/PT$"62BY)4HIOUID?>B-]M7<9M7ZOH6'=P=4"'&P/)P,75\
M=F.(2&:1P#'+Y9T,G-@,&;R+K'%_P'HFTS>^H&+L:<@:LJ'SSR>S:\!=T< ME\>@O=+VGD'O\NR@B7SE[OV"NOUGQOD.?DQ*^O@KY,$;+YE=9Q'ISB7X,\@ M+\"D0_?@QJ!<Y9'!+NJ>/1("4>?8A4\G!T!!^4[5MQY)[.'-,TP5=-^<3,!> MX&:>RZDD*5"L=Y#$MV?ZZ?,F)FB0#S,LUW_\;X(5.@I>_$P16_BD0@NZ#.< MJX[-U#_PE=;4<9?$![Y#3!B#FVHUV!"C6)6<X&F/+3'JXZJ,,3@UV]3*BMHE MPXXN%Z[]\:XK9A4IX:R8]S5<]W[I,=L1XU2IQ;ZIW9,7A/ZDL3D%7A2"M]> MO(C52(5B<'IP0A$X+NQ]H*H[K-N@_.);/&YL>E8,9S(?!HLFT&*L/Z<GY(^ M"6;5\"+Z'<LYSU94,?91H.KCHMNSVSC]^<W#^6D'"$7S?#;JOE$#A^-<_@8D MN4I23_3I;E\RX&I7)_K44-#<L?5<5;>V8');P?R-VJ9B3HUR$7;<\>Y1QV;\
MWMEM:QBV]VT'KA_D%%4A&[0!.7<XT&R);6H9:O@[#9/?SD$9^O)\]\Y/L/8 M_00:8[/J9>Z(S3M%L5?V",FC"<@"\P/+("NKP"O:4LC@^/3LJ'UUL.@0]6FW M%YUN^ZKSK^\@B__9LP'W='^AU\.::>SGOC]\=GP[.3C,0Q1$\.,_$&N$D3 M60YQJ\-97+;!M_V)3^L!8VV*L0P;3W%UD$1:N2;,RZS..P8?^Q7/;9NT%7! M$-/DQ^;'UYT>2E>='J.ER,#&XE3?P\;,ZNKRQF7RZA7Q7C;+KW]=>^^:W3]1 M.^M4P\$.U(GC:!]R0XFW,O/+YDA^_!,1(\JB[.T(=BN*J)<"/'8]"J$N(;; M%-\Z,?5;)Q#3&J/5?,IB]>Y&ZJV:M4>.L4K$I_83]3HOM"`N5(8%V*4R.*Z MQP/)4I*-L'5Z4+]?L;ZEIP0"'VUBX7DE@TJ]!P*5JVJC8E&Y>IVB*C:>FFZ1 M@)H6&SJ;KQCLZ/O4KZJH$>"-=.%P@=W@'XA^0G5?:7"YI[O!%(89I/[<#V#( M(OC&X&N(;L*DH\8YM\9I;&V-O!'IZD>OCX=#LV>_1B@2";;S5E[&^&"E"V8" M();%/32QQ/0XAQ+3-BVO;F_N$<:<IXIB6&$JH6'+))O.T&L+;,_HMPWNC7'X MOM#-**>?<N^*<K5D]NO^8WZ]BD,8@WXZ/QO^?'1Z?MQJ.\\__()QYH29YJ-M M<<!,E4DQTVOTG,!2M80^-7+ZT\7R_K4&B4P)8W+X6^5K&3[W>D/P\'Q=T?G MIV?#_YX?GQ^W/Q[]YZ?/PQ\_=PX@S@ZC<AI:"7QC:EOX)^5_:]>T@TB,,VB 3FK[V>!@X^#KL_P!<D_!-'2P`````
`
end

Quoted 8 lines

Quoted 8 lines

--
Vojtech Pavlik
SuSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

--
Vojtech Pavlik
SuSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Well, probably yes, but still hd.c is a glacial relict, a driver nobody (almost - it's for non-IDE "two ribbon" AT harddrives) uses. So this driver is probably not enough justification for a global (as in all archs) spinlock being added.

Quoted 4 lines · Vojtech Pavlik

Well, probably yes, but still hd.c is a glacial relict, a driver nobody (almost - it's for non-IDE "two ribbon" AT harddrives) uses. So this driver is probably not enough justification for a global (as in all archs) spinlock being added.

It only uses the timer in the case that HD_DELAY > 0. This code is ultimately used for timing functions. A better approach would be to remove the use of the timer chip from the file entirely and use the perfectly adequate udelay() function instead.

That would also conveniently make it do cpu_relax properly improving the performance of your ancient IDE controller when plugged into hyperthreading pentium IV 8)