Re: RE: [PATCH] qe_ic: Do a sync when masking interrupts.

Re: RE: [PATCH] qe_ic: Do a sync when masking interrupts.

[Michael R. Zucca]

>From: Li Yang-r58472 <LeoLi@freescale.com>
>
>But an i/o read will be considerably slower than a sync, and it is in
>the critical path of interrupt.  I have tested the patch under
>relatively heavy Ethernet load, and there is no spurious interrupt.
>Maybe it is because the device is an SOC device and MMIO store completes
>faster.  I'm wondering if there is a standard test method to show if the
>faster approach is sufficient or not.

All a sync tells you is that an I/O made it out of the CPU. The problem is, there may be other places a write could get hung up. For instance, sometimes devices sit behind a bridge with a write FIFO. In such a scenario, you can't be sure a write has made it to the device until you do a read to flush the FIFO.

If you're trying to figure out the minimum thing to do (eieio, sync, read-back, etc.) you have to understand what your system is doing between the store and the bits going into the register.

It may be that a sync is enough, but you won't know until you fully understand the system's bus/bridge topolgy between the CPU and the device.

Re: [PATCH] qe_ic: Do a sync when masking interrupts.

[Segher Boessenkool]

> ll a sync tells you is that an I/O made it out of the CPU. The  
> problem is, there may be other places a write could get hung up.  
> For instance, sometimes devices sit behind a bridge with a write  
> FIFO. In such a scenario, you can't be sure a write has made it to  
> the device until you do a read to flush the FIFO.

It's not enough that a write made it to the device even -- you
have to make sure the device has acted on it.

> If you're trying to figure out the minimum thing to do (eieio,  
> sync, read-back, etc.) you have to understand what your system is  
> doing between the store and the bits going into the register.

What the system is doing, and also what exactly you want to
accomplish (what ordering and what completion you depend on).

> It may be that a sync is enough, but you won't know until you fully  
> understand the system's bus/bridge topolgy between the CPU and the  
> device.

If a sync after an MMIO write is enough, then (in almost all
cases) so is an eieio.


Segher

Re: [PATCH] qe_ic: Do a sync when masking interrupts.

[Scott Wood]

Segher Boessenkool wrote:
> If a sync after an MMIO write is enough, then (in almost all
> cases) so is an eieio.

In this case, the spurious interrupts still happen with eieio, but not 
with sync.  It's probably synchronizing the MMIO write with some 
unrelated load from memory (such as reading the stack frame to return 
from the mask function, or reading action->flags to determine whether to 
check for IRQ_DISABLED).

-Scott

Re: [PATCH] qe_ic: Do a sync when masking interrupts.

[Segher Boessenkool]

>> If a sync after an MMIO write is enough, then (in almost all
>> cases) so is an eieio.
>
> In this case, the spurious interrupts still happen with eieio, but  
> not with sync.  It's probably synchronizing the MMIO write with  
> some unrelated load from memory (such as reading the stack frame to  
> return from the mask function, or reading action->flags to  
> determine whether to check for IRQ_DISABLED).

Yes, it sure sounds like the only reason the sync insn helps
is that it causes a (small) delay.  If you document this (and
also that it isn't required for correctness) in the code, I
don't think anyone will complain / be curious about it anymore.


Segher

RE: [PATCH] qe_ic: Do a sync when masking interrupts.

[Li Yang-r58472]

> -----Original Message-----
> From: Scott Wood [mailto:scottwood@freescale.com]
> Sent: Tuesday, October 24, 2006 2:27 AM
> To: Segher Boessenkool
> Cc: mrz5149@acm.org; Li Yang-r58472; Paul Mackerras;
linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] qe_ic: Do a sync when masking interrupts.
>=20
> Segher Boessenkool wrote:
> > If a sync after an MMIO write is enough, then (in almost all
> > cases) so is an eieio.
>=20
> In this case, the spurious interrupts still happen with eieio, but not
> with sync. =20

eieio on e300 core is just a no-op.

- Leo

RE: [PATCH] qe_ic: Do a sync when masking interrupts.

[Benjamin Herrenschmidt]

