linux-kernel - Re: [RFC PATCH v2 0/1] of: easier debugging for node life cycle issues

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 25 Jan 2018 17:08:56 -0800
From:   Frank Rowand <frowand.list@...il.com>
To:     Tyrel Datwyler <tyreld@...ux.vnet.ibm.com>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>
Cc:     devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Steven Rostedt <rostedt@...dmis.org>,
        linux-renesas-soc@...r.kernel.org,
        Rob Herring <robh+dt@...nel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        linuxppc-dev@...ts.ozlabs.org
Subject: Re: [RFC PATCH v2 0/1] of: easier debugging for node life cycle
 issues

On 01/25/18 15:53, Tyrel Datwyler wrote:
> On 01/25/2018 01:49 PM, Frank Rowand wrote:
>> Hi Wolfram,
>>
>> On 01/25/18 03:03, Steven Rostedt wrote:
>>> On Wed, 24 Jan 2018 22:55:13 -0800
>>> Frank Rowand <frowand.list@...il.com> wrote:
>>>
>>>> Hi Steve,
>>>
>>>>
>>>> Off the top of your head, can you tell me know early in the boot
>>>> process a trace_event can be called and successfully provide the
>>>> data to someone trying to debug early boot issues?
>>>
>>> The trace events are enabled by early_initcall().
>>
>> < snip >
>>
>> This means that ftrace can not be used for the of_node_get(),
>> of_node_put(), and of_node_release() debug info, because
>> these functions are called before early_initcall().  Please
>> use pr_debug() for these functions.
> 
> I would argue that early boot debugging doesn't completely negate the
> usefulness of this tracing infrastructure.

I did not say or imply that it did.  I am pointing out that this
implementation does not meet the needs of other use cases.  And
potentially provides misleading information (or more precisely
misleading lack of information) in some other use cases.


> I get that no information
> is available in the trace up until ftrace is setup by its
> early_initcall, but I still found issues after early boot using this
> patch and I would hope that it would be somewhat obvious if
> references are out of whack once the ftrace data becomes available.
> In the dynamic case on Power we often do reconfig well after boot on
> live systems which produces a lot of reference put/gets. This patch
> made it easy to identify several reference leaks and underflows in
> our attach and detach logic with the added aid of being able to turn
> on the stacktrace for each call in the ftrace data.

Yes, you can get stacktraces relatively easily.  This is the strongest
argument for using ftrace.

My assumption has been that the stack trace is useful for of_node_get()
and of_node_put().  Is there _large_ value to the stack trace for
of_reconfig_notify()?


> Another thought is it would be nice if we could have the best of both
> worlds such that the tracepoints were pr_debugs up until the ftrace
> early_initcall. Or, I suppose we could ifdef it and make the ftrace
> tracepoints a configuration option, such that if it wasn't configured
> we implement the tracepoint functions as pr_debugs. This makes early
> boot an option. Just spit balling ideas.

An overly complex solution.  This is just debug.  Worst case alternative
is that the patches live on, out of tree.  So nope.


> 
> -Tyrel
> 
>>
>> As far as I know, the of_reconfig_notify() could remain an
>> ftrace instrumented function.  But now that the only thing
>> that would be ftrace instrumented is of_reconfig_notify(),
>> I don't see a strong justification for changing the existing
>> pr_debug() calls to an ftrace alternative.  Though I suspect
>> the original author of the patch still might desire to have
>> the "#ifdef DEBUG" surrounding the pr_debug() calls removed
>> since one of his issues was having to recompile his kernel
>> to do his debugging.
>>
>> -Frank
>>
> 
>