All of lore.kernel.org
 help / color / mirror / Atom feed
From: Darren Hart <dvhltc@us.ibm.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Jan-Bernd Themann <THEMANN@de.ibm.com>,
	dvhltc@linux.vnet.ibm.com, linux-kernel@vger.kernel.org,
	Will Schmidt <will_schmidt@vnet.ibm.com>,
	Brian King <brking@linux.vnet.ibm.com>,
	niv@linux.vnet.ibm.com, Doug Maxey <doug.maxey@us.ibm.com>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH RT] ehea: make receive irq handler non-threaded (IRQF_NODELAY)
Date: Thu, 20 May 2010 07:39:27 -0700	[thread overview]
Message-ID: <4BF5499F.8050203@us.ibm.com> (raw)
In-Reply-To: <alpine.LFD.2.00.1005200950410.3368@localhost.localdomain>

On 05/20/2010 01:14 AM, Thomas Gleixner wrote:
> On Thu, 20 May 2010, Jan-Bernd Themann wrote:
>>>> 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
>>>> 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.

I was going to suggest that because these are threaded handlers, perhaps 
they are rescheduled on a different CPU and then receive the interrupt 
for the other CPU/queue that Jan was mentioning.

But, the handlers are affined if I remember correctly, and we aren't 
running with multiple receive queues. So, we're back to the same 
question, why are we seeing another irq. It comes in before 
napi_complete() and therefor before the ehea_reset*() block of calls 
which do the equivalent of re-enabling interrupts.

--
Darren

>
>> 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.
>
> Thanks,
>
> 	tglx


-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team

WARNING: multiple messages have this Message-ID (diff)
From: Darren Hart <dvhltc@us.ibm.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Jan-Bernd Themann <THEMANN@de.ibm.com>,
	michael@ellerman.id.au, Brian King <brking@linux.vnet.ibm.com>,
	Doug Maxey <doug.maxey@us.ibm.com>,
	dvhltc@linux.vnet.ibm.com, linux-kernel@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, niv@linux.vnet.ibm.com,
	Will Schmidt <will_schmidt@vnet.ibm.com>
Subject: Re: [PATCH RT] ehea: make receive irq handler non-threaded (IRQF_NODELAY)
Date: Thu, 20 May 2010 07:39:27 -0700	[thread overview]
Message-ID: <4BF5499F.8050203@us.ibm.com> (raw)
In-Reply-To: <alpine.LFD.2.00.1005200950410.3368@localhost.localdomain>

On 05/20/2010 01:14 AM, Thomas Gleixner wrote:
> On Thu, 20 May 2010, Jan-Bernd Themann wrote:
>>>> 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
>>>> 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.

I was going to suggest that because these are threaded handlers, perhaps 
they are rescheduled on a different CPU and then receive the interrupt 
for the other CPU/queue that Jan was mentioning.

But, the handlers are affined if I remember correctly, and we aren't 
running with multiple receive queues. So, we're back to the same 
question, why are we seeing another irq. It comes in before 
napi_complete() and therefor before the ehea_reset*() block of calls 
which do the equivalent of re-enabling interrupts.

--
Darren

>
>> 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.
>
> Thanks,
>
> 	tglx


-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team

  parent reply	other threads:[~2010-05-20 14:39 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-18 21:33 [PATCH RT] ehea: make receive irq handler non-threaded (IRQF_NODELAY) Darren Hart
2010-05-18 21:52 ` Brian King
2010-05-18 21:52   ` Brian King
2010-05-18 22:19   ` Nivedita Singhvi
2010-05-18 22:19     ` Nivedita Singhvi
2010-05-18 22:22   ` Darren Hart
2010-05-18 22:22     ` Darren Hart
2010-05-19  1:25     ` Michael Ellerman
2010-05-19  1:25       ` Michael Ellerman
2010-05-19 14:16       ` Darren Hart
2010-05-19 14:16         ` Darren Hart
2010-05-19 14:38         ` Thomas Gleixner
2010-05-19 14:38           ` Thomas Gleixner
2010-05-19 21:08           ` Thomas Gleixner
2010-05-19 21:08             ` Thomas Gleixner
2010-05-20  1:34             ` Michael Ellerman
2010-05-20  1:34               ` Michael Ellerman
2010-05-20  7:37               ` Jan-Bernd Themann
2010-05-20  7:37                 ` Jan-Bernd Themann
2010-05-20  8:14                 ` Thomas Gleixner
2010-05-20  8:14                   ` Thomas Gleixner
2010-05-20  9:05                   ` Jan-Bernd Themann
2010-05-20  9:05                     ` Jan-Bernd Themann
2010-05-20  9:19                     ` Thomas Gleixner
2010-05-20  9:19                       ` Thomas Gleixner
2010-05-20 14:26                       ` Nivedita Singhvi
2010-05-20 14:26                         ` Nivedita Singhvi
2010-05-20 14:53                     ` Will Schmidt
2010-05-20 14:53                       ` Will Schmidt
2010-05-20 14:39                   ` Darren Hart [this message]
2010-05-20 14:39                     ` Darren Hart
2010-05-20 14:45                     ` Thomas Gleixner
2010-05-20 14:45                       ` Thomas Gleixner
2010-05-20 21:44                       ` Will Schmidt
2010-05-20 21:44                         ` Will Schmidt
2010-05-20  1:32           ` Michael Ellerman
2010-05-20  1:32             ` Michael Ellerman
2010-05-20  8:21             ` Thomas Gleixner
2010-05-20  8:21               ` Thomas Gleixner
2010-05-21  9:18               ` [PATCH RT] ehea: make receive irq handler non-threaded (IRQF_NODELAY), " Milton Miller
2010-05-21  9:18                 ` Milton Miller
2010-09-20 14:26                 ` Jan-Bernd Themann
2010-09-20 14:26                   ` Jan-Bernd Themann
2010-05-20  1:28         ` Michael Ellerman
2010-05-20  1:28           ` Michael Ellerman
2010-05-21  9:02           ` Milton Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BF5499F.8050203@us.ibm.com \
    --to=dvhltc@us.ibm.com \
    --cc=THEMANN@de.ibm.com \
    --cc=brking@linux.vnet.ibm.com \
    --cc=doug.maxey@us.ibm.com \
    --cc=dvhltc@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=niv@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=will_schmidt@vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.