Message-ID: <CALwA+NYQsKAtrME-dgZcBH=+KKU0duHAjxu-X3PhckJmbwz60A@mail.gmail.com>
Date: Wed, 17 Jul 2024 12:38:53 +0200
From: Łukasz Bartosik <ukaszb@...omium.org>
To: jim.cromie@...il.com
Cc: Luis Chamberlain <mcgrof@...nel.org>, daniel.vetter@...ll.ch, 
	tvrtko.ursulin@...ux.intel.com, jani.nikula@...el.com, 
	ville.syrjala@...ux.intel.com, jbaron@...mai.com, gregkh@...uxfoundation.org, 
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	amd-gfx@...ts.freedesktop.org, intel-gvt-dev@...ts.freedesktop.org, 
	intel-gfx@...ts.freedesktop.org, linux@...musvillemoes.dk, joe@...ches.com
Subject: Re: [PATCH v9 00/53] fix CONFIG_DRM_USE_DYNAMIC_DEBUG=y

On Mon, Jul 15, 2024 at 8:00 PM <jim.cromie@...il.com> wrote:
>
> On Mon, Jul 15, 2024 at 4:05 AM Łukasz Bartosik <ukaszb@...omium.org> wrote:
> >
> > On Sat, Jul 13, 2024 at 11:45 PM <jim.cromie@...il.com> wrote:
> > >
> > > On Fri, Jul 12, 2024 at 9:44 AM Łukasz Bartosik <ukaszb@...omium.org> wrote:
> > > >
> > > > On Wed, Jul 3, 2024 at 12:14 AM <jim.cromie@...il.com> wrote:
> > > > >
> > > > > On Tue, Jul 2, 2024 at 4:01 PM Luis Chamberlain <mcgrof@...nel.org> wrote:
> > > > > >
> > > > > > On Tue, Jul 02, 2024 at 03:56:50PM -0600, Jim Cromie wrote:
> > > > > > > This fixes dynamic-debug support for DRM.debug, added via classmaps.
> > > > > > > commit bb2ff6c27bc9 (drm: Disable dynamic debug as broken)
> > > > > > >
> > > > > > > CONFIG_DRM_USE_DYNAMIC_DEBUG=y was marked broken because drm.debug=val
> > > > > > > was applied when drm.ko was modprobed; too early for the yet-to-load
> > > > > > > drivers, which thus missed the enablement.  My testing with
> > > > > > > /etc/modprobe.d/ entries and modprobes with dyndbg=$querycmd options
> > > > > > > obscured this omission.
> > > > > > >
> > > > > > > The fix is to replace invocations of DECLARE_DYNDBG_CLASSMAP with
> > > > > > > DYNDBG_CLASSMAP_DEFINE for core, and DYNDBG_CLASSMAP_USE for drivers.
> > > > > > > The distinction allows dyndbg to also handle the users properly.
> > > > > > >
> > > > > > > DRM is the only current classmaps user, and is not really using it,
> > > > > > > so if you think DRM could benefit from zero-off-cost debugs based on
> > > > > > > static-keys, please test.
> > > > > > >
> > > > > > > HISTORY
> > > > > > >
> > > > > > > 9/4/22  - ee879be38bc8..ace7c4bbb240 commited - classmaps-v1 dyndbg parts
> > > > > > > 9/11/22 - 0406faf25fb1..16deeb8e18ca commited - classmaps-v1 drm parts
> > > > > > >
> > > > > > > https://lore.kernel.org/lkml/Y3XUrOGAV4I7bB3M@kroah.com/
> > > > > > > greg k-h says:
> > > > > > > This should go through the drm tree now.  The rest probably should also
> > > > > > > go that way and not through my tree as well.
> > > > > >
> > > > > > Can't this just be defined as a coccinelle smpl patch? Must easier
> > > > > > to read than 53 patches?
> > > > > >
> > > > >
> > > > > perhaps it could - Im not sure that would be easier to review
> > > > > than a file-scoped struct declaration or reference per driver
> > > > >
> > > > > Also, I did it hoping to solicit more Tested-by:s with drm.debug=0x1ff
> > > > >
> > > > > Jim
> > > > >
> > > >
> > > > Jim,
> > > >
> > > > When testing different combinations of Y/M for TEST_DYNAMIC_DEBUG and
> > > > TEST_DYNAMIC_DEBUG_SUBMOD in virtme-ng I spotted test failures:
> > > >
> > > > When the TEST_DYNAMIC_DEBUG=M and TEST_DYNAMIC_DEBUG_SUBMOD=M -
> > > > BASIC_TESTS, COMMA_TERMINATOR_TESTS, TEST_PERCENT_SPLITTING,
> > > > TEST_MOD_SUBMOD selftests passed
> > > > When the TEST_DYNAMIC_DEBUG=Y and TEST_DYNAMIC_DEBUG_SUBMOD=M -
> > > > BASIC_TESTS, COMMA_TERMINATOR_TESTS selftests passed, however
> > > > TEST_PERCENT_SPLITTING selftest fails with ": ./dyndbg_selftest.sh:270
> > > > check failed expected 1 on =pf, got 0"
> > > > When the TEST_DYNAMIC_DEBUG=Y and TEST_DYNAMIC_DEBUG_SUBMOD=Y -
> > > > BASIC_TESTS, COMMA_TERMINATOR_TESTS selftests passed, however
> > > > TEST_PERCENT_SPLITTING selftest fails also with ":
> > > > ./dyndbg_selftest.sh:270 check failed expected 1 on =pf, got 0"
> > > >
> > > > Have I missed something ?
> > > >
> > >
> > > I am not seeing those 2 failures on those 2 configs.
> > >
> > > most of my recent testing has been on x86-defconfig + minimals,
> > > built and run using/inside virtme-ng
> > >
> > > the last kernel I installed on this hw was june 16, I will repeat that,
> > > and report soon if I see the failure outside the vm
> > >
> > > I'll also send you my script, to maybe speed isolation of the differences.
> > >
> >
> > Jim,
> >
> > I know why I saw these failures.
> > I ran dyndbg_selftest.sh directly in thw directory
> > tools/testing/selftests/dynamic_debug/.
>
> thats odd.
> I mostly run it from src-root,
> also whereever make selftest target is/works (I forgot)
>
> I went into that subdir and ran it there
> I got no test differences / failures.
>

