Re: [PATCH v2] net: fec: Use a spinlock to guard `fep->ptp_clk_on`

[Francesco Dolcini <francesco.dolcini@toradex.com>]

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