This replaces ',' with '.' as the char that ends the +T:name.
This allows a later patch to treat ',' as a space, which mostly
eliminates the need to quote query/rules. And this in turn avoids
quoting hassles:
modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p
It is particularly good for passing boot-args into test-scripts.
vng -p 4 -v \
-a test_dynamic_debug.dyndbg=class,D2_CORE,+p
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 2ac1bd7f105f..a5fc80edd24c 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -172,7 +172,7 @@ char *read_T_args(const char *str, struct flag_settings *modifiers)
}
str += 2;
- end = strchr(str, ',');
+ end = strchr(str, '.');
if (end && *(end + 1) == '\0')
return NULL;
@@ -264,7 +264,7 @@ static char *ddebug_describe_ctrl(struct dd_ctrl *ctrl, struct ctrlbuf *cb)
for (i = 0; i < ARRAY_SIZE(opt_array); ++i)
if (ctrl->flags & opt_array[i].flag) {
if (show_args)
- *p++ = ',';
+ *p++ = '.';
*p++ = opt_array[i].opt_char;
show_args = opt_array[i].show_args;
if (show_args)
--
2.43.0On Thu 2023-12-07 17:15:08, Jim Cromie wrote:
> This replaces ',' with '.' as the char that ends the +T:name.
>
> This allows a later patch to treat ',' as a space, which mostly
> eliminates the need to quote query/rules. And this in turn avoids
> quoting hassles:
>
> modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p
>
> It is particularly good for passing boot-args into test-scripts.
>
> vng -p 4 -v \
> -a test_dynamic_debug.dyndbg=class,D2_CORE,+p
Could you please add example how it looked before and after?
Is this format documented somewhere?
Will the documentation get updated?
Could it break existing scripts? [*]
The dynamic debug interface is really hard to use for me
as an occasional user. I always have to look into
Documentation/admin-guide/dynamic-debug-howto.rst
Anyway, there should be a good reason to change the interface.
And the exaplantion:
"Let's use '.' instead of ',' so that we could later
treat ',' as space"
sounds scarry. It does not explain what is the advantage at all.
[*] Some scripts are using the interface even in the mainline,
for example:
$> git grep "dynamic_debug" tools/testing/
tools/testing/selftests/bpf/test_tunnel.sh: echo 'file ip_gre.c +p' > /sys/kernel/debug/dynamic_debug/control
tools/testing/selftests/bpf/test_tunnel.sh: echo 'file ip6_gre.c +p' > /sys/kernel/debug/dynamic_debug/control
tools/testing/selftests/bpf/test_tunnel.sh: echo 'file geneve.c +p' > /sys/kernel/debug/dynamic_debug/control
tools/testing/selftests/bpf/test_tunnel.sh: echo 'file ipip.c +p' > /sys/kernel/debug/dynamic_debug/control
tools/testing/selftests/livepatch/functions.sh: DYNAMIC_DEBUG=$(grep '^kernel/livepatch' /sys/kernel/debug/dynamic_debug/control | \
tools/testing/selftests/livepatch/functions.sh: echo -n "$DYNAMIC_DEBUG" > /sys/kernel/debug/dynamic_debug/control
tools/testing/selftests/livepatch/functions.sh:function set_dynamic_debug() {
tools/testing/selftests/livepatch/functions.sh: cat <<-EOF > /sys/kernel/debug/dynamic_debug/control
tools/testing/selftests/livepatch/functions.sh: set_dynamic_debug
Best Regards,
Petr
pon., 18 gru 2023 o 18:29 Petr Mladek <pmladek@suse.com> napisał(a):
>
> On Thu 2023-12-07 17:15:08, Jim Cromie wrote:
> > This replaces ',' with '.' as the char that ends the +T:name.
> >
> > This allows a later patch to treat ',' as a space, which mostly
> > eliminates the need to quote query/rules. And this in turn avoids
> > quoting hassles:
> >
> > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p
> >
> > It is particularly good for passing boot-args into test-scripts.
> >
> > vng -p 4 -v \
> > -a test_dynamic_debug.dyndbg=class,D2_CORE,+p
>
> Could you please add example how it looked before and after?
Before a user had to issue a command in the format
modprobe test_dynamic_debug dyndbg="class D2_CORE +p"
Now a use can use either
modprobe test_dynamic_debug dyndbg="class D2_CORE +p"
or
modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p
> Is this format documented somewhere?
> Will the documentation get updated?
Documentation will be updated.
> Could it break existing scripts? [*]
>
It should not break any scripts as this change does not change the
interface but extends it.
> The dynamic debug interface is really hard to use for me
> as an occasional user. I always have to look into
> Documentation/admin-guide/dynamic-debug-howto.rst
>
> Anyway, there should be a good reason to change the interface.
> And the exaplantion:
>
> "Let's use '.' instead of ',' so that we could later
> treat ',' as space"
>
> sounds scarry. It does not explain what is the advantage at all.
>
I will clarify in the commit message that this change allows to use
two formats either
modprobe test_dynamic_debug dyndbg="class D2_CORE +p"
or
modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p
>
> [*] Some scripts are using the interface even in the mainline,
> for example:
>
> $> git grep "dynamic_debug" tools/testing/
> tools/testing/selftests/bpf/test_tunnel.sh: echo 'file ip_gre.c +p' > /sys/kernel/debug/dynamic_debug/control
> tools/testing/selftests/bpf/test_tunnel.sh: echo 'file ip6_gre.c +p' > /sys/kernel/debug/dynamic_debug/control
> tools/testing/selftests/bpf/test_tunnel.sh: echo 'file geneve.c +p' > /sys/kernel/debug/dynamic_debug/control
> tools/testing/selftests/bpf/test_tunnel.sh: echo 'file ipip.c +p' > /sys/kernel/debug/dynamic_debug/control
> tools/testing/selftests/livepatch/functions.sh: DYNAMIC_DEBUG=$(grep '^kernel/livepatch' /sys/kernel/debug/dynamic_debug/control | \
> tools/testing/selftests/livepatch/functions.sh: echo -n "$DYNAMIC_DEBUG" > /sys/kernel/debug/dynamic_debug/control
> tools/testing/selftests/livepatch/functions.sh:function set_dynamic_debug() {
> tools/testing/selftests/livepatch/functions.sh: cat <<-EOF > /sys/kernel/debug/dynamic_debug/control
> tools/testing/selftests/livepatch/functions.sh: set_dynamic_debug
>
Good to know. I will use these scripts to make sure the dynamic debug
interface is not broken.
Thanks,
Lukasz
>
> Best Regards,
> Petr
On Thu 2023-12-21 16:21:49, Łukasz Bartosik wrote: > pon., 18 gru 2023 o 18:29 Petr Mladek <pmladek@suse.com> napisał(a): > > > > On Thu 2023-12-07 17:15:08, Jim Cromie wrote: > > > This replaces ',' with '.' as the char that ends the +T:name. > > > > > > This allows a later patch to treat ',' as a space, which mostly > > > eliminates the need to quote query/rules. And this in turn avoids > > > quoting hassles: > > > > > > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p > > > > > > It is particularly good for passing boot-args into test-scripts. > > > > > > vng -p 4 -v \ > > > -a test_dynamic_debug.dyndbg=class,D2_CORE,+p > > > > Could you please add example how it looked before and after? > > Before a user had to issue a command in the format > modprobe test_dynamic_debug dyndbg="class D2_CORE +p" > > Now a use can use either > modprobe test_dynamic_debug dyndbg="class D2_CORE +p" > or > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p I see. This was not clear to me. Please, mention it in the commit message. That said, I am not sure if it is worth it and if it is a good idea. Supporting more formats adds complexity and confusion. It is the reason why people hate perl. I agree that quoting in scripts is complicated. Well, a sane approach is to use quotes everywhere where possible. If a script works correctly only with class,D2_CORE,+p and breaks with "class D2_CORE +p" then it is a ticking bomb. People might try to use "class D2_CORE +p" one day because they would cut&paste the string from the internet. > > Is this format documented somewhere? > > Will the documentation get updated? > > Documentation will be updated. > > > Could it break existing scripts? [*] > > It should not break any scripts as this change does not change the > interface but extends it. > > > The dynamic debug interface is really hard to use for me > > as an occasional user. I always have to look into > > Documentation/admin-guide/dynamic-debug-howto.rst > > > > Anyway, there should be a good reason to change the interface. > > And the exaplantion: > > > > "Let's use '.' instead of ',' so that we could later > > treat ',' as space" > > > > sounds scarry. It does not explain what is the advantage at all. > > > I will clarify in the commit message that this change allows to use > two formats either > modprobe test_dynamic_debug dyndbg="class D2_CORE +p" > or > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p But the patch replaces ',' with '.'. It looks like it modifies some existing syntax. Note that I am not familiar with this code. And I even do not see the patched function in the current Linus' tree. Best Regards, Petr
czw., 4 sty 2024 o 08:54 Petr Mladek <pmladek@suse.com> napisał(a): > > On Thu 2023-12-21 16:21:49, Łukasz Bartosik wrote: > > pon., 18 gru 2023 o 18:29 Petr Mladek <pmladek@suse.com> napisał(a): > > > > > > On Thu 2023-12-07 17:15:08, Jim Cromie wrote: > > > > This replaces ',' with '.' as the char that ends the +T:name. > > > > > > > > This allows a later patch to treat ',' as a space, which mostly > > > > eliminates the need to quote query/rules. And this in turn avoids > > > > quoting hassles: > > > > > > > > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p > > > > > > > > It is particularly good for passing boot-args into test-scripts. > > > > > > > > vng -p 4 -v \ > > > > -a test_dynamic_debug.dyndbg=class,D2_CORE,+p > > > > > > Could you please add example how it looked before and after? > > > > Before a user had to issue a command in the format > > modprobe test_dynamic_debug dyndbg="class D2_CORE +p" > > > > Now a use can use either > > modprobe test_dynamic_debug dyndbg="class D2_CORE +p" > > or > > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p > > I see. This was not clear to me. Please, mention it in > the commit message. > In the patchset v3 I squashed "dyndbg: change +T:name_terminator to dot" with "dyndbg: add processing of T(race) flag argument" but I will clarify this topic in the "dyndbg: treat comma as a token separator" > That said, I am not sure if it is worth it and if it > is a good idea. Supporting more formats adds complexity > and confusion. It is the reason why people hate perl. > > I agree that quoting in scripts is complicated. Well, > a sane approach is to use quotes everywhere where possible. > > If a script works correctly only with class,D2_CORE,+p > and breaks with "class D2_CORE +p" then it is a ticking > bomb. People might try to use "class D2_CORE +p" > one day because they would cut&paste the string > from the internet. > Addition of new format of dynamic debug queries does not obsolete the existing format. > > > Is this format documented somewhere? > > > Will the documentation get updated? > > > > Documentation will be updated. > > > > > Could it break existing scripts? [*] > > > > It should not break any scripts as this change does not change the > > interface but extends it. > > > > > The dynamic debug interface is really hard to use for me > > > as an occasional user. I always have to look into > > > Documentation/admin-guide/dynamic-debug-howto.rst > > > > > > Anyway, there should be a good reason to change the interface. > > > And the exaplantion: > > > > > > "Let's use '.' instead of ',' so that we could later > > > treat ',' as space" > > > > > > sounds scarry. It does not explain what is the advantage at all. > > > > > I will clarify in the commit message that this change allows to use > > two formats either > > modprobe test_dynamic_debug dyndbg="class D2_CORE +p" > > or > > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p > > But the patch replaces ',' with '.'. It looks like it modifies > some existing syntax. > > Note that I am not familiar with this code. And I even do not see > the patched function in the current Linus' tree. > Next patchset will include updated dynamic debug documentation. Hopefully this will help to resolve doubts. Regards, Lukasz > Best Regards, > Petr >
© 2016 - 2025 Red Hat, Inc.