Re: [RFC PATCH net-next 1/6] net: implement threaded-able napi poll loop support

[Wei Wang <weiwan@google.com>]

On Fri, Sep 25, 2020 at 12:46 PM Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
>
> Hello,
>
> Happy to see this work being resurrected (in case it is useful). :)
>
> On Mon, Sep 14, 2020, at 19:24, Wei Wang wrote:
> >
> > [...]
> >
> > +static void napi_thread_start(struct napi_struct *n)
> > +{
> > +     if (test_bit(NAPI_STATE_THREADED, &n->state) && !n->thread)
> > +             n->thread = kthread_create(napi_threaded_poll, n, "%s-%d",
> > +                                        n->dev->name, n->napi_id);
> > +}
> > +
>
> The format string is only based on variable strings. To ease a quick
> grep for napi threads with ps I would propose to use "napi-%s-%d" or
> something alike to distinguish all threads created that way.
>

Ack. Will add this in the next version.

> Some other comments and questions:
>
> Back then my plan was to get this somewhat integrated with the
> `threadirqs` kernel boot option because triggering the softirq from
> threaded context (if this option is set) seemed wrong to me. Maybe in
> theory the existing interrupt thread could already be used in this case.
> This would also allow for fine tuning the corresponding threads as in
> this patch series.
>
> Maybe the whole approach of threaded irqs plus the already existing
> infrastructure could also be used for this series if it wouldn't be an
> all or nothing opt-in based on the kernel cmd line parameter? napi would
> then be able to just poll directly inline in the interrupt thread.
>

I took a look at the current "threadirqs" implementation. From my
understanding, the kthread used there is to handle irq from the
driver, and needs driver-specific thread_fn to be used. It is not as
generic as in the napi layer where a common napi_poll() related
function could be used as the thread handler. Or did I misunderstand
your point?


> The difference for those kthreads and the extra threads created here
> would be that fifo scheduling policy is set by default and they seem to
> automatically get steered to the appropriate CPUs via the IRQTF_AFFINITY
> mechanism. Maybe this approach is useful here as well?
>
> I hadn't had a look at the code for a while thus my memories might be
> wrong here.

Yes. Using a higher priority thread policy and doing pinning could be
beneficial in certain workloads. But I think this should be left to
the user/admin to do the tuning accordingly.

>
> Thanks,
> Hannes