2021-09-14 01:16:32

by Andrew Halaney

[permalink] [raw]
Subject: [PATCH v2 3/3] Documentation: dyndbg: Improve cli param examples

Jim pointed out that using $module.dyndbg= is always a more flexible
choice for using dynamic debug on the command line. The $module.dyndbg
style is checked at boot and handles if $module is a builtin. If it is
actually a loadable module, it is handled again later when the module is
loaded.

If you just use dyndbg="module $module +p" dynamic debug is only enabled
when $module is a builtin.

It was recommended to illustrate wildcard usage as well.

Signed-off-by: Andrew Halaney <[email protected]>
Suggested-by: Jim Cromie <[email protected]>
---
Documentation/admin-guide/dynamic-debug-howto.rst | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index d0911e7cc271..4bfb23ed64ec 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -357,7 +357,10 @@ Examples
Kernel command line: ...
// see whats going on in dyndbg=value processing
dynamic_debug.verbose=1
- // enable pr_debugs in 2 builtins, #cmt is stripped
- dyndbg="module params +p #cmt ; module sys +p"
+ // Enable pr_debugs in the params builtin
+ params.dyndbg="+p"
+ // enable pr_debugs in all files under init/
+ // and the function pc87360_init_device, #cmt is stripped
+ dyndbg="file init/* +p #cmt ; func pc87360_init_device +p"
// enable pr_debugs in 2 functions in a module loaded later
pc87360.dyndbg="func pc87360_init_device +p; func pc87360_find +p"
--
2.31.1


2021-09-18 04:36:55

by Andrew Halaney

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Documentation: dyndbg: Improve cli param examples

On Fri, Sep 17, 2021 at 02:30:09PM -0600, [email protected] wrote:
> On Fri, Sep 17, 2021 at 1:50 PM Jason Baron <[email protected]> wrote:
> >
> >
> >
> > On 9/13/21 6:24 PM, Andrew Halaney wrote:
> > > Jim pointed out that using $module.dyndbg= is always a more flexible
> > > choice for using dynamic debug on the command line. The $module.dyndbg
> > > style is checked at boot and handles if $module is a builtin. If it is
> > > actually a loadable module, it is handled again later when the module is
> > > loaded.
> > >
> > > If you just use dyndbg="module $module +p" dynamic debug is only enabled
> > > when $module is a builtin.
> > >
> > > It was recommended to illustrate wildcard usage as well.
> > >
> > > Signed-off-by: Andrew Halaney <[email protected]>
> > > Suggested-by: Jim Cromie <[email protected]>
> > > ---
> > > Documentation/admin-guide/dynamic-debug-howto.rst | 7 +++++--
> > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
> > > index d0911e7cc271..4bfb23ed64ec 100644
> > > --- a/Documentation/admin-guide/dynamic-debug-howto.rst
> > > +++ b/Documentation/admin-guide/dynamic-debug-howto.rst
> > > @@ -357,7 +357,10 @@ Examples
> > > Kernel command line: ...
> > > // see whats going on in dyndbg=value processing
> > > dynamic_debug.verbose=1
> > > - // enable pr_debugs in 2 builtins, #cmt is stripped
> > > - dyndbg="module params +p #cmt ; module sys +p"
> > > + // Enable pr_debugs in the params builtin
> > > + params.dyndbg="+p"
> >
> > If we are going out of our way to change this to indicate that it works
> > for builtin and modules, it seems like the comment above should reflect
> > that? IE, something like this?
> >
> > '// Enable pr_debugs in the params module or if params is builtin.
> >
>
> I dont think params can be a loadable module, so its not a great
> example of this.
> it should be one that "everyone" knows is usually loaded.
>
> conversely, bare dyndbg example should have only builtin modules,
> then the contrast between 2 forms is most evident.
>

Thank you both for the feedback, good points.

Does something like:

// Enable pr_debugs in the btrfs module (can be builtin or loadable)
btrfs.dyndbg="+p"
// enable pr_debugs in all files under init/
// and the function parse_one, #cmt is stripped
dyndbg="file init/* +p #cmt ; func parse_one +p"

Work for you both? I think that makes the advantages of $module.dyndbg=
more clear and makes the usage of dyndbg= stick to strictly builtins.
If so I'll respin this patch in v3 of the series.

Thanks,
Andrew

