On Mon, Sep 11, 2023 at 07:11:02PM +0200, Uwe Kleine-König wrote:
> Hello,
>
> this patch became commit 133466c3bbe171f826294161db203f7670bb30c8 and is
> part of v6.6-rc1.
>
> On my arm/stm32mp157 based machine using NFS root this commit makes the
> following appear in the kernel log:
>
> INFO: trying to register non-static key.
> The code is fine but needs lockdep annotation, or maybe
> you didn't initialize this object before use?
> turning off the locking correctness validator.
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc1-00449-g133466c3bbe1-dirty #21
> Hardware name: STM32 (Device Tree Support)
> unwind_backtrace from show_stack+0x18/0x1c
> show_stack from dump_stack_lvl+0x60/0x90
> dump_stack_lvl from register_lock_class+0x98c/0x99c
> register_lock_class from __lock_acquire+0x74/0x293c
> __lock_acquire from lock_acquire+0x134/0x398
> lock_acquire from stmmac_get_stats64+0x2ac/0x2fc
> stmmac_get_stats64 from dev_get_stats+0x44/0x130
> dev_get_stats from rtnl_fill_stats+0x38/0x120
> rtnl_fill_stats from rtnl_fill_ifinfo+0x834/0x17f4
> rtnl_fill_ifinfo from rtmsg_ifinfo_build_skb+0xc0/0x144
> rtmsg_ifinfo_build_skb from rtmsg_ifinfo+0x50/0x88
> rtmsg_ifinfo from __dev_notify_flags+0xc0/0xec
> __dev_notify_flags from dev_change_flags+0x50/0x5c
> dev_change_flags from ip_auto_config+0x2f4/0x1260
> ip_auto_config from do_one_initcall+0x70/0x35c
> do_one_initcall from kernel_init_freeable+0x2ac/0x308
> kernel_init_freeable from kernel_init+0x1c/0x138
> kernel_init from ret_from_fork+0x14/0x2c
> Exception stack(0xe0815fb0 to 0xe0815ff8)
> 5fa0: 00000000 00000000 00000000 00000000
> 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> dwc2 49000000.usb-otg: new device is high-speed
>
> I didn't try understand this problem, it's too close to quitting time
> :-)
Thanks for the bug report, I'm checking the code.
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | https://www.pengutronix.de/ |
Am Dienstag, dem 12.09.2023 um 16:01 +0800 schrieb Jisheng Zhang:
> On Mon, Sep 11, 2023 at 07:11:02PM +0200, Uwe Kleine-König wrote:
> > Hello,
> >
> > this patch became commit 133466c3bbe171f826294161db203f7670bb30c8 and is
> > part of v6.6-rc1.
> >
> > On my arm/stm32mp157 based machine using NFS root this commit makes the
> > following appear in the kernel log:
> >
> > INFO: trying to register non-static key.
> > The code is fine but needs lockdep annotation, or maybe
> > you didn't initialize this object before use?
> > turning off the locking correctness validator.
> > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc1-00449-g133466c3bbe1-dirty #21
> > Hardware name: STM32 (Device Tree Support)
> > unwind_backtrace from show_stack+0x18/0x1c
> > show_stack from dump_stack_lvl+0x60/0x90
> > dump_stack_lvl from register_lock_class+0x98c/0x99c
> > register_lock_class from __lock_acquire+0x74/0x293c
> > __lock_acquire from lock_acquire+0x134/0x398
> > lock_acquire from stmmac_get_stats64+0x2ac/0x2fc
> > stmmac_get_stats64 from dev_get_stats+0x44/0x130
> > dev_get_stats from rtnl_fill_stats+0x38/0x120
> > rtnl_fill_stats from rtnl_fill_ifinfo+0x834/0x17f4
> > rtnl_fill_ifinfo from rtmsg_ifinfo_build_skb+0xc0/0x144
> > rtmsg_ifinfo_build_skb from rtmsg_ifinfo+0x50/0x88
> > rtmsg_ifinfo from __dev_notify_flags+0xc0/0xec
> > __dev_notify_flags from dev_change_flags+0x50/0x5c
> > dev_change_flags from ip_auto_config+0x2f4/0x1260
> > ip_auto_config from do_one_initcall+0x70/0x35c
> > do_one_initcall from kernel_init_freeable+0x2ac/0x308
> > kernel_init_freeable from kernel_init+0x1c/0x138
> > kernel_init from ret_from_fork+0x14/0x2c
> > Exception stack(0xe0815fb0 to 0xe0815ff8)
> > 5fa0: 00000000 00000000 00000000 00000000
> > 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > dwc2 49000000.usb-otg: new device is high-speed
> >
> > I didn't try understand this problem, it's too close to quitting time
> > :-)
>
> Thanks for the bug report, I'm checking the code.
The newly added "struct u64_stats_sync syncp" uses a seqlock
internally, which is broken into multiple words on 32bit machines, and
needs to be initialized properly. You need to call u64_stats_init on
syncp before first usage.
Regards,
Lucas
> >
> > Best regards
> > Uwe
> >
> > --
> > Pengutronix e.K. | Uwe Kleine-König |
> > Industrial Linux Solutions | https://www.pengutronix.de/ |
>
>
>
Hello,
On Tue, Sep 12, 2023 at 11:04:16AM +0200, Lucas Stach wrote:
> Am Dienstag, dem 12.09.2023 um 16:01 +0800 schrieb Jisheng Zhang:
> > On Mon, Sep 11, 2023 at 07:11:02PM +0200, Uwe Kleine-K?nig wrote:
> > > Hello,
> > >
> > > this patch became commit 133466c3bbe171f826294161db203f7670bb30c8 and is
> > > part of v6.6-rc1.
> > >
> > > On my arm/stm32mp157 based machine using NFS root this commit makes the
> > > following appear in the kernel log:
> > >
> > > INFO: trying to register non-static key.
> > > The code is fine but needs lockdep annotation, or maybe
> > > you didn't initialize this object before use?
> > > turning off the locking correctness validator.
> > > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc1-00449-g133466c3bbe1-dirty #21
> > > Hardware name: STM32 (Device Tree Support)
> > > unwind_backtrace from show_stack+0x18/0x1c
> > > show_stack from dump_stack_lvl+0x60/0x90
> > > dump_stack_lvl from register_lock_class+0x98c/0x99c
> > > register_lock_class from __lock_acquire+0x74/0x293c
> > > __lock_acquire from lock_acquire+0x134/0x398
> > > lock_acquire from stmmac_get_stats64+0x2ac/0x2fc
> > > stmmac_get_stats64 from dev_get_stats+0x44/0x130
> > > dev_get_stats from rtnl_fill_stats+0x38/0x120
> > > rtnl_fill_stats from rtnl_fill_ifinfo+0x834/0x17f4
> > > rtnl_fill_ifinfo from rtmsg_ifinfo_build_skb+0xc0/0x144
> > > rtmsg_ifinfo_build_skb from rtmsg_ifinfo+0x50/0x88
> > > rtmsg_ifinfo from __dev_notify_flags+0xc0/0xec
> > > __dev_notify_flags from dev_change_flags+0x50/0x5c
> > > dev_change_flags from ip_auto_config+0x2f4/0x1260
> > > ip_auto_config from do_one_initcall+0x70/0x35c
> > > do_one_initcall from kernel_init_freeable+0x2ac/0x308
> > > kernel_init_freeable from kernel_init+0x1c/0x138
> > > kernel_init from ret_from_fork+0x14/0x2c
> > > Exception stack(0xe0815fb0 to 0xe0815ff8)
> > > 5fa0: 00000000 00000000 00000000 00000000
> > > 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > dwc2 49000000.usb-otg: new device is high-speed
> > >
> > > I didn't try understand this problem, it's too close to quitting time
> > > :-)
> >
> > Thanks for the bug report, I'm checking the code.
>
> The newly added "struct u64_stats_sync syncp" uses a seqlock
> internally, which is broken into multiple words on 32bit machines, and
> needs to be initialized properly. You need to call u64_stats_init on
> syncp before first usage.
This is done. The problematic thing is that in stmmac_open() ->
__stmmac_open() the syncp initialized before is overwritten by
memcpy(&priv->dma_conf, dma_conf, sizeof(*dma_conf));
Do I need to point out that this is ugly?
Together with Johannes Berg I debugged that in #netdev and we came up
with:
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 9a3182b9e767..a3481b48c77f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3893,7 +3893,7 @@ static int stmmac_open(struct net_device *dev)
{
struct stmmac_priv *priv = netdev_priv(dev);
struct stmmac_dma_conf *dma_conf;
- int ret;
+ int ret, i;
dma_conf = stmmac_setup_dma_desc(priv, dev->mtu);
if (IS_ERR(dma_conf))
@@ -3904,6 +3904,13 @@ static int stmmac_open(struct net_device *dev)
free_dma_desc_resources(priv, dma_conf);
kfree(dma_conf);
+
+ dma_conf = &priv->dma_conf;
+ for (i = 0; i < MTL_MAX_RX_QUEUES; i++)
+ u64_stats_init(&dma_conf->rx_queue[i].rxq_stats.syncp);
+ for (i = 0; i < MTL_MAX_TX_QUEUES; i++)
+ u64_stats_init(&dma_conf->tx_queue[i].txq_stats.syncp);
+
return ret;
}
which works around the problem. Note however that the u64_stats_init()
calls must not be deleted from stmmac_dvr_probe() because it's used
there once before __stmmac_open() overwrites it.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |