Re: [Intel-gfx] [PATCH v2 20/20] jump_label: RFC - tolerate toggled state
On 1/17/23 6:57 AM, Peter Zijlstra wrote: On Fri, Jan 13, 2023 at 12:30:16PM -0700, Jim Cromie wrote: __jump_label_patch currently will "crash the box" if it finds a jump_entry not as expected. ISTM this overly harsh; it doesn't distinguish between "alternate/opposite" state, and truly "insane/corrupted". The "opposite" (but well-formed) state is a milder mis-initialization problem, and some less severe mitigation seems practical. ATM this just warns about it; a range/enum of outcomes: warn, crash, silence, ok, fixup-continue, etc, are possible on a case-by-case basis. Ive managed to create this mis-initialization condition with test_dynamic_debug.ko & _submod.ko. These replicate DRM's regression on DRM_USE_DYNAMIC_DEBUG=y; drm.debug callsites in drivers/helpers (dependent modules) are not enabled along with those in drm.ko itself. Ive hit this case a few times, but havent been able to isolate the when and why. warn-only is something of a punt, and I'm still left with remaining bugs which are likely related; I'm able to toggle the p-flag on callsites in the submod, but their enablement still doesn't yield logging activity. Right; having been in this is state is bad since it will generate inconsistent code-flow. Full on panic *might* not be warranted (as it does for corrupted text) but it is still a fairly bad situation -- so I'm not convinced we want to warn and carry on. It would be really good to figure out why the site was skipped over and got out of skew. Given it's all module stuff, the 'obvious' case would be something like a race between adding the new sites and flipping it, but I'm not seeing how -- things are rather crudely serialized by jump_label_mutex. Indeed, looks like dynamic debug introduces a new path in this series to that tries to toggle the jump label sites before they have been initialized, which ends up with the jump label key enabled but only some of the sites in the correct state. Then when the key is subsequently disabled it finds some in the wrong state. I just posted a patch for dynamic debug to use the module callback notifiers so it's ordered properly against jump label. Note this isn't an issue in the current tree b/c there is no path to toggle the sites currently before they are initialized, but Jim's series here adds such a path. Thanks, -Jason The only other option I can come up with is that somehow the update condition in jump_label_add_module() is somehow wrong.
Re: [Intel-gfx] [PATCH v7 0/9] dyndbg: drm.debug adaptation
On 10/31/22 6:11 PM, jim.cro...@gmail.com wrote:
On Mon, Oct 31, 2022 at 7:07 AM Ville Syrjälä
wrote:
On Sun, Oct 30, 2022 at 08:42:52AM -0600, jim.cro...@gmail.com wrote:
On Thu, Oct 27, 2022 at 2:10 PM Ville Syrjälä
wrote:
On Thu, Oct 27, 2022 at 01:55:39PM -0600, jim.cro...@gmail.com wrote:
On Thu, Oct 27, 2022 at 9:59 AM Ville Syrjälä
wrote:
On Thu, Oct 27, 2022 at 09:37:52AM -0600, jim.cro...@gmail.com wrote:
On Thu, Oct 27, 2022 at 9:08 AM Jason Baron wrote:
On 10/21/22 05:18, Jani Nikula wrote:
On Thu, 20 Oct 2022, Ville Syrjälä wrote:
On Sat, Sep 24, 2022 at 03:02:34PM +0200, Greg KH wrote:
On Sun, Sep 11, 2022 at 11:28:43PM -0600, Jim Cromie wrote:
hi Greg, Dan, Jason, DRM-folk,
heres follow-up to V6:
rebased on driver-core/driver-core-next for -v6 applied bits (thanks)
rework drm_debug_enabled{_raw,_instrumented,} per Dan.
It excludes:
nouveau parts (immature)
tracefs parts (I missed --to=Steve on v6)
split _ddebug_site and de-duplicate experiment (way unready)
IOW, its the remaining commits of V6 on which Dan gave his Reviewed-by.
If these are good to apply, I'll rebase and repost the rest separately.
All now queued up, thanks.
This stuff broke i915 debugs. When I first load i915 no debug prints are
produced. If I then go fiddle around in /sys/module/drm/parameters/debug
the debug prints start to suddenly work.
Wait what? I always assumed the default behaviour would stay the same,
which is usually how we roll. It's a regression in my books. We've got a
CI farm that's not very helpful in terms of dmesg logging right now
because of this.
BR,
Jani.
That doesn't sound good - so you are saying that prior to this change some
of the drm debugs were default enabled. But now you have to manually enable
them?
Thanks,
-Jason
Im just seeing this now.
Any new details ?
No. We just disabled it as BROKEN for now. I was just today thinking
about sending that patch out if no solutin is forthcoming soon since
we need this working before 6.1 is released.
Pretty sure you should see the problem immediately with any driver
(at least if it's built as a module, didn't try builtin). Or at least
can't think what would make i915 any more special.
So, I should note -
99% of my time & energy on this dyndbg + drm patchset
has been done using virtme,
so my world-view (and dev-hack-test env) has been smaller, simpler
maybe its been fatally simplistic.
ive just rebuilt v6.0 (before the trouble)
and run it thru my virtual home box,
I didnt see any unfamiliar drm-debug output
that I might have inadvertently altered somehow
I have some real HW I can put a reference kernel on,0
to look for the missing output, but its all gonna take some time,
esp to tighten up my dev-test-env
in the meantime, there is:
config DRM_USE_DYNAMIC_DEBUG
bool "use dynamic debug to implement drm.debug"
default y
depends on DRM
depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
depends on JUMP_LABEL
help
Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
bytes per callsite, the .data costs can be substantial, and
are therefore configurable.
Does changing the default fix things for i915 dmesg ?
I think we want to mark it BROKEN in addition to make sure no one
Ok, I get the distinction now.
youre spelling that
depends on BROKEN
I have a notional explanation, and a conflating commit:
can you eliminate
git log -p ccc2b496324c13e917ef05f563626f4e7826bef1
as the cause ?
Reverting that doesn't help.
thanks for eliminating it.
I do need to clarify, I dont know exactly what debug/logging output
is missing such that CI is failing
CI isn't failing. But any logs it produces are 100% useless,
as are any user reported logs.
The debugs that are missing are anything not coming directly
from drm.ko.
The stuff that I see being printed by i915.ko are drm_info()
and the drm_printer stuff from i915_welcome_messages(). That
also implies that drm_debug_enabled(DRM_UT_DRIVER) does at
least still work correctly.
I suspect that the problem is just that the debug calls
aren't getting patched in when a module loads. And fiddling
with the modparam after the fact does trigger that somehow.
ok, heres the 'tape' of a virtme boot,
then modprobe going wrong.
[1.785873] dyndbg: 2 debug prints in module intel_rapl_msr
[2.040598] virtme-init: udev is done
virtme-init: console is ttyS0
load drm driver
bash-5.2# modprobe i915
drm module is loaded 1st
[6.549451] dyndbg: add-module: drm.302 sites
[6.549991] dyndbg: class[0]: module:drm base:0 len:10 ty:0
[6.550647] dyndbg: 0: 0 DRM_UT_CORE
[6.551097] dyndbg: 1: 1 DRM_UT_DRIVER
[6.551531] dyndbg: 2: 2 DRM_UT_KMS
[6.551931] dyndbg: 3: 3 DRM_UT_PRIME
[6.552402] dyndbg: 4: 4 DRM_UT_ATOMIC
[6.552799] dyndbg: 5: 5 DRM_UT_VBL
[6.553270] dyndbg: 6: 6 DRM_
Re: [Intel-gfx] [PATCH v7 0/9] dyndbg: drm.debug adaptation
On 10/21/22 05:18, Jani Nikula wrote:
> On Thu, 20 Oct 2022, Ville Syrjälä wrote:
>> On Sat, Sep 24, 2022 at 03:02:34PM +0200, Greg KH wrote:
>>> On Sun, Sep 11, 2022 at 11:28:43PM -0600, Jim Cromie wrote:
hi Greg, Dan, Jason, DRM-folk,
heres follow-up to V6:
rebased on driver-core/driver-core-next for -v6 applied bits (thanks)
rework drm_debug_enabled{_raw,_instrumented,} per Dan.
It excludes:
nouveau parts (immature)
tracefs parts (I missed --to=Steve on v6)
split _ddebug_site and de-duplicate experiment (way unready)
IOW, its the remaining commits of V6 on which Dan gave his Reviewed-by.
If these are good to apply, I'll rebase and repost the rest separately.
>>>
>>> All now queued up, thanks.
>>
>> This stuff broke i915 debugs. When I first load i915 no debug prints are
>> produced. If I then go fiddle around in /sys/module/drm/parameters/debug
>> the debug prints start to suddenly work.
>
> Wait what? I always assumed the default behaviour would stay the same,
> which is usually how we roll. It's a regression in my books. We've got a
> CI farm that's not very helpful in terms of dmesg logging right now
> because of this.
>
> BR,
> Jani.
>
>
That doesn't sound good - so you are saying that prior to this change some
of the drm debugs were default enabled. But now you have to manually enable
them?
Thanks,
-Jason
Re: [Intel-gfx] [PATCH v6 17/57] dyndbg: validate class FOO by checking with module
On 9/9/22 16:44, jim.cro...@gmail.com wrote:
> On Wed, Sep 7, 2022 at 12:19 PM Jason Baron wrote:
>>
>>
>>
>> On 9/4/22 17:40, Jim Cromie wrote:
>>> 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.
>>
>>
>> Hi Jim,
>>
>> I'm wondering about the case where we have a callsite which is marked
>> as 'class foo', but the query string is done by say module and file, so:
>>
>> # echo "module bar file foo.c +p" > /proc/dynamic_debug_control
>>
>> With the proposed code, I think this ends up not enabling anything right?
>
> correct - the only way to enable :pr_debug_cls(CL_FOO, " ...")
> is
>echo class CL_FOO +p > control
>
> 1st, existing dyndbg query uses, whether ad-hoc or scripted,
> were not written in anticipation of new / classified subsystems.
>
> 2nd, new class users dont want to sit in coach. no damn legroom.
>
> 3rd, consider DRM, which already has drm.debug
> ie: /sys/module/drm/parameters/debug
> and prefers it, at least by inertia.
> protecting these new class'd callsites (3-5k of them)
> from casual (unintended) manipulations of the kernel-wide
> dyndbg state seems prudent, and a usability win.
>
> Not everyone will use module bar, requiring "class foo"
> guarantees that changes are intentional.
>
I sort of get that your trying to protect these from unintended toggling,
but I would say it's that's not really new with these statements,
prr_debug() come and go before and I'm not aware of this is an issue.
And in any case, a query can be modified.
I think what bugs me is now query stuff works differently. Previously,
all the query strings - 'module', 'file', 'line', 'format', were
used as additional selectors, but now we have this new one 'class'
that works differently as it's requited for pr_debug_cls() statements.
>
>
>> Because valid class is set to _DPRINTK_CLASS_DFLT and then:
>> 'dp->class_id != valid_class' is true?
>>
>> This seems confusing to me as a user as this doesn't work like the
>> other queriesso maybe we should only do the
>> 'dp->class_id != valid_class' check *if* query->class_string is set,
>> see below.
>>
>
> Could you clarify whether you think this is a logic error
> or a frame-of-reference difference as elaborated above ?
'frame-of-reference' I'm questioning the how 'class' works as mentioned
above not the implementation.
Thanks,
-Jason
>
> ISTM theres a place for a well-worded paragraph in doc
> about the class distinction, perhaps a whole for-authors section.
>
>
>
>>
>>
>>>
>>> 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
>>> ---
>>> lib/dynamic_debug.c | 76 -
>>> 1 file changed, 68 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
>>> index b71efd0b491d..db96ded78c3f 100644
>>> --- a/lib/dynamic_debug.c
>>> +++ b/lib/dynamic_debug.c
>>> @@ -56,6 +56,7 @@ struct ddebug_query {
>>>
Re: [Intel-gfx] [PATCH v6 17/57] dyndbg: validate class FOO by checking with module
On 9/4/22 17:40, Jim Cromie wrote:
> 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.
Hi Jim,
I'm wondering about the case where we have a callsite which is marked
as 'class foo', but the query string is done by say module and file, so:
# echo "module bar file foo.c +p" > /proc/dynamic_debug_control
With the proposed code, I think this ends up not enabling anything right?
Because valid class is set to _DPRINTK_CLASS_DFLT and then:
'dp->class_id != valid_class' is true?
This seems confusing to me as a user as this doesn't work like the
other queriesso maybe we should only do the
'dp->class_id != valid_class' check *if* query->class_string is set,
see below.
>
> 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
> ---
> lib/dynamic_debug.c | 76 -
> 1 file changed, 68 insertions(+), 8 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index b71efd0b491d..db96ded78c3f 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -56,6 +56,7 @@ struct ddebug_query {
> const char *module;
> const char *function;
> const char *format;
> + const char *class_string;
> unsigned int first_lineno, last_lineno;
> };
>
> @@ -136,15 +137,33 @@ static void vpr_info_dq(const struct ddebug_query
> *query, const char *msg)
> fmtlen--;
> }
>
> - v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\"
> lineno=%u-%u\n",
> - msg,
> - query->function ?: "",
> - query->filename ?: "",
> - query->module ?: "",
> - fmtlen, query->format ?: "",
> - query->first_lineno, query->last_lineno);
> + v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\"
> lineno=%u-%u class=%s\n",
> + msg,
> + query->function ?: "",
> + query->filename ?: "",
> + query->module ?: "",
> + fmtlen, query->format ?: "",
> + query->first_lineno, query->last_lineno, query->class_string);
> }
>
> +static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table
> const *dt,
> + const char
> *class_string, int *class_id)
> +{
> + struct ddebug_class_map *map;
> + int idx;
> +
> + list_for_each_entry(map, &dt->maps, link) {
> + idx = match_string(map->class_names, map->length, class_string);
> + if (idx >= 0) {
> + *class_id = idx + map->base;
> + return map;
> + }
> + }
> + *class_id = -ENOENT;
> + return NULL;
> +}
> +
> +#define __outvar /* filled by callee */
> /*
> * Search the tables for _ddebug's which match the given `query' and
> * apply the `flags' and `mask' to them. Returns number of matching
> @@ -159,6 +178,8 @@ static int ddebug_change(const struct ddebug_query *query,
> unsigned int newflags;
> unsigned int nfound = 0;
> struct flagsbuf fbuf, nbuf;
> + struct ddebug_class_map *map = NULL;
> + int __outvar valid_class;
>
> /* search for matching ddebugs */
> mutex_lock(&ddebug_lock);
> @@ -169,9 +190,22 @@ static int ddebug_change(const struct ddebug_query
> *query,
> !match_wildcard(query->module, dt->mod_name))
> continue;
>
> + if (query->class_string) {
> + map = ddebug_find_valid_class(dt, query->class_string,
> &valid_class);
> + if (!map)
> + continue;
So remove the else here.
> + } else {
> + /* constrain query, do not touch class'd callsites */
> + valid_class = _DPRINTK_CLA
Re: [Intel-gfx] [PATCH v6 10/57] dyndbg: cleanup auto vars in dynamic_debug_init
On 9/4/22 17:40, Jim Cromie wrote:
> 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
>
> Signed-off-by: Jim Cromie
> ---
> lib/dynamic_debug.c | 33 +
> 1 file changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index e96dc216463b..2e8ebef3bd0d 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -1059,11 +1059,10 @@ static int __init dynamic_debug_init_control(void)
>
> static int __init dynamic_debug_init(void)
> {
> - struct _ddebug *iter, *iter_start;
> - const char *modname = NULL;
> + struct _ddebug *iter, *iter_mod_start;
> + int ret, i, mod_sites, mod_ct;
> + const char *modname;
> char *cmdline;
> - int ret = 0;
> - int n = 0, entries = 0, modct = 0;
>
> if (&__start___dyndbg == &__stop___dyndbg) {
> if (IS_ENABLED(CONFIG_DYNAMIC_DEBUG)) {
> @@ -1074,30 +1073,32 @@ static int __init dynamic_debug_init(void)
> ddebug_init_success = 1;
> return 0;
> }
> - iter = __start___dyndbg;
> +
> + iter = iter_mod_start = __start___dyndbg;
> modname = iter->modname;
> - iter_start = iter;
> - for (; iter < __stop___dyndbg; iter++) {
> - entries++;
> + i = mod_sites = mod_ct = 0;
> +
> + for (; iter < __stop___dyndbg; iter++, i++, mod_sites++) {
> +
Just a small nit here - not sure why there's an extra blank line here?
But either way:
Acked-by: Jason Baron
> if (strcmp(modname, iter->modname)) {
> - modct++;
> - ret = ddebug_add_module(iter_start, n, modname);
> + mod_ct++;
> + ret = ddebug_add_module(iter_mod_start, mod_sites,
> modname);
> if (ret)
> goto out_err;
> - n = 0;
> +
> + mod_sites = 0;
> modname = iter->modname;
> - iter_start = iter;
> + iter_mod_start = iter;
> }
> - n++;
> }
> - ret = ddebug_add_module(iter_start, n, modname);
> + ret = ddebug_add_module(iter_mod_start, mod_sites, modname);
> if (ret)
> goto out_err;
>
> ddebug_init_success = 1;
> vpr_info("%d prdebugs in %d modules, %d KiB in ddebug tables, %d kiB in
> __dyndbg section\n",
> - entries, modct, (int)((modct * sizeof(struct ddebug_table)) >>
> 10),
> - (int)((entries * sizeof(struct _ddebug)) >> 10));
> + i, mod_ct, (int)((mod_ct * sizeof(struct ddebug_table)) >> 10),
> + (int)((i * sizeof(struct _ddebug)) >> 10));
>
> /* now that ddebug tables are loaded, process all boot args
>* again to find and activate queries given in dyndbg params.
Re: [Intel-gfx] [PATCH v4 16/41] dyndbg: add drm.debug style bitmap support
On 7/20/22 11:32, Jim Cromie wrote:
> Add kernel_param_ops and callbacks to apply a class-map to a
> sysfs-node, which then can 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_classes_bitmap_param, initialized by DYNAMIC_DEBUG_CLASSBITS
> macro, which contains:
>
> BITS: a pointer to the user module's ulong holding the bits/state. By
> ref'g the client's bit-state _var, we coordinate with existing code
> (such as drm_debug_enabled) which uses the _var, so it works
> unchanged, even as the foundation is switched out underneath it..
> Using 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 DEFINE_DYNDBG_CLASSMAP.
>
> map-type: add support here for DD_CLASS_DISJOINT, DD_CLASS_VERBOSE.
>
> These 2 class-types both expect an integer; _DISJOINT treats input
> like a bit-vector (ala drm.debug), and sets each bit accordingly.
>
> _VERBOSE treats input like a bit-pos:N, then sets bits(0..N)=1, and
> bits(N+1..max)=0. This applies (bit bits.
>
> cases DD_CLASS_SYMBOLIC, DD_CLASS_LEVELS are included for the complete
> picture, with commented out call to a following commit.
>
> NOTES:
>
> this now includes SYMBOLIC/LEVELS support, too tedious to keep
> separate thru all the tweaking.
>
> get-param undoes the bit-pos -> bitmap transform that set-param does
> on VERBOSE inputs, this gives the read-what-was-written property.
>
> _VERBOSE is overlay on _DISJOINT:
>
> verbose-maps still need class-names, even though theyre not usable at
> the sysfs interface (unlike with _SYMBOLIC/_LEVELS).
>
> - 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
>
> IOW this is possible, and maybe confusing:
>
> echo class V3 +p > control
> echo class V1 -p > control
>
> IMO thats ok, relative verbosity is an interface property.
>
> Signed-off-by: Jim Cromie
> ---
> . drop kp->mod->name as unneeded (build-dependent)
> ---
> include/linux/dynamic_debug.h | 18
> lib/dynamic_debug.c | 193 ++
> 2 files changed, 211 insertions(+)
>
> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> index f57076e02767..b50bdd5c8184 100644
> --- a/include/linux/dynamic_debug.h
> +++ b/include/linux/dynamic_debug.h
> @@ -113,6 +113,12 @@ struct ddebug_class_map {
> #define NUM_TYPE_ARGS(eltype, ...) \
> (sizeof((eltype[]) {__VA_ARGS__}) / sizeof(eltype))
>
> +struct ddebug_classes_bitmap_param {
> + unsigned long *bits;
> + char flags[8];
> + const struct ddebug_class_map *map;
> +};
> +
> #if defined(CONFIG_DYNAMIC_DEBUG_CORE)
>
> int ddebug_add_module(struct _ddebug *tab, unsigned int num_debugs,
> @@ -274,6 +280,10 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
> KERN_DEBUG, prefix_str, prefix_type, \
> rowsize, groupsize, buf, len, ascii)
>
> +struct kernel_param;
> +int param_set_dyndbg_classes(const char *instr, const struct kernel_param
> *kp);
> +int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp);
> +
> /* for test only, generally expect drm.debug style macro wrappers */
> #define __pr_debug_cls(cls, fmt, ...) do { \
> BUILD_BUG_ON_MSG(!__builtin_constant_p(cls),\
> @@ -322,6 +332,14 @@ static inline int ddebug_dyndbg_module_param_cb(char
> *param, char *val,
> rowsize, groupsize, buf, len, ascii); \
> } while (0)
>
> +struct kernel_param;
> +static inline int param_set_dyndbg_classes(const char *instr, const struct
> kernel_param *kp)
> +{ return 0; }
> +static inline int param_get_dyndbg_classes(char *buffer, const struct
> kernel_param *kp)
> +{ return 0; }
> +
> #endif /* !CONFIG_DYNAMIC_DEBUG_CORE */
>
> +extern const struct kernel_param_ops param_ops_dyndbg_classes;
> +
> #endif
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 4c27bbe5187e..dd27dc514aa3 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -596,6 +596,199 @@ static int ddebug_exec_queries(char *query, const char
> *modname)
> return nfound;
> }
>
> +static int ddebug_apply_cl
Re: [Intel-gfx] [PATCH v4 00/41] DYNDBG: opt-in class'd debug for modules, use in drm.
On 8/3/22 15:56, jim.cro...@gmail.com wrote: > On Wed, Jul 20, 2022 at 9:32 AM Jim Cromie wrote: >> > >> Hi Jason, Greg, DRM-folk, >> >> This adds 'typed' "class FOO" support to dynamic-debug, where 'typed' >> means either DISJOINT (like drm debug categories), or VERBOSE (like >> nouveau debug-levels). Use it in DRM modules: core, helpers, and in >> drivers i915, amdgpu, nouveau. >> > > This revision fell over, on a conflict with something in drm-MUMBLE > > Error: patch > https://urldefense.com/v3/__https://patchwork.freedesktop.org/api/1.0/series/106427/revisions/2/mbox/__;!!GjvTz_vk!UCPl5Uf32cDVwwysMTfaLwoGLWomargFXuR8HjBA3xsUOjxXHXC5hneAkP4iWK91yc-LjjJxWW89-51Z$ > > not applied > Applying: dyndbg: fix static_branch manipulation > Applying: dyndbg: fix module.dyndbg handling > Applying: dyndbg: show both old and new in change-info > Applying: dyndbg: reverse module walk in cat control > Applying: dyndbg: reverse module.callsite walk in cat control > Applying: dyndbg: use ESCAPE_SPACE for cat control > Applying: dyndbg: let query-modname override actual module name > Applying: dyndbg: add test_dynamic_debug module > Applying: dyndbg: drop EXPORTed dynamic_debug_exec_queries > > Jason, > those above are decent maintenance patches, particularly the drop export. > It would be nice to trim this unused api this cycle. Hi Jim, Agreed - I was thinking the same thing. Feel free to add Acked-by: Jason Baron to those first 9. > > Applying: dyndbg: add class_id to pr_debug callsites > Applying: dyndbg: add __pr_debug_cls for testing > Applying: dyndbg: add DECLARE_DYNDBG_CLASSMAP > Applying: kernel/module: add __dyndbg_classes section > Applying: dyndbg: add ddebug_attach_module_classes > Applying: dyndbg: validate class FOO by checking with module > Applying: dyndbg: add drm.debug style bitmap support > Applying: dyndbg: test DECLARE_DYNDBG_CLASSMAP, sysfs nodes > Applying: doc-dyndbg: describe "class CLASS_NAME" query support > Applying: doc-dyndbg: edit dynamic-debug-howto for brevity, audience > Applying: drm_print: condense enum drm_debug_category > Applying: drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers. > Applying: drm_print: interpose drm_*dbg with forwarding macros > Applying: drm_print: wrap drm_*_dbg in dyndbg descriptor factory macro > Using index info to reconstruct a base tree... > M drivers/gpu/drm/Kconfig > M drivers/gpu/drm/Makefile > Falling back to patching base and 3-way merge... > Auto-merging drivers/gpu/drm/Makefile > Auto-merging drivers/gpu/drm/Kconfig > CONFLICT (content): Merge conflict in drivers/gpu/drm/Kconfig > error: Failed to merge in the changes. > > > Before I resend, I should sort out that possible conflict > which tree is patchwork applied upon ? > > or was it just transient ? after 5.19 I rebased a copy onto drm-next/drm-next, > and there was nothing to fix - I will revisit presently.. Not sure, if it's a minor conflict maybe Greg KH can sort it when he pulls it in? If not yeah might be important to rebase first...Greg? Thanks, -Jason
Re: [Intel-gfx] [PATCH v4 12/41] dyndbg: add DECLARE_DYNDBG_CLASSMAP
On 7/20/22 11:32, Jim Cromie wrote: > DECLARE_DYNDBG_CLASSMAP lets modules declare a set of classnames, this > opt-in authorizes dyndbg to allow enabling of prdbgs by their class: > >:#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control > > This is just the setup; following commits deliver. > > The macro declares and initializes a static struct ddebug_class_map: > > - maps approved class-names to class_ids used in module, >by array order. forex: DRM_UT_* > - class-name vals allow validation of "class FOO" queries >using macro is opt-in > - enum class_map_type - determines interface, behavior > > Each module has its own .class_id space, and only known class-names > will be authorized for a manipulation. Only DRM stuff should know this: > > :#> echo class DRM_UT_CORE +p > control # across all modules > > pr_debugs (with default class_id) are still controllable as before. > > DECLARE_DYNDBG_CLASSMAP(_var, _maptype, _base, classes...) is:: > > _var: name of the static struct var. user passes to module_param_cb() >if they want a sysfs node. (ive only done it this way). > > _maptype: this is hard-coded to DD_CLASS_TYPE_DISJOINT for now. > > _base: usually 0, it allows splitting 31 classes into subranges, so > that multiple classes / sysfs-nodes can share the module's > class-id space. > > classes: list of class_name strings, these are mapped to class-ids > starting at _base. This class-names list must have a > corresponding ENUM, with SYMBOLS that match the literals, > and 1st enum val = _base. > > enum class_map_type has 4 values, on 2 factors:: > > - classes are disjoint/independent vs relative/xdisjoint is basis, verbosity by overlay. > > - input NUMBERS vs [+-]CLASS_NAMES >uints, ideally hex. vs +DRM_UT_CORE,-DRM_UT_KMS > Could the naming here directly reflect the 2 factors? At least for me I think it's more readable. So something like: > DD_CLASS_TYPE_DISJOINT: classes are separate, one per bit. >expecting hex input. basis for others. DD_CLASS_TYPE_DISJOINT_NUM > > DD_CLASS_TYPE_SYMBOLIC: input is a CSV of [+-]CLASS_NAMES, >classes are independent, like DISJOINT > DD_CLASS_TYPE_DISJOINT_NAME > DD_CLASS_TYPE_VERBOSE: input is numeric level, 0-N. >0 implies silence. use printk to break that. >relative levels applied on bitmaps. > DD_CLASS_TYPE_LEVEL_NUM > DD_CLASS_TYPE_LEVELS: input is a CSV of [+-]CLASS_NAMES, >names like: ERR,WARNING,NOTICE,INFO,DEBUG >avoiding EMERG,ALERT,CRIT,ERR - no point. > DD_CLASS_TYPE_LEVEL_NAME > NOTES: > > The macro places the initialized struct ddebug_class_map into the > __dyndbg_classes section. That draws a 'orphan' warning which we > handle in next commit. The struct attributes are necessary: > __aligned(8) stopped null-ptr derefs (why?), __used is needed by drm > drivers, which declare class-maps, but don't also declare a > sysfs-param, and thus dont ref the classmap; __used insures that the > linkage is made, then the class-map is found at load-time. > > While its possible to handle both NAMES and NUMBERS in the same > sys-interface, there is ambiguity to avoid, by disallowing them > together. Later, if ambiguities are resolved, 2 new enums can permit > both inputs, on verbose & independent types separately, and authors > can select the interface they like. > > RFC: > > My plan is to implement LEVELS in the callbacks, outside of > ddebug_exec_query(), which for simplicity will treat the CLASSES in > the map as disjoint. This should handle most situations. > > The callbacks can see map-type, and apply ++/-- loops (or bitops) to > force the relative meanings across the class-bitmap. > > That leaves 2 issues: > > 1. doing LEVELs in callbacks means that the native >control interface > doesn't enforce the LEVELS relationship, so you could confusingly have > V3 enabled, but V1 disabled. OTOH, the control iface already allows > infinite variety in the underlying callsites, despite the veneer of > uniformity suggested by the bitmap overlay, and LEVELS over that. > > 2. All dyndbg >control reduces to a query/command, includes +/-, which > is at-root a kernel patching operation with +/- semantics. And the > symbolic sys-node handling exposes it to the user: > > Consider whether these are/should-be 'exactly' the same: > ># force both 2 "half-duplex" relations >echo +V3,-V4 > /sys/module/test_dynamic_debug/p_VX > ># should these both do the same ? >echo +V3 > /sys/module/test_dynamic_debug/p_VX >echo -V4 > /sys/module/test_dynamic_debug/p_VX > > IOW, half relations are suggested by the +/-, and enum control of > individual behaviors leaves some room for this, especially wrt > handling [+-]SYMBOLIC inputs from the user. > > Signed-off-by: Jim Cromie > --- > include/linux/dynamic_debug.h | 55 +++ > 1 file changed, 55 insertions(+) > > diff --git a/include/linux/dynamic_d
Re: [Intel-gfx] [PATCH 2/5] dyndbg: add class_id field and query support
On 3/10/22 23:47, Jim Cromie wrote:
> DRM defines/uses 10 enum drm_debug_category's to create exclusive
> classes of debug messages. To support this directly in dynamic-debug,
> add the following:
>
> - struct _ddebug.class_id:4 - 4 bits is enough
> - define _DPRINTK_SITE_UNCLASSED 15 - see below
>
> and the query support:
> - struct _ddebug_query.class_id
> - ddebug_parse_query - looks for "class" keyword, then calls..
> - parse_class - accepts uint: 0-15, sets query.class_id and marker
> - vpr_info_dq - displays new field
> - ddebug_proc_show- append column with "cls:%d" if !UNCLASSED
>
> With the patch, one can enable current/unclassed callsites by:
>
> #> echo drm class 15 +p > /proc/dynamic_debug/control
>
To me, this is hard to read, what the heck is '15'? I have to go look it
up in the control file and it's not descriptive. I think that using
classes/categories makes sense but I'm wondering if it can be a bit more
user friendly? Perhaps, we can pass an array of strings that is indexed
by the class id to each pr_debug() site that wants to use class. So
something like:
enum levels {
LOW,
MEDIUM,
HIGH
};
static const char * const level_to_strings[] = {
[LOW] = "low",
[MEDIUM] = "medium",
[HIGH] = "high",
};
And then you'd have a wrapper macros in your driver:
#define module_foo_pr_debug_class(level, fmt, args...)
pr_debug_class(level, level_to_strings, fmt, args);
Such that call sites look like:
module_foo_pr_debug_class(LOW, fmt, args...);
Such that you're not always passing the strings array around. Now, this
does mean another pointer for struct _ddebug and most wouldn't have it.
Maybe we could just add another linker section for these so as to save
space.
> parse_class() accepts 0 .. _DPRINTK_SITE_UNCLASSED, which allows the
>> control interface to explicitly manipulate unclassed callsites.
>
> After parsing keywords, ddebug_parse_query() sets .class_id=15, iff it
> wasnt explicitly set. This allows future classed/categorized
> callsites to be untouched by legacy (class unaware) queries.
>
> DEFINE_DYNAMIC_DEBUG_METADATA gets _CLS(cls,) suffix and arg, and
> initializes the new .class_id=cls field. The old name gets the default.
>
> Then, these _CLS(cls,...) modifications are repeated up through the
> stack of *dynamic_func_call* macros that use the METADATA initializer,
> so as to actually supply the category into it.
>
> NOTES:
>
> _DPRINTK_SITE_UNCLASSED: this symbol is used to initialize all
> existing/unclassed pr-debug callsites. Normally, the default would be
> zero, but DRM_UT_CORE "uses" that value, in the sense that 0 is
> exposed as a bit position in drm.debug. Using 15 allows identity
> mapping from category to class, avoiding fiddly offsets.
>
> Default .class_id = 15 means that ``echo +p > control`` no longer
> toggles ALL the callsites, only the unclassed ones. This was only
> useful for static-branch toggle load testing anyway.
>
I think that # echo +p > control should continue to work as is, why
should the introduction of classes change that ?
> RFC:
>
> The new _CLS macro flavor gets a warning from DRM/dri-devel's CI,
> but not from checkpatch (on this subject).
>
> a8f6c71f283e dyndbg: add class_id field and query support
> -:141: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'id' - possible
> side-effects?
> +#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do {\
> + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt);\
> + if (DYNAMIC_DEBUG_BRANCH(id)) \
> + func(&id, ##__VA_ARGS__); \
> } while (0)
>
> I couldn't fix it with a ``typeof(id) _id = id`` construct. I haven't
> seen the warning myself, on the _CLS extended macro, nor the original.
>
> CC: Rasmus Villemoes
> Signed-off-by: Jim Cromie
> ---
> .../admin-guide/dynamic-debug-howto.rst | 7 +++
> include/linux/dynamic_debug.h | 54 ++-
> lib/dynamic_debug.c | 48 ++---
> 3 files changed, 88 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst
> b/Documentation/admin-guide/dynamic-debug-howto.rst
> index a89cfa083155..8ef8d7dcd140 100644
> --- a/Documentation/admin-guide/dynamic-debug-howto.rst
> +++ b/Documentation/admin-guide/dynamic-debug-howto.rst
> @@ -35,6 +35,7 @@ Dynamic debug has even more useful features:
> - line number (including ranges of line numbers)
> - module name
> - format string
> + - class number:0-15
>
> * Provides a debugfs control file: ``/dynamic_debug/control``
> which can be read to display the complete list of known debug
> @@ -143,6 +144,7 @@ against. Possible keywords are:::
>'module' string |
>'format' string |
>'line' line-range
> + 'class' integer:[0-15]
>
>line-range ::= linen
Re: [Intel-gfx] [PATCH 1/5] dyndbg: fix static_branch manipulation
On 3/10/22 23:47, Jim Cromie wrote:
> In
> https://urldefense.com/v3/__https://lore.kernel.org/lkml/20211209150910.ga23...@axis.com/__;!!GjvTz_vk!HGKKoni4RVdEBgv_V0zPSNSX428bpf02zkCy2WbeQkBdVtp1QJqGX-lJYlRDGg$
>
>
> 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 work
> properly, 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: vincent.whitchu...@axis.com
> Signed-off-by: Jim Cromie
>
> --
> .drop @stable, no exposed bug.
> ---
> lib/dynamic_debug.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index dd7f56af9aed..a56c1286ffa4 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -211,10 +211,11 @@ static int ddebug_change(const struct ddebug_query
> *query,
> continue;
> #ifdef CONFIG_JUMP_LABEL
> if (dp->flags & _DPRINTK_FLAGS_PRINT) {
> - if (!(modifiers->flags & _DPRINTK_FLAGS_PRINT))
> + if (!(newflags & _DPRINTK_FLAGS_PRINT))
>
> static_branch_disable(&dp->key.dd_key_true);
> - } else if (modifiers->flags & _DPRINTK_FLAGS_PRINT)
> + } else if (newflags & _DPRINTK_FLAGS_PRINT) {
> static_branch_enable(&dp->key.dd_key_true);
> + }
> #endif
> dp->flags = newflags;
> v4pr_info("changed %s:%d [%s]%s =%s\n",
Hi Jim,
If iiuc this is currently a bug but could be if we add a second 'print' bit
such as for printing to the tracing logs. That said I agree that using
'newflags'
here makes the code more straightforward/readable. So this one is fine with
me.
Acked-by: Jason Baron
Thanks,
-Jason
Re: [Intel-gfx] [PATCH 2/5] dyndbg: add class_id field and query support
On 3/11/22 20:06, jim.cro...@gmail.com wrote:
> On Fri, Mar 11, 2022 at 12:06 PM Jason Baron wrote:
>>
>>
>>
>> On 3/10/22 23:47, Jim Cromie wrote:
>>> DRM defines/uses 10 enum drm_debug_category's to create exclusive
>>> classes of debug messages. To support this directly in dynamic-debug,
>>> add the following:
>>>
>>> - struct _ddebug.class_id:4 - 4 bits is enough
>>> - define _DPRINTK_SITE_UNCLASSED 15 - see below
>>>
>>> and the query support:
>>> - struct _ddebug_query.class_id
>>> - ddebug_parse_query - looks for "class" keyword, then calls..
>>> - parse_class - accepts uint: 0-15, sets query.class_id and marker
>>> - vpr_info_dq - displays new field
>>> - ddebug_proc_show- append column with "cls:%d" if !UNCLASSED
>>>
>>> With the patch, one can enable current/unclassed callsites by:
>>>
>>> #> echo drm class 15 +p > /proc/dynamic_debug/control
>>>
>>
>> To me, this is hard to read, what the heck is '15'? I have to go look it
>> up in the control file and it's not descriptive. I think that using
>> classes/categories makes sense but I'm wondering if it can be a bit more
>> user friendly? Perhaps, we can pass an array of strings that is indexed
>> by the class id to each pr_debug() site that wants to use class. So
>> something like:
>>
>
> Im not at all averse to nice names, but as something added on.
>
> 1st, the interface to make friendlier is DRM's
>
> echo 0x04 > /sys/module/drm/parameters/debug # which category ?
>
> parm: debug:Enable debug output, where each bit enables a
> debug category.
> Bit 0 (0x01) will enable CORE messages (drm core code)
> Bit 1 (0x02) will enable DRIVER messages (drm controller code)
> Bit 2 (0x04) will enable KMS messages (modesetting code)
>
> echo DRM_UT_DRIVER,DRM_UT_KMS > /sys/module/drm/parameters/debug #
> now its pretty clear
>
> If that works, its cuz DEFINE_DYNAMIC_DEBUG_CLASSBITS()
> plucks class symbols out of its __VA_ARGS__, and #stringifes them.
> So that macro could then build the 1-per-module static constant string array
> and (only) the callbacks would be able to use it.
>
> from there, it shouldnt be hard to ref that from the module's ddebug_table,
> so parse_query could validate class args against the module given (or
> fail if none given)
>
> Speaking strictly, theres a gap:
>echo module * class DRM_UT_KMS +p > control
> is nonsense for * other than drm + drivers,
> but its fair/safe to just disallow wildcards on modname for this purpose.
>
> it does however imply that module foo must exist for FOO_CAT_1 to be usable.
> thats not currently the case:
> bash-5.1# echo module foo +p > /proc/dynamic_debug/control
> [ 15.403749] dyndbg: read 14 bytes from userspace
> [ 15.405413] dyndbg: query 0: "module foo +p" mod:*
> [ 15.406486] dyndbg: split into words: "module" "foo" "+p"
> [ 15.407070] dyndbg: op='+'
> [ 15.407388] dyndbg: flags=0x1
> [ 15.407809] dyndbg: *flagsp=0x1 *maskp=0x
> [ 15.408300] dyndbg: parsed: func="" file="" module="foo" format=""
> lineno=0-0 class=15
> [ 15.409151] dyndbg: no matches for query
> [ 15.409591] dyndbg: no-match: func="" file="" module="foo"
> format="" lineno=0-0 class=15
> [ 15.410524] dyndbg: processed 1 queries, with 0 matches, 0 errs
> bash-5.1#
>
> ISTM we can keep that "0 errs" response for that case, but still reject this:
>
>echo module foo class FOO_NOT_HERE +p > /proc/dynamic_debug/control
>
>
Ok, yeah so I guess I'm thinking about the 'class' more as global namespace,
so that for example, it could span modules, or be specific to certain modules.
I'm also thinking of it as a 'string' which is maybe hierarchical, so that it's
clear what sub-system, or sub-sub-system it belongs to. So for DRM for example,
it could be a string like "DRM:CORE". The index num I think is still helpful for
implementation so we don't have to store a pointer size, but I don't think it's
really exposed (except perhaps in module params b/c drm is doing that already?).
>> enum levels {
>> LOW,
>> MEDIUM,
>> HIGH
>> };
>
> I want to steer clear of "level" anything,
> since 2>1 implies non independence of the categories
>
>
Agreed, that was a bad example on my part.
I've put together a rou
Re: [Intel-gfx] [PATCH v10 08/10] dyndbg: add print-to-tracefs, selftest with it - RFC
On 11/18/21 10:24 AM, Pekka Paalanen wrote: > On Thu, 18 Nov 2021 09:29:27 -0500 > Jason Baron wrote: > >> On 11/16/21 3:46 AM, Pekka Paalanen wrote: >>> On Fri, 12 Nov 2021 10:08:41 -0500 >>> Jason Baron wrote: >>> >>>> On 11/12/21 6:49 AM, Vincent Whitchurch wrote: >>>>> On Thu, Nov 11, 2021 at 03:02:04PM -0700, Jim Cromie wrote: >>>>>> Sean Paul proposed, in: >>>>>> https://urldefense.com/v3/__https://patchwork.freedesktop.org/series/78133/__;!!GjvTz_vk!HcKnMRByYkIdyF1apqQjlN5aBIomzJR1an3YWXM6KXs0EftVMQdrewRA8Dki4A$ >>>>>> >>>>>> drm/trace: Mirror DRM debug logs to tracefs >>>>>> >>>>>> His patchset's objective is to be able to independently steer some of >>>>>> the drm.debug stream to an alternate tracing destination, by splitting >>>>>> drm_debug_enabled() into syslog & trace flavors, and enabling them >>>>>> separately. 2 advantages were identified: >>>>>> >>>>>> 1- syslog is heavyweight, tracefs is much lighter >>>>>> 2- separate selection of enabled categories means less traffic >>>>>> >>>>>> Dynamic-Debug can do 2nd exceedingly well: >>>>>> >>>>>> A- all work is behind jump-label's NOOP, zero off cost. >>>>>> B- exact site selectivity, precisely the useful traffic. >>>>>>can tailor enabled set interactively, at shell. >>>>>> >>>>>> Since the tracefs interface is effective for drm (the threads suggest >>>>>> so), adding that interface to dynamic-debug has real potential for >>>>>> everyone including drm. >>>>>> >>>>>> if CONFIG_TRACING: >>>>>> >>>>>> Grab Sean's trace_init/cleanup code, use it to provide tracefs >>>>>> available by default to all pr_debugs. This will likely need some >>>>>> further per-module treatment; perhaps something reflecting hierarchy >>>>>> of module,file,function,line, maybe with a tuned flattening. >>>>>> >>>>>> endif CONFIG_TRACING >>>>>> >>>>>> Add a new +T flag to enable tracing, independent of +p, and add and >>>>>> use 3 macros: dyndbg_site_is_enabled/logging/tracing(), to encapsulate >>>>>> the flag checks. Existing code treats T like other flags. >>>>> >>>>> I posted a patchset a while ago to do something very similar, but that >>>>> got stalled for some reason and I unfortunately didn't follow it up: >>>>> >>>>> >>>>> https://urldefense.com/v3/__https://lore.kernel.org/lkml/20200825153338.17061-1-vincent.whitchu...@axis.com/__;!!GjvTz_vk!HcKnMRByYkIdyF1apqQjlN5aBIomzJR1an3YWXM6KXs0EftVMQdrewRGytKHPg$ >>>>> >>>>> >>>>> A key difference between that patchset and this patch (besides that >>>>> small fact that I used +x instead of +T) was that my patchset allowed >>>>> the dyndbg trace to be emitted to the main buffer and did not force them >>>>> to be in an instance-specific buffer. >>>> >>>> Yes, I agree I'd prefer that we print here to the 'main' buffer - it >>>> seems to keep things simpler and easier to combine the output from >>>> different sources as you mentioned. >>> >>> Hi, >>> >>> I'm not quite sure I understand this discussion, but I would like to >>> remind you all of what Sean's original work is about: >>> >>> Userspace configures DRM tracing into a flight recorder buffer (I guess >>> this is what you refer to "instance-specific buffer"). >>> >>> Userspace runs happily for months, and then hits a problem: a failure >>> in the DRM sub-system most likely, e.g. an ioctl that should never >>> fail, failed. Userspace handles that failure by dumping the flight >>> recorder buffer into a file and saving or sending a bug report. The >>> flight recorder contents give a log of all relevant DRM in-kernel >>> actions leading to the unexpected failure to help developers debug it. >>> >>> I don't mind if one can additionally send the flight recorder stream to >>> the main buffer, but I do want the separate flight recorder buffer to >>> be an option so that a) unrelated things cannot fl
Re: [Intel-gfx] [PATCH v10 08/10] dyndbg: add print-to-tracefs, selftest with it - RFC
On 11/16/21 3:46 AM, Pekka Paalanen wrote: > On Fri, 12 Nov 2021 10:08:41 -0500 > Jason Baron wrote: > >> On 11/12/21 6:49 AM, Vincent Whitchurch wrote: >>> On Thu, Nov 11, 2021 at 03:02:04PM -0700, Jim Cromie wrote: >>>> Sean Paul proposed, in: >>>> https://urldefense.com/v3/__https://patchwork.freedesktop.org/series/78133/__;!!GjvTz_vk!HcKnMRByYkIdyF1apqQjlN5aBIomzJR1an3YWXM6KXs0EftVMQdrewRA8Dki4A$ >>>> >>>> drm/trace: Mirror DRM debug logs to tracefs >>>> >>>> His patchset's objective is to be able to independently steer some of >>>> the drm.debug stream to an alternate tracing destination, by splitting >>>> drm_debug_enabled() into syslog & trace flavors, and enabling them >>>> separately. 2 advantages were identified: >>>> >>>> 1- syslog is heavyweight, tracefs is much lighter >>>> 2- separate selection of enabled categories means less traffic >>>> >>>> Dynamic-Debug can do 2nd exceedingly well: >>>> >>>> A- all work is behind jump-label's NOOP, zero off cost. >>>> B- exact site selectivity, precisely the useful traffic. >>>>can tailor enabled set interactively, at shell. >>>> >>>> Since the tracefs interface is effective for drm (the threads suggest >>>> so), adding that interface to dynamic-debug has real potential for >>>> everyone including drm. >>>> >>>> if CONFIG_TRACING: >>>> >>>> Grab Sean's trace_init/cleanup code, use it to provide tracefs >>>> available by default to all pr_debugs. This will likely need some >>>> further per-module treatment; perhaps something reflecting hierarchy >>>> of module,file,function,line, maybe with a tuned flattening. >>>> >>>> endif CONFIG_TRACING >>>> >>>> Add a new +T flag to enable tracing, independent of +p, and add and >>>> use 3 macros: dyndbg_site_is_enabled/logging/tracing(), to encapsulate >>>> the flag checks. Existing code treats T like other flags. >>> >>> I posted a patchset a while ago to do something very similar, but that >>> got stalled for some reason and I unfortunately didn't follow it up: >>> >>> >>> https://urldefense.com/v3/__https://lore.kernel.org/lkml/20200825153338.17061-1-vincent.whitchu...@axis.com/__;!!GjvTz_vk!HcKnMRByYkIdyF1apqQjlN5aBIomzJR1an3YWXM6KXs0EftVMQdrewRGytKHPg$ >>> >>> >>> A key difference between that patchset and this patch (besides that >>> small fact that I used +x instead of +T) was that my patchset allowed >>> the dyndbg trace to be emitted to the main buffer and did not force them >>> to be in an instance-specific buffer. >> >> Yes, I agree I'd prefer that we print here to the 'main' buffer - it >> seems to keep things simpler and easier to combine the output from >> different sources as you mentioned. > > Hi, > > I'm not quite sure I understand this discussion, but I would like to > remind you all of what Sean's original work is about: > > Userspace configures DRM tracing into a flight recorder buffer (I guess > this is what you refer to "instance-specific buffer"). > > Userspace runs happily for months, and then hits a problem: a failure > in the DRM sub-system most likely, e.g. an ioctl that should never > fail, failed. Userspace handles that failure by dumping the flight > recorder buffer into a file and saving or sending a bug report. The > flight recorder contents give a log of all relevant DRM in-kernel > actions leading to the unexpected failure to help developers debug it. > > I don't mind if one can additionally send the flight recorder stream to > the main buffer, but I do want the separate flight recorder buffer to > be an option so that a) unrelated things cannot flood the interesting > bits out of it, and b) the scope of collected information is relevant. > > The very reason for this work is problems that are very difficult to > reproduce in practice, either because the problem itself is triggered > very rarely and randomly, or because the end users of the system have > either no knowledge or no access to reconfigure debug logging and then > reproduce the problem with good debug logs. > > Thank you very much for pushing this work forward! > > So I think Vincent (earlier in the thread) was saying that he finds it very helpful have dynamic debug output go to the 'main' trace buffer, while you seem to be saying you'd prefer it just go to dynamic debug specific trace buffer. So we certainly can have dynamic output potentially go to both places - although I think this would mean two tracepoints? But I really wonder if we really need a separate tracing buffer for dynamic debug when what goes to the 'main' buffer can be controlled and filtered to avoid your concern around a 'flood'? Thanks, -Jason
Re: [Intel-gfx] [PATCH v10 08/10] dyndbg: add print-to-tracefs, selftest with it - RFC
On 11/12/21 6:49 AM, Vincent Whitchurch wrote: > On Thu, Nov 11, 2021 at 03:02:04PM -0700, Jim Cromie wrote: >> Sean Paul proposed, in: >> https://urldefense.com/v3/__https://patchwork.freedesktop.org/series/78133/__;!!GjvTz_vk!HcKnMRByYkIdyF1apqQjlN5aBIomzJR1an3YWXM6KXs0EftVMQdrewRA8Dki4A$ >> >> drm/trace: Mirror DRM debug logs to tracefs >> >> His patchset's objective is to be able to independently steer some of >> the drm.debug stream to an alternate tracing destination, by splitting >> drm_debug_enabled() into syslog & trace flavors, and enabling them >> separately. 2 advantages were identified: >> >> 1- syslog is heavyweight, tracefs is much lighter >> 2- separate selection of enabled categories means less traffic >> >> Dynamic-Debug can do 2nd exceedingly well: >> >> A- all work is behind jump-label's NOOP, zero off cost. >> B- exact site selectivity, precisely the useful traffic. >>can tailor enabled set interactively, at shell. >> >> Since the tracefs interface is effective for drm (the threads suggest >> so), adding that interface to dynamic-debug has real potential for >> everyone including drm. >> >> if CONFIG_TRACING: >> >> Grab Sean's trace_init/cleanup code, use it to provide tracefs >> available by default to all pr_debugs. This will likely need some >> further per-module treatment; perhaps something reflecting hierarchy >> of module,file,function,line, maybe with a tuned flattening. >> >> endif CONFIG_TRACING >> >> Add a new +T flag to enable tracing, independent of +p, and add and >> use 3 macros: dyndbg_site_is_enabled/logging/tracing(), to encapsulate >> the flag checks. Existing code treats T like other flags. > > I posted a patchset a while ago to do something very similar, but that > got stalled for some reason and I unfortunately didn't follow it up: > > > https://urldefense.com/v3/__https://lore.kernel.org/lkml/20200825153338.17061-1-vincent.whitchu...@axis.com/__;!!GjvTz_vk!HcKnMRByYkIdyF1apqQjlN5aBIomzJR1an3YWXM6KXs0EftVMQdrewRGytKHPg$ > > > A key difference between that patchset and this patch (besides that > small fact that I used +x instead of +T) was that my patchset allowed > the dyndbg trace to be emitted to the main buffer and did not force them > to be in an instance-specific buffer. Yes, I agree I'd prefer that we print here to the 'main' buffer - it seems to keep things simpler and easier to combine the output from different sources as you mentioned. Thanks, -Jason > > That feature is quite important at least for my use case since I often > use dyndbg combined with function tracing, and the latter doesn't work > on non-main instances according to Documentation/trace/ftrace.rst. > > For example, here's a random example of a bootargs from one of my recent > debugging sessions: > > trace_event=printk:* ftrace_filter=_mmc*,mmc*,sd*,dw_mci*,mci* > ftrace=function trace_buf_size=20M dyndbg="file drivers/mmc/* +x" >
Re: [Intel-gfx] [PATCH v10 08/10] dyndbg: add print-to-tracefs, selftest with it - RFC
On 11/12/21 12:07 PM, Steven Rostedt wrote: > On Fri, 12 Nov 2021 10:08:41 -0500 > Jason Baron wrote: > >>> A key difference between that patchset and this patch (besides that >>> small fact that I used +x instead of +T) was that my patchset allowed >>> the dyndbg trace to be emitted to the main buffer and did not force them >>> to be in an instance-specific buffer. >> >> Yes, I agree I'd prefer that we print here to the 'main' buffer - it seems >> to keep things simpler and easier to combine the output from different >> sources as you mentioned. > > I do not want anything to print to the "main buffer" that can not be > filtered or turned off by the tracing infrastructure itself (aka tracefs > file system). > > Once we allow that, then the trace file will become useless because > everything will write to the main buffer and fill it with noise. > > Events that can be enabled and disabled from tracefs are fine, as they can > be limited. This is why I added that nasty warning if people leave around > trace_printk(), because it does exactly this (write to the main buffer). > It's fine for debugging a custom kernel, but should never be enabled in > something that is shipped, or part of mainline. > > -- Steve > Ok, it looks like Vincent's patch defines a dyndbg event and then uses 'trace_dyndbg()' to output to the 'main' log. So all dynamic output to the 'main' ftrace buffer goes through that event if I understand it correctly. Here's a pointer to it for reference: https://lore.kernel.org/lkml/20200825153338.17061-3-vincent.whitchu...@axis.com/ Would you be ok with that approach? Thanks, -Jason
Re: [Intel-gfx] [PATCH v9 10/10] drm: use DEFINE_DYNAMIC_DEBUG_TRACE_CATEGORIES bitmap to tracefs
On 10/27/21 12:36 AM, Jim Cromie wrote:
> Use new macro to create a sysfs control bitmap knob to control
> print-to-trace in: /sys/module/drm/parameters/trace
>
> todo: reconsider this api, ie a single macro expecting both debug &
> trace terms (2 each), followed by a single description and the
> bitmap-spec::
>
> Good: declares bitmap once for both interfaces
>
> Bad: arg-type/count handling (expecting 4 args) is ugly,
> especially preceding the bitmap-init var-args.
>
Hi Jim,
I agree having the bitmap declared twice seems redundant. But I like having
fewer args and not necessarily combining the trace/log variants of
DEBUG_CATEGORIES. hmmm...what if the DEFINE_DYNAMIC_DEBUG_CATEGORIES() took a
pointer to the array of struct dyndbg_bitdesc map[] directly as the
final argument instead of the __VA_ARGS__? Then, we could just declare the map
once?
Thanks,
-Jason
> Signed-off-by: Jim Cromie
> ---
> drivers/gpu/drm/drm_print.c | 19 +++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index ce662d0f339b..7b49fbc5e21d 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -73,6 +73,25 @@ DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug, __drm_debug,
> [7] = { DRM_DBG_CAT_LEASE },
> [8] = { DRM_DBG_CAT_DP },
> [9] = { DRM_DBG_CAT_DRMRES });
> +
> +#ifdef CONFIG_TRACING
> +unsigned long __drm_trace;
> +EXPORT_SYMBOL(__drm_trace);
> +DEFINE_DYNAMIC_DEBUG_TRACE_CATEGORIES(trace, __drm_trace,
> + DRM_DEBUG_DESC,
> + [0] = { DRM_DBG_CAT_CORE },
> + [1] = { DRM_DBG_CAT_DRIVER },
> + [2] = { DRM_DBG_CAT_KMS },
> + [3] = { DRM_DBG_CAT_PRIME },
> + [4] = { DRM_DBG_CAT_ATOMIC },
> + [5] = { DRM_DBG_CAT_VBL },
> + [6] = { DRM_DBG_CAT_STATE },
> + [7] = { DRM_DBG_CAT_LEASE },
> + [8] = { DRM_DBG_CAT_DP },
> + [9] = { DRM_DBG_CAT_DRMRES });
> +
> +struct trace_array *trace_arr;
> +#endif
> #endif
>
> void __drm_puts_coredump(struct drm_printer *p, const char *str)
>
Re: [Intel-gfx] [PATCH v6 01/11] moduleparam: add data member to struct kernel_param
On 8/22/21 6:19 PM, Jim Cromie wrote:
> Add a const void* data member to the struct, to allow attaching
> private data that will be used soon by a setter method (via kp->data)
> to perform more elaborate actions.
>
> To attach the data at compile time, add new macros:
>
> module_param_cb_data() derives from module_param_cb(), adding data
> param, and latter is redefined to use former.
>
> It calls __module_param_call_with_data(), which accepts new data param
> and inits .data with it. Re-define __module_param_call() to use it.
>
> Use of this new data member will be rare, it might be worth redoing
> this as a separate/sub-type to de-bloat the base case.
>
> Signed-off-by: Jim Cromie
> ---
> v6:
> . const void* data -
> . better macro names s/_cbd/_cb_data/, s/_wdata/_with_data/
> . more const, no cast - Willy
> ---
> include/linux/moduleparam.h | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index eed280fae433..b8871e514de5 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -78,6 +78,7 @@ struct kernel_param {
> const struct kparam_string *str;
> const struct kparam_array *arr;
> };
> + const void *data;
> };
>
I wonder if kp->arg can just be used for all this and avoid this patch entirely?
define something like:
struct dd_bitmap_param {
int bitmap;
struct dyndbg_bitdesc *bitmap_arr;
};
and then just pass a pointer to it as 'arg' for module_param_cb? And then in
the get/set callbacks you can use kp->bitmap and kp->bitmap_arr.
Thanks,
-Jason
> extern const struct kernel_param __start___param[], __stop___param[];
> @@ -175,6 +176,9 @@ struct kparam_array
> #define module_param_cb(name, ops, arg, perm)
> \
> __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1, 0)
>
> +#define module_param_cb_data(name, ops, arg, perm, data)
> \
> + __module_param_call_with_data(MODULE_PARAM_PREFIX, name, ops, arg,
> perm, -1, 0, data)
> +
> #define module_param_cb_unsafe(name, ops, arg, perm) \
> __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1,\
> KERNEL_PARAM_FL_UNSAFE)
> @@ -284,14 +288,17 @@ struct kparam_array
>
> /* This is the fundamental function for registering boot/module
> parameters. */
> -#define __module_param_call(prefix, name, ops, arg, perm, level, flags)
> \
> +#define __module_param_call(prefix, name, ops, arg, perm, level, flags) \
> + __module_param_call_with_data(prefix, name, ops, arg, perm, level,
> flags, NULL)
> +
> +#define __module_param_call_with_data(prefix, name, ops, arg, perm, level,
> flags, data) \
> /* Default value instead of permissions? */ \
> static const char __param_str_##name[] = prefix #name; \
> static struct kernel_param __moduleparam_const __param_##name \
> __used __section("__param") \
> __aligned(__alignof__(struct kernel_param)) \
> = { __param_str_##name, THIS_MODULE, ops, \
> - VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg } }
> + VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg }, data }
>
> /* Obsolete - use module_param_cb() */
> #define module_param_call(name, _set, _get, arg, perm)
> \
>
Re: [Intel-gfx] [PATCH v6 02/11] dyndbg: add DEFINE_DYNAMIC_DEBUG_CATEGORIES and callbacks
On 8/22/21 6:20 PM, Jim Cromie wrote:
> DEFINE_DYNAMIC_DEBUG_CATEGORIES(name, var, bitmap_desc, @bit_descs)
> allows users to define a drm.debug style (bitmap) sysfs interface, and
> to specify the desired mapping from bits[0-N] to the format-prefix'd
> pr_debug()s to be controlled.
>
> DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug_gvt, __gvt_debug,
> "i915/gvt bitmap desc",
> /**
>* search-prefixes, passed to dd-exec_queries
>* defines bits 0-N in order.
>* leading ^ is tacitly inserted (by callback currently)
>* trailing space used here excludes subcats.
>* helper macro needs more work
>* macro to autogen ++$i, 0x%x$i ?
>*/
> _DD_cat_("gvt:cmd: "),
> _DD_cat_("gvt:core: "),
> _DD_cat_("gvt:dpy: "),
> _DD_cat_("gvt:el: "),
> _DD_cat_("gvt:irq: "),
> _DD_cat_("gvt:mm: "),
> _DD_cat_("gvt:mmio: "),
> _DD_cat_("gvt:render: "),
> _DD_cat_("gvt:sched: "));
>
> dynamic_debug.c: add 3 new elements:
>
> - int param_set_dyndbg()
> - int param_get_dyndbg()
> - struct kernel_param_ops param_ops_dyndbg
>
> Following the model of kernel/params.c STANDARD_PARAM_DEFS, All 3 are
> non-static and exported.
>
> dynamic_debug.h:
>
> Add DEFINE_DYNAMIC_DEBUG_CATEGORIES() described above, and a do-nothing stub.
>
> Note that it also calls MODULE_PARM_DESC for the user, but expects the
> user to catenate all the bit-descriptions together (as is done in
> drm.debug), and in the following uses in amdgpu, i915.
>
> This in the hope that someone can offer an auto-incrementing
> label-generating macro, producing "\tbit-4 0x10\t" etc, and can show
> how to apply it to __VA_ARGS__.
>
> Also extern the struct kernel_param param_ops_dyndbg symbol, as is
> done in moduleparams.h for all the STANDARD params.
>
> USAGE NOTES:
>
> Using dyndbg to query on "format ^$prefix" requires that the prefix be
> present in the compiled-in format string; where run-time prefixing is
> used, that format would be "%s...", which is not usefully selectable.
>
> Adding structural query terms (func,file,lineno) could help (module is
> already done), but DEFINE_DYNAMIC_DEBUG_CATEGORIES can't do that now,
> adding it needs a better reason imo.
>
> Dyndbg is completely agnostic wrt the categorization scheme used, to
> play well with any prefix convention already in use. Ad-hoc
> categories and sub-categories are implicitly allowed, author
> discipline and review is expected.
>
> Here are some examples:
>
> "1","2","3" 2 doesn't imply 1.
> otherwize, sorta like printk levels
> "1:","2:","3:"are better, avoiding [1-9]\d+ ambiguity
> "hi:","mid:","low:" are reasonable, and imply independence
> "todo:","rfc:","fixme:" might be handy
> "A:".."Z:"uhm, yeah
>
> Hierarchical classes/categories are natural:
>
> "drm::" is used in later commit
> "drm:::"is a natural extension.
> "drm:atomic:fail:"has been proposed, sounds directly useful
>
> Some properties of a hierarchical category deserve explication:
>
> Trailing spaces matter !
>
> With 1..3-space ("drm: ", "drm:atomic: ", "drm:atomic:fail: "), the
> ":" doesn't terminate the search-space, the trailing space does. So a
> "drm:" search spec will match all DRM categories & subcategories, and
> will not be useful in an interface where all categories are already
> controlled together. That said, "drm:atomic:" & "drm:atomic: " are
> different, and both are useful in cases.
>
> Ad-Hoc sub-categories:
>
> These have a caveat wrt wrapper macros adding prefixes like
> "drm:atomic: "; the trailing space in the prefix means that
> drm_dbg_atomic("fail: ...") pastes as "drm:atomic: fail: ", which
> obviously isn't ideal wrt clear and simple bitmaps.
>
> A possible solution is to have a FOO_() version of every FOO() macro
> which (anti-mnemonically) elides the trailing space, which is normally
> inserted by a modified FOO(). Doing this would enforce a policy
> decision that "debug categories will be space terminated", with an
> pressure-relief valve.
>
> Summarizing:
>
> - "drm:kms: " & "drm:kms:" are different
> - "drm:kms" also different - includes drm:kms2:
> - "drm:kms:\t" also different
> - "drm:kms:*"doesn't work, no wildcard on format atm.
>
> Order matters in DEFINE_DYNAMIC_DEBUG_CATEGORIES(... @bit_descs)
>
> @bit_descs (array) position determines the bit mapping to the prefix,
> so to keep a stable map, new categories or 3rd level categories must
> be added to the end.
>
> Since bits are/will-stay applied 0-N, the later bits can countermand
> the earlier ones, but its tricky - consider;
>
> DD_CATs(... "drm:atomic:", "drm:atomic:fail:" ) // misleading
>
> The 1st search-term is misleading, because it includes (modifies)
> subcategories, but then 2nd overrides it. So don't do that.
>
> For "drm:atomic:fail:" in particular, its best not to add
Re: [Intel-gfx] [PATCH v6 00/11] use DYNAMIC_DEBUG to implement DRM.debug
On 8/22/21 6:19 PM, Jim Cromie wrote: > This patchset does 3 main things. > > Adds DEFINE_DYNAMIC_DEBUG_CATEGORIES to define bitmap => category > control of pr_debugs, and to create their sysfs entries. > > Uses it in amdgpu, i915 to control existing pr_debugs according to > their ad-hoc categorizations. > > Plugs dyndbg into drm-debug framework, in a configurable manner. > > v6: cleans up per v5 feedback, and adds RFC stuff: > > - test_dynamic_debug.ko: uses tracer facility added in v5:8/9 > - prototype print-once & rate-limiting > > Hopefully adding RFC stuff doesnt distract too much. Hi Jim, Yeah, I feel like the RFC patches should be in a separate series unless there is a drm dependency for them? Thanks, -Jason > > Jim Cromie (11): > moduleparam: add data member to struct kernel_param > dyndbg: add DEFINE_DYNAMIC_DEBUG_CATEGORIES and callbacks > i915/gvt: remove spaces in pr_debug "gvt: core:" etc prefixes > i915/gvt: use DEFINE_DYNAMIC_DEBUG_CATEGORIES to create "gvt:core:" > etc categories > amdgpu: use DEFINE_DYNAMIC_DEBUG_CATEGORIES to control categorized > pr_debugs > drm_print: add choice to use dynamic debug in drm-debug > drm_print: instrument drm_debug_enabled > amdgpu_ucode: reduce number of pr_debug calls > nouveau: fold multiple DRM_DEBUG_DRIVERs together > dyndbg: RFC add debug-trace callback, selftest with it. RFC > dyndbg: RFC add print-once and print-ratelimited features. RFC. > > drivers/gpu/drm/Kconfig | 13 + > drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 293 --- > .../gpu/drm/amd/display/dc/core/dc_debug.c| 44 ++- > drivers/gpu/drm/drm_print.c | 49 ++- > drivers/gpu/drm/i915/gvt/Makefile | 4 + > drivers/gpu/drm/i915/gvt/debug.h | 18 +- > drivers/gpu/drm/i915/i915_params.c| 35 ++ > drivers/gpu/drm/nouveau/nouveau_drm.c | 36 +- > include/drm/drm_print.h | 148 ++-- > include/linux/dynamic_debug.h | 81 - > include/linux/moduleparam.h | 11 +- > lib/Kconfig.debug | 11 + > lib/Makefile | 1 + > lib/dynamic_debug.c | 336 -- > lib/test_dynamic_debug.c | 279 +++ > 15 files changed, 1117 insertions(+), 242 deletions(-) > create mode 100644 lib/test_dynamic_debug.c >
