Re: [PATCH 2/3] lib/Kconfig.debug: disable LOCK_DEBUGGING_SUPPORT under KMSAN

[Boqun Feng <boqun.feng@gmail.com>]

On Fri, Jun 21, 2024 at 09:23:25AM -0700, Dave Hansen wrote:
> On 6/21/24 02:49, Alexander Potapenko wrote:
> >  config LOCK_DEBUGGING_SUPPORT
> >  	bool
> > -	depends on TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
> > +	depends on TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT && !KMSAN
> >  	default y
> 
> This kinda stinks.  Practically, it'll mean that anyone turning on KMSAN
> will accidentally turn off lockdep.  That's really nasty, especially for
> folks who are turning on debug options left and right to track down
> nasty bugs.
> 
> I'd *MUCH* rather hide KMSAN:
> 
> config KMSAN
>         bool "KMSAN: detector of uninitialized values use"
>         depends on HAVE_ARCH_KMSAN && HAVE_KMSAN_COMPILER
>         depends on DEBUG_KERNEL && !KASAN && !KCSAN
>         depends on !PREEMPT_RT
> +	depends on !LOCKDEP
> 
> Because, frankly, lockdep is way more important than KMSAN.
> 
> But ideally, we'd allow them to coexist somehow.  Have we even discussed
> the problem with the lockdep folks?  For instance, I'd much rather have
> a relaxed lockdep with no checking in pfn_valid() than no lockdep at all.

The only locks used in pfn_valid() are rcu_read_lock_sched(), right? If
so, could you try (don't tell Paul ;-)) replace rcu_read_lock_sched()
with preempt_disable() and rcu_read_unlock_sched() with
preempt_enable()? That would avoid calling into lockdep. If that works
for KMSAN, we can either have a special rcu_read_lock_sched() or call
lockdep_recursion_inc() in instrumented pfn_valid() to disable lockdep
temporarily.

[Cc Paul]

Regards,
Boqun