From: Sebastian Andrzej Siewior <[email protected]> Sent: Wednesday, April 1, 2026 8:15 AM >
Nit: For historical consistency, use "Drivers: hv: vmbus:" as the prefix for the patch "Subject:" line. Same in Patch 2 of this series. Also, any reason not to have copied [email protected]? I know this is pretty much just a Hyper-V thing, but I would have liked to see what the Sashiko AI did with these two patches. :-) > lockdep_hardirq_threaded() is supposed to be used within IRQ core code > and not within drivers. It is not obvious from within the driver, that > this is the only interrupt service routing and that it is not shared > handler. I presume you meant "routine". And what do you mean by "the only interrupt service routine"? And why is the lack of obviousness relevant here? I don't have deep expertise in lockdep, but evidently there's some conclusion to reach and it would have helped me to have it spelled out. > > Replace lockdep_hardirq_threaded() with a lockdep annotation limiting > threaded context on PREEMPT_RT to __vmbus_isr(). Again, I'm not clear what "limiting threaded context" means. But see my additional comment further down. > > Fixes: f8e6343b7a89c ("Drivers: hv: vmbus: Use kthread for vmbus interrupts > on PREEMPT_RT") > Signed-off-by: Sebastian Andrzej Siewior <[email protected]> > --- > drivers/hv/vmbus_drv.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index bc4fc1951ae1c..e44275370ac2a 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -1407,8 +1407,11 @@ void vmbus_isr(void) > if (IS_ENABLED(CONFIG_PREEMPT_RT)) { > vmbus_irqd_wake(); > } else { > - lockdep_hardirq_threaded(); I see two similar occurrences of LD_WAIT_CONFIG in the kernel: __kfree_rcu_sheaf() and adjacent to printk_legacy_allow_spinlock_enter(). Both occurrences have a multi-line comment explaining the "why". I'd like to see a similar comment here so that drive-by readers of the code have some idea of what's going on. My suggestion is something like this: vmbus_isr() always runs at hard IRQ level -- the interrupt is not threaded. It calls __vmbus_isr() here, which may obtain the spinlock_t sched_lock for a VMBus channel in vmbus_chan_sched(). If CONFIG_PROVE_LOCKING=y, lockdep complains because obtaining spinlock_t's is not permitted at hard IRQ level in PREEMPT_RT configurations. However, the PREEMPT_RT path is handled separately above, so there's actually not a problem. Tell lockdep that acquiring the spinlock_t is valid by temporarily raising the wait-type to LD_WAIT_CONFIG using the "fake" lock vmbus_map. If lockdep is not enabled, the acquire & release of the fake lock are no-ops, so performance is not impacted. Please review my suggested text and revise as appropriate, as I'm far from an expert on any of this. The above is based on what I've learned just now from a bit of research. And thanks for jumping in and making all this better .... Michael > + static DEFINE_WAIT_OVERRIDE_MAP(vmbus_map, LD_WAIT_CONFIG); > + > + lock_map_acquire_try(&vmbus_map); > __vmbus_isr(); > + lock_map_release(&vmbus_map); > } > } > EXPORT_SYMBOL_FOR_MODULES(vmbus_isr, "mshv_vtl"); > -- > 2.53.0
