Messages in this thread |  | | From | Jim Cromie <> | Subject | [RFC PATCH v6 00/34] DYNAMIC_DEBUG diet progress, dropped 30kb | Date | Sat, 29 May 2021 13:59:55 -0600 |
| |
DYNAMIC_DEBUG plants a vector/table of struct _ddebug's in kernel data; its (module,filename,function) columns have ordered, repeating data (90%,55%,45%), which is readily compressible.
This patchset restructures the table to isolate that redundant data, then packs out the duplicates, achieving 40% reduction in the memory needed for that data, about 30kb on my laptop. No de/compression used.
On my i7 laptop kernel:
dyndbg: 62 44 2899 1728 mptcp.check_fully_established.895 <== 1728/2899 compresion dyndbg: 63 debug prints in module mptcp (in 44 functions) dyndbg: 2900 prdebugs in 277 modules, \ 12 KiB in ddebug tables, \ 90 KiB in __dyndbg section, \ 67 KiB in __dyndbg_sites section <== on this data
It should be practical to actually reclaim that memory; RFC on how that could be done. On a hail-mary, I kreallocd sites on a module, it panic'd.
To isolate the compressible part, we split the table/struct in 2. The redundant columns (modname, filename, function) are moved to struct _ddebug_site ("__dyndbg_sites" elf-section), and refd by a new/temporary _ddebug.site pointer. The site data can then be made optional (6 patches), managed separately, and "compressed".
The pointer adds 8/56 to the memory footprint, we want to replace it with indexing; the _index only needs to span ~10k callsites, and comes "free" from unused bits in struct _ddebug.
The difficulty with this, and the reason for the multi-commit adaptation, is that we have only a struct _ddebug pointer. We know its in __dyndbgs[], but we dont know &__dyndbg_sites[], which we need to finally get __dyndbg_sites[_index].
We get it with an "up-over-and-down" maneuver;
1. __dyndbgs[._index] ==> __dyndbgs[0] pretty trivial with ._index 2. __dyndbgs[0].site indirectly use __dyndbgs[0].site instead of [._index].site 3. insert "header" record - just _ddebug, renamed uses .gnu.linkonce to create headers, linker script to insert it - RFC becomes __dyndbgs[0] provides .site for [2] 4. __dyndbgs[0].site->__dyndbg_sites[_index] trivial
With "up-over-and-down" working, we can get rid of .site pointer
Next is specialization and unionization:
We create a header union, containing a struct _ddebug to make it "fit" in __dyndbgs[], and an anonymous struct with .site and a several other struct _ddebug fields checked to validate headers.
This allows the header to keep its .site pointer, while struct _ddebug drops it, shrinking memory usage 8/56 back to parity.
It is working, but Im certain it can be restructured more tightly. In particular, [header,__dyndbgs[]] fits well in a flex-array. My anonymous struct/union fu is insufficient, hints or examples are solicited.
Finally comes "consolidation" (not compression). To do this, we split/clone ._index to its two tasks (1,4 above); ._back (to n=0), then ._map (to soon to be repacked site). When adding each module, we scan __dyndbg_sites[], repack it with its unique entries, and ._map it and the function's remaining _ddebugs to use that unique entry in the repacked vector.
Version-5 is here: https://lore.kernel.org/lkml/CAJfuBxyjfopQMzMyFrrZK7YppsL8kh0VVWySrJDXeUB15uwFag@mail.gmail.com/#t
Version-6 differs from -v5 by:
It works so far.. it's been surviving 0day tests:
It compresses, can save 30kb of 70kb table, of 200kb total data overhead.
Ive resolved most of my previous QsRFCs, but Im using .gnu.linkonce sections and linker script tweaks for the 1st time and figured it needs more scrutiny than usual. Several of the commit messages say "RFC" where I have most uncertainty.
The "most broken" part of the patchset is the specialization & unionization, Im certain theres a tighter way to construct it. ISTM theres a good chance that [header,__dyndbgs[]] fits well in a flex-array, and container_of could help. Hints, tips, pointers to examples are welcome.
Patch 34 - prototype pr_debug_once(), pr_debug_ratelimited()
= print-once.
$> echo function foo lineno 1234 +po > control
Doable with just a new flag, and a new "did_print" data bit, both which will fit in struct _ddebug, no separate storage needed. Its just one more flag check for dynamic_pr_debug, AFTER its +p enabled.
Unsetting flag and reseting it would clear the "did_print" bit. Maybe just resetting "+o" too.
= Rate-Limiting
We don't need this often, so allocate RLs only when enabled & needed, and store in hash on [ &builtin|&module, ._back|._map ]
$> echo module serio_raw +rp > control
By default, we could allocate a rate-limiting (RL) structure for every pr-debug flagged, store them in a RLs-hash (on ._back + module), and retrieve and evaluate them only when the pr-debug is enabled. We could even defer RL alloc/attach until the pr-debug is actually called.
If we also (optionally) hash RLs on (._map + module), then they are shared across a function. This could be useful behavior, and would save lots of memory where it is appropriate.
$> echo function foo +rg > control - on function alone, RL-group capable - flag modifier. use _map not _back.
Next ? More Dieting
_ddebug as 3 vectors of components.
We have split _ddebug in 2, maybe also split out static-keys to a 3rd vector ? This would smoothe the path towards a single struct _ddebug per function, which controls all N pr_debugs in that function.
We could recapitulate the repack, with some changes in details; - we're packing the referring table, not the referred table. ie: we'd condense __dyndbgs[], keep the vector of static-keys. - we'd lose meaning of lineno, could reclaim bits for something else.
New API elements ?
. pr_debug_once() etal . pr_debug_ratelimited() etal . pr_debug_rl_grouped() etal ??
This is good for signaling programmer intent. A tweak to DD_METADATA could set the corresponding bits at compile-time, causing its provisioning when prdebug'd.
RL contol - Some speculation:
$> echo module serio_raw +r rate 10 burst 3 > control - after required flags. - only legal with +r - suggests sharing across group (by default) - should rates scale per callsite count ? or only by user ?
$> echo module serio_raw lineno 333 +r rate 100 burst 3 > control - individual RLs are possible
Jim Cromie (33):
Jim Cromie (34):
3 trivial patches, 2 are in -rc3: dyndbg: avoid calling dyndbg_emit_prefix when it has no work dyndbg: drop uninformative vpr_info dyndbg: display KiB of data memory used.
split struct _ddebug in 2:
dyndbg: split struct _ddebug's display fields to new _ddebug_site dyndbg: __init iterate over __dyndbg & __dyndbg_site in parallel dyndbg+module: expose ddebug_sites to modules
make site info optional: Even though !.site is impossible, by linker init + DD_METADATA, lets allow it, to shorten codepaths and work towards dropping site data.
dyndbg: refactor part of ddebug_change to ddebug_match_site dyndbg: accept null site in ddebug_match_site dyndbg: hoist ->site out of ddebug_match_site dyndbg: accept null site in ddebug_change dyndbg: accept null site in dynamic_emit_prefix dyndbg: accept null site in ddebug_proc_show dyndbg: refactor ddebug_alter_site out of ddebug_change dyndbg: allow deleting site info via control interface
decoupling: dyndbg: add ddebug_site(_get|_put) abstraction cleanup, tidy: dyndbg: ddebug_add_module avoid adding empty modules
do the "up-over-down" maneuver: dyndbg: add _index to struct _ddebug dyndbg: prevent build bugs via -DNO_DYNAMIC_DEBUG_TABLE dyndbg: RFC - DEFINE_DYNAMIC_DEBUG_TABLE dyndbg: RFC handle __dyndbg* sections in module.lds.h dyndbg: ddebug_add_module() handle headers. dyndbg: validate ddebug_site_get invariants dyndbg: fix NULL deref after deleting sites dyndbg: dont show header records in control dyndbg: make site pointer and checks on it optional (almost) dyndbg: swap WARN_ON for BUG_ON see what 0-day says dyndbg: fixup protect header when deleting site dyndbg: unionize _ddebug*_headers with struct _ddebug* dyndbg: RFC drop _ddebug.site pointer
condense unique site data: dyndbg: split/copy ._index into 2 new fields: ._back, ._map dyndbg: detect repeated site recs in add_module dyndbg: pack module pr_debug sites dyndbg: pack pr-debug site-recs in builtin modules
new features - _once, _ratelimited: dyndbg: prototype print-once and print-ratelimited RFC
arch/arm/boot/compressed/Makefile | 2 + arch/sparc/vdso/Makefile | 2 + arch/x86/boot/compressed/Makefile | 1 + arch/x86/entry/vdso/Makefile | 3 + arch/x86/purgatory/Makefile | 1 + drivers/firmware/efi/libstub/Makefile | 3 +- include/asm-generic/module.lds.h | 9 +- include/asm-generic/vmlinux.lds.h | 24 +- include/linux/dynamic_debug.h | 143 +++++++-- kernel/module-internal.h | 1 + kernel/module.c | 11 +- lib/dynamic_debug.c | 421 ++++++++++++++++++++------ 12 files changed, 499 insertions(+), 122 deletions(-)
-- 2.31.1
|  |