Re: [PATCH v2 21/27] coresight: Convert driver messages to dev_dbg

[Robin Murphy]

On 02/05/18 04:55, Kim Phillips wrote:
> On Tue, 1 May 2018 10:10:51 +0100
> Suzuki K Poulose <suzuki.poulose@xxxxxxx> wrote:
>
> > Convert component enable/disable messages from dev_info to dev_dbg.
> > This is required to prevent LOCKDEP splats when operating in perf
> > mode where we could be called with locks held to enable a coresight
>
> Can we see the splats?  Doesn't lockdep turn itself off if it starts
> triggering too many splats?

Without some very careful and robust reasoning for why the condition 
being reported by lockdep could not actually occur in practice, 
"avoiding the splats" is far, far less important than "avoiding the 
potential deadlock that they are reporting".

> > path. If someone wants to really see the messages, they can always
> > enable it at runtime via dynamic_debug.
>
> Won't the splats still occur when the messages are enabled with
> dynamic_debug?
>
> So in effect this patch only tries to mitigate the splats, all the
> while making things harder for regular users that now have to recompile
> their kernels, in exchange for a very small convenience for kernel
> developers that happen to see a splat or two with DEBUG_LOCKDEP set?

FWIW, if "regular users" means people running distro kernels, then 
chances are that they probably have DYNAMIC_DEBUG set already (100% of 
my local sample of 2 - Ubuntu x86_64 and Arch aarch64 - certainly do). 
Either way, though, this particular log spam really does only look 
vaguely useful to people debugging the coresight stack itself, so anyone 
going out of their way to turn it on has surely already gone beyond 
regular use (even if they're just reproducing an issue with additional 
logging at the request of kernel developers, rather than debugging it 
themselves).

Reducing the scope for possible deadlock from the general case to just 
debugging scenarios is certainly not a bad thing, but as you say I think 
we need a closer look at the underlying issue to know whether even 
dev_dbg() is wise.

Robin.