Message-Id: <20210317065412.2890414-1-jim.cromie@gmail.com>
Date:   Wed, 17 Mar 2021 00:53:53 -0600
From:   Jim Cromie <jim.cromie@...il.com>
To:     jbaron@...mai.com, gregkh@...uxfoundation.org,
        linux-kernel@...r.kernel.org
Cc:     Jim Cromie <jim.cromie@...il.com>,
        kernel test robot <lkp@...el.com>
Subject: [RFC PATCH v4 00/19] dynamic debug diet plan

v4 fixes 'series grooming errors',
Reported-by: kernel test robot <lkp@...el.com>

CONFIG_DYNAMIC_DEBUG creates a struct _ddebug (56 bytes) for each
callsite, which includes 3 pointers to: module, filename, function.
These are repetetive, and compressible, this patch series goes about
doing that, it:

- splits struct _ddebug and __dyndbg[] section/array into 2
  creating struct _ddebug_site and __dyndbg_sites[]
  temporary _ddebug.site connects them.
  
- makes _ddebug_site data optional
- minor optimizations
- makes _ddebug_site data deleteable
  not necessary, proof of optionality

The RFC stuff comes at the end:

- attach __dyndbg_sites[] to module-info, like __dyndbg[]
- add index to struct _ddebug, use it for builtins
- add ddebug_site(_get|_put) abstraction to hide _ddebug.site

At this point, its practical to: compress __dyndbg_sites[] & replace
the section, then expand it on-demand to serve ddebug_site_get()
calls.  For enabled callsites (with decorations flags), the retrieved
records can be cached in a hash after theyre 1st needed/used (when
callsite is actually executed).

Whats my (ideal?) decompressing interface ?
And whats the name of the api call ? I couldnt suss it out yet.

For any control read, a simple block decompress and cache is close to
ideal; all site data is then available to iterate over, and each
ddebug_site_get() just indexes into it.  A stream of decompressed site
records would also work well, with less lumpy memory allocs and frees.

Actually dropping _ddebug.site is not yet possible.  While we could
drop it for builtin modules, thats cuz we know __start___dyndbg_sites.
For loaded modules, I need the elf section: __dyndbg_sites.  This is
in load-info, but I dont have a path to it.  In:

- add _ddebug_header/table

That adds a single header entry pair (structs _ddebug*s with special
initialization) into the arrays, and it links to the front of the
array, where its useful.  But creating this header entry only works
for vmlinux itself (because of vmlinux.ld.h patch), not for loadable
modules.

- add linker rules to module.lds.h

I tried to re-use the DYMAMIC_DEBUG_DATA macro added above for
modules, this failed, commit msg guesses the cause.  This breaks with
a linker script syntax error

TODO

Presuming the fixed header can be linked reliably in front doing
something like I tried, it can be recast as a ddebug_header and
unionized with struct _ddebug, and the _ddebugs[] will fit nicely as a
flex-array:

struct ddebug_table2 {
       struct ddebug_header foo;
       struct _ddebug ddebugs[];
}

A header would have 40 bytes, room to contain most/all of struct
ddebug_table's fields, a pointer to the __dyndbg_sites[] table, and a
list-head too, meaning it supports essential and nice-to-have
properties:

- the mapping: __dyndbg[N] --> __dyndbg_sites[N]	 # NEEDed to drop .site
  using container_of_flex() for flex-arrays

- we can add them directly to ddebug_tables list	 # freebie
  ie avoid the kzalloc in ddebug_add_module()  

If not all fields fit in the space available in __dyndbg[0], there is
space available in __dyndbg_sites[0].

Additionally, at the end of __init, ddebug_tables list is composed of
memory entirely in __dyndbg[], which could then be make readonly (by
means I dont know).  If this breaks insertions of loadable modules, we
can easily a 2nd list for that.


Jim Cromie (19):
  dyndbg: split struct _ddebug, move display fields to new _ddebug_site
  dyndbg: __init iterate over __dyndbg & __dyndbg_site in parallel
  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: optimize ddebug_emit_prefix
  dyndbg: avoid calling dyndbg_emit_prefix when it has no work
  dyndbg: refactor ddebug_alter_site out of ddebug_change
  dyndbg: allow deleting site info via control interface
  dyndbg+module: expose ddebug_sites to modules
  dyndbg: add ddebug_site(_get|_put) abstraction
  dyndbg: add _index to struct _ddebug
  dyndbg: prevent build bugs via -DNO_DYNAMIC_DEBUG_TABLE
  dyndbg: RFC - DECLARE/DEFINE_DYNAMIC_DEBUG_TABLE
  dyndbg: shuffle ddebug_table fields
  dyndbg: RFC add linker rules to module.lds.h

 arch/x86/boot/compressed/Makefile     |   1 +
 arch/x86/entry/vdso/Makefile          |   3 +
 arch/x86/purgatory/Makefile           |   1 +
 drivers/firmware/efi/libstub/Makefile |   3 +-
 drivers/gpu/drm/i915/i915_drv.c       |   2 +
 include/asm-generic/module.lds.h      |  21 ++
 include/asm-generic/vmlinux.lds.h     |  24 +-
 include/linux/dynamic_debug.h         | 180 +++++++++++++--
 kernel/module-internal.h              |   1 +
 kernel/module.c                       |   9 +-
 lib/dynamic_debug.c                   | 313 +++++++++++++++++++-------
 scripts/Makefile.lib                  |   2 +
 12 files changed, 449 insertions(+), 111 deletions(-)

-- 
2.29.2

Powered by blists - more mailing lists