Received-SPF: pass (google.com: domain of linux-kernel+bounces-172948-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Wed, 8 May 2024 10:55:05 +0200
From: Erhard Furtner <erhard_f@mailbox.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
 linux-kernel@vger.kernel.org
Subject: Re: WARNING: CPU: 1 PID: 1 at net/core/netpoll.c:370
 netpoll_send_skb+0x1fc/0x20c at boot when netconsole is enabled (kernel
 v6.9-rc5, v6.8.7, sungem, PowerMac G4 DP)
Message-ID: <20240508105505.098efd6c@yea>
In-Reply-To: <20240506181020.292b25f0@kernel.org>
References: <20240428125306.2c3080ef@legion>
	<20240429183630.399859e2@kernel.org>
	<20240505232713.46c03b30@yea>
	<20240506072645.448bc49f@kernel.org>
	<20240507024258.07980f55@yea>
	<20240506181020.292b25f0@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Mon, 6 May 2024 18:10:20 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> Excellent! Do you want to submit that as an official patch?
> The explanation is that we can't call disable_irq() from atomic
> context (which which netpoll runs). But the callback is no longer
> necessary as we can depend on NAPI to do the polling these days.

I could do that with the explanation you stated. But should any further questions arise in this process I would also lack the technical background to deal with them. ;)

I also noticed a similar #ifdef CONFIG_NET_POLL_CONTROLLER logic shows up in
many network drivers, e.g. net/ethernet/realtek/8139too.c:

#ifdef CONFIG_NET_POLL_CONTROLLER
static void rtl8139_poll_controller(struct net_device *dev);
#endif
[...]
#ifdef CONFIG_NET_POLL_CONTROLLER
/*
 * Polling receive - used by netconsole and other diagnostic tools
 * to allow network i/o with interrupts disabled.
 */
static void rtl8139_poll_controller(struct net_device *dev)
{
        struct rtl8139_private *tp = netdev_priv(dev);
       	const int irq = tp->pci_dev->irq;

       	disable_irq_nosync(irq);
       	rtl8139_interrupt(irq, dev);
       	enable_irq(irq);
}
#endif
[...]
#ifdef CONFIG_NET_POLL_CONTROLLER
       	.ndo_poll_controller    = rtl8139_poll_controller,
#endif


Should it be removed here too? This would be more cards I can test. So far I only see this on my G4 and I think something similar on an old Pentium4 box I no longer have. 
 
> > What I still get with 'modprobe -v dev_addr_lists_test', even with gem_poll_controller() removed is:
> > 
> > [...]
> > KTAP version 1
> > 1..1
> >     KTAP version 1
> >     # Subtest: dev-addr-list-test
> >     # module: dev_addr_lists_test
> >     1..6
> > 
> > ====================================
> > WARNING: kunit_try_catch/1770 still has locks held!
> > 6.9.0-rc6-PMacG4-dirty #5 Tainted: G        W        N
> > ------------------------------------
> > 1 lock held by kunit_try_catch/1770:
> >  #0: c0dbfce4 (rtnl_mutex){....}-{3:3}, at: dev_addr_test_init+0xbc/0xc8 [dev_addr_lists_test]  
> 
> I think that's fixed in net-next.

Ah, good to hear!

Regards,
Erhard F.