Jan-Bernd,

On Thu, 20 May 2010, Jan-Bernd Themann wrote:

Hi Thomas

quoted

Re: [PATCH RT] ehea: make receive irq handler non-threaded (IRQF_NODELAY)

On Thu, 20 May 2010, Jan-Bernd Themann wrote:

quoted

quoted

quoted

Thought more about that. The case at hand (ehea) is nasty:

The driver does _NOT_ disable the rx interrupt in the card in the

rx

quoted

quoted

quoted

quoted

interrupt handler - for whatever reason.

Yeah I saw that, but I don't know why it's written that way. Perhaps
Jan-Bernd or Doug will chime in and enlighten us? :)

From our perspective there is no need to disable interrupts for the
RX side as the chip does not fire further interrupts until we tell
the chip to do so for a particular queue. We have multiple receive

The traces tell a different story though:

    ehea_recv_irq_handler()
      napi_reschedule()
    eoi()
    ehea_poll()
      ...
      ehea_recv_irq_handler()    <---------------- ???
        napi_reschedule()
      ...
      napi_complete()

Can't tell whether you can see the same behaviour in mainline, but I
don't see a reason why not.

Is this the same interrupt we are seeing here, or do we see a second other
interrupt popping up on the same CPU? As I said, with multiple receive
queues
(if enabled) you can have multiple interrupts in parallel.

According to the traces it's the very same interrupt number.

Pleaes check if multiple queues are enabled. The following module parameter
is used for that:

MODULE_PARM_DESC(use_mcs, " 0:NAPI, 1:Multiple receive queues, Default = 0
");

you should also see the number of used HEA interrupts in /proc/interrupts

I leave that for Will and Darren, they have the hardware :)

quoted

quoted

queues with an own interrupt each so that the interrupts can arrive
on multiple CPUs in parallel.  Interrupts are enabled again when we
leave the NAPI Poll function for the corresponding receive queue.

I can't see a piece of code which does that, but that's probably just
lack of detailed hardware knowledge on my side.

If you mean the "re-enable" piece of code, it is not very obvious,
you are right.  Interrupts are only generated if a particular
register for our completion queues is written. We do this in the
following line:

          ehea_reset_cq_ep(pr->recv_cq);
          ehea_reset_cq_ep(pr->send_cq);
          ehea_reset_cq_n1(pr->recv_cq);
          ehea_reset_cq_n1(pr->send_cq);

So this is in a way an indirect way to ask for interrupts when new
completions were written to memory. We don't really disable/enable
interrupts on the HEA chip itself.

Ah, ok. That's after the napi_complete which looks correct.
 

I think there are some mechanisms build in the HEA chip that should
prevent that interrupts don't get lost. But that is something that
is / was completely hidden from us, so my skill is very limited
there.

If more details are needed here we should involve the PHYP guys +
eHEA HW guys if not already done. Did anyone already talk to them?

Will or Darren might have, but lets gather more information first
before we rack their nerves :)

Thanks,

	tglx