In some configurations, a single TAI can be responsible for multiple
mvpp2 interfaces. However, the mvpp2 driver will call mvpp22_tai_stop
and mvpp22_tai_start per interface RX timestamp disable/enable.
As a result, disabling timestamping for one interface would stop the
worker and corrupt the other interface's RX timestamps.
This commit solves the issue by introducing a simpler ref count for each
TAI instance.
Due to the ref count, we need now to lock tai->lock before doing
anything, as a result, we can't update the current ts using
mvpp22_tai_gettimex64 as it will cause a deadlock. Therefore, we will
just schedule the worker to start immediately.
Fixes: ce3497e2072e ("net: mvpp2: ptp: add support for receive timestamping")
Signed-off-by: Shmuel Hazan <[email protected]>
---
.../net/ethernet/marvell/mvpp2/mvpp2_tai.c | 30 ++++++++++++++++---
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c
index 95862aff49f1..2e3d43b1bac1 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c
@@ -61,6 +61,7 @@ struct mvpp2_tai {
u64 period; // nanosecond period in 32.32 fixed point
/* This timestamp is updated every two seconds */
struct timespec64 stamp;
+ u16 poll_worker_refcount;
};
static void mvpp2_tai_modify(void __iomem *reg, u32 mask, u32 set)
@@ -368,18 +369,39 @@ void mvpp22_tai_tstamp(struct mvpp2_tai *tai, u32 tstamp,
hwtstamp->hwtstamp = timespec64_to_ktime(ts);
}
+static void mvpp22_tai_start_unlocked(struct mvpp2_tai *tai)
+{
+ tai->poll_worker_refcount++;
+ if (tai->poll_worker_refcount > 1)
+ return;
+
+ ptp_schedule_worker(tai->ptp_clock, 0);
+}
+
void mvpp22_tai_start(struct mvpp2_tai *tai)
{
- long delay;
+ unsigned long flags;
- delay = mvpp22_tai_aux_work(&tai->caps);
+ spin_lock_irqsave(&tai->lock, flags);
+ mvpp22_tai_start_unlocked(tai);
+ spin_unlock_irqrestore(&tai->lock, flags);
+}
- ptp_schedule_worker(tai->ptp_clock, delay);
+static void mvpp22_tai_stop_unlocked(struct mvpp2_tai *tai)
+{
+ tai->poll_worker_refcount--;
+ if (tai->poll_worker_refcount)
+ return;
+ ptp_cancel_worker_sync(tai->ptp_clock);
}
void mvpp22_tai_stop(struct mvpp2_tai *tai)
{
- ptp_cancel_worker_sync(tai->ptp_clock);
+ unsigned long flags;
+
+ spin_lock_irqsave(&tai->lock, flags);
+ mvpp22_tai_stop_unlocked(tai);
+ spin_unlock_irqrestore(&tai->lock, flags);
}
static void mvpp22_tai_remove(void *priv)
--
2.40.0
On Wed, 19 Apr 2023 18:14:55 +0300 Shmuel Hazan wrote:
> +static void mvpp22_tai_stop_unlocked(struct mvpp2_tai *tai)
> +{
> + tai->poll_worker_refcount--;
> + if (tai->poll_worker_refcount)
> + return;
> + ptp_cancel_worker_sync(tai->ptp_clock);
How can you cancel it _sync() when the work takes the same
lock you're already holding?
https://elixir.bootlin.com/linux/v6.3-rc7/source/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c#L246
> void mvpp22_tai_stop(struct mvpp2_tai *tai)
> {
> - ptp_cancel_worker_sync(tai->ptp_clock);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&tai->lock, flags);
> + mvpp22_tai_stop_unlocked(tai);
--
pw-bot: cr
On Thu, 2023-04-20 at 20:20 -0700, Jakub Kicinski wrote:
> > Caution: This is an external email. Please take care when clicking
> > links or opening attachments.
> >
> >
> > On Wed, 19 Apr 2023 18:14:55 +0300 Shmuel Hazan wrote:
> > > > +static void mvpp22_tai_stop_unlocked(struct mvpp2_tai *tai)
> > > > +{
> > > > + tai->poll_worker_refcount--;
> > > > + if (tai->poll_worker_refcount)
> > > > + return;
> > > > + ptp_cancel_worker_sync(tai->ptp_clock);
> >
> > How can you cancel it _sync() when the work takes the same
> > lock you're already holding?
> >
> > https://elixir.bootlin.com/linux/v6.3-rc7/source/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c#L246
Hi Jakub,
Thanks for finding that. Strange that I have not encountered any
deadlocks while testing; I will apply a fix and resend after testing
it.
> >
> > > > void mvpp22_tai_stop(struct mvpp2_tai *tai)
> > > > {
> > > > - ptp_cancel_worker_sync(tai->ptp_clock);
> > > > + unsigned long flags;
> > > > +
> > > > + spin_lock_irqsave(&tai->lock, flags);
> > > > + mvpp22_tai_stop_unlocked(tai);
> >
> > --
> > pw-bot: cr