dyndbg: use seq_putc() in ddebug_proc_show()

Single characters should be put into a sequence. Thus use the
corresponding function "seq_putc".

This issue was transformed by using the Coccinelle sof

dyndbg: use seq_putc() in ddebug_proc_show()

Single characters should be put into a sequence. Thus use the
corresponding function "seq_putc".

This issue was transformed by using the Coccinelle software.

Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Markus Elfring <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: Jim Cromie <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>

show more ...

dyndbg: fix old BUG_ON in >control parser

Fix a BUG_ON from 2009. Even if it looks "unreachable" (I didn't
really look), lets make sure by removing it, doing pr_err and return
-EINVAL instead.

Cc:

dyndbg: fix old BUG_ON in >control parser

Fix a BUG_ON from 2009. Even if it looks "unreachable" (I didn't
really look), lets make sure by removing it, doing pr_err and return
-EINVAL instead.

Cc: stable <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: replace kstrdup() + strchr() with kstrdup_and_replace()

Replace open coded functionalify of kstrdup_and_replace() with a call.

Link: https://lkml.kernel.org/r/20240213162741.3102810-1-andri

dyndbg: replace kstrdup() + strchr() with kstrdup_and_replace()

Replace open coded functionalify of kstrdup_and_replace() with a call.

Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Andy Shevchenko <[email protected]>
Reviewed-by: Luis Chamberlain <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: Jim Cromie <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>

show more ...

dyndbg: add source filename to prefix

Printing the line number without the file is of limited usefulness.

Knowing the filename also makes it also easier to relate the logged
information to the cont

dyndbg: add source filename to prefix

Printing the line number without the file is of limited usefulness.

Knowing the filename also makes it also easier to relate the logged
information to the controlfile.

Example:

# modprobe test_dynamic_debug
# echo 'file test_dynamic_debug.c =pfsl' > /proc/dynamic_debug/control
# echo 1 > /sys/module/test_dynamic_debug/parameters/do_prints
# dmesg | tail -2
[ 71.802212] do_cats:lib/test_dynamic_debug.c:103: test_dd: doing categories
[ 71.802227] do_levels:lib/test_dynamic_debug.c:123: test_dd: doing levels

Signed-off-by: Thomas Weißschuh <[email protected]>
Acked-by: Jim Cromie <[email protected]>
Acked-by: Jason Baron <[email protected]>
Reviewed-by: Luis Chamberlain <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: increase PREFIX_SIZE to 128

A follow-up patch will add the possibility to print the filename as part
of the prefix.
Increase the maximum prefix size to accommodate this.

Signed-off-by: Thom

dyndbg: increase PREFIX_SIZE to 128

A follow-up patch will add the possibility to print the filename as part
of the prefix.
Increase the maximum prefix size to accommodate this.

Signed-off-by: Thomas Weißschuh <[email protected]>
Reviewed-by: Luis Chamberlain <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: constify opt_array

It is never modified, so mark it const.

Signed-off-by: Thomas Weißschuh <[email protected]>
Reviewed-by: Luis Chamberlain <[email protected]>
Link: https://lore.kernel

dyndbg: constify opt_array

It is never modified, so mark it const.

Signed-off-by: Thomas Weißschuh <[email protected]>
Reviewed-by: Luis Chamberlain <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: use the module notifier callbacks

Bring dynamic debug in line with other subsystems by using the module
notifier callbacks. This results in a net decrease in core module
code.

Additionally,

dyndbg: use the module notifier callbacks

Bring dynamic debug in line with other subsystems by using the module
notifier callbacks. This results in a net decrease in core module
code.

Additionally, Jim Cromie has a new dynamic debug classmap feature,
which requires that jump labels be initialized prior to dynamic debug.
Specifically, the new feature toggles a jump label from the existing
dynamic_debug_setup() function. However, this does not currently work
properly, because jump labels are initialized via the
'module_notify_list' notifier chain, which is invoked after the
current call to dynamic_debug_setup(). Thus, this patch ensures that
jump labels are initialized prior to dynamic debug by setting the
dynamic debug notifier priority to 0, while jump labels have the
higher priority of 1.

