Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1398143 > unrolled thread

[RFC PATCH 0/2] net: threadable napi poll loop

Started byPaolo Abeni <pabeni@redhat.com>
First post2016-05-10 16:20 +0200
Last post2016-05-10 22:50 +0200
Articles 7 on this page of 47 — 8 participants

Back to article view | Back to linux.kernel


Contents

  [RFC PATCH 0/2] net: threadable napi poll loop Paolo Abeni <pabeni@redhat.com> - 2016-05-10 16:20 +0200
    [RFC PATCH 2/2] net: add sysfs attribute to control napi threaded mode Paolo Abeni <pabeni@redhat.com> - 2016-05-10 16:20 +0200
    Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-10 16:40 +0200
      Re: [RFC PATCH 0/2] net: threadable napi poll loop David Miller <davem@davemloft.net> - 2016-05-10 18:00 +0200
      Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-10 18:10 +0200
        Re: [RFC PATCH 0/2] net: threadable napi poll loop Paolo Abeni <pabeni@redhat.com> - 2016-05-10 22:30 +0200
          Re: [RFC PATCH 0/2] net: threadable napi poll loop David Miller <davem@davemloft.net> - 2016-05-10 22:50 +0200
            Re: [RFC PATCH 0/2] net: threadable napi poll loop David Miller <davem@davemloft.net> - 2016-05-10 23:00 +0200
              Re: [RFC PATCH 0/2] net: threadable napi poll loop Rik van Riel <riel@redhat.com> - 2016-05-10 23:10 +0200
            Re: [RFC PATCH 0/2] net: threadable napi poll loop Rik van Riel <riel@redhat.com> - 2016-05-10 23:00 +0200
      Re: [RFC PATCH 0/2] net: threadable napi poll loop Paolo Abeni <pabeni@redhat.com> - 2016-05-10 18:10 +0200
      Re: [RFC PATCH 0/2] net: threadable napi poll loop Hannes Frederic Sowa <hannes@stressinduktion.org> - 2016-05-10 22:50 +0200
        Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <edumazet@google.com> - 2016-05-10 23:10 +0200
          Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-10 23:40 +0200
            Re: [RFC PATCH 0/2] net: threadable napi poll loop Rik van Riel <riel@redhat.com> - 2016-05-10 23:40 +0200
              Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-11 00:00 +0200
                Re: [RFC PATCH 0/2] net: threadable napi poll loop Rik van Riel <riel@redhat.com> - 2016-05-11 00:10 +0200
                Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-11 00:10 +0200
                  Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-11 00:50 +0200
                Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-11 20:00 +0200
          Re: [RFC PATCH 0/2] net: threadable napi poll loop Hannes Frederic Sowa <hannes@stressinduktion.org> - 2016-05-11 00:40 +0200
            Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-11 01:00 +0200
              Re: [RFC PATCH 0/2] net: threadable napi poll loop Peter Zijlstra <peterz@infradead.org> - 2016-05-11 09:00 +0200
                Re: [RFC PATCH 0/2] net: threadable napi poll loop Hannes Frederic Sowa <hannes@stressinduktion.org> - 2016-05-11 15:20 +0200
                  Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <edumazet@google.com> - 2016-05-11 16:50 +0200
                    Re: [RFC PATCH 0/2] net: threadable napi poll loop Rik van Riel <riel@redhat.com> - 2016-05-11 17:10 +0200
                    Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-11 18:00 +0200
                Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-12 00:00 +0200
                  Re: [RFC PATCH 0/2] net: threadable napi poll loop Paolo Abeni <pabeni@redhat.com> - 2016-05-12 22:10 +0200
                    Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-12 22:50 +0200
                      Re: [RFC PATCH 0/2] net: threadable napi poll loop Paolo Abeni <pabeni@redhat.com> - 2016-05-12 23:00 +0200
                        Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-12 23:10 +0200
                  Re: [RFC PATCH 0/2] net: threadable napi poll loop Paolo Abeni <pabeni@redhat.com> - 2016-05-13 19:00 +0200
                    Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <edumazet@google.com> - 2016-05-13 19:10 +0200
                      Re: [RFC PATCH 0/2] net: threadable napi poll loop Paolo Abeni <pabeni@redhat.com> - 2016-05-13 19:20 +0200
                        Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <edumazet@google.com> - 2016-05-13 19:40 +0200
                          Re: [RFC PATCH 0/2] net: threadable napi poll loop Paolo Abeni <pabeni@redhat.com> - 2016-05-16 15:20 +0200
                            Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <edumazet@google.com> - 2016-05-16 15:40 +0200
              Re: [RFC PATCH 0/2] net: threadable napi poll loop Paolo Abeni <pabeni@redhat.com> - 2016-05-11 11:50 +0200
                Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <eric.dumazet@gmail.com> - 2016-05-11 15:10 +0200
                  Re: [RFC PATCH 0/2] net: threadable napi poll loop Hannes Frederic Sowa <hannes@stressinduktion.org> - 2016-05-11 15:40 +0200
                    Re: [RFC PATCH 0/2] net: threadable napi poll loop Hannes Frederic Sowa <hannes@stressinduktion.org> - 2016-05-11 15:50 +0200
                  Re: [RFC PATCH 0/2] net: threadable napi poll loop Paolo Abeni <pabeni@redhat.com> - 2016-05-11 16:40 +0200
                    Re: [RFC PATCH 0/2] net: threadable napi poll loop Eric Dumazet <edumazet@google.com> - 2016-05-11 16:50 +0200
                      Re: [RFC PATCH 0/2] net: threadable napi poll loop Hannes Frederic Sowa <hannes@stressinduktion.org> - 2016-05-12 00:50 +0200
    Re: [RFC PATCH 0/2] net: threadable napi poll loop Thomas Gleixner <tglx@linutronix.de> - 2016-05-10 18:00 +0200
      Re: [RFC PATCH 0/2] net: threadable napi poll loop Paolo Abeni <pabeni@redhat.com> - 2016-05-10 22:50 +0200