>
> > The first two patches look fine to me, so if you agree maybe just
> > re-spin this one?
> >
> > Thanks,
> >
> > -Jason
> >
> > > + // enable pr_debugs in all files under init/
> > > + // and the function pc87360_init_device, #cmt is stripped
> > > + dyndbg="file init/* +p #cmt ; func pc87360_init_device +p"
> > > // enable pr_debugs in 2 functions in a module loaded later
> > > pc87360.dyndbg="func pc87360_init_device +p; func pc87360_find +p"
> >
>

2021-09-18 05:08:42

by Jason Baron

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Documentation: dyndbg: Improve cli param examples



On 9/17/21 4:53 PM, Andrew Halaney wrote:
> On Fri, Sep 17, 2021 at 02:30:09PM -0600, [email protected] wrote:
>> On Fri, Sep 17, 2021 at 1:50 PM Jason Baron <[email protected]> wrote:
>>>
>>>
>>> On 9/13/21 6:24 PM, Andrew Halaney wrote:
>>>> Jim pointed out that using $module.dyndbg= is always a more flexible
>>>> choice for using dynamic debug on the command line. The $module.dyndbg
>>>> style is checked at boot and handles if $module is a builtin. If it is
>>>> actually a loadable module, it is handled again later when the module is
>>>> loaded.
>>>>
>>>> If you just use dyndbg="module $module +p" dynamic debug is only enabled
>>>> when $module is a builtin.
>>>>
>>>> It was recommended to illustrate wildcard usage as well.
>>>>
>>>> Signed-off-by: Andrew Halaney <[email protected]>
>>>> Suggested-by: Jim Cromie <[email protected]>
>>>> ---
>>>> Documentation/admin-guide/dynamic-debug-howto.rst | 7 +++++--
>>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
>>>> index d0911e7cc271..4bfb23ed64ec 100644
>>>> --- a/Documentation/admin-guide/dynamic-debug-howto.rst
>>>> +++ b/Documentation/admin-guide/dynamic-debug-howto.rst
>>>> @@ -357,7 +357,10 @@ Examples
>>>> Kernel command line: ...
>>>> // see whats going on in dyndbg=value processing
>>>> dynamic_debug.verbose=1
>>>> - // enable pr_debugs in 2 builtins, #cmt is stripped
>>>> - dyndbg="module params +p #cmt ; module sys +p"
>>>> + // Enable pr_debugs in the params builtin
>>>> + params.dyndbg="+p"
>>> If we are going out of our way to change this to indicate that it works
>>> for builtin and modules, it seems like the comment above should reflect
>>> that? IE, something like this?
>>>
>>> '// Enable pr_debugs in the params module or if params is builtin.
>>>
>> I dont think params can be a loadable module, so its not a great
>> example of this.
>> it should be one that "everyone" knows is usually loaded.
>>
>> conversely, bare dyndbg example should have only builtin modules,
>> then the contrast between 2 forms is most evident.
>>
> Thank you both for the feedback, good points.
>
> Does something like:
>
> // Enable pr_debugs in the btrfs module (can be builtin or loadable)
> btrfs.dyndbg="+p"
> // enable pr_debugs in all files under init/
> // and the function parse_one, #cmt is stripped
> dyndbg="file init/* +p #cmt ; func parse_one +p"
>
> Work for you both? I think that makes the advantages of $module.dyndbg=
> more clear and makes the usage of dyndbg= stick to strictly builtins.
> If so I'll respin this patch in v3 of the series.

Fine with me.

Thanks,

-Jason

>
> Thanks,
> Andrew
>
>>> The first two patches look fine to me, so if you agree maybe just
>>> re-spin this one?
>>>
>>> Thanks,
>>>
>>> -Jason
>>>
>>>> + // enable pr_debugs in all files under init/
>>>> + // and the function pc87360_init_device, #cmt is stripped
>>>> + dyndbg="file init/* +p #cmt ; func pc87360_init_device +p"
>>>> // enable pr_debugs in 2 functions in a module loaded later
>>>> pc87360.dyndbg="func pc87360_init_device +p; func pc87360_find +p"

2021-09-18 08:34:40

by Jim Cromie

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Documentation: dyndbg: Improve cli param examples

