Re: [PATCH net-next v4 3/5] net: dsa: mv88e6xxx: decouple the PTP timecounter from the register lock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Calling cancel_delayed_work_sync() while holding a mutex that the work callback also acquires leads to an AB-BA deadlock.
--

--- Patch [3]: [PATCH net-next v4 3/5] net: dsa: mv88e6xxx: decouple the PTP timecounter from the register lock ---
commit 23ddfde2f70b36ac19aedfb1142d819436fc0957
Author: Luke Howard <lukeh@padl.com>

net: dsa: mv88e6xxx: decouple the PTP timecounter from the register lock

Use a dedicated spinlock, rather than the chip register mutex, to
protect read access to the hardware timestamp cycle counter.

> diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
> index f7603573d3a98..2a7f97625e375 100644
> --- a/drivers/net/dsa/mv88e6xxx/ptp.c
> +++ b/drivers/net/dsa/mv88e6xxx/ptp.c
> @@ -231,15 +231,37 @@ static void mv88e6352_tai_event_work(struct work_struct *ugly)
>  
>  	/* We only have one timestamping channel. */
>  	ev.index = 0;
> -	mv88e6xxx_reg_lock(chip);
> -	ev.timestamp = timecounter_cyc2time(&chip->tstamp_tc, raw_ts);
> -	mv88e6xxx_reg_unlock(chip);
> +	ev.timestamp = mv88e6xxx_timecounter_cyc2time(chip, raw_ts);
>  
>  	ptp_clock_event(chip->ptp_clock, &ev);
>  out:
>  	schedule_delayed_work(&chip->tai_event_work, TAI_EVENT_WORK_INTERVAL);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this worker I
noticed a potential deadlock.

If a userspace process calls the PTP_EXTTS_REQUEST ioctl to disable external
timestamps (on=0), it reaches mv88e6352_ptp_enable_extts() which takes the
reg_lock mutex:

drivers/net/dsa/mv88e6xxx/ptp.c:mv88e6352_ptp_enable_extts() {
...
	mv88e6xxx_reg_lock(chip);
...
	} else {
...
		cancel_delayed_work_sync(&chip->tai_event_work);
	}
...
}

Concurrently, the delayed worker tai_event_work can fire, and it attempts
to take reg_lock via mv88e6xxx_reg_lock() earlier in the function:

drivers/net/dsa/mv88e6xxx/ptp.c:mv88e6352_tai_event_work() {
...
	mv88e6xxx_reg_lock(chip);
	err = mv88e6xxx_tai_read(...);
...
}

Because cancel_delayed_work_sync() is waiting for the worker to finish while
holding the mutex the worker needs, can this lead to an AB-BA deadlock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-mv88e6xxx-ptp-fixes-v4-0-10b4020155fc@padl.com?part=3