Re: [RFC PATCH v2 1/1] of: introduce event tracepoints for dynamic device_node lifecyle

[Frank Rowand]

On 01/25/18 14:40, Tyrel Datwyler wrote:
> On 01/24/2018 10:48 PM, Frank Rowand wrote:
>> On 01/21/18 06:31, Wolfram Sang wrote:
>>> From: Tyrel Datwyler 
>>>
>>> This patch introduces event tracepoints for tracking a device_nodes
>>> reference cycle as well as reconfig notifications generated in response
>>> to node/property manipulations.
>>>
>>> With the recent upstreaming of the refcount API several device_node
>>> underflows and leaks have come to my attention in the pseries (DLPAR)
>>> dynamic logical partitioning code (ie. POWER speak for hotplugging
>>> virtual and physcial resources at runtime such as cpus or IOAs). These
>>> tracepoints provide a easy and quick mechanism for validating the
>>> reference counting of device_nodes during their lifetime.
>>>
>>> Further, when pseries lpars are migrated to a different machine we
>>> perform a live update of our device tree to bring it into alignment with
>>> the configuration of the new machine. The of_reconfig_notify trace point
>>> provides a mechanism that can be turned for debuging the device tree
>>> modifications with out having to build a custom kernel to get at the
>>> DEBUG code introduced by commit 00aa37206e1a54 ("of/reconfig: Add debug
>>> output for OF_RECONFIG notifiers").
>>>
>>> The following trace events are provided: of_node_get, of_node_put,
>>> of_node_release, and of_reconfig_notify. These trace points require a
>>
>> Please add a note that the of_reconfig_notify trace event is not an
>> added bit of debug info, but is instead replacing information that
>> was previously available via pr_debug() when DEBUG was defined.
>>
>>
>>> kernel built with ftrace support to be enabled. In a typical environment
>>> where debugfs is mounted at /sys/kernel/debug the entire set of
>>> tracepoints can be set with the following:
>>>
>>>   echo "of:*" > /sys/kernel/debug/tracing/set_event
>>>
>>> or
>>>
>>>   echo 1 > /sys/kernel/debug/tracing/events/of/enable
>>>
>>> The following shows the trace point data from a DLPAR remove of a cpu
>>> from a pseries lpar:
>>>
>>> cat /sys/kernel/debug/tracing/trace | grep "POWER8@10"
>>>
>>> cpuhp/23-147   [023]    128.324827:
>>> of_node_put: refcount=5, dn->full_name=/cpus/PowerPC,POWER8@10
>>> cpuhp/23-147   [023]    128.324829:
>>> of_node_put: refcount=4, dn->full_name=/cpus/PowerPC,POWER8@10
>>> cpuhp/23-147   [023]    128.324829:
>>> of_node_put: refcount=3, dn->full_name=/cpus/PowerPC,POWER8@10
>>> cpuhp/23-147   [023]    128.324831:
>>> of_node_put: refcount=2, dn->full_name=/cpus/PowerPC,POWER8@10
>>>drmgr-7284  [009]    128.439000:
>>> of_node_put: refcount=1, dn->full_name=/cpus/PowerPC,POWER8@10
>>>drmgr-7284  [009]    128.439002:
>>> of_reconfig_notify: action=DETACH_NODE, 
>>> dn->full_name=/cpus/PowerPC,POWER8@10,
>>> prop->name=null, old_prop->name=null
>>>drmgr-7284  [009]    128.439015:
>>> of_node_put: refcount=0, dn->full_name=/cpus/PowerPC,POWER8@10
>>>drmgr-7284  [009]    128.439016:
>>> of_node_release: dn->full_name=/cpus/PowerPC,POWER8@10, dn->_flags=4
>>>
>>> Signed-off-by: Tyrel Datwyler 
>>
>> The following belongs in a list of version 2 changes, below the "---" line:
>>
>>> [wsa: fixed commit abbrev and one of the sysfs paths in commit desc,
>>> removed trailing space and fixed pointer declaration in code]
>>
>>> Signed-off-by: Wolfram Sang 
>>> ---
>>>  drivers/of/dynamic.c  | 32 ++--
>>>  include/trace/events/of.h | 93 
>>> +++
>>>  2 files changed, 105 insertions(+), 20 deletions(-)
>>>  create mode 100644 include/trace/events/of.h
>>
>> mode looks incorrect.  Existing files in include/trace/events/ are -rw-rw
>>
>>
>>> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
>>> index ab988d88704da0..b0d6ab5a35b8c6 100644
>>> --- a/drivers/of/dynamic.c
>>> +++ b/drivers/of/dynamic.c
>>> @@ -21,6 +21,9 @@ static struct device_node *kobj_to_device_node(struct 
>>> kobject *kobj)
>>> return container_of(kobj, struct device_node, kobj);
>>>  }
>>>  
>>> +#define CREATE_TRACE_POINTS
>>> +#include 
>>> +
>>>  /**
>>>   * of_node_get() - Increment refcount of a node
>>>   * @node:  Node to inc refcount, NULL is supported to simplify writing of
>>> @@ -30,8 +33,10 @@ static struct device_node *kobj_to_device_node(struct 
>>> kobject *kobj)
>>>   */
>>>  struct device_node *of_node_get(struct device_node *node)
>>>  {
>>> -   if (node)
>>> +   if (node) {
>>> kobject_get(>kobj);
>>> +   trace_of_node_get(refcount_read(>kobj.kref.refcount), 
>>> node->full_name);
>>
>> See the comment from Ron that I mentioned in my previous email.
>>
>> Also, the path has been removed from node->full_name.  Does using it here
>> still give all of the information that is desired?  Same for 

Re: [RFC PATCH v2 1/1] of: introduce event tracepoints for dynamic device_node lifecyle

[Tyrel Datwyler]

On 01/24/2018 10:48 PM, Frank Rowand wrote:
> On 01/21/18 06:31, Wolfram Sang wrote:
>> From: Tyrel Datwyler 
>>
>> This patch introduces event tracepoints for tracking a device_nodes
>> reference cycle as well as reconfig notifications generated in response
>> to node/property manipulations.
>>
>> With the recent upstreaming of the refcount API several device_node
>> underflows and leaks have come to my attention in the pseries (DLPAR)
>> dynamic logical partitioning code (ie. POWER speak for hotplugging
>> virtual and physcial resources at runtime such as cpus or IOAs). These
>> tracepoints provide a easy and quick mechanism for validating the
>> reference counting of device_nodes during their lifetime.
>>
>> Further, when pseries lpars are migrated to a different machine we
>> perform a live update of our device tree to bring it into alignment with
>> the configuration of the new machine. The of_reconfig_notify trace point
>> provides a mechanism that can be turned for debuging the device tree
>> modifications with out having to build a custom kernel to get at the
>> DEBUG code introduced by commit 00aa37206e1a54 ("of/reconfig: Add debug
>> output for OF_RECONFIG notifiers").
>>
>> The following trace events are provided: of_node_get, of_node_put,
>> of_node_release, and of_reconfig_notify. These trace points require a
> 
> Please add a note that the of_reconfig_notify trace event is not an
> added bit of debug info, but is instead replacing information that
> was previously available via pr_debug() when DEBUG was defined.
> 
> 
>> kernel built with ftrace support to be enabled. In a typical environment
>> where debugfs is mounted at /sys/kernel/debug the entire set of
>> tracepoints can be set with the following:
>>
>>   echo "of:*" > /sys/kernel/debug/tracing/set_event
>>
>> or
>>
>>   echo 1 > /sys/kernel/debug/tracing/events/of/enable
>>
>> The following shows the trace point data from a DLPAR remove of a cpu
>> from a pseries lpar:
>>
>> cat /sys/kernel/debug/tracing/trace | grep "POWER8@10"
>>
>> cpuhp/23-147   [023]    128.324827:
>> of_node_put: refcount=5, dn->full_name=/cpus/PowerPC,POWER8@10
>> cpuhp/23-147   [023]    128.324829:
>> of_node_put: refcount=4, dn->full_name=/cpus/PowerPC,POWER8@10
>> cpuhp/23-147   [023]    128.324829:
>> of_node_put: refcount=3, dn->full_name=/cpus/PowerPC,POWER8@10
>> cpuhp/23-147   [023]    128.324831:
>> of_node_put: refcount=2, dn->full_name=/cpus/PowerPC,POWER8@10
>>drmgr-7284  [009]    128.439000:
>> of_node_put: refcount=1, dn->full_name=/cpus/PowerPC,POWER8@10
>>drmgr-7284  [009]    128.439002:
>> of_reconfig_notify: action=DETACH_NODE, 
>> dn->full_name=/cpus/PowerPC,POWER8@10,
>> prop->name=null, old_prop->name=null
>>drmgr-7284  [009]    128.439015:
>> of_node_put: refcount=0, dn->full_name=/cpus/PowerPC,POWER8@10
>>drmgr-7284  [009]    128.439016:
>> of_node_release: dn->full_name=/cpus/PowerPC,POWER8@10, dn->_flags=4
>>
>> Signed-off-by: Tyrel Datwyler 
> 
> The following belongs in a list of version 2 changes, below the "---" line:
> 
>> [wsa: fixed commit abbrev and one of the sysfs paths in commit desc,
>> removed trailing space and fixed pointer declaration in code]
> 
>> Signed-off-by: Wolfram Sang 
>> ---
>>  drivers/of/dynamic.c  | 32 ++--
>>  include/trace/events/of.h | 93 
>> +++
>>  2 files changed, 105 insertions(+), 20 deletions(-)
>>  create mode 100644 include/trace/events/of.h
> 
> mode looks incorrect.  Existing files in include/trace/events/ are -rw-rw
> 
> 
>> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
>> index ab988d88704da0..b0d6ab5a35b8c6 100644
>> --- a/drivers/of/dynamic.c
>> +++ b/drivers/of/dynamic.c
>> @@ -21,6 +21,9 @@ static struct device_node *kobj_to_device_node(struct 
>> kobject *kobj)
>>  return container_of(kobj, struct device_node, kobj);
>>  }
>>  
>> +#define CREATE_TRACE_POINTS
>> +#include 
>> +
>>  /**
>>   * of_node_get() - Increment refcount of a node
>>   * @node:   Node to inc refcount, NULL is supported to simplify writing of
>> @@ -30,8 +33,10 @@ static struct device_node *kobj_to_device_node(struct 
>> kobject *kobj)
>>   */
>>  struct device_node *of_node_get(struct device_node *node)
>>  {
>> -if (node)
>> +if (node) {
>>  kobject_get(>kobj);
>> +trace_of_node_get(refcount_read(>kobj.kref.refcount), 
>> node->full_name);
> 
> See the comment from Ron that I mentioned in my previous email.
> 
> Also, the path has been removed from node->full_name.  Does using it here
> still give all of the information that is desired?  Same for all others uses
> of full_name in this patch.
> 
> The trace point should have a single argument, node.  Accessing the two
> fields can be 

Re: [RFC PATCH v2 1/1] of: introduce event tracepoints for dynamic device_node lifecyle

[Frank Rowand]

On 01/21/18 06:31, Wolfram Sang wrote:
> From: Tyrel Datwyler 
> 
> This patch introduces event tracepoints for tracking a device_nodes
> reference cycle as well as reconfig notifications generated in response
> to node/property manipulations.
> 
> With the recent upstreaming of the refcount API several device_node
> underflows and leaks have come to my attention in the pseries (DLPAR)
> dynamic logical partitioning code (ie. POWER speak for hotplugging
> virtual and physcial resources at runtime such as cpus or IOAs). These
> tracepoints provide a easy and quick mechanism for validating the
> reference counting of device_nodes during their lifetime.
> 
> Further, when pseries lpars are migrated to a different machine we
> perform a live update of our device tree to bring it into alignment with
> the configuration of the new machine. The of_reconfig_notify trace point
> provides a mechanism that can be turned for debuging the device tree
> modifications with out having to build a custom kernel to get at the
> DEBUG code introduced by commit 00aa37206e1a54 ("of/reconfig: Add debug
> output for OF_RECONFIG notifiers").
> 
> The following trace events are provided: of_node_get, of_node_put,
> of_node_release, and of_reconfig_notify. These trace points require a
> kernel built with ftrace support to be enabled. In a typical environment
> where debugfs is mounted at /sys/kernel/debug the entire set of
> tracepoints can be set with the following:
> 
>   echo "of:*" > /sys/kernel/debug/tracing/set_event
> 
> or
> 
>   echo 1 > /sys/kernel/debug/tracing/events/of/enable
> 
> The following shows the trace point data from a DLPAR remove of a cpu
> from a pseries lpar:
> 
> cat /sys/kernel/debug/tracing/trace | grep "POWER8@10"
> 
> cpuhp/23-147   [023]    128.324827:
> of_node_put: refcount=5, dn->full_name=/cpus/PowerPC,POWER8@10
> cpuhp/23-147   [023]    128.324829:
> of_node_put: refcount=4, dn->full_name=/cpus/PowerPC,POWER8@10
> cpuhp/23-147   [023]    128.324829:
> of_node_put: refcount=3, dn->full_name=/cpus/PowerPC,POWER8@10
> cpuhp/23-147   [023]    128.324831:
> of_node_put: refcount=2, dn->full_name=/cpus/PowerPC,POWER8@10
>drmgr-7284  [009]    128.439000:
> of_node_put: refcount=1, dn->full_name=/cpus/PowerPC,POWER8@10
>drmgr-7284  [009]    128.439002:
> of_reconfig_notify: action=DETACH_NODE, 
> dn->full_name=/cpus/PowerPC,POWER8@10,
> prop->name=null, old_prop->name=null
>drmgr-7284  [009]    128.439015:
> of_node_put: refcount=0, dn->full_name=/cpus/PowerPC,POWER8@10
>drmgr-7284  [009]    128.439016:
> of_node_release: dn->full_name=/cpus/PowerPC,POWER8@10, dn->_flags=4
> 
> Signed-off-by: Tyrel Datwyler 
> [wsa: fixed commit abbrev and one of the sysfs paths in commit desc,
> removed trailing space and fixed pointer declaration in code]
> Signed-off-by: Wolfram Sang 
> ---
>  drivers/of/dynamic.c  | 32 ++--
>  include/trace/events/of.h | 93 
> +++
>  2 files changed, 105 insertions(+), 20 deletions(-)
>  create mode 100644 include/trace/events/of.h
> 
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index ab988d88704da0..b0d6ab5a35b8c6 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -21,6 +21,9 @@ static struct device_node *kobj_to_device_node(struct 
> kobject *kobj)
>   return container_of(kobj, struct device_node, kobj);
>  }
>  
> +#define CREATE_TRACE_POINTS
> +#include 
> +
>  /**
>   * of_node_get() - Increment refcount of a node
>   * @node:Node to inc refcount, NULL is supported to simplify writing of
> @@ -30,8 +33,10 @@ static struct device_node *kobj_to_device_node(struct 
> kobject *kobj)
>   */
>  struct device_node *of_node_get(struct device_node *node)
>  {
> - if (node)
> + if (node) {
>   kobject_get(>kobj);
> + trace_of_node_get(refcount_read(>kobj.kref.refcount), 
> node->full_name);
> + }
>   return node;
>  }
>  EXPORT_SYMBOL(of_node_get);
> @@ -43,8 +48,10 @@ EXPORT_SYMBOL(of_node_get);
>   */
>  void of_node_put(struct device_node *node)
>  {
> - if (node)
> + if (node) {
> + trace_of_node_put(refcount_read(>kobj.kref.refcount) - 1, 
> node->full_name);
>   kobject_put(>kobj);
> + }
>  }
>  EXPORT_SYMBOL(of_node_put);
>  
> @@ -75,24 +82,7 @@ const char *action_names[] = {
>  int of_reconfig_notify(unsigned long action, struct of_reconfig_data *p)
>  {
>   int rc;
> -#ifdef DEBUG
> - struct of_reconfig_data *pr = p;
> -
> - switch (action) {
> - case OF_RECONFIG_ATTACH_NODE:
> - case OF_RECONFIG_DETACH_NODE:
> - pr_debug("notify %-15s %pOF\n", action_names[action],
> - pr->dn);
> - break;
> - case 

Re: [RFC PATCH v2 1/1] of: introduce event tracepoints for dynamic device_node lifecyle

[Frank Rowand]

On 01/25/18 00:01, Geert Uytterhoeven wrote:
> Hi Frank,
> 
> On Thu, Jan 25, 2018 at 7:48 AM, Frank Rowand  wrote:
>>>  create mode 100644 include/trace/events/of.h
>>
>> mode looks incorrect.  Existing files in include/trace/events/ are -rw-rw
> 
> Not in my git clone ;-) 644 should be fine.
> 
> Gr{oetje,eeting}s,
> 
> Geert

Thanks!!!  I just learned something new about git.  Now I have to go experiment
a bit to make sure I learned properly.

-Frank

Re: [RFC PATCH v2 1/1] of: introduce event tracepoints for dynamic device_node lifecyle

[Geert Uytterhoeven]

Hi Frank,

On Thu, Jan 25, 2018 at 7:48 AM, Frank Rowand  wrote:
>>  create mode 100644 include/trace/events/of.h
>
> mode looks incorrect.  Existing files in include/trace/events/ are -rw-rw

Not in my git clone ;-) 644 should be fine.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [RFC PATCH v2 1/1] of: introduce event tracepoints for dynamic device_node lifecyle

[Frank Rowand]

On 01/24/18 22:48, Frank Rowand wrote:
> On 01/21/18 06:31, Wolfram Sang wrote:
>> From: Tyrel Datwyler 
>>
>> This patch introduces event tracepoints for tracking a device_nodes
>> reference cycle as well as reconfig notifications generated in response
>> to node/property manipulations.
>>
>> With the recent upstreaming of the refcount API several device_node
>> underflows and leaks have come to my attention in the pseries (DLPAR)
>> dynamic logical partitioning code (ie. POWER speak for hotplugging
>> virtual and physcial resources at runtime such as cpus or IOAs). These
>> tracepoints provide a easy and quick mechanism for validating the
>> reference counting of device_nodes during their lifetime.
>>
>> Further, when pseries lpars are migrated to a different machine we
>> perform a live update of our device tree to bring it into alignment with
>> the configuration of the new machine. The of_reconfig_notify trace point
>> provides a mechanism that can be turned for debuging the device tree
>> modifications with out having to build a custom kernel to get at the
>> DEBUG code introduced by commit 00aa37206e1a54 ("of/reconfig: Add debug
>> output for OF_RECONFIG notifiers").
>>
>> The following trace events are provided: of_node_get, of_node_put,
>> of_node_release, and of_reconfig_notify. These trace points require a
> 
> Please add a note that the of_reconfig_notify trace event is not an
> added bit of debug info, but is instead replacing information that
> was previously available via pr_debug() when DEBUG was defined.

I got a little carried away, "when DEBUG was defined" is extra
un-needed detail for the commit message.


> 
> 
>> kernel built with ftrace support to be enabled. In a typical environment
>> where debugfs is mounted at /sys/kernel/debug the entire set of
>> tracepoints can be set with the following:
>>
>>   echo "of:*" > /sys/kernel/debug/tracing/set_event
>>
>> or
>>
>>   echo 1 > /sys/kernel/debug/tracing/events/of/enable
>>
>> The following shows the trace point data from a DLPAR remove of a cpu
>> from a pseries lpar:
>>
>> cat /sys/kernel/debug/tracing/trace | grep "POWER8@10"
>>
>> cpuhp/23-147   [023]    128.324827:
>> of_node_put: refcount=5, dn->full_name=/cpus/PowerPC,POWER8@10
>> cpuhp/23-147   [023]    128.324829:
>> of_node_put: refcount=4, dn->full_name=/cpus/PowerPC,POWER8@10
>> cpuhp/23-147   [023]    128.324829:
>> of_node_put: refcount=3, dn->full_name=/cpus/PowerPC,POWER8@10
>> cpuhp/23-147   [023]    128.324831:
>> of_node_put: refcount=2, dn->full_name=/cpus/PowerPC,POWER8@10
>>drmgr-7284  [009]    128.439000:
>> of_node_put: refcount=1, dn->full_name=/cpus/PowerPC,POWER8@10
>>drmgr-7284  [009]    128.439002:
>> of_reconfig_notify: action=DETACH_NODE, 
>> dn->full_name=/cpus/PowerPC,POWER8@10,
>> prop->name=null, old_prop->name=null
>>drmgr-7284  [009]    128.439015:
>> of_node_put: refcount=0, dn->full_name=/cpus/PowerPC,POWER8@10
>>drmgr-7284  [009]    128.439016:
>> of_node_release: dn->full_name=/cpus/PowerPC,POWER8@10, dn->_flags=4
>>
>> Signed-off-by: Tyrel Datwyler 
> 
> The following belongs in a list of version 2 changes, below the "---" line:
> 
>> [wsa: fixed commit abbrev and one of the sysfs paths in commit desc,
>> removed trailing space and fixed pointer declaration in code]
> 
>> Signed-off-by: Wolfram Sang 
>> ---
>>  drivers/of/dynamic.c  | 32 ++--
>>  include/trace/events/of.h | 93 
>> +++
>>  2 files changed, 105 insertions(+), 20 deletions(-)
>>  create mode 100644 include/trace/events/of.h
> 
> mode looks incorrect.  Existing files in include/trace/events/ are -rw-rw
> 
> 
>> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
>> index ab988d88704da0..b0d6ab5a35b8c6 100644
>> --- a/drivers/of/dynamic.c
>> +++ b/drivers/of/dynamic.c
>> @@ -21,6 +21,9 @@ static struct device_node *kobj_to_device_node(struct 
>> kobject *kobj)
>>  return container_of(kobj, struct device_node, kobj);
>>  }
>>  
>> +#define CREATE_TRACE_POINTS
>> +#include 
>> +
>>  /**
>>   * of_node_get() - Increment refcount of a node
>>   * @node:   Node to inc refcount, NULL is supported to simplify writing of
>> @@ -30,8 +33,10 @@ static struct device_node *kobj_to_device_node(struct 
>> kobject *kobj)
>>   */
>>  struct device_node *of_node_get(struct device_node *node)
>>  {
>> -if (node)
>> +if (node) {
>>  kobject_get(>kobj);
>> +trace_of_node_get(refcount_read(>kobj.kref.refcount), 
>> node->full_name);
> 
> See the comment from Ron that I mentioned in my previous email.
   
   Rob, darn it.


> Also, the path has been removed from node->full_name.  Does using it here
> still give all of the information 

Re: [RFC PATCH v2 1/1] of: introduce event tracepoints for dynamic device_node lifecyle

[Frank Rowand]

On 01/21/18 06:31, Wolfram Sang wrote:
> From: Tyrel Datwyler 
> 
> This patch introduces event tracepoints for tracking a device_nodes
> reference cycle as well as reconfig notifications generated in response
> to node/property manipulations.
> 
> With the recent upstreaming of the refcount API several device_node
> underflows and leaks have come to my attention in the pseries (DLPAR)
> dynamic logical partitioning code (ie. POWER speak for hotplugging
> virtual and physcial resources at runtime such as cpus or IOAs). These
> tracepoints provide a easy and quick mechanism for validating the
> reference counting of device_nodes during their lifetime.
> 
> Further, when pseries lpars are migrated to a different machine we
> perform a live update of our device tree to bring it into alignment with
> the configuration of the new machine. The of_reconfig_notify trace point
> provides a mechanism that can be turned for debuging the device tree
> modifications with out having to build a custom kernel to get at the
> DEBUG code introduced by commit 00aa37206e1a54 ("of/reconfig: Add debug
> output for OF_RECONFIG notifiers").
> 
> The following trace events are provided: of_node_get, of_node_put,
> of_node_release, and of_reconfig_notify. These trace points require a

Please add a note that the of_reconfig_notify trace event is not an
added bit of debug info, but is instead replacing information that
was previously available via pr_debug() when DEBUG was defined.


> kernel built with ftrace support to be enabled. In a typical environment
> where debugfs is mounted at /sys/kernel/debug the entire set of
> tracepoints can be set with the following:
> 
>   echo "of:*" > /sys/kernel/debug/tracing/set_event
> 
> or
> 
>   echo 1 > /sys/kernel/debug/tracing/events/of/enable
> 
> The following shows the trace point data from a DLPAR remove of a cpu
> from a pseries lpar:
> 
> cat /sys/kernel/debug/tracing/trace | grep "POWER8@10"
> 
> cpuhp/23-147   [023]    128.324827:
> of_node_put: refcount=5, dn->full_name=/cpus/PowerPC,POWER8@10
> cpuhp/23-147   [023]    128.324829:
> of_node_put: refcount=4, dn->full_name=/cpus/PowerPC,POWER8@10
> cpuhp/23-147   [023]    128.324829:
> of_node_put: refcount=3, dn->full_name=/cpus/PowerPC,POWER8@10
> cpuhp/23-147   [023]    128.324831:
> of_node_put: refcount=2, dn->full_name=/cpus/PowerPC,POWER8@10
>drmgr-7284  [009]    128.439000:
> of_node_put: refcount=1, dn->full_name=/cpus/PowerPC,POWER8@10
>drmgr-7284  [009]    128.439002:
> of_reconfig_notify: action=DETACH_NODE, 
> dn->full_name=/cpus/PowerPC,POWER8@10,
> prop->name=null, old_prop->name=null
>drmgr-7284  [009]    128.439015:
> of_node_put: refcount=0, dn->full_name=/cpus/PowerPC,POWER8@10
>drmgr-7284  [009]    128.439016:
> of_node_release: dn->full_name=/cpus/PowerPC,POWER8@10, dn->_flags=4
> 
> Signed-off-by: Tyrel Datwyler 

The following belongs in a list of version 2 changes, below the "---" line:

> [wsa: fixed commit abbrev and one of the sysfs paths in commit desc,
> removed trailing space and fixed pointer declaration in code]

> Signed-off-by: Wolfram Sang 
> ---
>  drivers/of/dynamic.c  | 32 ++--
>  include/trace/events/of.h | 93 
> +++
>  2 files changed, 105 insertions(+), 20 deletions(-)
>  create mode 100644 include/trace/events/of.h

mode looks incorrect.  Existing files in include/trace/events/ are -rw-rw


> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index ab988d88704da0..b0d6ab5a35b8c6 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -21,6 +21,9 @@ static struct device_node *kobj_to_device_node(struct 
> kobject *kobj)
>   return container_of(kobj, struct device_node, kobj);
>  }
>  
> +#define CREATE_TRACE_POINTS
> +#include 
> +
>  /**
>   * of_node_get() - Increment refcount of a node
>   * @node:Node to inc refcount, NULL is supported to simplify writing of
> @@ -30,8 +33,10 @@ static struct device_node *kobj_to_device_node(struct 
> kobject *kobj)
>   */
>  struct device_node *of_node_get(struct device_node *node)
>  {
> - if (node)
> + if (node) {
>   kobject_get(>kobj);
> + trace_of_node_get(refcount_read(>kobj.kref.refcount), 
> node->full_name);

See the comment from Ron that I mentioned in my previous email.

Also, the path has been removed from node->full_name.  Does using it here
still give all of the information that is desired?  Same for all others uses
of full_name in this patch.

The trace point should have a single argument, node.  Accessing the two
fields can be done in the tracepoint assignment.  Or is there some
reason that can't be done?  Same for the trace_of_node_put() tracepoint.


> + }
>   return node;
>  }
>  

Re: [RFC PATCH v2 1/1] of: introduce event tracepoints for dynamic device_node lifecyle

[Steven Rostedt]

On Sun, 21 Jan 2018 15:31:17 +0100
Wolfram Sang  wrote:

> From: Tyrel Datwyler 
> 
> This patch introduces event tracepoints for tracking a device_nodes
> reference cycle as well as reconfig notifications generated in response
> to node/property manipulations.
> 
> With the recent upstreaming of the refcount API several device_node
> underflows and leaks have come to my attention in the pseries (DLPAR)
> dynamic logical partitioning code (ie. POWER speak for hotplugging
> virtual and physcial resources at runtime such as cpus or IOAs). These
> tracepoints provide a easy and quick mechanism for validating the
> reference counting of device_nodes during their lifetime.
> 
> Further, when pseries lpars are migrated to a different machine we
> perform a live update of our device tree to bring it into alignment with
> the configuration of the new machine. The of_reconfig_notify trace point
> provides a mechanism that can be turned for debuging the device tree
> modifications with out having to build a custom kernel to get at the
> DEBUG code introduced by commit 00aa37206e1a54 ("of/reconfig: Add debug
> output for OF_RECONFIG notifiers").
> 
> The following trace events are provided: of_node_get, of_node_put,
> of_node_release, and of_reconfig_notify. These trace points require a
> kernel built with ftrace support to be enabled. In a typical environment
> where debugfs is mounted at /sys/kernel/debug the entire set of
> tracepoints can be set with the following:
> 
>   echo "of:*" > /sys/kernel/debug/tracing/set_event
> 
> or
> 
>   echo 1 > /sys/kernel/debug/tracing/events/of/enable
> 
> The following shows the trace point data from a DLPAR remove of a cpu
> from a pseries lpar:
> 
> cat /sys/kernel/debug/tracing/trace | grep "POWER8@10"
> 
> cpuhp/23-147   [023]    128.324827:
> of_node_put: refcount=5, dn->full_name=/cpus/PowerPC,POWER8@10
> cpuhp/23-147   [023]    128.324829:
> of_node_put: refcount=4, dn->full_name=/cpus/PowerPC,POWER8@10
> cpuhp/23-147   [023]    128.324829:
> of_node_put: refcount=3, dn->full_name=/cpus/PowerPC,POWER8@10
> cpuhp/23-147   [023]    128.324831:
> of_node_put: refcount=2, dn->full_name=/cpus/PowerPC,POWER8@10
>drmgr-7284  [009]    128.439000:
> of_node_put: refcount=1, dn->full_name=/cpus/PowerPC,POWER8@10
>drmgr-7284  [009]    128.439002:
> of_reconfig_notify: action=DETACH_NODE, 
> dn->full_name=/cpus/PowerPC,POWER8@10,
> prop->name=null, old_prop->name=null
>drmgr-7284  [009]    128.439015:
> of_node_put: refcount=0, dn->full_name=/cpus/PowerPC,POWER8@10
>drmgr-7284  [009]    128.439016:
> of_node_release: dn->full_name=/cpus/PowerPC,POWER8@10, dn->_flags=4
> 
> Signed-off-by: Tyrel Datwyler 
> [wsa: fixed commit abbrev and one of the sysfs paths in commit desc,
> removed trailing space and fixed pointer declaration in code]
> Signed-off-by: Wolfram Sang 

From the tracing perspective:

Reviewed-by: Steven Rostedt (VMware) 

-- Steve

> ---