Page 3 of 3 — ← Prev page 1 2 [3]


#1399083

FromHannes Frederic Sowa <hannes@stressinduktion.org>
Date2016-05-11 15:40 +0200
Message-ID<rxCwP-Kt-11@gated-at.bofh.it>
In reply to#1399064
Hi all,

On 11.05.2016 15:08, Eric Dumazet wrote:
> On Wed, 2016-05-11 at 11:48 +0200, Paolo Abeni wrote:
>> Hi Eric,
>> On Tue, 2016-05-10 at 15:51 -0700, Eric Dumazet wrote:
>>> On Wed, 2016-05-11 at 00:32 +0200, Hannes Frederic Sowa wrote:
>>>
>>>> Not only did we want to present this solely as a bugfix but also as as
>>>> performance enhancements in case of virtio (as you can see in the cover
>>>> letter). Given that a long time ago there was a tendency to remove
>>>> softirqs completely, we thought it might be very interesting, that a
>>>> threaded napi in general seems to be absolutely viable nowadays and
>>>> might offer new features.
>>>
>>> Well, you did not fix the bug, you worked around by adding yet another
>>> layer, with another sysctl that admins or programs have to manage.
>>>
>>> If you have a special need for virtio, do not hide it behind a 'bug fix'
>>> but add it as a features request.
>>>
>>> This ksoftirqd issue is real and a fix looks very reasonable.
>>>
>>> Please try this patch, as I had very good success with it.
>>
>> Thank you for your time and your effort.
>>
>> I tested your patch on the bare metal "single core" scenario, disabling
>> the unneeded cores with:
>> CPUS=`nproc`
>> for I in `seq 1 $CPUS`; do echo 0  >  /sys/devices/system/node/node0/cpu$I/online; done
>>
>> And I got a:
>>
>> [   86.925249] Broke affinity for irq <num>
>>
> 
> Was it fatal, or simply a warning that you are removing the cpu that was
> the only allowed cpu in an affinity_mask ?
> 
> Looks another bug to fix then ? We disabled CPU hotplug here at Google
> for our production, as it was notoriously buggy. No time to fix dozens
> of issues added by a crowd of developers that do not even know a cpu can
> be unplugged.
> 
> Maybe some caller of local_bh_disable()/local_bh_enable() expected that
> current softirq would be processed. Obviously flaky even before the
> patches.

Yes, I fear this could come up. If we want to target net or stable maybe
we should maybe special case this patch specifically for net-rx?

>> for each irq number generated by a network device.
>>
>> In this scenario, your patch solves the ksoftirqd issue, performing
>> comparable to the napi threaded patches (with a negative delta in the
>> noise range) and introducing a minor regression with a single flow, in
>> the noise range (3%).
>>
>> As said in a previous mail, we actually experimented something similar,
>> but it felt quite hackish.
> 
> Right, we are networking guys, and we feel that messing with such core
> infra is not for us. So we feel comfortable adding a pure networking
> patch.

We posted this patch as an RFC. My initial internal proposal only had a
check in ___napi_schedule and completely relied on threaded irqs and
didn't spawn a thread per napi instance in the networking stack. I think
this is the better approach long term, as it allows to configure
threaded irqs per device and doesn't specifically deal with networking
only. NAPI must be aware of when to schedule, obviously, so we need
another check in napi_schedule.

My plan was definitely to go with something more generic, but we didn't
yet know how to express that in a generic way, but relied on the forced
threaded irqs kernel parameter.

>> AFAICS this patch adds three more tests in the fast path and affect all
>> other softirq use case. I'm not sure how to check for regression there.
> 
> It is obvious to me that ksoftird mechanism is not working as intended.

Yes.

> Fixing it might uncover bugs from parts of the kernel relying on the
> bug, indirectly or directly. Is it a good thing ?
> 
> I can not tell before trying.
> 
> Just by looking at /proc/{ksoftirqs_pid}/sched you can see the problem,
> as we normally schedule ksoftird under stress but most of the time,
> the softirq items were processed by another tasks as you found out.

Exactly, the pending mask gets reset by the task handling the softirq
inline and ksoftirqd runs dry too early not processing any more softirq
notifications.

>> The napi thread patches are actually a new feature, that also fixes the
>> ksoftirqd issue: hunting the ksoftirqd issue has been the initial
>> trigger for this work. I'm sorry for not being clear enough in the cover
>> letter.
>>
>> The napi thread patches offer additional benefits, i.e. an additional
>> relevant gain in the described test scenario, and do not impact on other
>> subsystems/kernel entities. 
>>
>> I still think they are worthy, and I bet you would disagree, but could
>> you please articulate more which parts concern you most and/or are more
>> bloated ?
> 
> Just look at the added code. napi_threaded_poll() is very buggy, but
> honestly I do not want to fix the bugs you added there. If you have only
> one vcpu, how jiffies can ever change since you block BH ?

I think the local_bh_disable/enable needs to be more fine granular,
correct (inside the loop).

> I was planning to remove cond_resched_softirq() that we no longer use
> after my recent changes to TCP stack,
> and you call it again (while it is obviously buggy since it does not
> check if a BH is pending, only if a thread needs the cpu)

Good point, thanks!

> I prefer fixing the existing code, really. It took us years to
> understand it and maybe fix it.

I agree, we should find a simple way to let ksoftirqd and netrx behave
better and target threaded napi as a new feature.

> Just think of what will happen if you have 10 devices (10 new threads in
> your model) and one cpu.
> 
> Instead of the nice existing netif_rx() doing 64 packets per device
> rounds, you'll now rely on process scheduler behavior that has no such
> granularity.

We didn't inspect all kinds of workloads right now but I hoped to see
benefits in forwarding with multiple interfaces. This is also why we
want to have some "easy" way to configure that for admins. Also
integration with RPS is on the todo list.

> Adding more threads is the natural answer of userland programmers, but
> in the kernel it is not the right answer. We already have mechanism,
> just use them and fix them if they are broken.
> 
> Sorry, I really do not think your patches are the way to go.
> But this thread is definitely interesting.

I am fine with that. It is us to show clear benefits or use cases for
that. If we fail with that, no problem at all that the patches get
rejected, we don't want to add bloat to the kernel, for sure! At this
point I still think a possibility to run napi in kthreads will allow
specific workloads to see an improvement. Maybe the simple branch in
napi_schedule is just worth so people can play around with it. As it
shouldn't change behavior we can later on simply remove it.

Thanks,
Hannes

[toc] | [prev] | [next] | [standalone]


#1399098

