[PATCH net-next v2 0/6] page_pool: a couple of assorted optimizations

* [PATCH net-next v2 0/6] page_pool: a couple of assorted optimizations
@ 2023-08-03 16:40 Alexander Lobakin
  2023-08-03 16:40 ` [PATCH net-next v2 1/6] page_pool: split types and declarations from page_pool.h Alexander Lobakin
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Alexander Lobakin @ 2023-08-03 16:40 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Alexander Lobakin, Maciej Fijalkowski, Larysa Zaremba,
	Yunsheng Lin, Alexander Duyck, Jesper Dangaard Brouer,
	Ilias Apalodimas, Simon Horman, netdev, linux-kernel

That initially was a spin-off of the IAVF PP series[0], but has grown
(and shrunk) since then a bunch. In fact, it consists of three
semi-independent blocks:

* #1-2: Compile-time optimization. Split page_pool.h into 2 headers to
  not overbloat the consumers not needing complex inline helpers and
  then stop including it in skbuff.h at all. The first patch is also
  prereq for the whole series.
* #3: Improve cacheline locality for users of the Page Pool frag API.
* #4-6: Use direct cache recycling more aggressively, when it is safe
  obviously. In addition, make sure nobody wants to use Page Pool API
  with disabled interrupts.

Patches #1 and #5 are authored by Yunsheng and Jakub respectively, with
small modifications from my side as per ML discussions.
For the perf numbers for #3-6, please see individual commit messages.

Also available on my GH with many more Page Pool goodies[1].

[0] https://lore.kernel.org/netdev/20230530150035.1943669-1-aleksander.lobakin@intel.com
[1] https://github.com/alobakin/linux/commits/iavf-pp-frag

Alexander Lobakin (4):
  net: skbuff: don't include <net/page_pool/types.h> to <linux/skbuff.h>
  page_pool: place frag_* fields in one cacheline
  net: skbuff: avoid accessing page_pool if !napi_safe when returning
    page
  net: skbuff: always try to recycle PP pages directly when in softirq

Jakub Kicinski (1):
  page_pool: add a lockdep check for recycling in hardirq

Yunsheng Lin (1):
  page_pool: split types and declarations from page_pool.h

 MAINTAINERS                                   |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c |   2 +-
 drivers/net/ethernet/engleder/tsnep_main.c    |   1 +
 drivers/net/ethernet/freescale/fec_main.c     |   1 +
 .../net/ethernet/hisilicon/hns3/hns3_enet.c   |   1 +
 .../net/ethernet/hisilicon/hns3/hns3_enet.h   |   2 +-
 drivers/net/ethernet/marvell/mvneta.c         |   2 +-
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |   2 +-
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   |   1 +
 .../marvell/octeontx2/nic/otx2_common.c       |   1 +
 .../ethernet/marvell/octeontx2/nic/otx2_pf.c  |   1 +
 drivers/net/ethernet/mediatek/mtk_eth_soc.c   |   1 +
 drivers/net/ethernet/mediatek/mtk_eth_soc.h   |   2 +-
 .../ethernet/mellanox/mlx5/core/en/params.c   |   1 +
 .../net/ethernet/mellanox/mlx5/core/en/trap.c |   1 -
 .../net/ethernet/mellanox/mlx5/core/en/xdp.c  |   1 +
 .../net/ethernet/mellanox/mlx5/core/en_main.c |   2 +-
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   |   2 +-
 .../ethernet/mellanox/mlx5/core/en_stats.c    |   2 +-
 .../ethernet/microchip/lan966x/lan966x_fdma.c |   1 +
 .../ethernet/microchip/lan966x/lan966x_main.h |   2 +-
 drivers/net/ethernet/socionext/netsec.c       |   2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |   2 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |   1 +
 drivers/net/ethernet/ti/cpsw.c                |   2 +-
 drivers/net/ethernet/ti/cpsw_new.c            |   2 +-
 drivers/net/ethernet/ti/cpsw_priv.c           |   2 +-
 drivers/net/ethernet/wangxun/libwx/wx_lib.c   |   2 +-
 drivers/net/veth.c                            |   2 +-
 drivers/net/wireless/mediatek/mt76/mac80211.c |   1 -
 drivers/net/wireless/mediatek/mt76/mt76.h     |   1 +
 drivers/net/xen-netfront.c                    |   2 +-
 include/linux/lockdep.h                       |   7 +
 include/linux/skbuff.h                        |   5 +-
 include/net/page_pool/helpers.h               | 193 ++++++++++++++++
 .../net/{page_pool.h => page_pool/types.h}    | 209 +-----------------
 include/trace/events/page_pool.h              |   2 +-
 net/bpf/test_run.c                            |   2 +-
 net/core/page_pool.c                          |  43 +---
 net/core/skbuff.c                             |  49 +++-
 net/core/xdp.c                                |   2 +-
 42 files changed, 297 insertions(+), 267 deletions(-)
 create mode 100644 include/net/page_pool/helpers.h
 rename include/net/{page_pool.h => page_pool/types.h} (51%)

---
From v1[2]:
* move the "avoid calling no-op DMA sync ops" piece out of the series --
  will join some other or transform into something else (Jakub et al.);
* #1: restore accidentally removed path in MAINTAINERS (Yunsheng);
* #1: prefer `include/net/page_pool/` over `include/net/page_pool/*` in
  MAINTAINERS (Yunsheng);
* #2: rename page_pool_return_skb_page() to napi_pp_put_page() -- the old
  name seems to not make any sense for some time already (Yunsheng);
* #2: guard napi_pp_put_page() with CONFIG_PAGE_POOL in skbuff.c (to not
  compile it when there are no users) (Yunsheng);

From RFC v2[3]:
* drop the dependency on the hybrid allocation series (and thus the
  "RFC" prefix) -- it wasn't a strict dep and it's not in the trees yet;
* add [slightly reworked] Yunsheng's patch which splits page_pool.h into
  2 headers -- merge conflict hell otherwise.
  Also fix a typo while nobody looks (Simon);
* #3 (former #2): word the commitmsg a bit better, mention the main
  reason for the change more clearly (Ilias);
* add Jakub's hardirq assertion as a prereq for the last patch;
* #9 (former #7): add comment mentioning that the hardirq case is not
  checked due to the assertion checking it later (yes, it is illegal to
  use Page Pool with the interrupts disabled or when in TH) (Jakub);

From RFC v1[4]:
* #1: move the entire function to skbuff.c, don't try to split it (Alex);
* #2-4: new;
* #5: use internal flags field added in #4 and don't modify driver-defined
  structure (Alex, Jakub);
* #6: new;
* drop "add new NAPI state" as a redundant complication;
* #7: replace the check for the new NAPI state to just in_softirq(), should
  be fine (Jakub).

[2] https://lore.kernel.org/netdev/20230727144336.1646454-1-aleksander.lobakin@intel.com
[3] https://lore.kernel.org/netdev/20230714170853.866018-1-aleksander.lobakin@intel.com
[4] https://lore.kernel.org/netdev/20230629152305.905962-1-aleksander.lobakin@intel.com
-- 
2.41.0

^ permalink raw reply	[flat|nested] 8+ messages in thread