All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Alexander Lobakin <aleksander.lobakin@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
	Larysa Zaremba <larysa.zaremba@intel.com>,
	Yunsheng Lin <linyunsheng@huawei.com>,
	Alexander Duyck <alexanderduyck@fb.com>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	"Ilias Apalodimas" <ilias.apalodimas@linaro.org>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH RFC net-next v2 7/7] net: skbuff: always try to recycle PP pages directly when in softirq
Date: Thu, 20 Jul 2023 11:00:27 -0700	[thread overview]
Message-ID: <20230720110027.4bd43ee7@kernel.org> (raw)
In-Reply-To: <8e65c3d3-c628-2176-2fc2-a1bc675ad607@intel.com>

On Thu, 20 Jul 2023 19:48:06 +0200 Alexander Lobakin wrote:
> >> My question was "how can two things race on one CPU in one context if it
> >> implies they won't ever happen simultaneously", but maybe my zero
> >> knowledge of netcons hides something from me.  
> > 
> > One of them is in hardirq.  
> 
> If I got your message correctly, that means softirq_count() can return
> `true` even if we're in hardirq context, but there are some softirqs
> pending? 

Not pending, being executed. Hardirq can come during softirq.

> I.e. if I call local_irq_save() inside NAPI poll loop,
> in_softirq() will still return `true`? (I'll check it myself in a bit,
> but why not ask).

Yes.

> Isn't checking for `interrupt_context_level() == 1` more appropriate
> then? Page Pool core code also uses in_softirq(), as well as a hellaton
> of other networking-related places.

Right now page pool only supports BH and process contexts. IOW the
"else" branch of if (in_softirq()) in page pool is expecting to be
in process context.

Supporting hard irq would mean we need to switch to _irqsave() locking.
That's likely way too costly.

Or stash the freed pages away and free them lazily.

Or add a lockdep warning and hope nobody will ever free a page-pool
backed skb from hard IRQ context :)

  reply	other threads:[~2023-07-20 18:00 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-14 17:08 [PATCH RFC net-next v2 0/7] net: page_pool: a couple of assorted optimizations Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 1/7] net: skbuff: don't include <net/page_pool.h> to <linux/skbuff.h> Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 2/7] net: page_pool: place frag_* fields in one cacheline Alexander Lobakin
2023-07-14 17:14   ` Alexander Lobakin
2023-07-14 18:37   ` Jesper Dangaard Brouer
2023-07-18 13:50     ` Alexander Lobakin
2023-07-26  8:13       ` Ilias Apalodimas
2023-07-26 10:39         ` Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 2/7] net: page_pool: shrink &page_pool_params a tiny bit Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 3/7] net: page_pool: place frag_* fields in one cacheline Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 3/7] net: page_pool: shrink &page_pool_params a tiny bit Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 4/7] net: page_pool: don't use driver-set flags field directly Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 5/7] net: page_pool: avoid calling no-op externals when possible Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 6/7] net: skbuff: avoid accessing page_pool if !napi_safe when returning page Alexander Lobakin
2023-07-14 17:08 ` [PATCH RFC net-next v2 7/7] net: skbuff: always try to recycle PP pages directly when in softirq Alexander Lobakin
2023-07-19  0:40   ` Jakub Kicinski
2023-07-19 16:34     ` Alexander Lobakin
2023-07-19 20:51       ` Jakub Kicinski
2023-07-20 16:46         ` Alexander Lobakin
2023-07-20 17:12           ` Jakub Kicinski
2023-07-20 17:48             ` Alexander Lobakin
2023-07-20 18:00               ` Jakub Kicinski [this message]
2023-07-20 18:01                 ` Alexander Lobakin
2023-07-20 18:13                   ` Alexander Lobakin
2023-07-20 19:20                     ` Jakub Kicinski
2023-07-20 19:33                       ` Alexander Lobakin
2023-07-20 19:46                         ` Jakub Kicinski
2023-07-21 11:53                           ` Yunsheng Lin
2023-07-21 15:37                             ` Alexander Lobakin
2023-07-21 16:01                               ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230720110027.4bd43ee7@kernel.org \
    --to=kuba@kernel.org \
    --cc=aleksander.lobakin@intel.com \
    --cc=alexanderduyck@fb.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hawk@kernel.org \
    --cc=ilias.apalodimas@linaro.org \
    --cc=larysa.zaremba@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linyunsheng@huawei.com \
    --cc=maciej.fijalkowski@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.