Tested by Jim using his new test case, and I've verfied the correct
printing via: # modprobe test_dynamic_debug dyndbg.

Link: https://lore.kernel.org/lkml/[email protected]/
Reported-by: kernel test robot <[email protected]>
Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Tested-by: Jim Cromie <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
CC: Jim Cromie <[email protected]>
Cc: Luis Chamberlain <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Jason Baron <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>

show more ...

dyndbg: remove unused 'base' arg from __ddebug_add_module()

The 'base' parameter to __ddebug_add_module() is no longer in use
after: Commit b7b4eebdba7b ("dyndbg: gather __dyndbg[] state into
struct

dyndbg: remove unused 'base' arg from __ddebug_add_module()

The 'base' parameter to __ddebug_add_module() is no longer in use
after: Commit b7b4eebdba7b ("dyndbg: gather __dyndbg[] state into
struct _ddebug_info").

Cc: Jim Cromie <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Tested-by: Jim Cromie <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
Signed-off-by: Jason Baron <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>

show more ...

dyndbg: add drm.debug style (drm/parameters/debug) bitmap support

Add kernel_param_ops and callbacks to use a class-map to validate and
apply input to a sysfs-node, which allows users to control cla

dyndbg: add drm.debug style (drm/parameters/debug) bitmap support

Add kernel_param_ops and callbacks to use a class-map to validate and
apply input to a sysfs-node, which allows users to control classes
defined in that class-map. This supports uses like:

echo 0x3 > /sys/module/drm/parameters/debug

IE add these:

- int param_set_dyndbg_classes()
- int param_get_dyndbg_classes()
- struct kernel_param_ops param_ops_dyndbg_classes

Following the model of kernel/params.c STANDARD_PARAM_DEFS, these are
non-static and exported. This might be unnecessary here.

get/set use an augmented kernel_param; the arg refs a new struct
ddebug_class_param, which contains:

- A ptr to user's state-store; a union of &ulong for drm.debug, &int
for nouveau level debug. By ref'g the client's bit-state _var, code
coordinates with existing code (like drm_debug_enabled) which uses
it, so existing/remaining calls can work unchanged. Changing
drm.debug to a ulong allows use of BIT() etc.

- FLAGS: dyndbg.flags toggled by changes to bitmap. Usually just "p".

- MAP: a pointer to struct ddebug_classes_map, which maps those
class-names to .class_ids 0..N that the module is using. This
class-map is declared & initialized by DECLARE_DYNDBG_CLASSMAP.

- map-type: 4 enums DD_CLASS_TYPE_* select 2 input forms and 2 meanings.

numeric input:
DD_CLASS_TYPE_DISJOINT_BITS integer input, independent bits. ie: drm.debug
DD_CLASS_TYPE_LEVEL_NUM integer input, 0..N levels

classnames-list (comma separated) input:
DD_CLASS_TYPE_DISJOINT_NAMES each name affects a bit, others preserved
DD_CLASS_TYPE_LEVEL_NAMES names have level meanings, like kern_levels.h

_NAMES - comma-separated classnames (with optional +-)
_NUM - numeric input, 0-N expected
_BITS - numeric input, 0x1F bitmap form expected

_DISJOINT - bits are independent
_LEVEL - (x<y) on bit-pos.

_DISJOINT treats input like a bit-vector (ala drm.debug), and sets
each bit accordingly. LEVEL is layered on top of this.

_LEVEL treats input like a bit-pos:N, then sets bits(0..N)=1, and
bits(N+1..max)=0. This applies (bit<N) semantics on top of disjoint
bits.

USAGES:

A potentially typical _DISJOINT_NAMES use:

echo +DRM_UT_CORE,+DRM_UT_KMS,-DRM_UT_DRIVER,-DRM_UT_ATOMIC \
> /sys/module/drm/parameters/debug_catnames

A naive _LEVEL_NAMES use, with one class, that sets all in the
class-map according to (x<y):

: problem seen
echo +L7 > /sys/module/test_dynamic_debug/parameters/p_level_names
: problem solved
echo -L1 > /sys/module/test_dynamic_debug/parameters/p_level_names

Note this artifact:

: this is same as prev cmd (due to +/-)
echo L0 > /sys/module/test_dynamic_debug/parameters/p_level_names

: this is "even-more" off, but same wo __pr_debug_class(L0, "..").
echo -L0 > /sys/module/test_dynamic_debug/parameters/p_level_names

A stress-test/make-work usage (kid toggling a light switch):

echo +L7,L0,L7,L0,L7,L0,L7,L0,L7,L0,L7,L0,L7 \
> /sys/module/test_dynamic_debug/parameters/p_level_names

ddebug_apply_class_bitmap(): inside-fn, works on bitmaps, receives
new-bits, finds diffs vs client-bitvector holding "current" state,
and issues exec_query to commit the adjustment.

param_set_dyndbg_classes(): interface fn, sends _NAMES to
param_set_dyndbg_classnames() and returns, falls thru to handle _BITS,
_NUM internally, and calls ddebug_apply_class_bitmap(). Finishes by
updating state.

param_set_dyndbg_classnames(): handles classnames-list in loop, calls
ddebug_apply_class_bitmap for each, then updates state.

NOTES:

_LEVEL_ is overlay on _DISJOINT_; inputs are converted to a bitmask,
by the callbacks. IOW this is possible, and possibly confusing:

echo class V3 +p > control
echo class V1 -p > control

IMO thats ok, relative verbosity is an interface property.

_LEVEL_NUM maps still need class-names, even though the names are not
usable at the sysfs interface (unlike with _NAMES style). The names
are the only way to >control the classes.

- It must have a "V0" name,
something below "V1" to turn "V1" off.
__pr_debug_cls(V0,..) is printk, don't do that.

- "class names" is required at the >control interface.
- relative levels are not enforced at >control

_LEVEL_NAMES bear +/- signs, which alters the on-bit-pos by 1. IOW,
+L2 means L0,L1,L2, and -L2 means just L0,L1. This kinda spoils the
readback fidelity, since the L0 bit gets turned on by any use of any
L*, except "-L0".

All the interface uncertainty here pertains to the _NAMES features.
Nobody has actually asked for this, so its practical (if a little
tedious) to split it out.

Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: validate class FOO by checking with module

Add module-to-class validation:

#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control

If a query has "class FOO", then ddebug_find_valid_cl

dyndbg: validate class FOO by checking with module

Add module-to-class validation:

#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control

If a query has "class FOO", then ddebug_find_valid_class(), called
from ddebug_change(), requires that FOO is known to module X,
otherwize the query is skipped entirely for X. This protects each
module's class-space, other than the default:31.

The authors' choice of FOO is highly selective, giving isolation
and/or coordinated sharing of FOOs. For example, only DRM modules
should know and respond to DRM_UT_KMS.

So this, combined with module's opt-in declaration of known classes,
effectively privatizes the .class_id space for each module (or
coordinated set of modules).

Notes:

For all "class FOO" queries, ddebug_find_valid_class() is called, it
returns the map matching the query, and sets valid_class via an
*outvar).

If no "class FOO" is supplied, valid_class = _CLASS_DFLT. This
insures that legacy queries do not trample on new class'd callsites,
as they get added.

Also add a new column to control-file output, displaying non-default
class-name (when found) or the "unknown _id:", if it has not been
(correctly) declared with one of the declarator macros.

Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: add ddebug_attach_module_classes

Add ddebug_attach_module_classes(), call it from ddebug_add_module().
It scans the classes/section its given, finds records where the
module-name matches the

dyndbg: add ddebug_attach_module_classes

Add ddebug_attach_module_classes(), call it from ddebug_add_module().
It scans the classes/section its given, finds records where the
module-name matches the module being added, and adds them to the
module's maps list. No locking here, since the record
isn't yet linked into the ddebug_tables list.

It is called indirectly from 2 sources:

- from load_module(), where it scans the module's __dyndbg_classes
section, which contains DYNAMIC_DEBUG_CLASSES definitions from just
the module.

- from dynamic_debug_init(), where all DYNAMIC_DEBUG_CLASSES
definitions of each builtin module have been packed together.
This is why ddebug_attach_module_classes() checks module-name.

NOTES

Its (highly) likely that builtin classes will be ordered by module
name (just like prdbg descriptors are in the __dyndbg section). So
the list can be replaced by a vector (ptr + length), which will work
for loaded modules too. This would imitate whats currently done for
the _ddebug descriptors.

That said, converting to vector,len is close to pointless; a small
minority of modules will ever define a class-map, and almost all of
them will have only 1 or 2 class-maps, so theres only a couple dozen
pointers to save. TODO: re-evaluate for lines removable.

Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

kernel/module: add __dyndbg_classes section

Add __dyndbg_classes section, using __dyndbg as a model. Use it:

vmlinux.lds.h:

KEEP the new section, which also silences orphan section warning on
load

kernel/module: add __dyndbg_classes section

Add __dyndbg_classes section, using __dyndbg as a model. Use it:

vmlinux.lds.h:

KEEP the new section, which also silences orphan section warning on
loadable modules. Add (__start_/__stop_)__dyndbg_classes linker
symbols for the c externs (below).

kernel/module/main.c:
- fill new fields in find_module_sections(), using section_objs()
- extend callchain prototypes
to pass classes, length
load_module(): pass new info to dynamic_debug_setup()
dynamic_debug_setup(): new params, pass through to ddebug_add_module()

dynamic_debug.c:
- add externs to the linker symbols.

ddebug_add_module():
- It currently builds a debug_table, and *will* find and attach classes.

dynamic_debug_init():
- add class fields to the _ddebug_info cursor var: di.

Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: gather __dyndbg[] state into struct _ddebug_info

This new struct composes the linker provided (vector,len) section,
and provides a place to add other __dyndbg[] state-data later:

descs -

dyndbg: gather __dyndbg[] state into struct _ddebug_info

This new struct composes the linker provided (vector,len) section,
and provides a place to add other __dyndbg[] state-data later:

descs - the vector of descriptors in __dyndbg section.
num_descs - length of the data/section.

Use it, in several different ways, as follows:

In lib/dynamic_debug.c:

ddebug_add_module(): Alter params-list, replacing 2 args (array,index)
with a struct _ddebug_info * containing them both, with room for
expansion. This helps future-proof the function prototype against the
looming addition of class-map info into the dyndbg-state, by providing
a place to add more member fields later.

NB: later add static struct _ddebug_info builtins_state declaration,
not needed yet.

ddebug_add_module() is called in 2 contexts:

In dynamic_debug_init(), declare, init a struct _ddebug_info di
auto-var to use as a cursor. Then iterate over the prdbg blocks of
the builtin modules, and update the di cursor before calling
_add_module for each.

Its called from kernel/module/main.c:load_info() for each loaded
module:

In internal.h, alter struct load_info, replacing the dyndbg array,len
fields with an embedded _ddebug_info containing them both; and
populate its members in find_module_sections().

The 2 calling contexts differ in that _init deals with contiguous
subranges of __dyndbgs[] section, packed together, while loadable
modules are added one at a time.

So rename ddebug_add_module() into outer/__inner fns, call __inner
from _init, and provide the offset into the builtin __dyndbgs[] where
the module's prdbgs reside. The cursor provides start, len of the
subrange for each. The offset will be used later to pack the results
of builtin __dyndbg_sites[] de-duplication, and is 0 and unneeded for
loadable modules,

Note:

kernel/module/main.c includes <dynamic_debug.h> for struct
_ddeubg_info. This might be prone to include loops, since its also
included by printk.h. Nothing has broken in robot-land on this.

cc: Luis Chamberlain <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: cleanup auto vars in dynamic_debug_init

rework var-names for clarity, regularity
rename variables
- n to mod_sites - it counts sites-per-module
- entries to i - display only
- iter_sta

dyndbg: cleanup auto vars in dynamic_debug_init

rework var-names for clarity, regularity
rename variables
- n to mod_sites - it counts sites-per-module
- entries to i - display only
- iter_start to iter_mod_start - marks start of each module's subrange
- modct to mod_ct - stylistic

new iterator var:
- site - cursor parallel to iter
1st step towards 'demotion' of iter->site, for removal later

treat vars as iters:
- drop init at top
init just above for-loop, in a textual block

Acked-by: Jason Baron <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: drop EXPORTed dynamic_debug_exec_queries

This exported fn is unused, and will not be needed. Lets dump it.

The export was added to let drm control pr_debugs, as part of using
them to avoid

dyndbg: drop EXPORTed dynamic_debug_exec_queries

This exported fn is unused, and will not be needed. Lets dump it.

The export was added to let drm control pr_debugs, as part of using
them to avoid drm_debug_enabled overheads. But its better to just
implement the drm.debug bitmap interface, then its available for
everyone.

Fixes: a2d375eda771 ("dyndbg: refine export, rename to dynamic_debug_exec_queries()")
Fixes: 4c0d77828d4f ("dyndbg: export ddebug_exec_queries")
Acked-by: Jason Baron <[email protected]>
Acked-by: Daniel Vetter <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: let query-modname override actual module name

dyndbg's control-parser: ddebug_parse_query(), requires that search
terms: module, func, file, lineno, are used only once in a query; a
thing ca

dyndbg: let query-modname override actual module name

dyndbg's control-parser: ddebug_parse_query(), requires that search
terms: module, func, file, lineno, are used only once in a query; a
thing cannot be named both foo and bar.

The cited commit added an overriding module modname, taken from the
module loader, which is authoritative. So it set query.module 1st,
which disallowed its use in the query-string.

But now, its useful to allow a module-load to enable classes across a
whole (or part of) a subsystem at once.

# enable (dynamic-debug in) drm only
modprobe drm dyndbg="class DRM_UT_CORE +p"

# get drm_helper too
modprobe drm dyndbg="class DRM_UT_CORE module drm* +p"

# get everything that knows DRM_UT_CORE
modprobe drm dyndbg="class DRM_UT_CORE module * +p"

# also for boot-args:
drm.dyndbg="class DRM_UT_CORE module * +p"

So convert the override into a default, by filling it only when/after
the query-string omitted the module.

NB: the query class FOO handling is forthcoming.

Fixes: 8e59b5cfb9a6 dynamic_debug: add modname arg to exec_query callchain
Acked-by: Jason Baron <[email protected]>
Acked-by: Daniel Vetter <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: use ESCAPE_SPACE for cat control

`cat control` currently does octal escape, so '\n' becomes "\012".
Change this to display as "\n" instead, which reads much cleaner.

:#> head -n7 /proc/d

dyndbg: use ESCAPE_SPACE for cat control

`cat control` currently does octal escape, so '\n' becomes "\012".
Change this to display as "\n" instead, which reads much cleaner.

:#> head -n7 /proc/dynamic_debug/control
# filename:lineno [module]function flags format
init/main.c:1179 [main]initcall_blacklist =_ "blacklisting initcall %s\n"
init/main.c:1218 [main]initcall_blacklisted =_ "initcall %s blacklisted\n"
init/main.c:1424 [main]run_init_process =_ " with arguments:\n"
init/main.c:1426 [main]run_init_process =_ " %s\n"
init/main.c:1427 [main]run_init_process =_ " with environment:\n"
init/main.c:1429 [main]run_init_process =_ " %s\n"

Acked-by: Jason Baron <[email protected]>
Acked-by: Daniel Vetter <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: reverse module.callsite walk in cat control

Walk the module's vector of callsites backwards; ie N..0. This
"corrects" the backwards appearance of a module's prdbg vector when
walked 0..N.

dyndbg: reverse module.callsite walk in cat control

Walk the module's vector of callsites backwards; ie N..0. This
"corrects" the backwards appearance of a module's prdbg vector when
walked 0..N. I think this is due to linker mechanics, which I'm
inclined to treat as immutable, and the order is fixable in display.

No functional changes.

Combined with previous commit, which reversed tables-list, we get:

:#> head -n7 /proc/dynamic_debug/control
# filename:lineno [module]function flags format
init/main.c:1179 [main]initcall_blacklist =_ "blacklisting initcall %s\012"
init/main.c:1218 [main]initcall_blacklisted =_ "initcall %s blacklisted\012"
init/main.c:1424 [main]run_init_process =_ " with arguments:\012"
init/main.c:1426 [main]run_init_process =_ " %s\012"
init/main.c:1427 [main]run_init_process =_ " with environment:\012"
init/main.c:1429 [main]run_init_process =_ " %s\012"

Acked-by: Jason Baron <[email protected]>
Acked-by: Daniel Vetter <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: reverse module walk in cat control

/proc/dynamic_debug/control walks the prdbg catalog in "reverse",
fix this by adding new ddebug_tables to tail of list.

This puts init/main.c entries 1st,

dyndbg: reverse module walk in cat control

/proc/dynamic_debug/control walks the prdbg catalog in "reverse",
fix this by adding new ddebug_tables to tail of list.

This puts init/main.c entries 1st, which looks more than coincidental.

no functional changes.

Acked-by: Jason Baron <[email protected]>
Acked-by: Daniel Vetter <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: show both old and new in change-info

print "old => new" flag values to the info("change") message.

no functional change.

Acked-by: Jason Baron <[email protected]>
Acked-by: Daniel Vetter <

dyndbg: show both old and new in change-info

print "old => new" flag values to the info("change") message.

no functional change.

Acked-by: Jason Baron <[email protected]>
Acked-by: Daniel Vetter <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: fix static_branch manipulation

In https://lore.kernel.org/lkml/[email protected]/

Vincent's patch commented on, and worked around, a bug toggling
static_branch's, when a 2nd P

dyndbg: fix static_branch manipulation

In https://lore.kernel.org/lkml/[email protected]/

Vincent's patch commented on, and worked around, a bug toggling
static_branch's, when a 2nd PRINTK-ish flag was added. The bug
results in a premature static_branch_disable when the 1st of 2 flags
was disabled.

The cited commit computed newflags, but then in the JUMP_LABEL block,
failed to use that result, instead using just one of the terms in it.
Using newflags instead made the code work properly.

This is Vincents test-case, reduced. It needs the 2nd flag to
demonstrate the bug, but it's explanatory here.

pt_test() {
echo 5 > /sys/module/dynamic_debug/verbose

site="module tcp" # just one callsite
echo " $site =_ " > /proc/dynamic_debug/control # clear it

# A B ~A ~B
for flg in +T +p "-T #broke here" -p; do
echo " $site $flg " > /proc/dynamic_debug/control
done;

# A B ~B ~A
for flg in +T +p "-p #broke here" -T; do
echo " $site $flg " > /proc/dynamic_debug/control
done
}
pt_test

Fixes: 84da83a6ffc0 dyndbg: combine flags & mask into a struct, simplify with it
CC: [email protected]
Acked-by: Jason Baron <[email protected]>
Acked-by: Daniel Vetter <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: refine verbosity 1-4 summary-detail

adjust current v*pr_info() calls to fit an overview..detail scheme:

1- module level activity: add/remove, etc
2- command ingest, splitting, summary of ef

dyndbg: refine verbosity 1-4 summary-detail

adjust current v*pr_info() calls to fit an overview..detail scheme:

1- module level activity: add/remove, etc
2- command ingest, splitting, summary of effects.
per >control write
3- command parsing: op, flags, search terms
4- per-site change msg
can yield ~3k x 2 logs per echo "+p;-p" > command.

Summarize these 4 levels in MODULE_PARM_DESC, and update verbose=3 in Doc.

2- is new, to isolate a problem where a stress-test script (which
feeds ~4kb multi-command strings) would produce short writes,
truncating last command and causing parsing errors, which confused
test results. The script fix was to use syswrite, to deliver full
proper commands.

4- gets per-callsite "changed:" pr-infos, which are very noisy during
stress tests, and formerly obscured v1-3 messages, and overwhelmed the
static-key workload being tested.

The verbose parameter has previously seen adjustment:
commit 481c0e33f1e7 ("dyndbg: refine debug verbosity; 1 is basic, 2 more chatty")

The script driving these adjustments is:

!/usr/bin/perl -w

=for Doc

1st purpose was to benchmark the effect of wildcard queries on query
performance; if wildcards are risk free cheap enough, we can deploy
them in the (floating) format search. 1st finding: wildcards take 2x
as long to process.

2nd purpose was to benchmark real static-key changes VS simple flag
changes. Found ~100x decrease for the hard work.

The script maximizes workload per >control by packing it a ~4kb
string of "+p; -p;" commands; this uncovered some broken stuff.

The 85th query failed, and appears to be truncated, so is gramatically
incorrect. Its either an error here, or in the kernel. Its not
happening atm, retest.

Plot thickens: fail only happens doing +-p, not +-mf, likely load
dependent. Error remains consistent. Looks like a short write,
longer on writer than kernel-reader. Try syswrite on handle to
control this. That fixed short write.

=cut

use Getopt::Std;

getopts('vN:k:', \my %opts) or die <<EOH;
$0 options:
-v verbose
-k=n kernel dyndbg verbosity
-N=n number of loops.. tbrc
EOH
$opts{N} //= 10; # !undef, 0 tests too long.

my $ctrl = '/proc/dynamic_debug/control';

vx($opts{k}) if defined $opts{k}; # works on -k0

open(my $CTL, '>', $ctrl) or die "cant open $ctrl for writing: $!\n";

sub vx {
my $arg = shift;
my $cmd = "echo $arg > /sys/module/dynamic_debug/parameters/verbose";
system($cmd);
warn("vx problem: rc:$? err:$! qry: $cmd\n") if ($?);
}

sub qryOK {
my $qry = shift;

print "syntax test: <\n$qry>\n" if $opts{v};
my $bytes = syswrite $CTL, $qry;
printf "short read: $bytes / %d\n", length $qry if $bytes < length $qry;
if ($?) {
warn "rc:$? err:$! qry: $qry\n";
return 0;
}
return 1;
}

sub build_queries {
my ($cmd, $flags, $ct) = @_;

# build experiment and reference queries

my $cycle = " $cmd +$flags # on ; $cmd -$flags # off \n";
my $ref = " +$flags ; -$flags \n";

my $len = length $cycle;
my $max = int(4096 / $len); # break/fit to buffer size
$ct |= $max;
print "qry: ct:$max x << \n$cycle >>\n";

return unless qryOK($ref);
return unless qryOK($cycle);

my $wild = $cycle x $ct;
my $empty = $ref x $ct;

printf "len: %d, %d\n", length $wild, length $empty;

return { trial => $wild,
ref => $empty,
probe => $cycle,
zero => $ref,
count => $ct,
max => $max
};
}

my $query_set = build_queries(' file "*" module "*" func "*" ', "mf");

qryOK($query_set->{zero});
qryOK($query_set->{probe});

qryOK($query_set->{ref});
qryOK($query_set->{trial});

use Benchmark;
sub dobatch {
my ($cmd, $flags, $reps, $ct) = @_;
$reps ||= $opts{N};

my $qs = build_queries($cmd, $flags, $ct);

timethese($reps,
{
wildcards => sub {
syswrite $CTL, $qs->{trial};
},
no_search => sub {
syswrite $CTL, $qs->{ref};
}
}
);
}

sub bench_static_key_toggle {
vx 0;
dobatch(' file "*" module "*" func "*" ', "mf");
dobatch(' file "*" module "*" func "*" ', "p");
}

sub bench_verbose_levels {
for my $i (0..4) {
vx $i;
dobatch(' file "*" module "*" func "*" ', "mf");
}
}

bench_static_key_toggle();

__END__

Heres how the test-script runs:

:: verbose=3 parsing info

[ 48.401646] dyndbg: query 95: "file "*" module "*" func "*" -mf # off " mod:*
[ 48.402040] dyndbg: split into words: "file" "*" "module" "*" "func" "*" "-mf"
[ 48.402456] dyndbg: op='-'
[ 48.402615] dyndbg: flags=0x6
[ 48.402779] dyndbg: *flagsp=0x0 *maskp=0xfffffff9
[ 48.403033] dyndbg: parsed: func="*" file="*" module="*" format="" lineno=0-0
[ 48.403674] dyndbg: applied: func="*" file="*" module="*" format="" lineno=0-0

:: verbose=2 >control summary.
~300k site matches/changes per 4kb command

[ 48.404063] dyndbg: processed 96 queries, with 296160 matches, 0 errs

:: 2 queries against each other, no-search vs all-wildcard-search

qry: ct:48 x <<
file "*" module "*" func "*" +mf # on ; file "*" module "*" func "*" -mf # off
>>
len: 4080, 576
Benchmark: timing 10 iterations of no_search, wildcards...
no_search: 0 wallclock secs ( 0.00 usr + 0.03 sys = 0.03 CPU) @ 333.33/s (n=10)
(warning: too few iterations for a reliable count)
wildcards: 0 wallclock secs ( 0.00 usr + 0.09 sys = 0.09 CPU) @ 111.11/s (n=10)
(warning: too few iterations for a reliable count)

:: 2 queries, both doing real work / changing stati-key states.

qry: ct:49 x <<
file "*" module "*" func "*" +p # on ; file "*" module "*" func "*" -p # off
>>
len: 4067, 490
Benchmark: timing 10 iterations of no_search, wildcards...
no_search: 20 wallclock secs ( 0.00 usr + 20.36 sys = 20.36 CPU) @ 0.49/s (n=10)
wildcards: 21 wallclock secs ( 0.00 usr + 21.08 sys = 21.08 CPU) @ 0.47/s (n=10)
bash-5.1#

Thats 150k static-key-toggles / sec
~600x slower than simple flags
on qemu --smp 3 run

Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: fix spurious vNpr_info change

The cited commit inadvertently altered the verbose level of a
vpr_info, restore it to original.

Fixes: 216a0fc40897 ("dyndbg: show module in vpr-info in dd-exe

dyndbg: fix spurious vNpr_info change

The cited commit inadvertently altered the verbose level of a
vpr_info, restore it to original.

Fixes: 216a0fc40897 ("dyndbg: show module in vpr-info in dd-exec-queries")
Signed-off-By: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: no vpr-info on empty queries

when `echo $cmd > control` contains multiple queries, extra query
separators (;\n) can parse as empty statements. This is normal, and a
vpr-info on an empty com

dyndbg: no vpr-info on empty queries

when `echo $cmd > control` contains multiple queries, extra query
separators (;\n) can parse as empty statements. This is normal, and a
vpr-info on an empty command is just noise.

Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

dyndbg: vpr-info on remove-module complete, not starting

On qemu --smp 3 runs, remove-module can get called 3 times.
So don't print on entry; instead print "removed" after entry is
found and removed

dyndbg: vpr-info on remove-module complete, not starting

On qemu --smp 3 runs, remove-module can get called 3 times.
So don't print on entry; instead print "removed" after entry is
found and removed, so just once.

Signed-off-by: Jim Cromie <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...