Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jim Cromie <jim.cromie@gmail.com>
To: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	amd-gfx@lists.freedesktop.org,
	intel-gvt-dev@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org
Cc: jani.nikula@intel.com, daniel.vetter@ffwll.ch, jbaron@akamai.com,
	Jim Cromie <jim.cromie@gmail.com>,
	seanpaul@chromium.org, gregkh@linuxfoundation.org
Subject: [Intel-gfx] [RFC PATCH 00/17] DRM_USE_DYNAMIC_DEBUG regression
Date: Mon,  5 Dec 2022 17:34:07 -0700	[thread overview]
Message-ID: <20221206003424.592078-1-jim.cromie@gmail.com> (raw)

Hi everyone,

DRM_USE_DYNAMIC_DEBUG=y has a regression on rc-*

Regression is due to a chicken-egg problem loading modules; on
`modprobe i915`, drm is loaded 1st, and drm.debug is set.  When
drm_debug_enabled() tested __drm_debug at runtime, that just worked.

But with DRM_USE_DYNAMIC_DEBUG=y, the runtime test is replaced with a
post-load enablement of drm_dbg/dyndbg callsites (static-keys), via
dyndbg's callback on __drm_debug.  Since all drm-drivers need drm.ko,
it is loaded 1st, then drm.debug=X is applied, then drivers load, but
too late for drm_dbgs to be enabled.

STATUS

For all-loadable drm,i915,amdgpu configs, it almost works, but
propagating drm.debug to dependent modules doesnt actually apply,
though the motions are there.  This is not the problem I want to chase
here.

The more basic trouble is:

For builtin drm + helpers, things are broken pretty early; at the
beginning of dynamic_debug_init().  As the ddebug_sanity() commit-msg
describes in some detail, the records added by _USE fail to reference
the struct ddebug_class_map created and exported by _DEFINE, but get
separate addresses to "other" data that segv's when used as the
expected pointer. FWIW, the pointer val starts with "revi".

OVERVIEW

DECLARE_DYNDBG_CLASSMAP is broken: it is one-size-fits-all-poorly.
It muddles the distinction between a (single) definition, and multiple
references.  Something exported should suffice.

The core of this patchset splits it into:

DYNDBG_CLASSMAP_DEFINE	used once per subsystem to define each classmap
DYNDBG_CLASSMAP_USE	declare dependence on a DEFINEd classmap

This makes the weird coordinated-changes-by-identical-classmaps
"feature" unnecessary; the DEFINE can export the var, and USE refers
to the exported var.

So this patchset adds another section: __dyndbg_class_refs.

It is like __dyndbg_classes; it is scanned under ddebug_add_module(),
and attached to each module's ddebug_table.  Once attached, it can be
used like classes to validate and apply class FOO >control queries.

It also maps the class user -> definer explicitly, so that when the
module is loaded, the section scan can find the kernel-param that is
wired to dyndbg's kparam-callback, and apply its state-var, forex:
__drm_debug to the just loaded helper/driver module.

Theres plenty to address Im sure.

Jim Cromie (17):
  test-dyndbg: fixup CLASSMAP usage error
  test-dyndbg: show that DEBUG enables prdbgs at compiletime
  dyndbg: fix readback value on LEVEL_NAMES interfaces
  dyndbg: replace classmap list with a vector
  dyndbg: make ddebug_apply_class_bitmap more selective
  dyndbg: dynamic_debug_init - use pointer inequality, not strcmp
  dyndbg: drop NUM_TYPE_ARRAY
  dyndbg: reduce verbose/debug clutter
  dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP with
    DYNDBG_CLASSMAP(_DEFINE|_USE)
  dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE)
  dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args
  dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements
  drm_print: fix stale macro-name in comment
  dyndbg: unwrap __ddebug_add_module inner function NOTYET
  dyndbg: ddebug_sanity()
  dyndbg: mess-w-dep-class
  dyndbg: miss-on HACK

 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  14 +-
 drivers/gpu/drm/display/drm_dp_helper.c |  14 +-
 drivers/gpu/drm/drm_crtc_helper.c       |  14 +-
 drivers/gpu/drm/drm_print.c             |  22 +--
 drivers/gpu/drm/i915/i915_params.c      |  14 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  14 +-
 include/asm-generic/vmlinux.lds.h       |   3 +
 include/drm/drm_print.h                 |   6 +-
 include/linux/dynamic_debug.h           |  57 ++++--
 include/linux/map.h                     |  54 ++++++
 kernel/module/main.c                    |   2 +
 lib/dynamic_debug.c                     | 240 +++++++++++++++++++-----
 lib/test_dynamic_debug.c                |  47 ++---
 13 files changed, 344 insertions(+), 157 deletions(-)
 create mode 100644 include/linux/map.h

-- 
2.38.1


             reply	other threads:[~2022-12-06  0:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-06  0:34 Jim Cromie [this message]
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 01/17] test-dyndbg: fixup CLASSMAP usage error Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 02/17] test-dyndbg: show that DEBUG enables prdbgs at compiletime Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 03/17] dyndbg: fix readback value on LEVEL_NAMES interfaces Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 04/17] dyndbg: replace classmap list with a vector Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 05/17] dyndbg: make ddebug_apply_class_bitmap more selective Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 06/17] dyndbg: dynamic_debug_init - use pointer inequality, not strcmp Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 07/17] dyndbg: drop NUM_TYPE_ARRAY Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 08/17] dyndbg: reduce verbose/debug clutter Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 09/17] dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP with DYNDBG_CLASSMAP(_DEFINE|_USE) Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 10/17] dyndbg-API: specialize DYNDBG_CLASSMAP_(DEFINE|USE) Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 11/17] dyndbg-API: DYNDBG_CLASSMAP_USE drop extra args Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 12/17] dyndbg-API: DYNDBG_CLASSMAP_DEFINE() improvements Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 13/17] drm_print: fix stale macro-name in comment Jim Cromie
2023-01-11 23:02   ` Daniel Vetter
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 14/17] dyndbg: unwrap __ddebug_add_module inner function NOTYET Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 15/17] dyndbg: ddebug_sanity() Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 16/17] dyndbg: mess-w-dep-class Jim Cromie
2022-12-06  0:34 ` [Intel-gfx] [RFC PATCH 17/17] dyndbg: miss-on HACK Jim Cromie
2022-12-06  2:16 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for DRM_USE_DYNAMIC_DEBUG regression Patchwork
2022-12-06  2:17 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-12-06  2:43 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-12-06  5:29 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2023-01-11 23:09 ` [Intel-gfx] [RFC PATCH 00/17] " Daniel Vetter
2023-01-13 18:29   ` jim.cromie
2023-01-13 18:48     ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221206003424.592078-1-jim.cromie@gmail.com \
    --to=jim.cromie@gmail.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-gvt-dev@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=jbaron@akamai.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=seanpaul@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox