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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEA6p_DyU7jyHEeRiWFtNZfMPQjJJEV2jN1MV-+5txumC5nmZg@mail.gmail.com>
Date:   Fri, 25 Sep 2020 16:50:56 -0700
From:   Wei Wang <weiwan@...gle.com>
To:     Hannes Frederic Sowa <hannes@...essinduktion.org>
Cc:     David Miller <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>, Felix Fietkau <nbd@....name>
Subject: Re: [RFC PATCH net-next 1/6] net: implement threaded-able napi poll
 loop support

On Fri, Sep 25, 2020 at 12:46 PM Hannes Frederic Sowa
<hannes@...essinduktion.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