Re: [PATCH v5 0/3] pps: improve PREEMPT_RT performance

Hi Sebastian,

Quick update: v7 is on the way, addressing your v5 feedback plus
a separate bug Calvin Owens found in v6 during testing with
CONFIG_DEBUG_ATOMIC_SLEEP.

The new bug: pps_event() in kapi.c was holding pps->lock (now
raw_spinlock_t) across wake_up_interruptible_all() and
kill_fasync(), both of which internally take regular spinlock_t
locks that become rt_mutexes on PREEMPT_RT. This is the same
illegal-nesting pattern Nikolaus Buchwitz caught for the pps_kc
path between v2 and v3 -- I should have audited pps_event() for
the same issue back then and didn't. Patch 3/3 in v7 moves the
unlock above the wakeup calls.

Your v5 feedback is fully addressed:

  1/3: commit message reworded to describe the split structurally
       first, then the PREEMPT_RT benefit.

  2/3: pps_kc_hardpps_lock conversion, now ordered before the
       pps_device.lock conversion as you noted. While here, also
       refactored pps_kc_bind() and pps_kc_remove() to use
       guard(raw_spinlock_irq), eliminating the awkward bracket
       placement you flagged.

  3/3: pps_device.lock conversion, now with Calvin's wake-up
       reordering. Rodolfo's Acked-by is dropped on this patch
       since the change is substantive; hoping he'll re-ack.

Calvin is relaying v7 the same way he relayed v6.

Thanks again for the review,
Michael



> On 23. May 2026, at 19:58, Michael Byczkowski <[email protected]> wrote:
> 
> Hi Sebastian,
> 
> Thanks very much for the review.
> 
> On 22 May 2026, at 16:13, Sebastian Andrzej Siewior wrote:
>> Was there a follow-up to the series?
> 
> Yes: v6 is on the way. Calvin Owens kindly offered to relay it for
> me (Apple Mail has been corrupting my patches in transit; I'm
> addressing that separately), so the v6 series will arrive from him
> shortly.
> 
> A summary of how I've addressed your v5 comments:
> 
> 1/3 (pps-gpio handler split): commit message reworded. The new
>   message describes the split structurally first -- primary hardirq
>   handler that captures the timestamp, threaded handler that
>   processes the event -- and then describes the PREEMPT_RT benefit
>   second (no more scheduling delay between IRQ entry and timestamp
>   capture). The patch itself is unchanged.
> 
> 2/3 (pps_kc_hardpps_lock -> raw_spinlock_t): I'd appreciate a quick
>   clarification here. You wrote that pps_kc_hardpps_lock "is
>   spinlock_t in my tree". My patch is against mainline at
>   0d9363a764d9 ("Input: xpad - add support for BETOP BTP-KP50B/C
>   controller's wireless mode"), where it is still DEFINE_SPINLOCK.
>   Could you point me at the branch you were looking at, so I can
>   check whether v6 needs to rebase on top of an existing conversion
>   in your tree?
> 
>   Separately: I've also taken your suggestion and refactored
>   pps_kc_bind() (and pps_kc_remove() while I was in the file) to use
>   guard(raw_spinlock_irq). That eliminates the four duplicated unlock
>   call sites in pps_kc_bind() and removes the ambiguous bracket
>   nesting. Thank you for that pointer.
> 
> 3/3 (pps_device.lock -> raw_spinlock_t): reordered so this comes
>   after the kc_hardpps conversion (which now becomes patch 2/3 in
>   v6), as you noted -- the previous ordering would briefly produce
>   a raw_spinlock holding a sleeping spinlock on PREEMPT_RT. Thank
>   you for catching that.
> 
> Also: thank you for auditing pps_gpio_echo() on my behalf, much
> appreciated.
> 
> Best regards,
> Michael
>