FromHannes Frederic Sowa <hannes@stressinduktion.org>
Date2016-05-11 15:50 +0200
Message-ID<rxCGv-Pf-27@gated-at.bofh.it>
In reply to#1399083
On 11.05.2016 15:39, Hannes Frederic Sowa wrote:
> I am fine with that. It is us to show clear benefits or use cases for
> that. If we fail with that, no problem at all that the patches get
> rejected, we don't want to add bloat to the kernel, for sure! At this
> point I still think a possibility to run napi in kthreads will allow
> specific workloads to see an improvement. Maybe the simple branch in
> napi_schedule is just worth so people can play around with it. As it
> shouldn't change behavior we can later on simply remove it.

Actually, I consider it a bug that when we force the kernel to use
threaded irqs that we only schedule the softirq for napi later on and
don't do the processing within the thread.

Bye,
Hannes

[toc] | [prev] | [next] | [standalone]


#1399179

FromPaolo Abeni <pabeni@redhat.com>
Date2016-05-11 16:40 +0200
Message-ID<rxDsS-1yk-1@gated-at.bofh.it>
In reply to#1399064
On Wed, 2016-05-11 at 06:08 -0700, Eric Dumazet wrote:
> On Wed, 2016-05-11 at 11:48 +0200, Paolo Abeni wrote:
> > Hi Eric,
> > On Tue, 2016-05-10 at 15:51 -0700, Eric Dumazet wrote:
> > > On Wed, 2016-05-11 at 00:32 +0200, Hannes Frederic Sowa wrote:
> > > 
> > > > Not only did we want to present this solely as a bugfix but also as as
> > > > performance enhancements in case of virtio (as you can see in the cover
> > > > letter). Given that a long time ago there was a tendency to remove
> > > > softirqs completely, we thought it might be very interesting, that a
> > > > threaded napi in general seems to be absolutely viable nowadays and
> > > > might offer new features.
> > > 
> > > Well, you did not fix the bug, you worked around by adding yet another
> > > layer, with another sysctl that admins or programs have to manage.
> > > 
> > > If you have a special need for virtio, do not hide it behind a 'bug fix'
> > > but add it as a features request.
> > > 
> > > This ksoftirqd issue is real and a fix looks very reasonable.
> > > 
> > > Please try this patch, as I had very good success with it.
> > 
> > Thank you for your time and your effort.
> > 
> > I tested your patch on the bare metal "single core" scenario, disabling
> > the unneeded cores with:
> > CPUS=`nproc`
> > for I in `seq 1 $CPUS`; do echo 0  >  /sys/devices/system/node/node0/cpu$I/online; done
> > 
> > And I got a:
> > 
> > [   86.925249] Broke affinity for irq <num>
> > 
> 
> Was it fatal, or simply a warning that you are removing the cpu that was
> the only allowed cpu in an affinity_mask ?

The above message is emitted with pr_notice() by the x86 version of
fixup_irqs(). It's not fatal, the host is alive and well after that. The
un-patched kernel does not emit it on cpus disabling.

I'll try to look into this later.

> Looks another bug to fix then ? We disabled CPU hotplug here at Google
> for our production, as it was notoriously buggy. No time to fix dozens
> of issues added by a crowd of developers that do not even know a cpu can
> be unplugged.
> 
> Maybe some caller of local_bh_disable()/local_bh_enable() expected that
> current softirq would be processed. Obviously flaky even before the
> patches.
> 
> > for each irq number generated by a network device.
> > 
> > In this scenario, your patch solves the ksoftirqd issue, performing
> > comparable to the napi threaded patches (with a negative delta in the
> > noise range) and introducing a minor regression with a single flow, in
> > the noise range (3%).
> > 
> > As said in a previous mail, we actually experimented something similar,
> > but it felt quite hackish.
> 
> Right, we are networking guys, and we feel that messing with such core
> infra is not for us. So we feel comfortable adding a pure networking
> patch.
> 
> > 
> > AFAICS this patch adds three more tests in the fast path and affect all
> > other softirq use case. I'm not sure how to check for regression there.
> 
> It is obvious to me that ksoftird mechanism is not working as intended.
> 
> Fixing it might uncover bugs from parts of the kernel relying on the
> bug, indirectly or directly. Is it a good thing ?
> 
> I can not tell before trying.
> 
> Just by looking at /proc/{ksoftirqs_pid}/sched you can see the problem,
> as we normally schedule ksoftird under stress but most of the time,
> the softirq items were processed by another tasks as you found out.
> 
> 
> > 
> > The napi thread patches are actually a new feature, that also fixes the
> > ksoftirqd issue: hunting the ksoftirqd issue has been the initial
> > trigger for this work. I'm sorry for not being clear enough in the cover
> > letter.
> > 
> > The napi thread patches offer additional benefits, i.e. an additional
> > relevant gain in the described test scenario, and do not impact on other
> > subsystems/kernel entities. 
> > 
> > I still think they are worthy, and I bet you would disagree, but could
> > you please articulate more which parts concern you most and/or are more
> > bloated ?
> 
> Just look at the added code. napi_threaded_poll() is very buggy, but
> honestly I do not want to fix the bugs you added there. If you have only
> one vcpu, how jiffies can ever change since you block BH ?

Uh, we have likely the same issue in the net_rx_action() function, which
also execute with bh disabled and check for jiffies changes even on
single core hosts ?!?

Aren't jiffies updated by the timer interrupt ? and thous even with
bh_disabled ?!?

> I was planning to remove cond_resched_softirq() that we no longer use
> after my recent changes to TCP stack,
> and you call it again (while it is obviously buggy since it does not
> check if a BH is pending, only if a thread needs the cpu)

I missed that, thank you for pointing out.

> I prefer fixing the existing code, really. It took us years to
> understand it and maybe fix it.
> 
> Just think of what will happen if you have 10 devices (10 new threads in
> your model) and one cpu.
> 
> Instead of the nice existing netif_rx() doing 64 packets per device
> rounds, you'll now rely on process scheduler behavior that has no such
> granularity.
> 
> Adding more threads is the natural answer of userland programmers, but
> in the kernel it is not the right answer. We already have mechanism,
> just use them and fix them if they are broken.
> 
> Sorry, I really do not think your patches are the way to go.
> But this thread is definitely interesting.

Oh, this is a far better comment that I would have expected ;-)

Cheers,

Paolo

[toc] | [prev] | [next] | [standalone]


#1399198

FromEric Dumazet <edumazet@google.com>
Date2016-05-11 16:50 +0200
Message-ID<rxDCy-1G3-5@gated-at.bofh.it>
In reply to#1399179
On Wed, May 11, 2016 at 7:38 AM, Paolo Abeni <pabeni@redhat.com> wrote:

> Uh, we have likely the same issue in the net_rx_action() function, which
> also execute with bh disabled and check for jiffies changes even on
> single core hosts ?!?

That is why we have a loop break after netdev_budget=300 packets.
And a sysctl to eventually tune this.

Same issue for softirq handler, look at commit
34376a50fb1fa095b9d0636fa41ed2e73125f214

Your questions about this central piece of networking code are worrying.

>
> Aren't jiffies updated by the timer interrupt ? and thous even with
> bh_disabled ?!?

Exactly my point : jiffie wont be updated in your code, since you block BH.

[toc] | [prev] | [next] | [standalone]


#1399563

FromHannes Frederic Sowa <hannes@stressinduktion.org>
Date2016-05-12 00:50 +0200
Message-ID<rxL74-U4-35@gated-at.bofh.it>
In reply to#1399198
On 11.05.2016 16:45, Eric Dumazet wrote:
> On Wed, May 11, 2016 at 7:38 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> 
>> Uh, we have likely the same issue in the net_rx_action() function, which
>> also execute with bh disabled and check for jiffies changes even on
>> single core hosts ?!?
> 
> That is why we have a loop break after netdev_budget=300 packets.
> And a sysctl to eventually tune this.
> 
> Same issue for softirq handler, look at commit
> 34376a50fb1fa095b9d0636fa41ed2e73125f214
> 
> Your questions about this central piece of networking code are worrying.
> 
>>
>> Aren't jiffies updated by the timer interrupt ? and thous even with
>> bh_disabled ?!?
> 
> Exactly my point : jiffie wont be updated in your code, since you block BH.

To be fair, jiffies get updated in hardirq and not softirq context. The
cond_resched_softirq not looking for pending softirqs is indeed a problem.

Thanks,
Hannes

[toc] | [prev] | [next] | [standalone]


#1398273

FromThomas Gleixner <tglx@linutronix.de>
Date2016-05-10 18:00 +0200
Message-ID<rxieK-5nU-19@gated-at.bofh.it>
In reply to#1398143
On Tue, 10 May 2016, Paolo Abeni wrote:

Nice patch set and very promising results! 