On Tue, 2006-10-24 at 15:17 +0800, Li Yang-r58472 wrote:
> > -----Original Message-----
> > From: Scott Wood [mailto:scottwood@freescale.com]
> > Sent: Tuesday, October 24, 2006 2:27 AM
> > To: Segher Boessenkool
> > Cc: mrz5149@acm.org; Li Yang-r58472; Paul Mackerras;
> linuxppc-dev@ozlabs.org
> > Subject: Re: [PATCH] qe_ic: Do a sync when masking interrupts.
> > 
> > Segher Boessenkool wrote:
> > > If a sync after an MMIO write is enough, then (in almost all
> > > cases) so is an eieio.
> > 
> > In this case, the spurious interrupts still happen with eieio, but not
> > with sync.  
> 
> eieio on e300 core is just a no-op.

It's not sent to the bus on non-cahed storage (like the PCI bridge ?)
That's pretty bad ... there is quite a bit of code that assumes that
eieio's will prevent write combining...

Ben.

RE: [PATCH] qe_ic: Do a sync when masking interrupts.

[Liu Dave-r63238]

> > > Segher Boessenkool wrote:
> > > > If a sync after an MMIO write is enough, then (in almost all
> > > > cases) so is an eieio.
> > >=20
> > > In this case, the spurious interrupts still happen with=20
> eieio, but=20
> > > not with sync.
> >=20
> > eieio on e300 core is just a no-op.
>=20
> It's not sent to the bus on non-cahed storage (like the PCI=20
> bridge ?) That's pretty bad ... there is quite a bit of code=20
> that assumes that eieio's will prevent write combining...

Due to the LSU, the e300 cored doesn't reorder non-cached
stroage, and doesn't implementation of write commbining.
So, actually its behavior likes eieio.

-Dave

[PATCH] qe_ic: Do a sync when masking interrupts.

[Scott Wood]

This patch causes a sync do be done after masking a QE interrupt, to
ensure that the masking has completed before interrupts are enabled.
This allows the masking of the cascade IRQ to be removed without causing
spurious interrupts.

The mask_and_ack function is also removed and set to the mask function,
as the two are identical.

This is the second version of this patch; this version clarifies why a
sync was chosen rather than a read-back, and it fixes unused variable
warnings where the cascade mask was removed.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/sysdev/qe_lib/qe_ic.c |   40 +++++++++---------------------------
 1 files changed, 10 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c b/arch/powerpc/sysdev/qe_lib/qe_ic.c
index 6995f51..74e48d9 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
+++ b/arch/powerpc/sysdev/qe_lib/qe_ic.c
@@ -223,23 +223,15 @@ static void qe_ic_mask_irq(unsigned int 
 	qe_ic_write(qe_ic->regs, qe_ic_info[src].mask_reg,
 		    temp & ~qe_ic_info[src].mask);
 
-	spin_unlock_irqrestore(&qe_ic_lock, flags);
-}
-
-static void qe_ic_mask_irq_and_ack(unsigned int virq)
-{
-	struct qe_ic *qe_ic = qe_ic_from_irq(virq);
-	unsigned int src = virq_to_hw(virq);
-	unsigned long flags;
-	u32 temp;
-
-	spin_lock_irqsave(&qe_ic_lock, flags);
-
-	temp = qe_ic_read(qe_ic->regs, qe_ic_info[src].mask_reg);
-	qe_ic_write(qe_ic->regs, qe_ic_info[src].mask_reg,
-		    temp & ~qe_ic_info[src].mask);
-
-	/* There is nothing to do for ack here, ack is handled in ISR */
+	/* Flush the above write before enabling interrupts; otherwise,
+	 * spurious interrupts will sometimes happen.  To be 100% sure
+	 * that the write has reached the device before interrupts are
+	 * enabled, the mask register would have to be read back; however,
+	 * this is not required for correctness, only to avoid wasting
+	 * time on a large number of spurious interrupts.  In testing,
+	 * a sync reduced the observed spurious interrupts to zero.
+	 */
+	mb();
 
 	spin_unlock_irqrestore(&qe_ic_lock, flags);
 }
