From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 55726C43334 for ; Mon, 6 Jun 2022 15:00:09 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id F142610FBBB; Mon, 6 Jun 2022 15:00:06 +0000 (UTC) Received: from mail-ed1-x52e.google.com (mail-ed1-x52e.google.com [IPv6:2a00:1450:4864:20::52e]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5079A10FB3C; Mon, 6 Jun 2022 15:00:05 +0000 (UTC) Received: by mail-ed1-x52e.google.com with SMTP id o10so19194005edi.1; Mon, 06 Jun 2022 08:00:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=A4brwVq972U5Cy5SArwJ+3RcY5BE7XJcKiLcZM5bXIg=; b=ebFb6iqNJFZiPZiWnmKDO7dEP68cK+FoW8vXzrP5Fks5IjVYQ5KD7xcjVGogrp1jaK tSTzUP+/ElsJCotyF7jOKZSs/ErxzErYTeTKBoyDUVifknmBzs3LEpH++LChHUyw8RFW kg2rLfXhL2CUSTb6yPjk0GxgqyqEIE+1EMegj2gmO7JcNw92PI60fXqUktgfshLjZNqz t6osXqlL17LYEgWsnrNrX5ikm15/pWjw7m8Z5TgVMLxcYeD7NnmHuRZcpQ5h2truhj54 LTCCwvcQIpR335afRgDDx1Yt9+IEhEOxCLCXYhjoG5bov+j38Bs4PqefbVaiNjRjfn09 5Pgg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=A4brwVq972U5Cy5SArwJ+3RcY5BE7XJcKiLcZM5bXIg=; b=od0xkI8hddwfYFz2C2a48ZkrGEIHr8ECxTFzDrJDpjJY9y4i7Ok8KoLJqkDjzBwv4B XArUdtDFs1b9T24DTPYXuprgH3Bi/dbt2ODWKnjoOnHNK6u6vQTxTxWqUEGGgbGKJ/ul l2zW9DvZunNENQxOJ7sVX9UuMJkoxJyfq2tlt/Kx/tW1H0iIqS3UUnk+JTWyiUpDSuKw ixpHMe/nk8PuqM2aTU5MpMSMnbuzkjZLUUdA2SekSjjkJN/YreGMzX3gQfqWxbQmZHac iKXnjCztmD6QoS1GIz8vVqk1jrlrhL6ST0JwQM5VwO9czR7jVx2x7IW3ZyWIogesREXG kYng== X-Gm-Message-State: AOAM5322fFzXgoQHHyVNRyolJUQJlx0oDd/Mv5cwgJnJZnJygfFZ/+Oj 2MDAucW7D37WIy1a+zy6tWctFSYKJP7DrcGN82c= X-Google-Smtp-Source: ABdhPJzLti6TUUZHJfMec1JUrCsTmwoizRibw8eAdJaxg3bN0xJ9XMOJVeqNHpQGX5I05TWYbejuSINHWA3H0CyqbSM= X-Received: by 2002:aa7:c846:0:b0:431:6c7b:26af with SMTP id g6-20020aa7c846000000b004316c7b26afmr3720487edt.123.1654527603550; Mon, 06 Jun 2022 08:00:03 -0700 (PDT) MIME-Version: 1.0 References: <20220516225640.3102269-1-jim.cromie@gmail.com> In-Reply-To: From: jim.cromie@gmail.com Date: Mon, 6 Jun 2022 08:59:36 -0600 Message-ID: Subject: Re: [RFC PATCH v2 00/27] DRM.debug on DYNAMIC_DEBUG, add trace events To: Jim Cromie , Jason Baron , LKML , dri-devel , amd-gfx mailing list , intel-gvt-dev@lists.freedesktop.org, Intel Graphics Development , Greg KH , Sean Paul , robdclark@gmail.com, Steven Rostedt , mathieu.desnoyers@efficios.com, quic_saipraka@quicinc.com, Will Deacon , Catalin Marinas , quic_psodagud@quicinc.com, Marc Zyngier , Arnd Bergmann , Linux ARM , linux-arm-msm@vger.kernel.org, Ingo Molnar , David Airlie , Jani Nikula , Joonas Lahtinen , Pekka Paalanen , Thomas Zimmermann , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Chris Wilson Content-Type: multipart/alternative; boundary="00000000000014abc905e0c8ba93" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Daniel Vetter Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" --00000000000014abc905e0c8ba93 Content-Type: text/plain; charset="UTF-8" On Wed, May 25, 2022 at 9:02 AM Daniel Vetter wrote: > On Mon, May 16, 2022 at 04:56:13PM -0600, Jim Cromie wrote: > > DRM.debug API is 23 macros, issuing 10 exclusive categories of debug > > messages. By rough count, they are used 5140 times in the kernel. > > These all call drm_dbg or drm_devdbg, which call drm_debug_enabled(), > > which checks bits in global __drm_debug. Some of these are page-flips > > and vblanks, and get called often. > > > > DYNAMIC_DEBUG (with CONFIG_JUMP_LABEL) is built to avoid this kind of > > work, with NOOPd jump/callsites. > > > > This patchset is RFC because: > > - it touches 2.5 subsystems: dyndbg, drm, tracefs (new events) > > - dyndbg class support is built for drm, needs it for validation > > - new api, used by drm > > - big memory impact, with 5100 new pr-debug callsites. > > - drm class bikeshedding opportunities > > - others, names etc. > > Thanks a lot for keeping on pushing this! > > > > DYNAMIC_DEBUG: > > > > RFC: > > > > dynamic_debug_register_classes() cannot act early enough to be in > > effect at module-load. So this will not work as you'd reasonably > > expect: > > > > modprobe test_dynamic_debug dyndbg='+pfm; class FOO +pfmlt' > > > > The 1st query:+pfm will be enabled during load, but in the 2nd query, > > "class FOO" will be unknown at load time. Early class enablement > > would be nice. DYNAMIC_DEBUG_CLASSES is a static initializer, which > > is certainly early enough, but Im missing a trick, suggestions? > > So maybe I'm just totally overloading this work here so feel free to > ignore or postpone, but: Could we do the dynamic_debug_register_classes() > automatically at module load as a new special section? And then throw in a > bit of kbuild so that in a given subsystem every driver gets the same > class names by default and everything would just work, without having to > sprinkle calls to dynamic_debug_register_classes() all over the place? > This is now done; Ive added __dyndbg_classes section. load_module() now grabs it from the .ko and ddebug_add_module() attaches it to the module's ddebug_table record. for builtins, dynamic_debug_init feeds the builtin class-maps to ddebug_add_module bash-5.1# modprobe test_dynamic_debug dyndbg="class FOO +p" [ 88.374722] dyndbg: class[0]: nm:test_dynamic_debug base:20 len:7 ty:1 [ 88.375158] dyndbg: 0: EMERG [ 88.375345] dyndbg: 1: DANGER [ 88.375540] dyndbg: 2: ERROR [ 88.375726] dyndbg: 3: WARNING [ 88.375930] dyndbg: 4: NOTICE [ 88.376130] dyndbg: 5: INFO [ 88.376310] dyndbg: 6: DEBUG [ 88.376499] dyndbg: class[1]: nm:test_dynamic_debug base:12 len:3 ty:1 [ 88.376903] dyndbg: 0: ONE [ 88.377079] dyndbg: 1: TWO [ 88.377253] dyndbg: 2: THREE [ 88.377441] dyndbg: class[2]: nm:test_dynamic_debug base:8 len:3 ty:0 [ 88.377837] dyndbg: 0: bing [ 88.378022] dyndbg: 1: bong [ 88.378203] dyndbg: 2: boom [ 88.378387] dyndbg: class[3]: nm:test_dynamic_debug base:4 len:3 ty:0 [ 88.378800] dyndbg: 0: Foo [ 88.378986] dyndbg: 1: Bar [ 88.379167] dyndbg: 2: Buzz [ 88.379348] dyndbg: class[4]: nm:test_dynamic_debug base:0 len:3 ty:0 [ 88.379757] dyndbg: 0: FOO [ 88.379938] dyndbg: 1: BAR [ 88.380136] dyndbg: 2: BUZZ [ 88.380410] dyndbg: module:test_dynamic_debug attached 5 classes [ 88.380881] dyndbg: 24 debug prints in module test_dynamic_debug [ 88.381315] dyndbg: module: test_dynamic_debug dyndbg="class FOO +p" [ 88.381714] dyndbg: query 0: "class FOO +p" mod:test_dynamic_debug [ 88.382109] dyndbg: split into words: "class" "FOO" "+p" [ 88.382445] dyndbg: op='+' [ 88.382616] dyndbg: flags=0x1 [ 88.382802] dyndbg: *flagsp=0x1 *maskp=0xffffffff [ 88.383101] dyndbg: parsed: func="" file="" module="test_dynamic_debug" format="" lineno=0-0 class=FOO [ 88.383740] dyndbg: applied: func="" file="" module="test_dynamic_debug" format="" lineno=0-0 class=FOO [ 88.384324] dyndbg: processed 1 queries, with 2 matches, 0 errs bash-5.1# so its working at module-load time. > For the entire class approach, did you spot another subsystem that could > benefit from this and maybe make a more solid case that this is something > good? > I had been working on the premise that ~4k drm*dbg callsites was a good case. verbosity-levels - with x /sys/module/foo/parameters/debug_verbosity and effectively does echo < /proc/dynamic_debug/control module foo class V1 +p module foo class V2 +p module foo class V3 +p module foo class V4 -p module foo class V5 -p module foo class V6 -p module foo class V7 -p module foo class V8 -p EOQRY since only real +/-p changes incur kernel-patching costs, the remaining overheads are minimal. > RFC for DRM: > > - decoration flags "fmlt" do not work on drm_*dbg(). > (drm_*dbg() dont use pr_debug, they *become* one flavor of them) > this could (should?) be added, and maybe tailored for drm. > some of the device prefixes are very long, a "d" flag could optionalize them. I'm lost what the fmlt decoration flags are? The flags are:: p enables the pr_debug() callsite. f Include the function name in the printed message l Include line number in the printed message m Include module name in the printed message t Include thread ID in messages not generated from interrupt context _ No flags are set. (Or'd with others on input) the fmlt flags add a "decoration" prefix to enabled/printed log messages > - api use needs review wrt drm life-cycle. > > enum drm_debug_category and DYNAMIC_DEBUG_CLASSES could be together? > > Hm if they're tied to module lifetime we should be good? Not sure what > could go wrong here. > > with the new __section, "life-cycle" doesnt really pertain. the new issue is how the class-maps are shared across the subsystem; the current class-maps list for each module will probably break; a list item cannot be on N different lists of different modules. Altering the list-items to ref the class-map (not contain it) should solve the problem. > > - class-names could stand review, perhaps extension > > "drm:core:" etc have appeared (maybe just from me) > > or a "plan" to look at it later > > Yeah it's been a bit sprawling. I'm kinda hoping that by firmly > establishing dyndbg as the drm debug approach we can cut down for the need > for ad-hoc flags a bit. > > yeah thats why I kept the DRM_UT_* names. OTOH - the symbolic names patch exposes the choices, which locks the names as API ?? > > - i915 & amdgpu have pr_debugs (DC_LOG_*, gvt_dbg_*) that have > > class-ish prefixes that are separate from, but similar to DRM_UT_*, > > and could stand review and possible unification with reformed or > > extended drm categories. > > Yeah drm is not entirely consistent with how exactly driver debug printing > should be done. Another reason why I'm hoping that the kitchen sync with > everything approach you're doing here could help unify things. > the decoration flags can help here; they loosely/precisely describe the elements of most/all the current debug format-prefix variations. So case by case, the ad-hoc variations should map onto these flags, The flags allow selectively dropping the prefix info from some of the log entries, after you've seen the module name and function a dozen times, it's helpful to reduce screen clutter. It might make sense to add a new flag for device, so that dev_dbg() flavors can shorten-or-skip the longer device strings, maybe some drm specific flavors. > > > - the change to enum drm_debug_category from bitmask values to 0..31 > > means that we foreclose this possiblility: > > > > drm_dbg(DRM_UT_CORE|DRM_UT_KMS, "wierd double-cat experiment"); > > Yeah no, that doesn't make much sense to me :-) > > no chuckles for the schrodinger's cat joke ? (maybe "yeah no" is the artful superpositional reply, I just caught :) > > - nouveau has very few drm.debug calls, > > has NV_DEBUG, VMM_DEBUG, nvkm_printk_, I havent looked deeply. > > nouveau has like levels, man .. test_dynamic_debug implements its priority-style names as a POC patch 18 converts nvkm_debug/trace to use dev_dbg instead of dev_info it probably could adapt to use drm_devdbg > Yeah see above. There's a pile more drivers (more on the armsoc side of > things) which are quite big on the raw debug call approach. > > LOW, MID, HI has been proposed at least once wrt dyndbg. that probably fits well with current disjoint classes. level/verbose classes should be practical too, as described above. NB: The symbolic names should also work echo +MID > /sys/module/foobar/parameters/debug_verbosity though theres some ambiguity with echo -V3 > /sys/module/foobar/parameters/debug_verbosity that should turn off V4,5,6, but what about V1,2 ? it could leave them alone (whatever their previous settings are) anyway, lkp-robot and igt-trybot should be grinding on the latest patchset soon, I'll send it after I fix whatever breaks. > Cheers, Daniel > thanks, Jim --00000000000014abc905e0c8ba93 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Wed, May 25, 2022 at 9:02 AM Danie= l Vetter <daniel@ff= wll.ch> wrote:
On Mon, May 16, 2022 at 04:56:13PM -0600, Jim Cromie wrote:
> DRM.debug API is 23 macros, issuing 10 exclusive categories of debug > messages.=C2=A0 By rough count, they are used 5140 times in the kernel= .
> These all call drm_dbg or drm_devdbg, which call drm_debug_enabled(),<= br> > which checks bits in global __drm_debug.=C2=A0 Some of these are page-= flips
> and vblanks, and get called often.
>
> DYNAMIC_DEBUG (with CONFIG_JUMP_LABEL) is built to avoid this kind of<= br> > work, with NOOPd jump/callsites.
>
> This patchset is RFC because:
> - it touches 2.5 subsystems: dyndbg, drm, tracefs (new events)
> - dyndbg class support is built for drm, needs it for validation
> - new api, used by drm
> - big memory impact, with 5100 new pr-debug callsites.
> - drm class bikeshedding opportunities
> - others, names etc.

Thanks a lot for keeping on pushing this!

>
> DYNAMIC_DEBUG:
>
=C2=A0
> RFC:
>
> dynamic_debug_register_classes() cannot act early enough to be in
> effect at module-load.=C2=A0 So this will not work as you'd reason= ably
> expect:
>
>=C2=A0 =C2=A0modprobe test_dynamic_debug dyndbg=3D'+pfm; class FOO = +pfmlt'
>
> The 1st query:+pfm will be enabled during load, but in the 2nd query,<= br> > "class FOO" will be unknown at load time.=C2=A0 Early class = enablement
> would be nice.=C2=A0 DYNAMIC_DEBUG_CLASSES is a static initializer, wh= ich
> is certainly early enough, but Im missing a trick, suggestions?

So maybe I'm just totally overloading this work here so feel free to ignore or postpone, but: Could we do the dynamic_debug_register_classes() automatically at module load as a new special section? And then throw in a<= br> bit of kbuild so that in a given subsystem every driver gets the same
class names by default and everything would just work, without having to sprinkle calls to dynamic_debug_register_classes() all over the place?
<= /blockquote>

This is now done; Ive added __dyndbg_classe= s section.
load_module() now grabs it from the .ko
and = ddebug_add_module() attaches it to the module's ddebug_table record.
for builtins, dynamic_debug_init feeds the builtin class-maps to dd= ebug_add_module

bash-5.1# modprobe test_dynamic_de= bug dyndbg=3D"class FOO +p"
[ =C2=A0 88.374722] dyndbg: class[= 0]: nm:test_dynamic_debug base:20 len:7 ty:1
[ =C2=A0 88.375158] dyndbg:= =C2=A00: EMERG
[ =C2=A0 88.375345] dyndbg: =C2=A01: DANGER
[ =C2=A0 = 88.375540] dyndbg: =C2=A02: ERROR
[ =C2=A0 88.375726] dyndbg: =C2=A03: W= ARNING
[ =C2=A0 88.375930] dyndbg: =C2=A04: NOTICE
[ =C2=A0 88.376130= ] dyndbg: =C2=A05: INFO
[ =C2=A0 88.376310] dyndbg: =C2=A06: DEBUG
[ = =C2=A0 88.376499] dyndbg: class[1]: nm:test_dynamic_debug base:12 len:3 ty:= 1
[ =C2=A0 88.376903] dyndbg: =C2=A00: ONE
[ =C2=A0 88.377079] dyndbg= : =C2=A01: TWO
[ =C2=A0 88.377253] dyndbg: =C2=A02: THREE
[ =C2=A0 88= .377441] dyndbg: class[2]: nm:test_dynamic_debug base:8 len:3 ty:0
[ =C2= =A0 88.377837] dyndbg: =C2=A00: bing
[ =C2=A0 88.378022] dyndbg: =C2=A01= : bong
[ =C2=A0 88.378203] dyndbg: =C2=A02: boom
[ =C2=A0 88.378387] = dyndbg: class[3]: nm:test_dynamic_debug base:4 len:3 ty:0
[ =C2=A0 88.37= 8800] dyndbg: =C2=A00: Foo
[ =C2=A0 88.378986] dyndbg: =C2=A01: Bar
[= =C2=A0 88.379167] dyndbg: =C2=A02: Buzz
[ =C2=A0 88.379348] dyndbg: cla= ss[4]: nm:test_dynamic_debug base:0 len:3 ty:0
[ =C2=A0 88.379757] dyndb= g: =C2=A00: FOO
[ =C2=A0 88.379938] dyndbg: =C2=A01: BAR
[ =C2=A0 88.= 380136] dyndbg: =C2=A02: BUZZ
[ =C2=A0 88.380410] dyndbg: module:test_dy= namic_debug attached 5 classes
[ =C2=A0 88.380881] dyndbg: =C2=A024 debu= g prints in module test_dynamic_debug
[ =C2=A0 88.381315] dyndbg: module= : test_dynamic_debug dyndbg=3D"class FOO +p"
[ =C2=A0 88.38171= 4] dyndbg: query 0: "class FOO +p" mod:test_dynamic_debug
[ = =C2=A0 88.382109] dyndbg: split into words: "class" "FOO&quo= t; "+p"
[ =C2=A0 88.382445] dyndbg: op=3D'+'
[ =C2= =A0 88.382616] dyndbg: flags=3D0x1
[ =C2=A0 88.382802] dyndbg: *flagsp= =3D0x1 *maskp=3D0xffffffff
[ =C2=A0 88.383101] dyndbg: parsed: func=3D&q= uot;" file=3D"" module=3D"test_dynamic_debug" form= at=3D"" lineno=3D0-0 class=3DFOO
[ =C2=A0 88.383740] dyndbg: a= pplied: func=3D"" file=3D"" module=3D"test_dynamic= _debug" format=3D"" lineno=3D0-0 class=3DFOO
[ =C2=A0 88.= 384324] dyndbg: processed 1 queries, with 2 matches, 0 errs
bash-5.1#=C2= =A0

so its working at module-load time.
<= div>

For the entire class approach, did you spot another subsystem that could benefit from this and maybe make a more solid case that this is something good?

I had been working on the premise= that ~4k drm*dbg callsites was a good case.

verbo= sity-levels - with x<y logic instead=C2=A0of=C2=A0x=3D=3Dy is what's= =C2=A0currently missing.

the next revision adds so= mething, which "kinda works".
But I think I'll rip = it out, and do this simpler approach instead:

impl= ement a verbose/levels=C2=A0 param & callback, which takes
=C2=A0 =C2=A0echo 3 > /sys/module/foo/parameters/debug_verb= osity

and effectively does

=C2=A0 echo <<EOQRY=C2=A0 > /proc/dynamic_debug/control
module foo class V1 +p
module foo class V2 +p
module foo class V3 +p
module foo class V4 -p
<= div>module foo class V5 -p
module foo class V6 -p
=
module foo class V7 -p
module foo class V8 -p
EOQRY

s= ince only real=C2=A0+/-p changes incur=C2=A0kernel-patching costs,
the remaining overheads are minimal.


> RFC for DRM:
>
> - decoration flags "fmlt" do not work on drm_*dbg().
>=C2=A0 =C2=A0(drm_*dbg() dont use pr_debug, they *become* one flavor of= them)
>=C2=A0 =C2=A0this could (should?) be added, and maybe tailored for drm.=
>=C2=A0 =C2=A0some of the device prefixes are very long, a "d"= flag could optionalize them.

I'm lost what the fmlt decoration flags are?


The flags are::

=C2=A0 p =C2=A0 =C2=A0enables the pr_d= ebug() callsite.
=C2=A0 f =C2=A0 =C2=A0Include the function name in the = printed message
=C2=A0 l =C2=A0 =C2=A0Include line number in the printed= message
=C2=A0 m =C2=A0 =C2=A0Include module name in the printed messag= e
=C2=A0 t =C2=A0 =C2=A0Include thread ID in messages not generated from= interrupt context
=C2=A0 _ =C2=A0 =C2=A0No flags are set. (Or'd wit= h others on input)


the fmlt flags add= a "decoration" prefix to enabled/printed log messages
=

> - api use needs review wrt drm life-cycle.
>=C2=A0 =C2=A0enum drm_debug_category and DYNAMIC_DEBUG_CLASSES could be= together?

Hm if they're tied to module lifetime we should be good? Not sure what<= br> could go wrong here.


with the new __section, "life-cyc= le" doesnt really pertain.
the new issue is how the class-ma= ps are shared across the subsystem;
the current class-maps list f= or each module will probably break;
a list item cannot be on N di= fferent lists of different modules.
Altering the list-items to re= f the class-map (not contain it) should solve the problem.


=C2=A0
> - class-names could stand review, perhaps extension
>=C2=A0 =C2=A0"drm:core:" etc have appeared (maybe just from m= e)
>=C2=A0 =C2=A0or a "plan" to look at it later

Yeah it's been a bit sprawling. I'm kinda hoping that by firmly
establishing dyndbg as the drm debug approach we can cut down for the need<= br> for ad-hoc flags a bit.

yeah thats why I kept the DRM_UT_* names.
O= TOH - the symbolic names patch exposes the choices,
which locks t= he names as API ??

=C2=A0
> - i915 & amdgpu have pr_debugs (DC_LOG_*, gvt_dbg_*) that have
> class-ish prefixes that are separate from, but similar to DRM_UT_*, > and could stand review and possible unification with reformed or
> extended drm categories.

Yeah drm is not entirely consistent with how exactly driver debug printing<= br> should be done. Another reason why I'm hoping that the kitchen sync wit= h
everything approach you're doing here could help unify things.


the decoration flags can help he= re; they loosely/precisely describe
the elements of most/all the = current debug format-prefix variations.
So case by case, the ad-h= oc variations should map onto these flags,

The fla= gs allow selectively dropping the prefix info from some of the log entries,=
after you've=C2=A0seen the module name and function a dozen = times,=C2=A0
it's helpful to reduce screen clutter.

It might make sense to add a new flag for device,
so that dev_dbg() flavors can shorten-or-skip the longer device strings,= =C2=A0
maybe some drm specific flavors.

= =C2=A0

> - the change to enum drm_debug_category from bitmask values to 0..31 >=C2=A0 =C2=A0means that we foreclose this possiblility:
>
>=C2=A0 =C2=A0 drm_dbg(DRM_UT_CORE|DRM_UT_KMS, "wierd double-cat ex= periment");

Yeah no, that doesn't make much sense to me :-)

no chuckles for the schrodinger's cat joke ?
(maybe "yeah no" is the artful superpositional reply, I jus= t caught :)
=C2=A0
> - nouveau has very few drm.debug calls,
>=C2=A0 =C2=A0has NV_DEBUG, VMM_DEBUG, nvkm_printk_, I havent looked dee= ply.


nouveau has like levels, man ..
<= div>test_dynamic_debug implements its priority-style names as a POC

patch 18 converts nvkm_debug/trace to use dev_dbg instead= of dev_info
it probably could adapt to use drm_devdbg
<= div>

=C2=A0
Yeah see above. There's a pile more drivers (more on the armsoc side of=
things) which are quite big on the raw debug call approach.


LOW, MID, HI has been proposed at= least once wrt dyndbg.
that probably fits well with current disj= oint classes.
level/verbose classes should be practical too, as d= escribed above.

NB: The symbolic names should also= work=C2=A0

=C2=A0 =C2=A0echo +MID > /sys/modul= e/foobar/parameters/debug_verbosity
=C2=A0
though= theres some ambiguity with

=C2=A0 =C2=A0echo= -V3 > /sys/module/foobar/parameters/debug_verbosity

that should turn off V4,5,6,=C2= =A0
but what about V1,2 ?
it could leave them alone (wh= atever their previous settings are)

anyway, lkp-ro= bot and igt-trybot should be grinding on the latest patchset soon,
I'll send it after I fix whatever breaks.

= =C2=A0
Cheers, Daniel

thanks,=C2=A0
= Jim=C2=A0
--00000000000014abc905e0c8ba93-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 07048C43334 for ; Mon, 6 Jun 2022 15:00:13 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 422B510FBC5; Mon, 6 Jun 2022 15:00:07 +0000 (UTC) Received: from mail-ed1-x52e.google.com (mail-ed1-x52e.google.com [IPv6:2a00:1450:4864:20::52e]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5079A10FB3C; Mon, 6 Jun 2022 15:00:05 +0000 (UTC) Received: by mail-ed1-x52e.google.com with SMTP id o10so19194005edi.1; Mon, 06 Jun 2022 08:00:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=A4brwVq972U5Cy5SArwJ+3RcY5BE7XJcKiLcZM5bXIg=; b=ebFb6iqNJFZiPZiWnmKDO7dEP68cK+FoW8vXzrP5Fks5IjVYQ5KD7xcjVGogrp1jaK tSTzUP+/ElsJCotyF7jOKZSs/ErxzErYTeTKBoyDUVifknmBzs3LEpH++LChHUyw8RFW kg2rLfXhL2CUSTb6yPjk0GxgqyqEIE+1EMegj2gmO7JcNw92PI60fXqUktgfshLjZNqz t6osXqlL17LYEgWsnrNrX5ikm15/pWjw7m8Z5TgVMLxcYeD7NnmHuRZcpQ5h2truhj54 LTCCwvcQIpR335afRgDDx1Yt9+IEhEOxCLCXYhjoG5bov+j38Bs4PqefbVaiNjRjfn09 5Pgg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=A4brwVq972U5Cy5SArwJ+3RcY5BE7XJcKiLcZM5bXIg=; b=od0xkI8hddwfYFz2C2a48ZkrGEIHr8ECxTFzDrJDpjJY9y4i7Ok8KoLJqkDjzBwv4B XArUdtDFs1b9T24DTPYXuprgH3Bi/dbt2ODWKnjoOnHNK6u6vQTxTxWqUEGGgbGKJ/ul l2zW9DvZunNENQxOJ7sVX9UuMJkoxJyfq2tlt/Kx/tW1H0iIqS3UUnk+JTWyiUpDSuKw ixpHMe/nk8PuqM2aTU5MpMSMnbuzkjZLUUdA2SekSjjkJN/YreGMzX3gQfqWxbQmZHac iKXnjCztmD6QoS1GIz8vVqk1jrlrhL6ST0JwQM5VwO9czR7jVx2x7IW3ZyWIogesREXG kYng== X-Gm-Message-State: AOAM5322fFzXgoQHHyVNRyolJUQJlx0oDd/Mv5cwgJnJZnJygfFZ/+Oj 2MDAucW7D37WIy1a+zy6tWctFSYKJP7DrcGN82c= X-Google-Smtp-Source: ABdhPJzLti6TUUZHJfMec1JUrCsTmwoizRibw8eAdJaxg3bN0xJ9XMOJVeqNHpQGX5I05TWYbejuSINHWA3H0CyqbSM= X-Received: by 2002:aa7:c846:0:b0:431:6c7b:26af with SMTP id g6-20020aa7c846000000b004316c7b26afmr3720487edt.123.1654527603550; Mon, 06 Jun 2022 08:00:03 -0700 (PDT) MIME-Version: 1.0 References: <20220516225640.3102269-1-jim.cromie@gmail.com> In-Reply-To: From: jim.cromie@gmail.com Date: Mon, 6 Jun 2022 08:59:36 -0600 Message-ID: To: Jim Cromie , Jason Baron , LKML , dri-devel , amd-gfx mailing list , intel-gvt-dev@lists.freedesktop.org, Intel Graphics Development , Greg KH , Sean Paul , robdclark@gmail.com, Steven Rostedt , mathieu.desnoyers@efficios.com, quic_saipraka@quicinc.com, Will Deacon , Catalin Marinas , quic_psodagud@quicinc.com, Marc Zyngier , Arnd Bergmann , Linux ARM , linux-arm-msm@vger.kernel.org, Ingo Molnar , David Airlie , Jani Nikula , Joonas Lahtinen , Pekka Paalanen , Thomas Zimmermann , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Chris Wilson Content-Type: multipart/alternative; boundary="00000000000014abc905e0c8ba93" Subject: Re: [Intel-gfx] [RFC PATCH v2 00/27] DRM.debug on DYNAMIC_DEBUG, add trace events X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Daniel Vetter Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" --00000000000014abc905e0c8ba93 Content-Type: text/plain; charset="UTF-8" On Wed, May 25, 2022 at 9:02 AM Daniel Vetter wrote: > On Mon, May 16, 2022 at 04:56:13PM -0600, Jim Cromie wrote: > > DRM.debug API is 23 macros, issuing 10 exclusive categories of debug > > messages. By rough count, they are used 5140 times in the kernel. > > These all call drm_dbg or drm_devdbg, which call drm_debug_enabled(), > > which checks bits in global __drm_debug. Some of these are page-flips > > and vblanks, and get called often. > > > > DYNAMIC_DEBUG (with CONFIG_JUMP_LABEL) is built to avoid this kind of > > work, with NOOPd jump/callsites. > > > > This patchset is RFC because: > > - it touches 2.5 subsystems: dyndbg, drm, tracefs (new events) > > - dyndbg class support is built for drm, needs it for validation > > - new api, used by drm > > - big memory impact, with 5100 new pr-debug callsites. > > - drm class bikeshedding opportunities > > - others, names etc. > > Thanks a lot for keeping on pushing this! > > > > DYNAMIC_DEBUG: > > > > RFC: > > > > dynamic_debug_register_classes() cannot act early enough to be in > > effect at module-load. So this will not work as you'd reasonably > > expect: > > > > modprobe test_dynamic_debug dyndbg='+pfm; class FOO +pfmlt' > > > > The 1st query:+pfm will be enabled during load, but in the 2nd query, > > "class FOO" will be unknown at load time. Early class enablement > > would be nice. DYNAMIC_DEBUG_CLASSES is a static initializer, which > > is certainly early enough, but Im missing a trick, suggestions? > > So maybe I'm just totally overloading this work here so feel free to > ignore or postpone, but: Could we do the dynamic_debug_register_classes() > automatically at module load as a new special section? And then throw in a > bit of kbuild so that in a given subsystem every driver gets the same > class names by default and everything would just work, without having to > sprinkle calls to dynamic_debug_register_classes() all over the place? > This is now done; Ive added __dyndbg_classes section. load_module() now grabs it from the .ko and ddebug_add_module() attaches it to the module's ddebug_table record. for builtins, dynamic_debug_init feeds the builtin class-maps to ddebug_add_module bash-5.1# modprobe test_dynamic_debug dyndbg="class FOO +p" [ 88.374722] dyndbg: class[0]: nm:test_dynamic_debug base:20 len:7 ty:1 [ 88.375158] dyndbg: 0: EMERG [ 88.375345] dyndbg: 1: DANGER [ 88.375540] dyndbg: 2: ERROR [ 88.375726] dyndbg: 3: WARNING [ 88.375930] dyndbg: 4: NOTICE [ 88.376130] dyndbg: 5: INFO [ 88.376310] dyndbg: 6: DEBUG [ 88.376499] dyndbg: class[1]: nm:test_dynamic_debug base:12 len:3 ty:1 [ 88.376903] dyndbg: 0: ONE [ 88.377079] dyndbg: 1: TWO [ 88.377253] dyndbg: 2: THREE [ 88.377441] dyndbg: class[2]: nm:test_dynamic_debug base:8 len:3 ty:0 [ 88.377837] dyndbg: 0: bing [ 88.378022] dyndbg: 1: bong [ 88.378203] dyndbg: 2: boom [ 88.378387] dyndbg: class[3]: nm:test_dynamic_debug base:4 len:3 ty:0 [ 88.378800] dyndbg: 0: Foo [ 88.378986] dyndbg: 1: Bar [ 88.379167] dyndbg: 2: Buzz [ 88.379348] dyndbg: class[4]: nm:test_dynamic_debug base:0 len:3 ty:0 [ 88.379757] dyndbg: 0: FOO [ 88.379938] dyndbg: 1: BAR [ 88.380136] dyndbg: 2: BUZZ [ 88.380410] dyndbg: module:test_dynamic_debug attached 5 classes [ 88.380881] dyndbg: 24 debug prints in module test_dynamic_debug [ 88.381315] dyndbg: module: test_dynamic_debug dyndbg="class FOO +p" [ 88.381714] dyndbg: query 0: "class FOO +p" mod:test_dynamic_debug [ 88.382109] dyndbg: split into words: "class" "FOO" "+p" [ 88.382445] dyndbg: op='+' [ 88.382616] dyndbg: flags=0x1 [ 88.382802] dyndbg: *flagsp=0x1 *maskp=0xffffffff [ 88.383101] dyndbg: parsed: func="" file="" module="test_dynamic_debug" format="" lineno=0-0 class=FOO [ 88.383740] dyndbg: applied: func="" file="" module="test_dynamic_debug" format="" lineno=0-0 class=FOO [ 88.384324] dyndbg: processed 1 queries, with 2 matches, 0 errs bash-5.1# so its working at module-load time. > For the entire class approach, did you spot another subsystem that could > benefit from this and maybe make a more solid case that this is something > good? > I had been working on the premise that ~4k drm*dbg callsites was a good case. verbosity-levels - with x /sys/module/foo/parameters/debug_verbosity and effectively does echo < /proc/dynamic_debug/control module foo class V1 +p module foo class V2 +p module foo class V3 +p module foo class V4 -p module foo class V5 -p module foo class V6 -p module foo class V7 -p module foo class V8 -p EOQRY since only real +/-p changes incur kernel-patching costs, the remaining overheads are minimal. > RFC for DRM: > > - decoration flags "fmlt" do not work on drm_*dbg(). > (drm_*dbg() dont use pr_debug, they *become* one flavor of them) > this could (should?) be added, and maybe tailored for drm. > some of the device prefixes are very long, a "d" flag could optionalize them. I'm lost what the fmlt decoration flags are? The flags are:: p enables the pr_debug() callsite. f Include the function name in the printed message l Include line number in the printed message m Include module name in the printed message t Include thread ID in messages not generated from interrupt context _ No flags are set. (Or'd with others on input) the fmlt flags add a "decoration" prefix to enabled/printed log messages > - api use needs review wrt drm life-cycle. > > enum drm_debug_category and DYNAMIC_DEBUG_CLASSES could be together? > > Hm if they're tied to module lifetime we should be good? Not sure what > could go wrong here. > > with the new __section, "life-cycle" doesnt really pertain. the new issue is how the class-maps are shared across the subsystem; the current class-maps list for each module will probably break; a list item cannot be on N different lists of different modules. Altering the list-items to ref the class-map (not contain it) should solve the problem. > > - class-names could stand review, perhaps extension > > "drm:core:" etc have appeared (maybe just from me) > > or a "plan" to look at it later > > Yeah it's been a bit sprawling. I'm kinda hoping that by firmly > establishing dyndbg as the drm debug approach we can cut down for the need > for ad-hoc flags a bit. > > yeah thats why I kept the DRM_UT_* names. OTOH - the symbolic names patch exposes the choices, which locks the names as API ?? > > - i915 & amdgpu have pr_debugs (DC_LOG_*, gvt_dbg_*) that have > > class-ish prefixes that are separate from, but similar to DRM_UT_*, > > and could stand review and possible unification with reformed or > > extended drm categories. > > Yeah drm is not entirely consistent with how exactly driver debug printing > should be done. Another reason why I'm hoping that the kitchen sync with > everything approach you're doing here could help unify things. > the decoration flags can help here; they loosely/precisely describe the elements of most/all the current debug format-prefix variations. So case by case, the ad-hoc variations should map onto these flags, The flags allow selectively dropping the prefix info from some of the log entries, after you've seen the module name and function a dozen times, it's helpful to reduce screen clutter. It might make sense to add a new flag for device, so that dev_dbg() flavors can shorten-or-skip the longer device strings, maybe some drm specific flavors. > > > - the change to enum drm_debug_category from bitmask values to 0..31 > > means that we foreclose this possiblility: > > > > drm_dbg(DRM_UT_CORE|DRM_UT_KMS, "wierd double-cat experiment"); > > Yeah no, that doesn't make much sense to me :-) > > no chuckles for the schrodinger's cat joke ? (maybe "yeah no" is the artful superpositional reply, I just caught :) > > - nouveau has very few drm.debug calls, > > has NV_DEBUG, VMM_DEBUG, nvkm_printk_, I havent looked deeply. > > nouveau has like levels, man .. test_dynamic_debug implements its priority-style names as a POC patch 18 converts nvkm_debug/trace to use dev_dbg instead of dev_info it probably could adapt to use drm_devdbg > Yeah see above. There's a pile more drivers (more on the armsoc side of > things) which are quite big on the raw debug call approach. > > LOW, MID, HI has been proposed at least once wrt dyndbg. that probably fits well with current disjoint classes. level/verbose classes should be practical too, as described above. NB: The symbolic names should also work echo +MID > /sys/module/foobar/parameters/debug_verbosity though theres some ambiguity with echo -V3 > /sys/module/foobar/parameters/debug_verbosity that should turn off V4,5,6, but what about V1,2 ? it could leave them alone (whatever their previous settings are) anyway, lkp-robot and igt-trybot should be grinding on the latest patchset soon, I'll send it after I fix whatever breaks. > Cheers, Daniel > thanks, Jim --00000000000014abc905e0c8ba93 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Wed, May 25, 2022 at 9:02 AM Danie= l Vetter <daniel@ff= wll.ch> wrote:
On Mon, May 16, 2022 at 04:56:13PM -0600, Jim Cromie wrote:
> DRM.debug API is 23 macros, issuing 10 exclusive categories of debug > messages.=C2=A0 By rough count, they are used 5140 times in the kernel= .
> These all call drm_dbg or drm_devdbg, which call drm_debug_enabled(),<= br> > which checks bits in global __drm_debug.=C2=A0 Some of these are page-= flips
> and vblanks, and get called often.
>
> DYNAMIC_DEBUG (with CONFIG_JUMP_LABEL) is built to avoid this kind of<= br> > work, with NOOPd jump/callsites.
>
> This patchset is RFC because:
> - it touches 2.5 subsystems: dyndbg, drm, tracefs (new events)
> - dyndbg class support is built for drm, needs it for validation
> - new api, used by drm
> - big memory impact, with 5100 new pr-debug callsites.
> - drm class bikeshedding opportunities
> - others, names etc.

Thanks a lot for keeping on pushing this!

>
> DYNAMIC_DEBUG:
>
=C2=A0
> RFC:
>
> dynamic_debug_register_classes() cannot act early enough to be in
> effect at module-load.=C2=A0 So this will not work as you'd reason= ably
> expect:
>
>=C2=A0 =C2=A0modprobe test_dynamic_debug dyndbg=3D'+pfm; class FOO = +pfmlt'
>
> The 1st query:+pfm will be enabled during load, but in the 2nd query,<= br> > "class FOO" will be unknown at load time.=C2=A0 Early class = enablement
> would be nice.=C2=A0 DYNAMIC_DEBUG_CLASSES is a static initializer, wh= ich
> is certainly early enough, but Im missing a trick, suggestions?

So maybe I'm just totally overloading this work here so feel free to ignore or postpone, but: Could we do the dynamic_debug_register_classes() automatically at module load as a new special section? And then throw in a<= br> bit of kbuild so that in a given subsystem every driver gets the same
class names by default and everything would just work, without having to sprinkle calls to dynamic_debug_register_classes() all over the place?
<= /blockquote>

This is now done; Ive added __dyndbg_classe= s section.
load_module() now grabs it from the .ko
and = ddebug_add_module() attaches it to the module's ddebug_table record.
for builtins, dynamic_debug_init feeds the builtin class-maps to dd= ebug_add_module

bash-5.1# modprobe test_dynamic_de= bug dyndbg=3D"class FOO +p"
[ =C2=A0 88.374722] dyndbg: class[= 0]: nm:test_dynamic_debug base:20 len:7 ty:1
[ =C2=A0 88.375158] dyndbg:= =C2=A00: EMERG
[ =C2=A0 88.375345] dyndbg: =C2=A01: DANGER
[ =C2=A0 = 88.375540] dyndbg: =C2=A02: ERROR
[ =C2=A0 88.375726] dyndbg: =C2=A03: W= ARNING
[ =C2=A0 88.375930] dyndbg: =C2=A04: NOTICE
[ =C2=A0 88.376130= ] dyndbg: =C2=A05: INFO
[ =C2=A0 88.376310] dyndbg: =C2=A06: DEBUG
[ = =C2=A0 88.376499] dyndbg: class[1]: nm:test_dynamic_debug base:12 len:3 ty:= 1
[ =C2=A0 88.376903] dyndbg: =C2=A00: ONE
[ =C2=A0 88.377079] dyndbg= : =C2=A01: TWO
[ =C2=A0 88.377253] dyndbg: =C2=A02: THREE
[ =C2=A0 88= .377441] dyndbg: class[2]: nm:test_dynamic_debug base:8 len:3 ty:0
[ =C2= =A0 88.377837] dyndbg: =C2=A00: bing
[ =C2=A0 88.378022] dyndbg: =C2=A01= : bong
[ =C2=A0 88.378203] dyndbg: =C2=A02: boom
[ =C2=A0 88.378387] = dyndbg: class[3]: nm:test_dynamic_debug base:4 len:3 ty:0
[ =C2=A0 88.37= 8800] dyndbg: =C2=A00: Foo
[ =C2=A0 88.378986] dyndbg: =C2=A01: Bar
[= =C2=A0 88.379167] dyndbg: =C2=A02: Buzz
[ =C2=A0 88.379348] dyndbg: cla= ss[4]: nm:test_dynamic_debug base:0 len:3 ty:0
[ =C2=A0 88.379757] dyndb= g: =C2=A00: FOO
[ =C2=A0 88.379938] dyndbg: =C2=A01: BAR
[ =C2=A0 88.= 380136] dyndbg: =C2=A02: BUZZ
[ =C2=A0 88.380410] dyndbg: module:test_dy= namic_debug attached 5 classes
[ =C2=A0 88.380881] dyndbg: =C2=A024 debu= g prints in module test_dynamic_debug
[ =C2=A0 88.381315] dyndbg: module= : test_dynamic_debug dyndbg=3D"class FOO +p"
[ =C2=A0 88.38171= 4] dyndbg: query 0: "class FOO +p" mod:test_dynamic_debug
[ = =C2=A0 88.382109] dyndbg: split into words: "class" "FOO&quo= t; "+p"
[ =C2=A0 88.382445] dyndbg: op=3D'+'
[ =C2= =A0 88.382616] dyndbg: flags=3D0x1
[ =C2=A0 88.382802] dyndbg: *flagsp= =3D0x1 *maskp=3D0xffffffff
[ =C2=A0 88.383101] dyndbg: parsed: func=3D&q= uot;" file=3D"" module=3D"test_dynamic_debug" form= at=3D"" lineno=3D0-0 class=3DFOO
[ =C2=A0 88.383740] dyndbg: a= pplied: func=3D"" file=3D"" module=3D"test_dynamic= _debug" format=3D"" lineno=3D0-0 class=3DFOO
[ =C2=A0 88.= 384324] dyndbg: processed 1 queries, with 2 matches, 0 errs
bash-5.1#=C2= =A0

so its working at module-load time.
<= div>

For the entire class approach, did you spot another subsystem that could benefit from this and maybe make a more solid case that this is something good?

I had been working on the premise= that ~4k drm*dbg callsites was a good case.

verbo= sity-levels - with x<y logic instead=C2=A0of=C2=A0x=3D=3Dy is what's= =C2=A0currently missing.

the next revision adds so= mething, which "kinda works".
But I think I'll rip = it out, and do this simpler approach instead:

impl= ement a verbose/levels=C2=A0 param & callback, which takes
=C2=A0 =C2=A0echo 3 > /sys/module/foo/parameters/debug_verb= osity

and effectively does

=C2=A0 echo <<EOQRY=C2=A0 > /proc/dynamic_debug/control
module foo class V1 +p
module foo class V2 +p
module foo class V3 +p
module foo class V4 -p
<= div>module foo class V5 -p
module foo class V6 -p
=
module foo class V7 -p
module foo class V8 -p
EOQRY

s= ince only real=C2=A0+/-p changes incur=C2=A0kernel-patching costs,
the remaining overheads are minimal.


> RFC for DRM:
>
> - decoration flags "fmlt" do not work on drm_*dbg().
>=C2=A0 =C2=A0(drm_*dbg() dont use pr_debug, they *become* one flavor of= them)
>=C2=A0 =C2=A0this could (should?) be added, and maybe tailored for drm.=
>=C2=A0 =C2=A0some of the device prefixes are very long, a "d"= flag could optionalize them.

I'm lost what the fmlt decoration flags are?


The flags are::

=C2=A0 p =C2=A0 =C2=A0enables the pr_d= ebug() callsite.
=C2=A0 f =C2=A0 =C2=A0Include the function name in the = printed message
=C2=A0 l =C2=A0 =C2=A0Include line number in the printed= message
=C2=A0 m =C2=A0 =C2=A0Include module name in the printed messag= e
=C2=A0 t =C2=A0 =C2=A0Include thread ID in messages not generated from= interrupt context
=C2=A0 _ =C2=A0 =C2=A0No flags are set. (Or'd wit= h others on input)


the fmlt flags add= a "decoration" prefix to enabled/printed log messages
=

> - api use needs review wrt drm life-cycle.
>=C2=A0 =C2=A0enum drm_debug_category and DYNAMIC_DEBUG_CLASSES could be= together?

Hm if they're tied to module lifetime we should be good? Not sure what<= br> could go wrong here.


with the new __section, "life-cyc= le" doesnt really pertain.
the new issue is how the class-ma= ps are shared across the subsystem;
the current class-maps list f= or each module will probably break;
a list item cannot be on N di= fferent lists of different modules.
Altering the list-items to re= f the class-map (not contain it) should solve the problem.


=C2=A0
> - class-names could stand review, perhaps extension
>=C2=A0 =C2=A0"drm:core:" etc have appeared (maybe just from m= e)
>=C2=A0 =C2=A0or a "plan" to look at it later

Yeah it's been a bit sprawling. I'm kinda hoping that by firmly
establishing dyndbg as the drm debug approach we can cut down for the need<= br> for ad-hoc flags a bit.

yeah thats why I kept the DRM_UT_* names.
O= TOH - the symbolic names patch exposes the choices,
which locks t= he names as API ??

=C2=A0
> - i915 & amdgpu have pr_debugs (DC_LOG_*, gvt_dbg_*) that have
> class-ish prefixes that are separate from, but similar to DRM_UT_*, > and could stand review and possible unification with reformed or
> extended drm categories.

Yeah drm is not entirely consistent with how exactly driver debug printing<= br> should be done. Another reason why I'm hoping that the kitchen sync wit= h
everything approach you're doing here could help unify things.


the decoration flags can help he= re; they loosely/precisely describe
the elements of most/all the = current debug format-prefix variations.
So case by case, the ad-h= oc variations should map onto these flags,

The fla= gs allow selectively dropping the prefix info from some of the log entries,=
after you've=C2=A0seen the module name and function a dozen = times,=C2=A0
it's helpful to reduce screen clutter.

It might make sense to add a new flag for device,
so that dev_dbg() flavors can shorten-or-skip the longer device strings,= =C2=A0
maybe some drm specific flavors.

= =C2=A0

> - the change to enum drm_debug_category from bitmask values to 0..31 >=C2=A0 =C2=A0means that we foreclose this possiblility:
>
>=C2=A0 =C2=A0 drm_dbg(DRM_UT_CORE|DRM_UT_KMS, "wierd double-cat ex= periment");

Yeah no, that doesn't make much sense to me :-)

no chuckles for the schrodinger's cat joke ?
(maybe "yeah no" is the artful superpositional reply, I jus= t caught :)
=C2=A0
> - nouveau has very few drm.debug calls,
>=C2=A0 =C2=A0has NV_DEBUG, VMM_DEBUG, nvkm_printk_, I havent looked dee= ply.


nouveau has like levels, man ..
<= div>test_dynamic_debug implements its priority-style names as a POC

patch 18 converts nvkm_debug/trace to use dev_dbg instead= of dev_info
it probably could adapt to use drm_devdbg
<= div>

=C2=A0
Yeah see above. There's a pile more drivers (more on the armsoc side of=
things) which are quite big on the raw debug call approach.


LOW, MID, HI has been proposed at= least once wrt dyndbg.
that probably fits well with current disj= oint classes.
level/verbose classes should be practical too, as d= escribed above.

NB: The symbolic names should also= work=C2=A0

=C2=A0 =C2=A0echo +MID > /sys/modul= e/foobar/parameters/debug_verbosity
=C2=A0
though= theres some ambiguity with

=C2=A0 =C2=A0echo= -V3 > /sys/module/foobar/parameters/debug_verbosity

that should turn off V4,5,6,=C2= =A0
but what about V1,2 ?
it could leave them alone (wh= atever their previous settings are)

anyway, lkp-ro= bot and igt-trybot should be grinding on the latest patchset soon,
I'll send it after I fix whatever breaks.

= =C2=A0
Cheers, Daniel

thanks,=C2=A0
= Jim=C2=A0
--00000000000014abc905e0c8ba93--