Re: netconsole: HARDIRQ-safe -> HARDIRQ-unsafe lock order warning

Hello John,

On Fri, Sep 05, 2025 at 02:54:32PM +0206, John Ogness wrote:

quoted

quoted

The bigger issue for the nbcon patch would seem to be the seemingly
required .write_atomic leading to landing here with disabled IRQs.

Using spin_lock_irqsave()/spin_unlock_irqrestore() within the
->device_lock() and ->device->unlock() callbacks is fine.

But it is not fine for netpoll, given that netpoll calls the network TX
path, that in some cases, tries to get a IRQ-unsafe locks, such as
&fq->lock. This is the current issue reported in this thread.

In other words, netconsole/netpoll cannot call the TX path with IRQ
disabled for some devices, due to some driver's TX path using IRQ unsafe
locks.

quoted

1) Decouple the SKB pool from netpoll and move it into netconsole

  * This makes netconsole behave like any other netpoll user,
    interacting with netpoll by sending SKBs.
	* The SKB population logic would then reside in netconsole, where it
	  logically belongs.

  * Enable NBCONS in netconsole, guarded by NETCONSOLE_NBCON
	* In normal .write_atomic() mode, messages should be queued in
	  a workqueue.

This is the wrong approach. It cannot be expected that the workqueue is
functional during panic. ->write_atomic() needs to be able to write
directly, most likely using pre-allocated SKBs and pre-setup dedicated
network queues.

Netpoll has pre-allocated SKBs and, although not the primary way
of allocating it, it can easily be set up to do so.

The problem happens later, when netpoll calls netdev_start_xmit(), which
calls ops->ndo_start_xmit(skb, dev), which might have some IRQ unsafe
locks (depending on the sub system).

To summarize the problem:

1) netpoll calls .ndo_start_xmit() with IRQ disabled, which causes the
lockdep problem reported in this thread. (current code)

2) moving netconsole to use NBCON will help in the thread context, given
that .write_thread() doesn't need to have IRQ disabled. (This requires
rework of netconsole target_list_lock)

3) In the atomic context, there is no easy solution so far. The options
are not good, but, I will list them here for the sake of getting things
clear:

  a) Defer the msg as proposed initially.
    Pro: If the machine is not crashing, it should simply work (?!)
    Cons: It cannot be expected that the workqueue is functional during panic, thus
          the messages might be lost
   
  b) Send the message anyway (and hope for the best)
    Cons: Netpoll will continue to call IRQ unsafe locks from IRQ safe
          context (lockdep will continue to be unhappy)
    Pro: This is how it works today already, so, it is not making the problem worse.
         In fact, it is narrowing the problem to only .write_atomic().

  c) Not implementing .write_atomic
    Cons: we lose the most important messages of the boot.

  c) Any other option I am not seeing?

Thanks for the insights,
--breno