netdev - Re: [PATCH net] net: fix race between napi kthread mode and busy poll

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20210224155237.221dd0c2@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Wed, 24 Feb 2021 15:52:37 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     Wei Wang <weiwan@...gle.com>,
        "David S . Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>, Paolo Abeni <pabeni@...hat.com>,
        Hannes Frederic Sowa <hannes@...essinduktion.org>,
        Alexander Duyck <alexanderduyck@...com>,
        Martin Zaharinov <micron10@...il.com>
Subject: Re: [PATCH net] net: fix race between napi kthread mode and busy
 poll

On Wed, 24 Feb 2021 23:30:23 +0100 Eric Dumazet wrote:
> On Wed, Feb 24, 2021 at 10:30 PM Jakub Kicinski <kuba@...nel.org> wrote:
> > Just to find out what the LoC is I sketched this out:
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index ddf4cfc12615..77f09ced9ee4 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -348,6 +348,7 @@ struct napi_struct {
> >         struct hlist_node       napi_hash_node;
> >         unsigned int            napi_id;
> >         struct task_struct      *thread;
> > +       struct list_head        thread_poll_list;
> >  };
> 
> offlist, since it seems this conversation is upsetting you.

Interesting, vger seems to be CCed but it isn't appearing on the ML.
Perhaps just a vger delay :S

Not really upsetting. I'm just trying to share what I learned devising
more advanced pollers. The bits get really messy really quickly.
Especially that the proposed fix adds a bit for a poor bystander (busy
poll) while it's the threaded IRQ that is incorrectly not preserving
its ownership.

> Additional 16 bytes here, possibly in a shared cache line, [1]
> I prefer using a bit in hot n->state, we have plenty of them available.

Right, presumably the location of the new member could be optimized.
I typed this proposal up in a couple of minutes.

> We worked hours with Alexander, Wei, I am sorry you think we did a poor job.
> I really thought we instead solved the issue at hand.
> 
> May I suggest you defer your idea of redesigning the NAPI model for
> net-next ?

Seems like you decided on this solution off list and now the fact that
there is a discussion on the list is upsetting you. May I suggest that
discussions should be conducted on list to avoid such situations?