All of lore.kernel.org
 help / color / mirror / Atom feed
From: jim.cromie@gmail.com
To: LKML <linux-kernel@vger.kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	maennich@google.com
Cc: Greg KH <gregkh@linuxfoundation.org>, Jessica Yu <jeyu@kernel.org>
Subject: use / misuse of EXPORT_SYMBOL ?
Date: Fri, 23 Dec 2022 18:31:53 -0700	[thread overview]
Message-ID: <CAJfuBxzzg=w-q5grXhXKU2bkROd0LoXiamj2r0ket5oPHKskKQ@mail.gmail.com> (raw)

hi folks,

so Im trying to fix a regression in
CONFIG_DRM_USE_DYNAMIC_DEBUG=y
drm.debugs in drivers and helpers arent enabled
cuz theyre not modprobed until after drm,
when its debug=0x1f is applied.
Previously, __drm_debug settings were seen by drm_dbg when called,
but now with dyndbg underneath, that runtime test is replaced
by a static-key, which missed the debug callback and enablement

I want to export a symbol / struct _var
from inside a macro, the _var is placed into its own
linker section, but this appears to make it un-linkable
(at least not the way Im expecting)

WIP patchset at
https://github.com/jimc/linux/tree/hf-5f

Heres the final change, and the ensuing compile errs,
which are more helpful than the previous: silence and misbehavior.

The change isnt correct, but I have a hunch that
the errs reveal something about the underlying problem.

