[PATCH] net: fec: allow to build without PAGE_POOL_STATS

[PATCH] net: fec: allow to build without PAGE_POOL_STATS

[Lucas Stach]

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(-)

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,
-- 
2.40.1

Re: [PATCH] net: fec: allow to build without PAGE_POOL_STATS

[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?

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

Re: [PATCH] net: fec: allow to build without PAGE_POOL_STATS

[Lucas Stach]

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

Re: [PATCH] net: fec: allow to build without PAGE_POOL_STATS

[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.
> 
> 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

Re: [PATCH] net: fec: allow to build without PAGE_POOL_STATS

[patchwork-bot+netdevbpf]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html