@@ -248,7 +240,7 @@ static struct irq_chip qe_ic_irq_chip = 
 	.typename = " QEIC  ",
 	.unmask = qe_ic_unmask_irq,
 	.mask = qe_ic_mask_irq,
-	.mask_ack = qe_ic_mask_irq_and_ack,
+	.mask_ack = qe_ic_mask_irq,
 };
 
 static int qe_ic_host_match(struct irq_host *h, struct device_node *node)
@@ -331,34 +323,22 @@ unsigned int qe_ic_get_high_irq(struct q
 	return irq_linear_revmap(qe_ic->irqhost, irq);
 }
 
-/* FIXME: We mask all the QE Low interrupts while handling.  We should
- * let other interrupt come in, but BAD interrupts are generated */
 void fastcall qe_ic_cascade_low(unsigned int irq, struct irq_desc *desc)
 {
 	struct qe_ic *qe_ic = desc->handler_data;
-	struct irq_chip *chip = irq_desc[irq].chip;
-
 	unsigned int cascade_irq = qe_ic_get_low_irq(qe_ic);
 
-	chip->mask_ack(irq);
 	if (cascade_irq != NO_IRQ)
 		generic_handle_irq(cascade_irq);
-	chip->unmask(irq);
 }
 
-/* FIXME: We mask all the QE High interrupts while handling.  We should
- * let other interrupt come in, but BAD interrupts are generated */
 void fastcall qe_ic_cascade_high(unsigned int irq, struct irq_desc *desc)
 {
 	struct qe_ic *qe_ic = desc->handler_data;
-	struct irq_chip *chip = irq_desc[irq].chip;
-
 	unsigned int cascade_irq = qe_ic_get_high_irq(qe_ic);
 
-	chip->mask_ack(irq);
 	if (cascade_irq != NO_IRQ)
 		generic_handle_irq(cascade_irq);
-	chip->unmask(irq);
 }
 
 void __init qe_ic_init(struct device_node *node, unsigned int flags)
-- 
1.4.2.3

[PATCH] qe_ic: Do a sync when masking interrupts.

[Scott Wood]

This patch causes a sync do be done after masking a QE interrupt, to
ensure that the masking has completed before interrupts are enabled.
This allows the masking of the cascade IRQ to be removed without causing
spurious interrupts.

The mask_and_ack function is also removed and set to the mask function,
as the two are identical.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/sysdev/qe_lib/qe_ic.c |   34 +++++++---------------------------
 1 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c b/arch/powerpc/sysdev/qe_lib/qe_ic.c
index 6995f51..d96e48d 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
+++ b/arch/powerpc/sysdev/qe_lib/qe_ic.c
@@ -222,24 +222,12 @@ static void qe_ic_mask_irq(unsigned int 
 	temp = qe_ic_read(qe_ic->regs, qe_ic_info[src].mask_reg);
 	qe_ic_write(qe_ic->regs, qe_ic_info[src].mask_reg,
 		    temp & ~qe_ic_info[src].mask);
-
-	spin_unlock_irqrestore(&qe_ic_lock, flags);
-}
-
-static void qe_ic_mask_irq_and_ack(unsigned int virq)
-{
-	struct qe_ic *qe_ic = qe_ic_from_irq(virq);
-	unsigned int src = virq_to_hw(virq);
-	unsigned long flags;
-	u32 temp;
-
-	spin_lock_irqsave(&qe_ic_lock, flags);
-
-	temp = qe_ic_read(qe_ic->regs, qe_ic_info[src].mask_reg);
-	qe_ic_write(qe_ic->regs, qe_ic_info[src].mask_reg,
-		    temp & ~qe_ic_info[src].mask);
-
-	/* There is nothing to do for ack here, ack is handled in ISR */
+	
+	/* Flush the above write before enabling interrupts;
+	 * otherwise, spurious interrupts will sometimes
+	 * happen
+	 */
+	mb();
 
 	spin_unlock_irqrestore(&qe_ic_lock, flags);
 }
@@ -248,7 +236,7 @@ static struct irq_chip qe_ic_irq_chip = 
 	.typename = " QEIC  ",
 	.unmask = qe_ic_unmask_irq,
 	.mask = qe_ic_mask_irq,
