[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 9 Sep 2022 15:56:33 +0200
From: Francesco Dolcini <francesco.dolcini@...adex.com>
To: Marc Kleine-Budde <mkl@...gutronix.de>,
Csókás Bence <csokas.bence@...lan.hu>
CC: netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
Richard Cochran <richardcochran@...il.com>,
kernel@...gutronix.de,
Francesco Dolcini <francesco.dolcini@...adex.com>,
Jakub Kicinski <kuba@...nel.org>,
"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH v2] net: fec: Use a spinlock to guard `fep->ptp_clk_on`
On Wed, Sep 07, 2022 at 04:39:15PM +0200, Marc Kleine-Budde wrote:
> On 05.09.2022 09:38:04, Marc Kleine-Budde wrote:
> > On 9/1/22 4:04 PM, Csókás Bence wrote:
> > > Mutexes cannot be taken in a non-preemptible context,
> > > causing a panic in `fec_ptp_save_state()`. Replacing
> > > `ptp_clk_mutex` by `tmreg_lock` fixes this.
> >
> > I was on holidays, but this doesn't look good.
>
> Does anyone care to fix this? Csókás?
I do care, however I do not really have resources for anything better than
reverting both of the problematic commits.
If we do not have a fix, I guess this is the only way. Csókás?
For some reason when I tested on imx6 :-( I had no warning, now the
change was merged and while doing a quick test on a imx7
I got new issues.
[ 37.061582] ========================================================
[ 37.070203] WARNING: possible irq lock inversion dependency detected
[ 37.078847] 6.0.0-rc4-00137-g506357871c18 #3 Tainted: G W
[ 37.087944] --------------------------------------------------------
[ 37.096615] systemd/580 just changed the state of lock:
[ 37.104160] c218f2a4 (&dev->tx_global_lock){+.-.}-{2:2}, at: dev_watchdog+0x18/0x2c4
[ 37.116691] but this lock took another, SOFTIRQ-unsafe lock in the past:
[ 37.125896] (&fep->tmreg_lock){+.+.}-{2:2}
[ 37.125921]
[ 37.125921]
[ 37.125921] and interrupts could create inverse lock ordering between them.
[ 37.125921]
[ 37.153018]
[ 37.153018] other info that might help us debug this:
[ 37.163824] Possible interrupt unsafe locking scenario:
[ 37.163824]
[ 37.174714] CPU0 CPU1
[ 37.181215] ---- ----
[ 37.187631] lock(&fep->tmreg_lock);
[ 37.193110] local_irq_disable();
[ 37.200841] lock(&dev->tx_global_lock);
[ 37.209153] lock(&fep->tmreg_lock);
[ 37.217061] <Interrupt>
[ 37.221328] lock(&dev->tx_global_lock);
[ 37.227167]
[ 37.227167] *** DEADLOCK ***
that just goes away reverting this patch.
Francesco
Powered by blists - more mailing lists