> At this point we are not really sure if we should go with this simpler
> approach by putting NAPI itself into kthreads or leverage the threadirqs
> function by putting the whole interrupt into a thread and signaling NAPI
> that it does not reschedule itself in a softirq but to simply run at
> this particular context of the interrupt handler.
> 
> While the threaded irq way seems to better integrate into the kernel and
> also other devices could move their interrupts into the threads easily
> on a common policy, we don't know how to really express the necessary
> knobs with the current device driver model (module parameters, sysfs
> attributes, etc.). This is where we would like to hear some opinions.
> NAPI would e.g. have to query the kernel if the particular IRQ/MSI if it
> should be scheduled in a softirq or in a thread, so we don't have to
> rewrite all device drivers. This might even be needed on a per rx-queue
> granularity.

Utilizing threaded irqs should be halfways simple even without touching the
device driver at all.

We can do the switch to threading in two ways:

1) Let the driver request the interrupt(s) as it does now and then have a
   /proc/irq/NNN/threaded file which converts it to a threaded interrupt on
   the fly. That should be fairly trivial.

2) Let the driver request the interrupt(s) as it does now and retrieve the
   interrupt number which belongs to the device/queue from the network core
   and let the irq core switch it over to threaded.

So the interrupt flow of the device would be:

interrupt
    IRQ_WAKE_THREAD
    
irq thread()
{ 
    local_bh_disable();
    action->thread_fn(action->irq, action->dev_id); <-- driver handler
    irq_finalize_oneshot(desc, action);
    local_bh_enable();
}

The driver irq handler calls napi_schedule(). So if your napi_struct is
flagged POLL_IRQ_THREAD then you can call your polling machinery from there
instead of raising the softirq.

You surely need some way to figure out whether the interrupt is threaded when
you set up the device in order to flag your napi struct, but that should be
not too hard to achieve.

Thanks,

	tglx



   

[toc] | [prev] | [next] | [standalone]


#1398517

FromPaolo Abeni <pabeni@redhat.com>
Date2016-05-10 22:50 +0200
Message-ID<rxmLo-1CV-9@gated-at.bofh.it>
In reply to#1398273
On Tue, 2016-05-10 at 17:57 +0200, Thomas Gleixner wrote:
> On Tue, 10 May 2016, Paolo Abeni wrote:
> 
> Nice patch set and very promising results! 
> 
> > At this point we are not really sure if we should go with this simpler
> > approach by putting NAPI itself into kthreads or leverage the threadirqs
> > function by putting the whole interrupt into a thread and signaling NAPI
> > that it does not reschedule itself in a softirq but to simply run at
> > this particular context of the interrupt handler.
> > 
> > While the threaded irq way seems to better integrate into the kernel and
> > also other devices could move their interrupts into the threads easily
> > on a common policy, we don't know how to really express the necessary
> > knobs with the current device driver model (module parameters, sysfs
> > attributes, etc.). This is where we would like to hear some opinions.
> > NAPI would e.g. have to query the kernel if the particular IRQ/MSI if it
> > should be scheduled in a softirq or in a thread, so we don't have to
> > rewrite all device drivers. This might even be needed on a per rx-queue
> > granularity.
> 
> Utilizing threaded irqs should be halfways simple even without touching the
> device driver at all.
> 
> We can do the switch to threading in two ways:
> 
> 1) Let the driver request the interrupt(s) as it does now and then have a
>    /proc/irq/NNN/threaded file which converts it to a threaded interrupt on
>    the fly. That should be fairly trivial.
> 
> 2) Let the driver request the interrupt(s) as it does now and retrieve the
>    interrupt number which belongs to the device/queue from the network core
>    and let the irq core switch it over to threaded.

Thank you for the feedback.

We actually experimented something similar to (2). In our implementation
we needed a per device chunk of code to do the actual irq number ->
queue mapping (and than we performed as well the switch in the device
code).

> You surely need some way to figure out whether the interrupt is threaded when
> you set up the device in order to flag your napi struct, but that should be
> not too hard to achieve.

This is the part that required per device changes and complicated a bit
the implementation. We can research further to simplify it, according to
the overall discussion.

Cheers,

Paolo

[toc] | [prev] | [standalone]


Page 3 of 3 — ← Prev page 1 2 [3]

Back to top | Article view | linux.kernel


csiph-web