Re: [PATCH v6 3/3] pps: convert pps_device.lock to raw_spinlock_t

[Sebastian Andrzej Siewior]

On 2026-05-28 08:57:37 [-0700], Calvin Owens wrote:
…
> > This does not work because the commit message claims to allow
> > pps_event() to be called from hardirq context. So if this is called
> > indeed from hardirq, context as claimed in the commit message, then
> > dropping the lock early does not help here.
> 
> It isn't called from hardirq AFAICS.
> 
> > Does it need to call pps_event() in hardirq? Patch #1 takes the
> > timestamp in hardirq deferring the remaining part to the thread. What is
> > the crucial part here that needs to happen in hardirq context?
> 
> This is the artificial testcase which just uses a ktimer, it both
> "samples" the time and calls pps_event() from the same timer callback.
> 
> But I understood you to be saying it's threaded above, which mirrors my
> own understanding. If that's true, I don't see why dropping the lock
> early wouldn't be sufficient here?

So lets look at the series:
- #1 splits the handler to take time stamp in hardirq and the remaining
  work in the thread. It seems to be crucial to do so because doing so
  in thread breaks something. Okay. Granted, makes a bit of sense.

- #2 ignore for argument's sake

- #3 says swap the lock so we can use pps_event() in hardirq context.

Now. Why or where do we need to use pps_event() in hardirq context?
If we use pps_event() is used in hardirq context, as claimed in the
commit message, then dropping the lock early here does not help and it
will lead to the same splat in wake_up_interruptible_all() because
interrupts are still disabled independent of the lock here. It is
hardirq-context after all.

You don't see this warning in your testcase because it gets here from a
timer, yes. But. pps_event() can't be used as-is from hardirq context
either.

> This is what lockdep says on v6:
> 
>     =============================
>     [ BUG: Invalid wait context ]
…
>     -----------------------------
>     ktimers/0/15 is trying to lock:
>     ffff888107303118 (&pps->queue){....}-{3:3}, at: __wake_up+0x1f/0x50
>     other info that might help us debug this:
…
> If I drop the lock early, lockdep and atomic_sleep are both slient.

Yes and might_sleep() would yell, too.

> Thanks,
> Calvin

Sebastian