On Fri, Sep 17, 2021 at 1:50 PM Jason Baron <[email protected]> wrote:
>
>
>
> On 9/13/21 6:24 PM, Andrew Halaney wrote:
> > Jim pointed out that using $module.dyndbg= is always a more flexible
> > choice for using dynamic debug on the command line. The $module.dyndbg
> > style is checked at boot and handles if $module is a builtin. If it is
> > actually a loadable module, it is handled again later when the module is
> > loaded.
> >
> > If you just use dyndbg="module $module +p" dynamic debug is only enabled
> > when $module is a builtin.
> >
> > It was recommended to illustrate wildcard usage as well.
> >
> > Signed-off-by: Andrew Halaney <[email protected]>
> > Suggested-by: Jim Cromie <[email protected]>
> > ---
> > Documentation/admin-guide/dynamic-debug-howto.rst | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
> > index d0911e7cc271..4bfb23ed64ec 100644
> > --- a/Documentation/admin-guide/dynamic-debug-howto.rst
> > +++ b/Documentation/admin-guide/dynamic-debug-howto.rst
> > @@ -357,7 +357,10 @@ Examples
> > Kernel command line: ...
> > // see whats going on in dyndbg=value processing
> > dynamic_debug.verbose=1
> > - // enable pr_debugs in 2 builtins, #cmt is stripped
> > - dyndbg="module params +p #cmt ; module sys +p"
> > + // Enable pr_debugs in the params builtin
> > + params.dyndbg="+p"
>
> If we are going out of our way to change this to indicate that it works
> for builtin and modules, it seems like the comment above should reflect
> that? IE, something like this?
>
> '// Enable pr_debugs in the params module or if params is builtin.
>

I dont think params can be a loadable module, so its not a great
example of this.
it should be one that "everyone" knows is usually loaded.

conversely, bare dyndbg example should have only builtin modules,
then the contrast between 2 forms is most evident.


> The first two patches look fine to me, so if you agree maybe just
> re-spin this one?
>
> Thanks,
>
> -Jason
>
> > + // enable pr_debugs in all files under init/
> > + // and the function pc87360_init_device, #cmt is stripped
> > + dyndbg="file init/* +p #cmt ; func pc87360_init_device +p"
> > // enable pr_debugs in 2 functions in a module loaded later
> > pc87360.dyndbg="func pc87360_init_device +p; func pc87360_find +p"
>

2021-09-18 09:11:33

by Jason Baron

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Documentation: dyndbg: Improve cli param examples



On 9/13/21 6:24 PM, Andrew Halaney wrote:
> Jim pointed out that using $module.dyndbg= is always a more flexible
> choice for using dynamic debug on the command line. The $module.dyndbg
> style is checked at boot and handles if $module is a builtin. If it is
> actually a loadable module, it is handled again later when the module is
> loaded.
>
> If you just use dyndbg="module $module +p" dynamic debug is only enabled
> when $module is a builtin.
>
> It was recommended to illustrate wildcard usage as well.
>
> Signed-off-by: Andrew Halaney <[email protected]>
> Suggested-by: Jim Cromie <[email protected]>
> ---
> Documentation/admin-guide/dynamic-debug-howto.rst | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
> index d0911e7cc271..4bfb23ed64ec 100644
> --- a/Documentation/admin-guide/dynamic-debug-howto.rst
> +++ b/Documentation/admin-guide/dynamic-debug-howto.rst
> @@ -357,7 +357,10 @@ Examples
> Kernel command line: ...
> // see whats going on in dyndbg=value processing
> dynamic_debug.verbose=1
> - // enable pr_debugs in 2 builtins, #cmt is stripped
> - dyndbg="module params +p #cmt ; module sys +p"
> + // Enable pr_debugs in the params builtin
> + params.dyndbg="+p"

If we are going out of our way to change this to indicate that it works
for builtin and modules, it seems like the comment above should reflect
that? IE, something like this?

'// Enable pr_debugs in the params module or if params is builtin.

The first two patches look fine to me, so if you agree maybe just
re-spin this one?

Thanks,

-Jason

> + // enable pr_debugs in all files under init/
> + // and the function pc87360_init_device, #cmt is stripped
> + dyndbg="file init/* +p #cmt ; func pc87360_init_device +p"
> // enable pr_debugs in 2 functions in a module loaded later
> pc87360.dyndbg="func pc87360_init_device +p; func pc87360_find +p"