mirrors/linux
2
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2026-08-30 16:53:20 -04:00
Code Issues Packages Projects Releases Wiki Activity

netconsole: take over skb pool lifecycle from netpoll

Browse Source 
The fallback skb pool fronted by find_skb() is netconsole's only client:
every other netpoll goes through __netpoll_setup() / netpoll_send_skb()
without ever touching np->skb_pool.

Today __netpoll_setup() and __netpoll_cleanup() create and destroy the
pool for everyone, paying ~48 KB of pre-allocated skbs per netpoll
instance that only netconsole uses, what a waste!

Move the responsibility to netconsole. __netpoll_setup() did this under
the RTNL, but netconsole enables targets from enabled_store() /
alloc_param_target() without it, while the teardown path flushes the pool
(cancel_work_sync() + skb_queue_purge()) under the RTNL from
netconsole_process_cleanups_core(). Initialising the queue head and the
refill work on every enable would therefore race that flush.

They only need initialising once: after a flush the queue head is left
valid and empty and cancel_work_sync() leaves the work re-armable. Set
them up in alloc_and_init(), while the target is not yet reachable, and
let the enable paths only refill the pool via refill_skbs(), which
serialises with the flush through skb_pool.lock. See discussions in [1]

Link: https://lore.kernel.org/all/alDMvD5S7TZnoD_V@gmail.com/ [1]
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260710-netconsole_move_more-v3-3-6f63f76b28bc@debian.org
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Breno Leitao
2026-07-10 04:38:55 -07:00
committed by Paolo Abeni
parent ede59d06c2
commit 1fee9a9c59
2 changed files with 52 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
drivers/net/netconsole.c
View File

