diff mbox series

net: fec: allow to build without PAGE_POOL_STATS

Message ID 20230616191832.2944130-1-l.stach@pengutronix.de (mailing list archive)
State Accepted
Commit 857922b16bb893d26d5ecd83acf9f20cb28eaea2
Delegated to: Netdev Maintainers
Headers show
Series net: fec: allow to build without PAGE_POOL_STATS | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 5 maintainers not CCed: hawk@kernel.org daniel@iogearbox.net john.fastabend@gmail.com bpf@vger.kernel.org ast@kernel.org
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 22 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Lucas Stach June 16, 2023, 7:18 p.m. UTC
Commit 6970ef27ff7f ("net: fec: add xdp and page pool statistics") selected
CONFIG_PAGE_POOL_STATS from the FEC driver symbol, making it impossible
to build without the page pool statistics when this driver is enabled. The
help text of those statistics mentions increased overhead. Allow the user
to choose between usefulness of the statistics and the added overhead.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/net/ethernet/freescale/Kconfig    | 2 +-
 drivers/net/ethernet/freescale/fec_main.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Andrew Lunn June 17, 2023, 3:48 p.m. UTC | #1
On Fri, Jun 16, 2023 at 09:18:32PM +0200, Lucas Stach wrote:
> Commit 6970ef27ff7f ("net: fec: add xdp and page pool statistics") selected
> CONFIG_PAGE_POOL_STATS from the FEC driver symbol, making it impossible
> to build without the page pool statistics when this driver is enabled. The
> help text of those statistics mentions increased overhead. Allow the user
> to choose between usefulness of the statistics and the added overhead.

Hi Lucas

Do you have any sort of numbers?

Object size should be easy to do.  How much difference does the #ifdef
CONFIG_PAGE_POOL_STATS make to the code segment? Those come with a
small amount of maintenance cost. And there does appear to be stubs
for when PAGE_POOL_STATS is disabled.

    Andrew
Lucas Stach June 19, 2023, 8:06 a.m. UTC | #2
Hi Andrew,

Am Samstag, dem 17.06.2023 um 17:48 +0200 schrieb Andrew Lunn:
> On Fri, Jun 16, 2023 at 09:18:32PM +0200, Lucas Stach wrote:
> > Commit 6970ef27ff7f ("net: fec: add xdp and page pool statistics") selected
> > CONFIG_PAGE_POOL_STATS from the FEC driver symbol, making it impossible
> > to build without the page pool statistics when this driver is enabled. The
> > help text of those statistics mentions increased overhead. Allow the user
> > to choose between usefulness of the statistics and the added overhead.
> 
> Hi Lucas
> 
> Do you have any sort of numbers?

I don't have any numbers. To be honest I only wrote this patch because
I was surprised to see CONFIG_PAGE_POOL_STATS being enabled via a
select after a kernel update, while the help text of that item suggests
that the user should have a choice here.

> 
> Object size should be easy to do.  How much difference does the #ifdef
> CONFIG_PAGE_POOL_STATS make to the code segment? Those come with a
> small amount of maintenance cost. And there does appear to be stubs
> for when PAGE_POOL_STATS is disabled.

Stubs aren't sufficient here, as the structures used as parameters to
those functions aren't defined when !CONFIG_PAGE_POOL_STATS.

Regards,
Lucas
Andrew Lunn June 20, 2023, 3:22 p.m. UTC | #3
On Fri, Jun 16, 2023 at 09:18:32PM +0200, Lucas Stach wrote:
> Commit 6970ef27ff7f ("net: fec: add xdp and page pool statistics") selected
> CONFIG_PAGE_POOL_STATS from the FEC driver symbol, making it impossible
> to build without the page pool statistics when this driver is enabled. The
> help text of those statistics mentions increased overhead. Allow the user
> to choose between usefulness of the statistics and the added overhead.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

I don't think it does any harm, even if the saving is minimal. And
0-day has not yet reported any build errors.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
patchwork-bot+netdevbpf@kernel.org June 20, 2023, 7:20 p.m. UTC | #4
Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 16 Jun 2023 21:18:32 +0200 you wrote:
> Commit 6970ef27ff7f ("net: fec: add xdp and page pool statistics") selected
> CONFIG_PAGE_POOL_STATS from the FEC driver symbol, making it impossible
> to build without the page pool statistics when this driver is enabled. The
> help text of those statistics mentions increased overhead. Allow the user
> to choose between usefulness of the statistics and the added overhead.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> 
> [...]

Here is the summary with links:
  - net: fec: allow to build without PAGE_POOL_STATS
    https://git.kernel.org/netdev/net-next/c/857922b16bb8

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index 1c78f66a89da..75401d2a5fb4 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -29,7 +29,7 @@  config FEC
 	select CRC32
 	select PHYLIB
 	select PAGE_POOL
-	select PAGE_POOL_STATS
+	imply PAGE_POOL_STATS
 	imply NET_SELFTESTS
 	help
 	  Say Y here if you want to use the built-in 10/100 Fast ethernet
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 38e5b5abe067..be1308295b11 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2810,6 +2810,7 @@  static void fec_enet_get_xdp_stats(struct fec_enet_private *fep, u64 *data)
 
 static void fec_enet_page_pool_stats(struct fec_enet_private *fep, u64 *data)
 {
+#ifdef CONFIG_PAGE_POOL_STATS
 	struct page_pool_stats stats = {};
 	struct fec_enet_priv_rx_q *rxq;
 	int i;
@@ -2824,6 +2825,7 @@  static void fec_enet_page_pool_stats(struct fec_enet_private *fep, u64 *data)
 	}
 
 	page_pool_ethtool_stats_get(data, &stats);
+#endif
 }
 
 static void fec_enet_get_ethtool_stats(struct net_device *dev,