Permalink
Commits on Mar 16, 2021
-
dyndbg: shuffle ddebug_table fields
In preparation to unionize structs _ddebug & ddebug_table, shuffle fields in latter so they match the layout of the former. This MAY simplify initialization of the header field, in particular by preserving *sites. It also sets up a later conversion to a flex-array ddebugs[]. This step is mostly to isolate/prove no breakage before HEAD++ Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: RFC - DECLARE/DEFINE_DYNAMIC_DEBUG_TABLE
V4-proto - now currently diet-3i Simplify: .gnu.linkonce._mod_.dyndbg -> .gnu.linkonce.dyndbg ie drop _mod_ This puts a single header record at front of the vectors, solving 2 problems (discussed below) simultaneously: - header in front, allowing flex-array rep of layout. - single header, not per-module. adequate for needs, no wasted space. this now appears to work - get: dp range-check good on builtin, !builtin. todo: - _sym_##_site symbols in init/main.i still busted - _index init in __ddebug_add_module - cleanup commentary below. DEFINE_DYNAMIC_DEBUG_TABLE is based on DECLARE_DYNAMIC_DEBUG_METADATA. Like its model, it creates/allocates a pair of structs: _ddebug & _ddebug_site, and inits them distinctively. Its purpose is to create an in-situ module-header-pair for each module's sub-vectors of _ddebug[], _ddebug_sites[]. Once the header-pair is reliably linked into the vectors, code can get from _ddebug[N] to the header-pair, then to _ddebug_sites[N] at runtime by saving N into _ddebug._index during init. With this, we can drop the site pointer in struct _ddebug. Eventually, this module-header-pair can be adapted to be an in-situ replacement for the separately allocated struct ddebug_tables, and be linked into the ddebug_tables list. RFC, NOTES: DYNAMIC_DEBUG is a 'transparent' facility, in that uses of pr_debug get extra features without additional api. Because of this, DEFINE_DYNAMIC_DEBUG_TABLE is 'transparently' invoked by dynamic_debug.h on behalf of all its (indirect) users, including printk.h. IOW this has wide effects; it results in multiple redundant declarations of header records, even single object files may get multiple copies. Using .gnu.linkonce._mod_.dyndbg(|_site) section names and "__used __weak " seems to resolve the redundancies. I havent tried with clang. In vmlinux-lds.h, the 2 KEEPs are modified to append those 2 new header sections to their respective existing __dyndbg* sections, in an interleaved manner. This places the header records immediately after the modules' block of _ddebug*s, in a knowable offset from &__dyndbg[0]. scripts/Makefile.lib gets a new -DKBUILD_MODSYM defn, with a value like KBUILD_MODNAME, except that its not __stringified. It is used to create a pair of module-ish named variables: _sym_##_dyndbg_base. For some non-obvious reason, the substitution doesnt work, resulting in per-module symbol names like KBUILD_MODSYM_dyndbg_base. This subtly alters the header initialization and is_dyndbg_header_pair(), which is used in __init to find the headers adjacent to each modules' block of _ddebug records. This isnt fatal to the plan; we just need the storage reserved where its accessible by known offset from the _ddebug[N] record of an enabled pr-debug. But it would be nice to have the symbol names consistent with the intent. I looked for a MODULE_SINGLETON(name_suffix) to use, similarly to how UNIQUE_ID is used to construct names, but found nothing. The .gnu.linkonce._mod_. works to eliminate all the extra headers in each module, but a problem remains; it adds unneeded headers for modules with zero pr-debugs. So Im seeing ~1500 extra headers. I tried several flavors of conditional linking, now think I want/need a linker-script language extension: KEEP ( *(__dyndbg) AND *(.gnu.linkonce.dyndbg) ) # my need KEEP ( *(foo_tbl) OR *(.gnu.linkonce.foo_alt) ) # for completeness I've managed to alter ld's grammar, but its only compile tested, and is missing the conditional linking pieces. Since this patchset's value proposition is a memory shrink, ~1500 extra headers is fatal, and this patchset must be a slow-cook. V4 todo: Time to simplify, drop _mod_, and change _ddebug.module_index to ._index, indicating that its no longer keyed to module, but to the whole compilation unit, which for the kernel includes all the builtin modules. loadable modules should get their own. tbt. this could obsolete all the above problems. Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: prevent build bugs via -DNO_DYNAMIC_DEBUG_TABLE
The next patch adds DEFINE_DYNAMIC_DEBUG_TABLE(), which breaks some subtrees with special compile constraints (efi etc). Avoid this by adding a define to suppress the *remote declaration* done by DEFINE_DYNAMIC_DEBUG_TABLE(), automatically, on behalf of all possible users of pr_debug. Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: add _index to struct _ddebug
We currently use dp->site to map: &__dyndbg[N] -> &__dyndbg_sites[N]. We want to drop site, new _ddebug._index provides the N. The mapping is done in ddebug_site_get(): For builtin modules, a _ddebug *ptr is between __start___dyndbg and __stop___dyndbg, and we can use &__start___dyndbg_sites[N] directly. For loadable modules, we still need work, so we print rubbish, and just return site pointer (which is correct). ddebug_add_module() handles _index initialization: Its new task is to number each module consecutively, so it gets new base arg to pass the next starting index. To actually drop site, We need both the module's __dyndbg* section addys, and we need their relative placement to have a base-to-base offset. PLAN - a table header connecting 2 tables. - ddebug_table points to both __dyndbgs & __dyndbg_sites. but *ddebugs & *sites are independent. no path from ddebugs[n] -> ddebug_sites[n] If we have a header record in-situ, which keeps the site pointer we seek to eliminate from _ddebug, and its in element[0] of both vectors, we can go: ddebugs[n] -> ddebugs[0] -> containerof -> site[n] union ddebug_table_header { struct ddebug_table *owner; struct _ddebug item; } and struct ddebug_table_vector { struct ddebug_table *owner; struct _ddebug vector[]; } Signed-off-by: Jim Cromie <jim.cromie@gmail.com> -
dyndbg: add ddebug_site(_get|_put) abstraction
Replace direct ->site refs with _get(),_put() internal API. Right now, _get() just returns ->site and _put() does nothing. Later we can replace that implementation with one using ->module_index to fetch then forget site data dynamically. Several approaches are possible: A: !site -> fill from backing store 1st try at this is/was using zram. At init, it copied each callsite into a zs-allocation, and all site-> refs afterward went thru _get/_put to zs-map on demand, and zs-unmap the site info. This worked until I tried to keep callsites mapped while they're enabled, when it gave lockdep warns/panics. IIRC theres a zram patchset doing something with locking; I need to retry this approach, even if other options are better, this might be a validating use case. B: block store Another approach is to compress the new linker section, using some algorithm thats good at indexed decompression. I probed this approach, using objcopy, unsuccessfully: objcopy --dump-section __dyndbg=dd \ --dump-section __dyndbg_sites=ddsites $IMG From vmlinux.o dumps were mostly empty (pre-link/reloc data?) and vmlinux didnt have the section. C: callsite composed from __dyndbg[N] & __dyndbg_site[N] We know _ddebug records are in a vector, either in the builtin __dyndbg linker section, or the same from a modprobed one. The builtin section has all builtin module sub-sections catenated dogether. At init, we iterate over the section, and "parse it" by creating a ddebug_table for each module with prdebugs. ddebug_table.num_debugs remembers the size of each modules' vector of prdebugs. We need a few things: - _ddebug.index field, which knows offset to start of this sub-vector. this new field will be "free" because the struct has padding. it can be initialized during init, then RO. - a back-pointer at the beginning of the sub-vector, to the ddebug_table "owning" (but not containing) this sub-vector of prdebugs. If we had both, we could get from the ddebug element to its vector root, back up to the owning ddebug_table, then down to the _callsite vector, and index to the right element. While slower than a pointer deref, this is a cold path, and it allows elimination of the per-callsite pointer member, thus greater density of the sections, and still can support sparse site info. That back-pointer feels tricky. It needs to be 1st in the sub-vector D: (C1?) add a header record to each sub-vector If we can insert a header record into each modules' __dyndbg* section sub-vectors, we can simplify the cold path above; a single sites* pointer in the header can give us access to __dyndbg_sites[N] Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg+module: expose ddebug_sites to modules
In order to drop the pointer connecting _ddebug's to _ddebug_sites, we need to elevate the latter; we need to track it in (internal) ddebug_tables, and set it in ddebug_add_module. That last part exposes it by interface to module.c, so we add a field to load_info, and adjust load_module to initialize it from the elf section. Its possible that this closes a "hole" created when __dyndbg_sites section was added, in that the section wasn't "managed" directly, and could conceivably get lost later. I never saw any misbehavior loading i915.ko into a vm, but still.. TBD/RFC: these 2 vectors should be in a single struct. if this struct can have ddebugs[], ie a flex-array, then container_of can get us to the sites*, yielding &ddebug_sites[N], and allowing to drop _ddebug.site The trouble with this is that ddebugs* now points to someone elses memory, and we cant just steal it and stomp on the memory just in front of it (for the sites ptr). rename n to numdbgs Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: allow deleting site info via control interface
Allow users & subsystems to selectively delete callsite info for pr-debug callsites. Hopefully, this can lead to actual recovery of memory. DRM is a potential user which would drop the sites: - has distinct categories for logging, and can map them over to a format prefix, like: "drm:core:", "drm:kms:", etc. - are happy with group control of all the callsites in a class/cateory. individual control is still possible using queries including line numbers - don't need dynamic "module:function:line:" prefixes in log messages - don't care about loss of context in /proc/dynamic_debug/control before: init/initramfs.c:485 [initramfs]unpack_to_rootfs =_ "Detected %s compressed data\012" init/main.c:1337 [main]run_init_process =pm " %s\012" init/main.c:1335 [main]run_init_process =pm " with environment:\012" init/main.c:1334 [main]run_init_process =pm " %s\012" init/main.c:1332 [main]run_init_process =pm " with arguments:\012" init/main.c:1121 [main]initcall_blacklisted =pm "initcall %s blacklisted\012" init/main.c:1082 [main]initcall_blacklist =pm "blacklisting initcall %s\012" then: bash-5.0# echo file init/main.c +D > /proc/dynamic_debug/control after: init/initramfs.c:485 [initramfs]unpack_to_rootfs =_ "Detected %s compressed data\012" [main]:1337 =pmD " %s\012" [main]:1335 =pmD " with environment:\012" [main]:1334 =pmD " %s\012" [main]:1332 =pmD " with arguments:\012" [main]:1121 =pmD "initcall %s blacklisted\012" [main]:1082 =pmD "blacklisting initcall %s\012" Notes: If Drm adopted dyndbg, i915 + drm* would add ~1600 prdebugs, amdgpu + drm* would add ~3200 callsites, so the additional memory costs are substantial. In trade, drm and drivers would avoid lots of calls to drm_debug_enabled(). This patch should reduce the costs. Using this interface, drm could drop site info for all categories / prefixes controlled by bits in drm.debug, while preserving site info and individual selectivity for any uncategorized prdebugs, and for all other modules. Lastly, because lineno field was not moved into _ddebug_callsite, it can be used to modify a single[*] callsite even if drm has dropped all the callsite data: echo module $mod format ^$prefix line $line +p >control Dropping site info is a one-way, information losing operation, so minor misuse is possible. Worst case is maybe (depending upon previous settings) some loss of logging context/decorations. echo +D > /proc/dynamic_debug/control [*] amdgpu has some macros invoking clusters of pr_debugs; each use of them creates a cluster of pr-debugs with the same line number. Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: refactor ddebug_alter_site out of ddebug_change
Move the JUMP_LABEL/static-key code to a separate function. no functional changes. Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: avoid calling dyndbg_emit_prefix when it has no work
Wrap function in a static-inline one, which checks flags to avoid calling the function unnecessarily. Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: optimize ddebug_emit_prefix
Add early return if no callsite info is specified in site-flags. This avoids fetching site info that isn't going to be printed. Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: accept null site in ddebug_proc_show
Accept a ddebug record with a null site pointer, and write abbreviated output for that record that doesn't include site info (but does include line-number, since that can be used in >control queries). Also add a 2nd header line with a template for the new output. Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: accept null site in dynamic_emit_prefix
2 prints use site->member, protect them with if site. no functional changes. Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: accept null site in ddebug_change
fix a debug-print that includes site info, by adding an alternate debug message that does not. no functional changes. Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: hoist ->site out of ddebug_match_site
A coming change adds _get/_put abstraction on the site pointer, to allow managing site info more flexibly. The get/put pattern is best done at a single lexical scope, where its more obviously correct, so hoist the ->site ref out of ddebug_match_site, and pass it in instead. no functional changes Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: accept null site in ddebug_match_site
basically, reorder the elements to minimize data fetches. 1- move format and line-number check code to the top of the function, since they don't use/check site info. 2- test site pointer: If its null, we return early, skipping 3: If the query tests against missing site info, fail the match. otherwize site matches. 3- rest of function (checking site vs query) is unchanged. ddebug_match_site ignores module, because it's tested already by the caller, where it is known from debug_tables. Signed-off-by: Jim Cromie <jim.cromie@gmail.com> -
dyndbg: refactor part of ddebug_change to ddebug_match_site
Move all the site-match logic into a separate function, reindent the code, and replace the continues with return falses. No functional changes. Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: __init iterate over __dyndbg & __dyndbg_site in parallel
In dynamic_debug_init(), rework for-loop; add 2nd 'site' var, and iterate over both __dyndbg* sections in parallel. Replace uses of iter->site with the new 'site' iter, add a BUG_ON to enforce the invariant given by HEAD~1's DECLARE_DYNAMIC_DEBUG_METADATA base->site initialization. 0- declare the new elf section start/stop, named in vmlinux.lds.h I disregarded a checkpatch warning about externs in c-files, stuck with current practice. 1- clean up use of 4 iterators for clarity: (iter, site), and ((iter, site)_mod_start) block markers. 2- iterate over __dyndbg_sites in parallel with __dyndbg s/iter->site/site/g; 3- add BUG_ON(iter->site != site) DECLARE_DYNAMIC_DEBUG_METADATA + linker insure this now. Maybe we can drop pointer, still get order. 4- var rename n to site_ct Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
-
dyndbg: split struct _ddebug, move display fields to new _ddebug_site
struct _ddebug has 2 flavors of fields: essential and optional. Split the 3 optional fields: module, function, file into a new struct _ddebug_site, and add pointer to it from _ddebug. These fields are optional in that they are primarily used to generate the optional "module:func:line" log prefix. They're also used to select callsites to >control. lineno is arguably optional too, but leaving it uses spare bytes in struct _ddebug. The new ptr increases memory footprint by 1 ptr per pr_debug, but I think its temporary, and the indirection gives several advantages: - we can drop sites and their storage opportunistically. this reduces per-site mem by 24/64. Subsystems may not need/want "module:func:line:" in their logs. If they already use format-prefixes such as "drm:kms:", they can select on those, and don't need the site info for that. forex: #> echo module drm format "^drm:kms: " +p >control ie: dynamic_debug_exec_queries("format '^drm:kms: '", "drm"); - the moved display fields are inherently hierarchical, and the linker section is ordered; so (module, file, function) have repeating values (90%, 85%, 45%). This is readily compressible, even with a simple field-wise run length encoding. Since I'm splitting the struct, I also reordered the fields to match the hierarchy. - the separate linker section sets up naturally for block compression. IFF we can on-demand map: ddebugs[N] -> ddebug_sites[N] - we can compress __dyndbg_sites during __init, and mark section __initdata - can decompress on-demand, say for `cat control` - can save chunks of decompressed buffer for enabled callsites - free chunks on site disable, or on memory pressure. Whats actually done here is rather mechanical, and preparatory. dynamic_debug.h: I cut struct _ddebug in half, renamed optional top-half to _ddebug_site, kept __align(8) for both halves. I added a forward decl for a unified comment for both head & body, and added head.site to point at body. DECLARE_DYNAMIC_DEBUG_METADATA does the core of the work; it declares and initializes both static struct variables together, and refs one to the other. dynamic_debug.c: dynamic_debug_init() mem-usage now also counts sites. dynamic_emit_prefix() & ddebug_change() use those moved fields; they get a new initialized auto-var, and the field refs get adjusted as needed to follow the move from one struct to the other. struct _ddebug_site *dc = dp->site; ddebug_proc_show() differs slightly; it assigns to (not initializes) the autovar, to avoid a panic when p == SEQ_START_TOKEN. vmlinux.lds.h: add __dyndbg_sites section, with the same align(8) and KEEP as used in the __dyndbg section. Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
