> On Thu, Nov 04, 2010 at 09:31:30PM -0400, Rick Macklem wrote:
quoted text
> > >
> > > If the counter was not wrapped, it seem you lost more than 10% out
> > > of
> > > total RX frames. This is a lot loss and there should be a way to
> > > mitigate it.
> > >
> > I've attached a patch (to the if_re.c in head, not your patched
> > variant)
> > that works a lot better (about 5Mbytes/sec read rate). To get that,
> > I
> > had to disable msi and not clear the RL_IMR register in re_intr(). I
> > suspect that a packet would be received between when the bits in
> > RL_IMR
> > were cleared and when they were set at the end of re_int_task() and
> > those
> > were getting lost.
> >
> > This patch doesn't completely fix the problem. (I added your stats
> > collecting
> > stuff to the if_re.c in head and attached the result, which still
> > shows some lost packets. One
> > thought is clearing the bits in RL_ISR in re_intr() instead of
> > re_int_task(),
> > but then I can't see a good way to pass the old value of the status
> > reg.
> > through to re_int_task()?
> >
> 
> Hmm, I still don't understand how the patch mitigates the issue. :-(
> The patch does not disable interrupts in interrupt handler so
> taskqueue runs with interrupt enabled. This may ensure not loosing
> interrupts but it may also generate many interrupts. By chance, did
> you check number of interrupts generated with/without your patch?
> 
I agree. I think all it did was create more interrupts so that re_rxeof()
got called more frequently. (see below)

quoted text
> The only guess I have at the moment is the writing RL_IMR in
> interrupt handler at the end of taskqueue might be not immediately
> reflected so controller can loose interrupts for the time window.
> Would you try attach patch and let me know it makes any difference?
> 
Nope, it didn't make any difference.

I've added a counter of how many times re_rxeof() gets called, but then
returns without handling any received packets (I think because
RL_RDESC_STAT_OWN is set on the first entry it looks at in the rcv. ring.)

This count comes out as almost the same as the # of missed frames (see
"rxe did 0:" in the attached stats).

So, I think what is happenning about 10% of the time is that re_rxeof()
is looking at the ring descriptor before it has been "updated" and then
returns without handling the packet and then it doesn't get called again
because the RL_ISR bit has been cleared.

When "options DEVICE_POLLING" is specified, it works ok, because it calls
re_rxeof() fairly frequently and it doesn't pay any attention to the RL_ISR
bits.

Now, I don't know if this is a hardware flaw on this machine or something
that can be fixed in software? I know diddly about the current driver
setup, but I assume something like this has to happen?
- chip (or PCIe handler) forces the DMA of the descriptor change to RAM
- then posts the interrupt
- and the driver must read the up to date descriptor from RAM
--> I notice that "volatile" isn't used anywhere in the driver. Wouldn't
    the descriptor ring memory have to be defined "volatile" somehow?
    (I don't know how to do this correctly?)

So, if you can think of anything that will ensure that re_rxeof() reads
"up to date" descriptor ring entries, please let me know.

Otherwise, I can live with "options DEVICE_POLLING".

Thanks for your help, rick