@@ -292,11 +292,33 @@ static void netcons_release_dev(struct netconsole_target *nt)
memset(&nt->np.dev_name, 0, IFNAMSIZ);
}
/* Seed the per-target skb pool that find_skb() falls back to. The queue
* head and refill work are set up once in alloc_and_init(); this only
* (re)fills the pool. Pair with netconsole_skb_pool_flush().
*/
static void netconsole_skb_pool_init(struct netconsole_target *nt)
{
refill_skbs(&nt->np);
}
static void netconsole_skb_pool_flush(struct netconsole_target *nt)
{
skb_pool_flush(&nt->np);
}
/* Attempts to resume logging to a deactivated target. */
static void resume_target(struct netconsole_target *nt)
{
/* Initialise the skb pool before netpoll_setup() makes nt->np.dev
* visible to target_list walkers (e.g. netconsole_netdev_event),
* which otherwise may move the target to the cleanup list and
* call netconsole_skb_pool_flush() on uninitialised state.
*/
netconsole_skb_pool_init(nt);
if (netpoll_setup(&nt->np)) {
/* netpoll fails setup once, do not try again. */
netconsole_skb_pool_flush(nt);
nt->state = STATE_DISABLED;
return;
}
@@ -358,6 +380,7 @@ static void process_resume_target(struct work_struct *work)
rtnl_lock();
if (nt->state == STATE_ENABLED && nt->np.dev &&
nt->np.dev->reg_state != NETREG_REGISTERED) {
netconsole_skb_pool_flush(nt);
netcons_release_dev(nt);
nt->state = STATE_DISABLED;
}
@@ -398,6 +421,9 @@ static struct netconsole_target *alloc_and_init(void)
eth_broadcast_addr(nt->np.remote_mac);
nt->state = STATE_DISABLED;
INIT_WORK(&nt->resume_wq, process_resume_target);
/* Set up the skb pool primitives once; enabling only refills it. */
skb_queue_head_init(&nt->np.skb_pool);
INIT_WORK(&nt->np.refill_wq, refill_skbs_work_handler);
return nt;
}
@@ -417,6 +443,7 @@ static void netconsole_process_cleanups_core(void)
list_for_each_entry_safe(nt, tmp, &target_cleanup_list, list) {
/* all entries in the cleanup_list needs to be disabled */
WARN_ON_ONCE(nt->state == STATE_ENABLED);
netconsole_skb_pool_flush(nt);
netcons_release_dev(nt);
/* moved the cleaned target to target_list. Need to hold both
* locks
@@ -758,9 +785,19 @@ static ssize_t enabled_store(struct config_item *item,
*/
netconsole_print_banner(&nt->np);
/* Initialise the skb pool before netpoll_setup() so the pool
* is valid as soon as nt->np.dev becomes visible to
* target_list walkers (netconsole_netdev_event), which would
* otherwise call netconsole_skb_pool_flush() on uninitialised
* state.
*/
netconsole_skb_pool_init(nt);
ret = netpoll_setup(&nt->np);
if (ret)
if (ret) {
netconsole_skb_pool_flush(nt);
goto out_unlock;
}
nt->state = STATE_ENABLED;
pr_info("network logging started\n");
@@ -1514,8 +1551,10 @@ static void drop_netconsole_target(struct config_group *group,
* netpoll_cleanup() is idempotent (it skips when np->dev is NULL), so
* it is safe even if the cleanup worker already tore the netpoll down.
*/
if (needs_cleanup)
if (needs_cleanup) {
netconsole_skb_pool_flush(nt);
netpoll_cleanup(&nt->np);
}
config_item_put(&nt->group.cg_item);
}
@@ -2330,10 +2369,18 @@ static struct netconsole_target *alloc_param_target(char *target_config,
if (err)
goto fail;
/* Initialise the skb pool before netpoll_setup() so the pool is
* valid as soon as nt->np.dev becomes visible. The target is not
* yet on target_list, so a netdev event cannot reach it here, but
* mirror the configfs path for symmetry.
*/
netconsole_skb_pool_init(nt);
err = netpoll_setup(&nt->np);
if (err) {
pr_err("Not enabling netconsole for %s%d. Netpoll setup failed\n",
NETCONSOLE_PARAM_TARGET_PREFIX, cmdline_count);
netconsole_skb_pool_flush(nt);
if (!IS_ENABLED(CONFIG_NETCONSOLE_DYNAMIC))
/* only fail if dynamic reconfiguration is set,
* otherwise, keep the target in the list, but disabled.
@@ -2355,6 +2402,8 @@ static struct netconsole_target *alloc_param_target(char *target_config,
static void free_param_target(struct netconsole_target *nt)
{
cancel_work_sync(&nt->resume_wq);
if (nt->state == STATE_ENABLED)
netconsole_skb_pool_flush(nt);
netpoll_cleanup(&nt->np);
#ifdef CONFIG_NETCONSOLE_DYNAMIC
kfree(nt->userdata);

12
net/core/netpoll.c
View File

@@ -377,9 +377,6 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
const struct net_device_ops *ops;
int err;
skb_queue_head_init(&np->skb_pool);
INIT_WORK(&np->refill_wq, refill_skbs_work_handler);
if (ndev->priv_flags & IFF_DISABLE_NETPOLL) {
np_err(np, "%s doesn't support polling, aborting\n",
ndev->name);
@@ -414,9 +411,6 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
np->dev = ndev;
strscpy(np->dev_name, ndev->name, IFNAMSIZ);
/* fill up the skb queue */
refill_skbs(np);
/* last thing to do is link it to the net device structure */
rcu_assign_pointer(ndev->npinfo, npinfo);
@@ -606,7 +600,7 @@ int netpoll_setup(struct netpoll *np)
err = __netpoll_setup(np, ndev);
if (err)
goto flush;
goto put;
rtnl_unlock();
/* Make sure all NAPI polls which started before dev->npinfo
@@ -617,8 +611,6 @@ int netpoll_setup(struct netpoll *np)
return 0;
flush:
skb_pool_flush(np);
put:
DEBUG_NET_WARN_ON_ONCE(np->dev);
if (ip_overwritten)
@@ -662,8 +654,6 @@ static void __netpoll_cleanup(struct netpoll *np)
disable_delayed_work_sync(&npinfo->tx_work);
call_rcu(&npinfo->rcu, rcu_cleanup_netpoll_info);
}
skb_pool_flush(np);
}
void __netpoll_free(struct netpoll *np)