Jim,

The dyndbg_selftest.sh checks the location of kernel .config if it is
configured and
if not it sets it to the current dir.

[ -f "$KCONFIG_CONFIG" ] || KCONFIG_CONFIG=".config"
if [ -f "$KCONFIG_CONFIG" ]; then

If it does not find the .config it will set the variables to:

    LACK_DD_BUILTIN=0
    LACK_TMOD=0
    LACK_TMOD_SUBMOD=0

and run all selftests no matter what the values (Y/M) of
TEST_DYNAMIC_DEBUG and TEST_DYNAMIC_DEBUG_SUBMOD are.

> IIRC, the failure was on line 270, just after a modprobe.
> can you further isolate it ?
>
> > All works as expected when I run it from the top kernel directory.
> > Here are the results:
> >
> > When the TEST_DYNAMIC_DEBUG=M and TEST_DYNAMIC_DEBUG_SUBMOD=M -
> > BASIC_TESTS, COMMA_TERMINATOR_TESTS, TEST_PERCENT_SPLITTING,
> > TEST_MOD_SUBMOD selftests passed
> >
> > When the TEST_DYNAMIC_DEBUG=Y and TEST_DYNAMIC_DEBUG_SUBMOD=M -
> > BASIC_TESTS and COMMA_TERMINATOR_TESTS selftests passed,
> > TEST_PERCENT_SPLITTING and TEST_PERCENT_SPLITTING selftests were
> > skipped
> >
> > When the TEST_DYNAMIC_DEBUG=Y and TEST_DYNAMIC_DEBUG_SUBMOD=Y -
> > BASIC_TESTS and COMMA_TERMINATOR_TESTS selftests passed,
> > TEST_PERCENT_SPLITTING and TEST_PERCENT_SPLITTING selftests were
> > skipped
>
>
> thank you for running these config-combo tests.
>
> are you doing these in a VM ?
> and since Im asking, Ive done these combos on virtme-ng builds,
> also installed & running on 2 x86 boxen.
>

Sorry I forgot to mention that I tested it using virtme-ng.

> could you add DRM=m and a driver too,
> and boot with drm.debug=0x1ff, dynamic_debug.verbose=3
> the debug output should show all the class-work on modprobe,
> for your easy inspection/grading ;-)
>

I will retest with your patchset v9.

Thanks,
Lukasz

> >
> > Based on that maybe it would be worth it for the script to fail when
> > it doesn't find a .config with an error message something like this:
>
> if no config - they get to see more errors now.
> if the solution isnt obvious to them, we can find out more ?
>
> > "Kernel .config not found. Are you running the script from the
> > kernel's top directory?"
> >
> > What do you think ?
>
> the -rc0 window is open, Id rather not fiddle with this now.
>
> Im gonna rebase onto 6.10, resend,
> copy and rebase onto drm-mumble-next
> then try to get into the DRM-CI river, see where that takes me.
>
> thanks Lukas,
> Jim
>
> >
> > Thanks,
> > Lukasz
> >
> > > > Thanks,
> > > > Lukasz
> > > >
> > > > > >   Luis
> > > > > >

Powered by blists - more mailing lists