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);
> netconsole_target_put(nt);
> }
> spin_unlock_irqrestore(&target_list_lock, flags);
> @@ -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
