Hi Thomas

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

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.

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

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.

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?

Regards,
Jan-Bernd