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