Hello Andre,
Given you are going to respin this oen due to the NIPA errors, let me get some nits I found.
On Sun, Nov 16, 2025 at 05:14:04PM +0000, Andre Carvalho wrote:
+/* Attempts to resume logging to a deactivated target. */ +static void maybe_resume_target(struct netconsole_target *nt,
struct net_device *ndev)
nit: s/maybe_resume_target/resume_target/
+{
- int ret;
- ret = __netpoll_setup(&nt->np, ndev);
- if (ret) {
/* netpoll fails setup once, do not try again. */nt->state = STATE_DISABLED;return;- }
- netdev_hold(ndev, &nt->np.dev_tracker, GFP_KERNEL);
In netpoll_setup(), it calls netdev_hold() first, and then call __netpoll_setup(). Shouldn't net device be held before trying to to setup netpoll()?
+static bool deactivated_target_match(struct netconsole_target *nt,
struct net_device *ndev)+{
- if (nt->state != STATE_DEACTIVATED)
return false;- if (bound_by_mac(nt))
return !memcmp(nt->np.dev_mac, ndev->dev_addr, ETH_ALEN);- return !strncmp(nt->np.dev_name, ndev->name, IFNAMSIZ);
+}
+/* Process targets in resume_list and returns then to target_list */
s/then/them
+static void process_resumable_targets(struct list_head *resume_list,
struct net_device *ndev)
nit: s/process_resumable_targets/netconsole_process_resumable_tagets/
The name is not the best, but it matches a similar function (netconsole_process_cleanups_core())
@@ -1475,6 +1537,11 @@ static int netconsole_netdev_event(struct notifier_block *this, stopped = true; } }
if (event == NETDEV_UP && deactivated_target_match(nt, dev))/* maybe_resume_target is IRQ unsafe, remove target from* target_list in order to resume it with IRQ enabled.*/list_move(&nt->list, &resume_list);@@ -1498,6 +1565,8 @@ static int netconsole_netdev_event(struct notifier_block *this, dev->name, msg); }
- process_resumable_targets(&resume_list, dev);
Nice, this new approach looks cleaner now.
--breno