{"thread":{"id":"64625","subject":"Re: [PATCH v6 00/31] drm/dyndbg: Fix dynamic debug classmap regression","startedAt":"2025-12-14T18:25:01Z","lastAt":"2025-12-15T08:14:23Z","messageCount":4,"participants":["jim.cromie@gmail.com","Jeff King"],"isPatch":true,"patchVersion":6,"patchTotal":31},"messages":[{"id":"532150","messageId":"CAJfuBxzW6TMmdS74ZPfPSe1w6S=oO17WYZc-Jgn_et=-Muw05A@mail.gmail.com","threadId":"64625","inReplyTo":"5b3d492c-7037-45a5-a001-0064f14d5f81@akamai.com","subject":"Re: [PATCH v6 00/31] drm/dyndbg: Fix dynamic debug classmap regression","fromName":"","fromEmail":"jim.cromie@gmail.com","sentAt":"2025-12-14T18:24:34Z","receivedAt":"2025-12-14T18:25:01Z","isPatch":true,"sender":{"key":"jim.cromie@gmail.com","avatar":null},"body":"for some reason I cannot grasp,\ngit am fails to process this mbox.\n\nIt entirely misses 13/31,\nthen fails to apply 14, which needs 13\n\nIm able to cherry-pick 13,\nbut then I cannot --continue with 14,\neven after bumping .git/rebase-apply/next (iirc)\n\njimc@frodo:~/projects/lx/linux.git$ git am --empty=drop\n~/Downloads/PATCH-v6-00-31-drm-dyndbg-Fix-dynamic-debug-classmap-regression.mbox\nSkipping: drm/dyndbg: Fix dynamic debug classmap regression\nApplying: dyndbg: factor ddebug_match_desc out from ddebug_change\nApplying: dyndbg: add stub macro for DECLARE_DYNDBG_CLASSMAP\nApplying: docs/dyndbg: update examples \\012 to \\n\nApplying: test-dyndbg: fixup CLASSMAP usage error\nApplying: dyndbg: reword \"class unknown,\" to \"class:_UNKNOWN_\"\nApplying: docs/dyndbg: explain flags parse 1st\nApplying: dyndbg: make ddebug_class_param union members same size\nApplying: dyndbg: drop NUM_TYPE_ARRAY\nApplying: dyndbg: tweak pr_fmt to avoid expansion conflicts\nApplying: dyndbg: reduce verbose/debug clutter\nApplying: dyndbg: refactor param_set_dyndbg_classes and below\nApplying: dyndbg: tighten fn-sig of ddebug_apply_class_bitmap\nApplying: dyndbg: macrofy a 2-index for-loop pattern\nerror: patch failed: lib/dynamic_debug.c:155\nerror: lib/dynamic_debug.c: patch does not apply\nPatch failed at 0014 dyndbg: macrofy a 2-index for-loop pattern\nhint: Use 'git am --show-current-patch=diff' to see the failed patch\nhint: When you have resolved this problem, run \"git am --continue\".\nhint: If you prefer to skip this patch, run \"git am --skip\" instead.\nhint: To restore the original branch and stop patching, run \"git am --abort\".\nhint: Disable this message with \"git config set advice.mergeConflict false\"\njimc@frodo:~/projects/lx/linux.git$ git help am\n\nIOW 1st below fails cuz 2nd was missed.\n\n9d3217b82474 dyndbg: macrofy a 2-index for-loop pattern\n0181185c3e75 dyndbg: replace classmap list with a vector\nef6ee2b321ce dyndbg: tighten fn-sig of ddebug_apply_class_bitmap\n804e6a0d59b6 dyndbg: refactor param_set_dyndbg_classes and below\n039806bc83dd dyndbg: reduce verbose/debug clutter\n162a0398fae9 dyndbg: tweak pr_fmt to avoid expansion conflicts\nd5524fc1ef31 dyndbg: drop NUM_TYPE_ARRAY\na6e1e7f4da90 dyndbg: make ddebug_class_param union members same size\na1d3e32dd906 dyndbg: reword \"class unknown,\" to \"class:_UNKNOWN_\"\n5692e955f0ce test-dyndbg: fixup CLASSMAP usage error\n3ee7e303e78e docs/dyndbg: explain flags parse 1st\n2f33390837fb docs/dyndbg: update examples \\012 to \\n\n256317aa5996 dyndbg: add stub macro for DECLARE_DYNDBG_CLASSMAP\n37bad039f6c7 dyndbg: factor ddebug_match_desc out from ddebug_change\n7d0a66e4bb90 (tag: v6.18, master) Linux 6.18\n\nOn Sat, Dec 13, 2025 at 4:57 AM Jason Baron wrote:\n>\n>\n>\n> On 12/10/25 4:12 PM, jim.cromie@gmail.com wrote:\n> > !-------------------------------------------------------------------|\n> > This Message Is From an External Sender\n> > This message came from outside your organization.\n> > |-------------------------------------------------------------------!\n> >\n> > On Thu, Dec 11, 2025 at 8:09 AM Jason Baron wrote:\n> >>\n> >>\n> >>\n> >> On 11/18/25 3:18 PM, Jim Cromie wrote:\n> >>> !-------------------------------------------------------------------|\n> >>> This Message Is From an External Sender\n> >>> This message came from outside your organization.\n> >>> |-------------------------------------------------------------------!\n> >>>\n> >>> hello all,\n> >>>\n> >>> commit aad0214f3026 (\"dyndbg: add DECLARE_DYNDBG_CLASSMAP macro\")\n> >>>\n> >>> added dyndbg's \"classmaps\" feature, which brought dyndbg's 0-off-cost\n> >>> debug to DRM. Dyndbg wired to /sys/module/drm/parameters/debug,\n> >>> mapped its bits to classes named \"DRM_UT_*\", and effected the callsite\n> >>> enablements only on updates to the sys-node (and underlying >control).\n> >>>\n> >>> Sadly, it hit a CI failure, resulting in:\n> >>> commit bb2ff6c27bc9 (\"drm: Disable dynamic debug as broken\")\n> >>>\n> >>> The regression was that drivers, when modprobed, did not get the\n> >>> drm.debug=0xff turn-on action, because that had already been done for\n> >>> drm.ko itself.\n> >>>\n> >>> The core design bug is in the DECLARE_DYNDBG_CLASSMAP macro. Its use\n> >>> in both drm.ko (ie core) and all drivers.ko meant that they couldn't\n> >>> fundamentally distinguish their respective roles. They each\n> >>> \"re-defined\" the classmap separately, breaking K&R-101.\n> >>>\n> >>> My ad-hoc test scripting helped to hide the error from me, by 1st\n> >>> testing various combos of boot-time module.dyndbg=... and\n> >>> drm.debug=... configurations, and then inadvertently relying upon\n> >>> those initializations.\n> >>>\n> >>> This series addresses both failings:\n> >>>\n> >>> It replaces DECLARE_DYNDBG_CLASSMAP with\n> >>>\n> >>> - `DYNAMIC_DEBUG_CLASSMAP_DEFINE`: Used by core modules (e.g.,\n> >>> `drm.ko`) to define their classmaps. Based upon DECLARE, it exports\n> >>> the classmap so USE can use it.\n> >>>\n> >>> - `DYNAMIC_DEBUG_CLASSMAP_USE`: this lets other \"subsystem\" users\n> >>> create a linkage to the classmap defined elsewhere (ie drm.ko).\n> >>> These users can then find their \"parent\" and apply its settings.\n> >>>\n> >>> It adds a selftest script, and a 2nd \"sub-module\" to recapitulate\n> >>> DRM's multi-module \"subsystem\" use-case, including the specific\n> >>> failure scenario.\n> >>>\n> >>> It also adds minor parsing enhancements, allowing easier construction\n> >>> of multi-part debug configurations. These enhancements are used to\n> >>> test classmaps in particular, but are not otherwize required.\n> >>>\n> >>> Thank you for your review.\n> >>>\n> >>> P.S. Id also like to \"tease\" some other work:\n> >>>\n> >>> 1. patchset to send pr_debugs to tracefs on +T flag\n> >>>\n> >>> allows 63 \"private\" tracebufs, 1 \"common\" one (at 0)\n> >>> \"drm.debug_2trace=0x1ff\" is possible\n> >>> from Lukas Bartoski\n> >>>\n> >>> 2. patchset to save 40% of DATA_DATA footprint\n> >>>\n> >>> move (modname,filename,function) to struct _ddebug_site\n> >>> save their descriptor intervals to 3 maple-trees\n> >>> 3 accessors fetch on descriptor, from trees\n> >>> move __dyndbg_sites __section to INIT_DATA\n> >>>\n> >>> 3. patchset to cache dynamic-prefixes\n> >>> should hide 2.s cost increase.\n> >>>\n> >>>\n> >>\n> >> Hi Jim,\n> >>\n> >> I just wanted to confirm my understanding that the class names here are\n> >> 'global'. That is if say two different modules both used say the name\n> >> \"core\" in their DYNAMIC_DEBUG_CLASSMAP_DEFINE() name array, then if the\n> >> user did: echo \"class core +p > control\", then that would enable all the\n> >> sites that had the class name \"core\" in both modules. One could add the\n> >> \"module\" modifier to the request if needed.\n> >>\n> >> One could prepend the module name to the class names to make them unique\n> >> but it's not much different from adding a separate 'module blah' in the\n> >> request. So probably fine as is, but maybe worth calling out in the docs\n> >> a bit?\n> >>\n> >\n> > Yes. that is correct. class CORE is global.\n> > If 2 different DEFINE()s give that classname,\n> > the defining modules will both respond to `class CORE +p > control`\n> > but they will get independent int values (which could be the same, but\n> > dont have to be)\n> >\n> > DRM is our case in point.\n> > I reused DRM_UT_CORE...\n> > because I didnt have a good reason to change it\n> > that said, Daniel Vetter noted that the _UT_ part doesnt have a real reason.\n> > So theres some space for a discussion, when I resend that patchset.\n> >\n> > `module drm class DRM_UT_CORE +p > control`\n> > will narrow the query and avoid all the drivers/helpers,\n> > which could be what someone wants.\n> > class DRM_UT_CORE would select drivers and helpers too,\n> > so the DRM_UT_ disambiguation is appropriate.\n> >\n> > I'll reread the docs to see if theres a bit more I can add to further\n> > explain this.\n> > Do you have any suggestions for wording to achieve this ?\n> >\n>\n>\n> Ok, so sounds like DRM_ prefix is already adding some scoping vs. just\n> the simple 'CORE' name. So maybe just something like:\n>\n> Note that class names exist in a 'global' namespace. Thus, if two\n> different modules share a common class name such as 'core' both modules\n> will have sites enabled via: echo 'class core +p > control'. Thus, you\n> may wish to scope any new class names to a specific use-case or module.\n> For example, drm uses the 'DRM_' prefix, as in 'DRM_CORE'.\n>\n> Thanks,\n>\n> -Jason\n"},{"id":"532151","messageId":"20251214195420.GA791422@coredump.intra.peff.net","threadId":"64625","inReplyTo":"CAJfuBxzW6TMmdS74ZPfPSe1w6S=oO17WYZc-Jgn_et=-Muw05A@mail.gmail.com","subject":"Re: [PATCH v6 00/31] drm/dyndbg: Fix dynamic debug classmap regression","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-12-14T19:54:20Z","receivedAt":"2025-12-14T19:54:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 15, 2025 at 07:24:34AM +1300, jim.cromie@gmail.com wrote:\n\n> for some reason I cannot grasp,\n> git am fails to process this mbox.\n> \n> It entirely misses 13/31,\n> then fails to apply 14, which needs 13\n\nCan you show the exact input you fed to git-am?\n\nEverything applied fine for me using this workflow:\n\n - grab the thread mbox from\n https://lore.kernel.org/dri-devel/CAJfuBxzW6TMmdS74ZPfPSe1w6S=oO17WYZc-Jgn_et=-Muw05A@mail.gmail.com/t.mbox.gz\n\n - view that mbox in mutt, tagging all of the 31 messages and then\n copying them into their own mbox \n\n - git checkout v6.18 && git am wrote:\n>\n> On Mon, Dec 15, 2025 at 07:24:34AM +1300, jim.cromie@gmail.com wrote:\n>\n> > for some reason I cannot grasp,\n> > git am fails to process this mbox.\n> >\n> > It entirely misses 13/31,\n> > then fails to apply 14, which needs 13\n>\n> Can you show the exact input you fed to git-am?\n>\n\nin the 1st report, I got mbox.gz from:\nhttps://lore.kernel.org/lkml/20251118201842.1447666-1-jim.cromie@gmail.com/\n\nusing the mbox.gz from your link, I get a different failure, this time\non patch 11\n\n\njimc@frodo:~/projects/lx/linux.git$ git am --abort\njimc@frodo:~/projects/lx/linux.git$ git describe\nv6.18\njimc@frodo:~/projects/lx/linux.git$ cksum\n~/Downloads/PATCH-v6-00-31-drm-dyndbg-Fix-dynamic-debug-classmap-regression.mbox.gz\n540358004 206558\n/home/jimc/Downloads/PATCH-v6-00-31-drm-dyndbg-Fix-dynamic-debug-classmap-regression.mbox.gz\n\njimc@frodo:~/projects/lx/linux.git$ gunzip\n~/Downloads/PATCH-v6-00-31-drm-dyndbg-Fix-dynamic-debug-classmap-regression.mbox.gz\ngzip: /home/jimc/Downloads/PATCH-v6-00-31-drm-dyndbg-Fix-dynamic-debug-classmap-regression.mbox\nalready exists; do you wish to overwrite (y or n)? y\njimc@frodo:~/projects/lx/linux.git$ git am --empty=drop\n~/Downloads/PATCH-v6-00-31-drm-dyndbg-Fix-dynamic-debug-classmap-regression.mbox\nSkipping: drm/dyndbg: Fix dynamic debug classmap regression\nApplying: dyndbg: factor ddebug_match_desc out from ddebug_change\nApplying: docs/dyndbg: explain flags parse 1st\nApplying: test-dyndbg: fixup CLASSMAP usage error\nApplying: dyndbg: add stub macro for DECLARE_DYNDBG_CLASSMAP\nApplying: dyndbg: make ddebug_class_param union members same size\nApplying: dyndbg: tweak pr_fmt to avoid expansion conflicts\nApplying: dyndbg: refactor param_set_dyndbg_classes and below\nApplying: dyndbg: reduce verbose/debug clutter\nApplying: dyndbg: drop NUM_TYPE_ARRAY\nApplying: dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module\nerror: patch failed: lib/dynamic_debug.c:170\nerror: lib/dynamic_debug.c: patch does not apply\nPatch failed at 0011 dyndbg: hoist classmap-filter-by-modname up to\nddebug_add_module\nhint: Use 'git am --show-current-patch=diff' to see the failed patch\nhint: When you have resolved this problem, run \"git am --continue\".\nhint: If you prefer to skip this patch, run \"git am --skip\" instead.\nhint: To restore the original branch and stop patching, run \"git am --abort\".\nhint: Disable this message with \"git config set advice.mergeConflict false\"\njimc@frodo:~/projects/lx/linux.git$\n\nUpon closer inspection, it misses several patches, and reorders others ??\n\nin particular, the reported 0011 patch above is numbered 16 in the mbox.\n\n\n2025-11-18 20:18 ` [PATCH v6 02/31] dyndbg: add stub macro for\nDECLARE_DYNDBG_CLASSMAP Jim Cromie\n2025-11-18 20:18 ` [PATCH v6 03/31] docs/dyndbg: update examples \\012\nto \\n Jim Cromie\n2025-11-20 9:30 ` Bagas Sanjaya\n2025-11-18 20:18 ` [PATCH v6 06/31] dyndbg: reword \"class unknown,\" to\n\"class:_UNKNOWN_\" Jim Cromie\n2025-11-18 20:18 ` [PATCH v6 09/31] dyndbg: tweak pr_fmt to avoid\nexpansion conflicts Jim Cromie\n2025-11-18 20:18 ` [PATCH v6 12/31] dyndbg: tighten fn-sig of\nddebug_apply_class_bitmap Jim Cromie\n2025-11-18 20:18 ` [PATCH v6 13/31] dyndbg: replace classmap list with\na vector Jim Cromie\n2025-11-18 20:18 ` [PATCH v6 14/31] dyndbg: macrofy a 2-index for-loop\npattern Jim Cromie\n2025-11-18 20:18 ` [PATCH v6 15/31] dyndbg,module: make proper\nsubstructs in _ddebug_info Jim Cromie\n\n2025-11-18 20:18 ` [PATCH v6 16/31] dyndbg: hoist\nclassmap-filter-by-modname up to ddebug_add_module Jim Cromie\n\njimc@frodo:~/projects/lx/linux.git$ git --version\ngit version 2.52.0\nIm using fedora packaged git.\n"},{"id":"532177","messageId":"20251215080736.GA809641@coredump.intra.peff.net","threadId":"64625","inReplyTo":"CAJfuBxx-_Z_hCoqdj2Lma7oP6LhCM6Pz=afe2P=wKO41T7R3mA@mail.gmail.com","subject":"Re: [PATCH v6 00/31] drm/dyndbg: Fix dynamic debug classmap regression","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-12-15T08:07:36Z","receivedAt":"2025-12-15T08:14:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 15, 2025 at 11:52:38AM +1300, jim.cromie@gmail.com wrote:\n\n> using the mbox.gz from your link, I get a different failure, this time\n> on patch 11\n> [...]\n> jimc@frodo:~/projects/lx/linux.git$ gunzip\n> ~/Downloads/PATCH-v6-00-31-drm-dyndbg-Fix-dynamic-debug-classmap-regression.mbox.gz\n> gzip: /home/jimc/Downloads/PATCH-v6-00-31-drm-dyndbg-Fix-dynamic-debug-classmap-regression.mbox\n> already exists; do you wish to overwrite (y or n)? y\n> jimc@frodo:~/projects/lx/linux.git$ git am --empty=drop\n\nAh, that is the difference: you are applying directly from the\ndownloaded mbox file, whereas I picked out the messages using mutt.\n\nThe mbox provided by lore is generally in the order the messages were\nreceived, which does not necessarily correspond to the order they were\nsent, or the rfc822 dates, or the subject lines. But \"git am\" does not\ndo any sorting; it applies the messages in the order it finds them in\nthe input mbox. So you get out-of-order patch application.\n\nThere's another possible gotcha, as well. The mbox for the thread will\ncontain other non-patch messages like the cover letter and any review\nresponses. Adding --empty=drop as you did will generally skip past\nthose, but not always. If somebody responds and says \"Maybe do it like\nthis\" with an inline patch, then \"git am\" will pick up that patch, too!\n\n\nIt worked for me because when I picked the patches out of the thread in\nmutt, it showed them sorted by rfc822 date header and used that same\nordering to dump them to the new, filtered mbox. And of course I\nmanually decided on which messages were part of the patch series and\nexcluded the rest (based on subject lines).\n\nIt would probably be possible to teach \"git am\" to sort by date header,\nbut that's not always right, either (you could have a local series with\nout-of-order author dates due to rebasing). You could use the subject\nlines as heuristics, if you know that the sender didn't use any exotic\nformat-patch options. So there are probably some heuristics at play.\n\nAnd none of those ideas helps with the selection problem, which is\nanother heuristics ball of wax.\n\nFortunately, I think b4 has melted that wax for us already (OK, maybe\nI'm losing the metaphor). If you do:\n\n b4 mbox https://lore.kernel.org/lkml/20251118201842.1447666-1-jim.cromie@gmail.com/\n\nyou'll get that unordered mbox again. But if you use the \"am\" command:\n\n b4 am https://lore.kernel.org/lkml/20251118201842.1447666-1-jim.cromie@gmail.com/\n\nit figures everything out and gives you the clean series in an mbox. It\nalso knows how to pick the latest version of the series (your v6 is in\nits own thread here, but if it were in a thread with v1..v5, you again\nget into another message-selection problem).\n\n-Peff\n"}]}