-	.mask_ack = qe_ic_mask_irq_and_ack,
+	.mask_ack = qe_ic_mask_irq,
 };
 
 static int qe_ic_host_match(struct irq_host *h, struct device_node *node)
@@ -331,8 +319,6 @@ unsigned int qe_ic_get_high_irq(struct q
 	return irq_linear_revmap(qe_ic->irqhost, irq);
 }
 
-/* FIXME: We mask all the QE Low interrupts while handling.  We should
- * let other interrupt come in, but BAD interrupts are generated */
 void fastcall qe_ic_cascade_low(unsigned int irq, struct irq_desc *desc)
 {
 	struct qe_ic *qe_ic = desc->handler_data;
@@ -340,14 +326,10 @@ void fastcall qe_ic_cascade_low(unsigned
 
 	unsigned int cascade_irq = qe_ic_get_low_irq(qe_ic);
 
-	chip->mask_ack(irq);
 	if (cascade_irq != NO_IRQ)
 		generic_handle_irq(cascade_irq);
-	chip->unmask(irq);
 }
 
-/* FIXME: We mask all the QE High interrupts while handling.  We should
- * let other interrupt come in, but BAD interrupts are generated */
 void fastcall qe_ic_cascade_high(unsigned int irq, struct irq_desc *desc)
 {
 	struct qe_ic *qe_ic = desc->handler_data;
@@ -355,10 +337,8 @@ void fastcall qe_ic_cascade_high(unsigne
 
 	unsigned int cascade_irq = qe_ic_get_high_irq(qe_ic);
 
-	chip->mask_ack(irq);
 	if (cascade_irq != NO_IRQ)
 		generic_handle_irq(cascade_irq);
-	chip->unmask(irq);
 }
 
 void __init qe_ic_init(struct device_node *node, unsigned int flags)
-- 
1.4.2.3

RE: [PATCH] qe_ic: Do a sync when masking interrupts.

[Li Yang-r58472]

> -----Original Message-----
> From: linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org
> [mailto:linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org] On =
Behalf
Of Scott
> Wood
> Sent: Friday, October 20, 2006 2:03 AM
> To: linuxppc-dev@ozlabs.org
> Subject: [PATCH] qe_ic: Do a sync when masking interrupts.
>=20
> This patch causes a sync do be done after masking a QE interrupt, to
> ensure that the masking has completed before interrupts are enabled.
> This allows the masking of the cascade IRQ to be removed without
causing
> spurious interrupts.
>=20
> The mask_and_ack function is also removed and set to the mask
function,
> as the two are identical.
>=20
> Signed-off-by: Scott Wood <scottwood@freescale.com>

Acked-by: Li Yang <leoli@freescale.com>
> ---
>  arch/powerpc/sysdev/qe_lib/qe_ic.c |   34
+++++++---------------------------
>  1 files changed, 7 insertions(+), 27 deletions(-)
>=20
> diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c
> b/arch/powerpc/sysdev/qe_lib/qe_ic.c
> index 6995f51..d96e48d 100644
> --- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
> +++ b/arch/powerpc/sysdev/qe_lib/qe_ic.c
> @@ -222,24 +222,12 @@ static void qe_ic_mask_irq(unsigned int
>  	temp =3D qe_ic_read(qe_ic->regs, qe_ic_info[src].mask_reg);
>  	qe_ic_write(qe_ic->regs, qe_ic_info[src].mask_reg,
>  		    temp & ~qe_ic_info[src].mask);
> -
> -	spin_unlock_irqrestore(&qe_ic_lock, flags);
> -}
> -
> -static void qe_ic_mask_irq_and_ack(unsigned int virq)
> -{
> -	struct qe_ic *qe_ic =3D qe_ic_from_irq(virq);
> -	unsigned int src =3D virq_to_hw(virq);
> -	unsigned long flags;
> -	u32 temp;
> -
> -	spin_lock_irqsave(&qe_ic_lock, flags);
> -
> -	temp =3D qe_ic_read(qe_ic->regs, qe_ic_info[src].mask_reg);
> -	qe_ic_write(qe_ic->regs, qe_ic_info[src].mask_reg,
> -		    temp & ~qe_ic_info[src].mask);
> -
> -	/* There is nothing to do for ack here, ack is handled in ISR */
> +
> +	/* Flush the above write before enabling interrupts;
> +	 * otherwise, spurious interrupts will sometimes
> +	 * happen
> +	 */
> +	mb();
>=20
>  	spin_unlock_irqrestore(&qe_ic_lock, flags);
>  }
> @@ -248,7 +236,7 @@ static struct irq_chip qe_ic_irq_chip =3D
>  	.typename =3D " QEIC  ",
>  	.unmask =3D qe_ic_unmask_irq,
>  	.mask =3D qe_ic_mask_irq,
> -	.mask_ack =3D qe_ic_mask_irq_and_ack,
> +	.mask_ack =3D qe_ic_mask_irq,
>  };
>=20
>  static int qe_ic_host_match(struct irq_host *h, struct device_node
*node)
> @@ -331,8 +319,6 @@ unsigned int qe_ic_get_high_irq(struct q
>  	return irq_linear_revmap(qe_ic->irqhost, irq);
>  }
>=20
> -/* FIXME: We mask all the QE Low interrupts while handling.  We
should
> - * let other interrupt come in, but BAD interrupts are generated */
>  void fastcall qe_ic_cascade_low(unsigned int irq, struct irq_desc
*desc)
>  {
>  	struct qe_ic *qe_ic =3D desc->handler_data;
> @@ -340,14 +326,10 @@ void fastcall qe_ic_cascade_low(unsigned
>=20
>  	unsigned int cascade_irq =3D qe_ic_get_low_irq(qe_ic);
>=20
> -	chip->mask_ack(irq);
>  	if (cascade_irq !=3D NO_IRQ)
>  		generic_handle_irq(cascade_irq);
> -	chip->unmask(irq);
>  }
>=20
> -/* FIXME: We mask all the QE High interrupts while handling.  We
should
> - * let other interrupt come in, but BAD interrupts are generated */
>  void fastcall qe_ic_cascade_high(unsigned int irq, struct irq_desc
*desc)
>  {
>  	struct qe_ic *qe_ic =3D desc->handler_data;
> @@ -355,10 +337,8 @@ void fastcall qe_ic_cascade_high(unsigne
>=20
>  	unsigned int cascade_irq =3D qe_ic_get_high_irq(qe_ic);
>=20
> -	chip->mask_ack(irq);
>  	if (cascade_irq !=3D NO_IRQ)
>  		generic_handle_irq(cascade_irq);
> -	chip->unmask(irq);
>  }
>=20
>  void __init qe_ic_init(struct device_node *node, unsigned int flags)
> --
> 1.4.2.3
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] qe_ic: Do a sync when masking interrupts.

[Paul Mackerras]

Scott Wood writes:

> This patch causes a sync do be done after masking a QE interrupt, to
> ensure that the masking has completed before interrupts are enabled.
> This allows the masking of the cascade IRQ to be removed without causing
> spurious interrupts.

Hmmm.  In general a sync having completed doesn't mean that previous
MMIO stores have actually got to the device.  Reading from a device
register generally does ensure that previous writes have actually got
to the device though - could you do that instead?  I'm concerned that
adding the sync is not a robust fix and is possibly only working due
to fortuitous timing.

Paul.

RE: [PATCH] qe_ic: Do a sync when masking interrupts.

[Li Yang-r58472]

> -----Original Message-----
> From: linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org
> [mailto:linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org] On =
Behalf
Of Paul
> Mackerras
> Sent: Monday, October 23, 2006 12:00 PM
> To: Wood Scott-B07421
> Cc: linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] qe_ic: Do a sync when masking interrupts.
>=20
> Scott Wood writes:
>=20
> > This patch causes a sync do be done after masking a QE interrupt, to
> > ensure that the masking has completed before interrupts are enabled.
> > This allows the masking of the cascade IRQ to be removed without
causing
> > spurious interrupts.
>=20
> Hmmm.  In general a sync having completed doesn't mean that previous
> MMIO stores have actually got to the device.  Reading from a device
> register generally does ensure that previous writes have actually got
> to the device though - could you do that instead?  I'm concerned that
> adding the sync is not a robust fix and is possibly only working due
> to fortuitous timing.

But an i/o read will be considerably slower than a sync, and it is in
the critical path of interrupt.  I have tested the patch under
relatively heavy Ethernet load, and there is no spurious interrupt.
Maybe it is because the device is an SOC device and MMIO store completes
faster.  I'm wondering if there is a standard test method to show if the
faster approach is sufficient or not.

- Leo=20

Re: [PATCH] qe_ic: Do a sync when masking interrupts.

[Scott Wood]

Paul Mackerras wrote:
> Hmmm.  In general a sync having completed doesn't mean that previous
> MMIO stores have actually got to the device.  Reading from a device
> register generally does ensure that previous writes have actually got
> to the device though - could you do that instead?  I'm concerned that
> adding the sync is not a robust fix and is possibly only working due
> to fortuitous timing.

Possibly -- but the only problem on the rare occasions where the timing 
is not fortuitous is a spurious interrupt; the only reason the sync 
needs to be there at all is to avoid the overhead of the extra interrupt 
(and to avoid user complaints that they're getting "BAD" interrupts).

Note that many/most interrupt controller drivers don't even do that 
much; they just have more fortuitous timing than the QE (and/or the 
spurious interrupt is non-cascaded, and thus gets caught in 
handle_level_irq and doesn't increment the spurious interrupt count).

-Scott

Re: [PATCH] qe_ic: Do a sync when masking interrupts.

[Scott Wood]

Scott Wood wrote:
> Note that many/most interrupt controller drivers don't even do that 
> much; they just have more fortuitous timing than the QE (and/or the 
> spurious interrupt is non-cascaded, and thus gets caught in 
> handle_level_irq and doesn't increment the spurious interrupt count).

Ignore that last bit about non-cascaded interrupts; if it makes a 
difference at all, it's because of added delays (getting back to the 
fortuitous timing thing).

-Scott

Re: [PATCH] qe_ic: Do a sync when masking interrupts.

[Kumar Gala]

On Oct 23, 2006, at 10:19 AM, Scott Wood wrote:

> Paul Mackerras wrote:
>> Hmmm.  In general a sync having completed doesn't mean that previous
>> MMIO stores have actually got to the device.  Reading from a device
>> register generally does ensure that previous writes have actually got
>> to the device though - could you do that instead?  I'm concerned that
>> adding the sync is not a robust fix and is possibly only working due
>> to fortuitous timing.
>
> Possibly -- but the only problem on the rare occasions where the  
> timing
> is not fortuitous is a spurious interrupt; the only reason the sync
> needs to be there at all is to avoid the overhead of the extra  
> interrupt
> (and to avoid user complaints that they're getting "BAD" interrupts).

Why wouldn't the read accomplish the same thing in a more robust way  
than the sync?

- k

Re: [PATCH] qe_ic: Do a sync when masking interrupts.

[Scott Wood]

Kumar Gala wrote:
> 
> On Oct 23, 2006, at 10:19 AM, Scott Wood wrote:
>> Possibly -- but the only problem on the rare occasions where the  timing
>> is not fortuitous is a spurious interrupt; the only reason the sync
>> needs to be there at all is to avoid the overhead of the extra  interrupt
>> (and to avoid user complaints that they're getting "BAD" interrupts).
> 
> 
> Why wouldn't the read accomplish the same thing in a more robust way  
> than the sync?

It would.  However, it also adds a small amount of overhead to every QE 
interrupt, and the only thing that that overhead buys is avoiding 
possible but empirically very rare spurious interrupts; it'd cost more 
than simply accepting that a spurious interrupt might happen once in a 
great while.

Without any type of sync, spurious interrupts happen fairly regularly 
(about 5-10% of legitimate interrupts), so adding the sync should be a 
net gain over doing nothing.

If the consensus is that a read should be done anyway, I can resumbit 
the patch that way; I just think it's overkill given that a 100% 
guarantee isn't required for correctness.

-Scott