Re: [PATCH] net: remove lockdep asserts from ____napi_schedule()

[Jason A. Donenfeld]

Hi Sebastian,

On Sat, Mar 19, 2022 at 6:01 AM Sebastian Andrzej Siewior
 wrote:
>
> On 2022-03-18 18:47:38 [-0600], Jason A. Donenfeld wrote:
> > This reverts commit fbd9a2ceba5c ("net: Add lockdep asserts to
> > napi_schedule()."). While good in theory, in practice it causes
> > issues with various drivers, and so it can be revisited earlier in the
> > cycle where those drivers can be adjusted if needed.
>
> Do you plan to address to address the wireguard warning?

It seemed to me like you had a lot of interesting ideas regarding
packet batching and performance and whatnot around when bh is enabled
or not. I'm waiting for a patch from you on this, as I mentioned in my
previous email. There is definitely a lot of interesting potential
performance work here. I am curious to play around with it too, of
course, but it sounded to me like you had very specific ideas. I'm not
really sure how to determine how many packets to batch, except for
through empirical observation or some kind of crazy dql thing. Or
maybe there's some optimal quantity due to the way napi works in the
first place. Anyway, there's some research to do here.

>
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4277,9 +4277,6 @@ static inline void napi_schedule(struct 
> > softnet_data *sd,
> >  {
> >   struct task_struct *thread;
> >
> > - lockdep_assert_softirq_will_run();
> > - lockdep_assert_irqs_disabled();
>
> Could you please keep that lockdep_assert_irqs_disabled()? That is
> needed regardless of the upper one.

Feel free to send in a more specific revert if you think it's
justifiable. I just sent in the thing that reverted the patch that
caused the regression - the dumb brute approach.

Jason

Re: [PATCH] net: remove lockdep asserts from ____napi_schedule()

[Sebastian Andrzej Siewior]

On 2022-03-18 18:47:38 [-0600], Jason A. Donenfeld wrote:
> This reverts commit fbd9a2ceba5c ("net: Add lockdep asserts to
> napi_schedule()."). While good in theory, in practice it causes
> issues with various drivers, and so it can be revisited earlier in the
> cycle where those drivers can be adjusted if needed.

Do you plan to address to address the wireguard warning?

> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4277,9 +4277,6 @@ static inline void napi_schedule(struct 
> softnet_data *sd,
>  {
>   struct task_struct *thread;
>  
> - lockdep_assert_softirq_will_run();
> - lockdep_assert_irqs_disabled();

Could you please keep that lockdep_assert_irqs_disabled()? That is
needed regardless of the upper one.

Sebastian

Re: [PATCH] net: remove lockdep asserts from ____napi_schedule()

[Jakub Kicinski]

On Fri, 18 Mar 2022 18:50:08 -0600 Jason A. Donenfeld wrote:
> Hi Jakub,
> 
> Er, I forgot to mark this as net-next, but as it's connected to the
> discussion we were just having, I think you get the idea. :)

Yup, patchwork bot figured it out, too. All good :)

Re: [PATCH] net: remove lockdep asserts from ____napi_schedule()

[Jason A. Donenfeld]

Hi Jakub,

Er, I forgot to mark this as net-next, but as it's connected to the
discussion we were just having, I think you get the idea. :)

Jason