[jimc@frodo f2]$ git diff
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 02a66fd047b2..1d88ebf00bb1 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -111,7 +111,7 @@ struct ddebug_class_map {
                .length = ARRAY_SIZE(_var##_classnames),                \
                .class_names = _var##_classnames,                       \
        };                                                              \
-       EXPORT_SYMBOL(_var)
+       __EXPORT_SYMBOL(_var, "__dyndbg_classes", "")

 struct ddebug_class_user {
        const char *user_mod_name;
[jimc@frodo f2]$ make
....
  LD      vmlinux.o
  OBJCOPY modules.builtin.modinfo
  GEN     modules.builtin
  MODPOST Module.symvers
ERROR: modpost: Could not update namespace() for symbol map_disjoint_bits
ERROR: modpost: Could not update namespace() for symbol map_disjoint_names
ERROR: modpost: Could not update namespace() for symbol map_level_num
ERROR: modpost: Could not update namespace() for symbol map_level_names
ERROR: modpost: Could not update namespace() for symbol drm_debug_classes
ERROR: modpost: "map_level_names" [lib/test_dynamic_debug_submod.ko] undefined!
ERROR: modpost: "map_level_num" [lib/test_dynamic_debug_submod.ko] undefined!
ERROR: modpost: "map_disjoint_names"
[lib/test_dynamic_debug_submod.ko] undefined!
ERROR: modpost: "map_disjoint_bits" [lib/test_dynamic_debug_submod.ko]
undefined!
ERROR: modpost: "drm_debug_classes"
[drivers/gpu/drm/display/drm_display_helper.ko] undefined!
ERROR: modpost: "drm_debug_classes"
[drivers/gpu/drm/drm_kms_helper.ko] undefined!
ERROR: modpost: "drm_debug_classes"
[drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: modpost: "drm_debug_classes" [drivers/gpu/drm/i915/i915.ko] undefined!
ERROR: modpost: "drm_debug_classes"
[drivers/gpu/drm/nouveau/nouveau.ko] undefined!
make[1]: *** [/home/jimc/projects/lx/wk-test/scripts/Makefile.modpost:126:
Module.symvers] Error 1
make: *** [/home/jimc/projects/lx/wk-test/Makefile:1944: modpost] Error 2
[jimc@frodo f2]$


Heres the relevant macros, including above change.

/**
 * DYNDBG_CLASSMAP_DEFINE - define the debug classes used in this module.
 * This tells dyndbg what debug classes it should control for the client.
 *
 * @_var:    struct ddebug_class_map, as passed to module_param_cb
 * @_type:   enum ddebug_class_map_type, chooses bits/verbose, numeric/symbolic
 * @classes: enum class values used in module, such as: DRM_UT_*
 */
#define DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, ...)              \
        const char *_var##_classnames[] = {                             \
                iMAP_LIST(__stringify, _base, __VA_ARGS__) };           \
        struct ddebug_class_map __aligned(8) __used                     \
                __section("__dyndbg_classes") _var = {                  \
                .mod = THIS_MODULE,                                     \
                .mod_name = KBUILD_MODNAME,                             \
                .base = _base,                                          \
                .map_type = _maptype,                                   \
                .length = ARRAY_SIZE(_var##_classnames),                \
                .class_names = _var##_classnames,                       \
        };                                                              \
        __EXPORT_SYMBOL(_var, "__dyndbg_classes", "")

struct ddebug_class_user {
        const char *user_mod_name;
        const struct ddebug_class_map *map;
};
/**
 * DYNDBG_CLASSMAP_USE - refer to a classmap, DEFINEd elsewhere.
 * @_var: name of the exported classmap
 *
 * This registers the module's use of another module's classmap defn,
 * allowing dyndbg to find the controlling kparam, and propagate its
 * settings to the dependent module being loaded.
 */
#define DYNDBG_CLASSMAP_USE(_var)                                       \
        DYNDBG_CLASSMAP_USE_(_var, __UNIQUE_ID(ddebug_class_user))
#define DYNDBG_CLASSMAP_USE_(_var, _uname)                              \
        extern struct ddebug_class_map _var;                            \
        struct ddebug_class_user __used                                 \
        __section("__dyndbg_class_refs") _uname = {                     \
                .user_mod_name = KBUILD_MODNAME,                        \
                .map = &_var,                                           \
        }

Translating that for the skimmers -
_DEFINE creates a record in __dyndbg_classes section, exports it.
_USE creates a user-record, in __dyndbg_class_refs section,
which refs the 1st record.

plain old EXPORT_SYMBOL doesnt know the __dyndbg_classes section,
so users dont get it either, they somehow get new records,
which are "wrong" (have pointer to disallowed addr)full
The "ddebug_sanity" patch - 21/22 - isolates the problem linkage
to very early in dynamic_debug_init()


delving into the error itself, its from modpost.c:
static void sym_update_namespace(const char *symname, const char *namespace)

which is called from:
static void read_symbols(const char *modname)
...
        for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
                symname = remove_dot(info.strtab + sym->st_name);

                /* Apply symbol namespaces from __kstrtabns_<symbol> entries. */
                if (strstarts(symname, "__kstrtabns_"))
                        sym_update_namespace(symname + strlen("__kstrtabns_"),
                                             sym_get_data(&info, sym));
        }

consulting git blame leads me to:

commit 69923208431e097ce3830647aee98e5bd3e889c8
Author: Matthias Maennich <maennich@google.com>
Date:   Fri Oct 18 10:31:42 2019 +0100

    symbol namespaces: revert to previous __ksymtab name scheme

    The introduction of Symbol Namespaces changed the naming schema of the
    __ksymtab entries from __kysmtab__symbol to __ksymtab_NAMESPACE.symbol.

    That caused some breakages in tools that depend on the name layout in
    either the binaries(vmlinux,*.ko) or in System.map. E.g. kmod's depmod
    would not be able to read System.map without a patch to support symbol
    namespaces. A warning reported by depmod for namespaced symbols would
    look like

      depmod: WARNING: [...]/uas.ko needs unknown symbol usb_stor_adjust_quirks

    In order to address this issue, revert to the original naming scheme and
    rather read the __kstrtabns_<symbol> entries and their corresponding
    values from __ksymtab_strings to update the namespace values for
    symbols. After having read all symbols and handled them in
    handle_modversions(), the symbols are created. In a second pass, read
    the __kstrtabns_ entries and update the namespaces accordingly.

    Fixes: 8651ec01daed ("module: add support for symbol namespaces.")
    Reported-by: Stefan Wahren <stefan.wahren@i2se.com>
    Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
    Acked-by: Will Deacon <will@kernel.org>
    Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>
    Signed-off-by: Matthias Maennich <maennich@google.com>
    Signed-off-by: Jessica Yu <jeyu@kernel.org>


Im well out of my depth here, calling the experts

I think what I want is EXPORT_SYMBOL_FROM_SECTION ??
am I just doing it wrong ?

                 reply	other threads:[~2022-12-24  1:38 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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='CAJfuBxzzg=w-q5grXhXKU2bkROd0LoXiamj2r0ket5oPHKskKQ@mail.gmail.com' \
    --to=jim.cromie@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jeyu@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maennich@google.com \
    --cc=masahiroy@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.