Re: [PATCH] dyndbg: add special aux_print framework
Hi Jim,
I love your patch! Yet something to improve:
[auto build test ERROR on tegra-drm/drm/tegra/for-next]
[also build test ERROR on linux/master linus/master v5.14-rc3 next-20210730]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Jim-Cromie/dyndbg-add-special-aux_print-framework/20210801-113749
base: git://anongit.freedesktop.org/tegra/linux.git drm/tegra/for-next
config: i386-randconfig-r024-20210801 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce (this is a W=1 build):
#
https://github.com/0day-ci/linux/commit/1bf22862d95adc83487ade34fe1c42707f94ac4d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Jim-Cromie/dyndbg-add-special-aux_print-framework/20210801-113749
git checkout 1bf22862d95adc83487ade34fe1c42707f94ac4d
# save the attached .config to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
lib/dynamic_debug.c: In function '__dynamic_pr_debug':
>> lib/dynamic_debug.c:651:17: error: 'struct _ddebug' has no member named
>> 'aux_channel'
651 | if (descriptor->aux_channel) {
| ^~
lib/dynamic_debug.c:653:16: error: 'struct _ddebug' has no member named
'aux_channel'
653 |(*descriptor->aux_channel)
|^~
vim +651 lib/dynamic_debug.c
626
627 void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt,
...)
628 {
629 va_list args;
630 struct va_format vaf;
631 char buf[PREFIX_SIZE] = "";
632
633 BUG_ON(!descriptor);
634 BUG_ON(!fmt);
635
636 va_start(args, fmt);
637
638 vaf.fmt = fmt;
639 vaf.va =
640
641 printk(KERN_DEBUG "%s%pV", dynamic_emit_prefix(descriptor,
buf), );
642
643 if (descriptor->flags & _DPRINTK_FLAGS_PRINT_AUX) {
644 /* our model:
645 drm_trace_printf("%s%s[" DRM_NAME ":%ps] %pV",
646 dev ? dev_name(dev) : "", dev ? " " :
"",
647 __builtin_return_address(0), );
648 */
649 pr_info("reached check aux\n");
650
> 651 if (descriptor->aux_channel) {
652 pr_info("calling aux\n");
653 (*descriptor->aux_channel)
654 ("%s[DRM_mumble :%ps] %pV", buf,
655 __builtin_return_address(0), );
656 }
657 }
658 va_end(args);
659 }
660 EXPORT_SYMBOL(__dynamic_pr_debug);
661
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
[PATCH] dyndbg: add special aux_print framework
Sean Paul seanp...@chromium.org proposed, in
https://patchwork.freedesktop.org/series/78133/
drm/trace: Mirror DRM debug logs to tracefs
The problem with the approach is that its built upon splitting
drm_debug_enabled() into syslog & trace flavors, which clashes rather
profoundly with the strategy of obsoleting it using dyndbg.
Instead, this puts the print-to-trace decision after the is-it-enabled
test (which is a noop), so it has near zero additional cost.
This is preliminary, Proof-of-Concept, and about 2 hrs old.
But its surprisingly simple:
- add a new struct _ddebug member: (*aux_print)(char *format, ...)
- add a new S/special flag to check !!aux_print
- if S, invoke the function to handle the prepared vaf
It intrinsically allows any number of alternate printf-ish consumers,
but only 1 active per callsite. I have another patchset that
eliminates some of the data redundancies like this, it can be
extended.
It may also prove to be a generic way to implement the netdev & ibdev
variants of __dynamic_pr_debug.
It just needs a mechanism to set the per-callsite pointer to a
printf-ish function to consume the pr_debug output, a tighter/better
function prototype, and a wrapper on drm_trace_printf to bundle up the
args and comport with the prototype, which can evolve to suit this 1st
client.
it is on top of:
https://patchwork.freedesktop.org/series/92544/
(v4 on lkml, v2 in patchwork)
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 7 ++-
lib/dynamic_debug.c | 22 +++---
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 677ad176b167..0d068e8ed7aa 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -22,6 +22,7 @@ struct _ddebug {
const char *function;
const char *filename;
const char *format;
+ int (*aux_print)(char *, void *, void *);
unsigned int lineno:18;
/*
* The flags field controls the behaviour at the callsite.
@@ -29,7 +30,11 @@ struct _ddebug {
* writes commands to /dynamic_debug/control
*/
#define _DPRINTK_FLAGS_NONE0
-#define _DPRINTK_FLAGS_PRINT (1<<0) /* printk() a message using the format */
+#define _DPRINTK_FLAGS_PRINT (1<<0) /* printk() a message */
+#define _DPRINTK_FLAGS_PRINT_AUX (1<<5) /* call (*aux_print) */
+
+#define _DPRINTK_ENABLED (_DPRINTK_FLAGS_PRINT | _DPRINTK_FLAGS_PRINT_AUX)
+
#define _DPRINTK_FLAGS_INCL_MODNAME(1<<1)
#define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
#define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 045e1cf92c44..7bbdedabe6f1 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -85,6 +85,7 @@ static inline const char *trim_prefix(const char *path)
static struct { unsigned flag:8; char opt_char; } opt_array[] = {
{ _DPRINTK_FLAGS_PRINT, 'p' },
+ { _DPRINTK_FLAGS_PRINT_AUX, 'S' },
{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
@@ -206,10 +207,10 @@ static int ddebug_change(const struct ddebug_query *query,
if (newflags == dp->flags)
continue;
#ifdef CONFIG_JUMP_LABEL
- if (dp->flags & _DPRINTK_FLAGS_PRINT) {
- if (!(modifiers->flags & _DPRINTK_FLAGS_PRINT))
+ if (dp->flags & _DPRINTK_ENABLED) {
+ if (!(modifiers->flags & _DPRINTK_ENABLED))
static_branch_disable(>key.dd_key_true);
- } else if (modifiers->flags & _DPRINTK_FLAGS_PRINT)
+ } else if (modifiers->flags & _DPRINTK_ENABLED)
static_branch_enable(>key.dd_key_true);
#endif
dp->flags = newflags;
@@ -639,6 +640,21 @@ void __dynamic_pr_debug(struct _ddebug *descriptor, const
char *fmt, ...)
printk(KERN_DEBUG "%s%pV", dynamic_emit_prefix(descriptor, buf), );
+ if (descriptor->flags & _DPRINTK_FLAGS_PRINT_AUX) {
+ /* our model:
+ drm_trace_printf("%s%s[" DRM_NAME ":%ps] %pV",
+dev ? dev_name(dev) : "", dev ? " " : "",
+__builtin_return_address(0), );
+ */
+ pr_info("reached check aux\n");
+
+ if (descriptor->aux_channel) {
+ pr_info("calling aux\n");
+ (*descriptor->aux_channel)
+ ("%s[DRM_mumble :%ps] %pV", buf,
+__builtin_return_address(0), );
+ }
+ }
va_end(args);
}
EXPORT_SYMBOL(__dynamic_pr_debug);
--
2.31.1
[PATCH] dyndbg: add special aux_print framework
Sean Paul seanp...@chromium.org proposed, in
https://patchwork.freedesktop.org/series/78133/
drm/trace: Mirror DRM debug logs to tracefs
The problem with the approach is that its built upon splitting
drm_debug_enabled() into syslog & trace flavors, which clashes rather
profoundly with the strategy of obsoleting it using dyndbg.
Instead, this puts the print-to-trace decision after the is-it-enabled
test (which is a noop), so it has near zero additional cost.
This is preliminary, Proof-of-Concept, and about 2 hrs old.
But its surprisingly simple:
- add a new struct _ddebug member: (*aux_print)(char *format, ...)
- add a new S/special flag to check !!aux_print
- if S, invoke the function to handle the prepared vaf
It intrinsically allows any number of alternate printf-ish consumers,
but only 1 active per callsite. I have another patchset that
eliminates some of the data redundancies like this, it can be
extended.
It may also prove to be a generic way to implement the netdev & ibdev
variants of __dynamic_pr_debug.
It just needs a mechanism to set the per-callsite pointer to a
printf-ish function to consume the pr_debug output, a tighter/better
function prototype, and a wrapper on drm_trace_printf to bundle up the
args and comport with the prototype, which can evolve to suit this 1st
client.
it is on top of:
https://patchwork.freedesktop.org/series/92544/
(v4 on lkml, v2 in patchwork)
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 7 ++-
lib/dynamic_debug.c | 22 +++---
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 677ad176b167..0d068e8ed7aa 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -22,6 +22,7 @@ struct _ddebug {
const char *function;
const char *filename;
const char *format;
+ int (*aux_print)(char *, void *, void *);
unsigned int lineno:18;
/*
* The flags field controls the behaviour at the callsite.
@@ -29,7 +30,11 @@ struct _ddebug {
* writes commands to /dynamic_debug/control
*/
#define _DPRINTK_FLAGS_NONE0
-#define _DPRINTK_FLAGS_PRINT (1<<0) /* printk() a message using the format */
+#define _DPRINTK_FLAGS_PRINT (1<<0) /* printk() a message */
+#define _DPRINTK_FLAGS_PRINT_AUX (1<<5) /* call (*aux_print) */
+
+#define _DPRINTK_ENABLED (_DPRINTK_FLAGS_PRINT | _DPRINTK_FLAGS_PRINT_AUX)
+
#define _DPRINTK_FLAGS_INCL_MODNAME(1<<1)
#define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
#define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 045e1cf92c44..7bbdedabe6f1 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -85,6 +85,7 @@ static inline const char *trim_prefix(const char *path)
static struct { unsigned flag:8; char opt_char; } opt_array[] = {
{ _DPRINTK_FLAGS_PRINT, 'p' },
+ { _DPRINTK_FLAGS_PRINT_AUX, 'S' },
{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
@@ -206,10 +207,10 @@ static int ddebug_change(const struct ddebug_query *query,
if (newflags == dp->flags)
continue;
#ifdef CONFIG_JUMP_LABEL
- if (dp->flags & _DPRINTK_FLAGS_PRINT) {
- if (!(modifiers->flags & _DPRINTK_FLAGS_PRINT))
+ if (dp->flags & _DPRINTK_ENABLED) {
+ if (!(modifiers->flags & _DPRINTK_ENABLED))
static_branch_disable(>key.dd_key_true);
- } else if (modifiers->flags & _DPRINTK_FLAGS_PRINT)
+ } else if (modifiers->flags & _DPRINTK_ENABLED)
static_branch_enable(>key.dd_key_true);
#endif
dp->flags = newflags;
@@ -639,6 +640,21 @@ void __dynamic_pr_debug(struct _ddebug *descriptor, const
char *fmt, ...)
printk(KERN_DEBUG "%s%pV", dynamic_emit_prefix(descriptor, buf), );
+ if (descriptor->flags & _DPRINTK_FLAGS_PRINT_AUX) {
+ /* our model:
+ drm_trace_printf("%s%s[" DRM_NAME ":%ps] %pV",
+dev ? dev_name(dev) : "", dev ? " " : "",
+__builtin_return_address(0), );
+ */
+ pr_info("reached check aux\n");
+
+ if (descriptor->aux_channel) {
+ pr_info("calling aux\n");
+ (*descriptor->aux_channel)
+ ("%s[DRM_mumble :%ps] %pV", buf,
+__builtin_return_address(0), );
+ }
+ }
va_end(args);
}
EXPORT_SYMBOL(__dynamic_pr_debug);
--
2.31.1
[PATCH v4 7/7] amdgpu: define a dydbg-bitmap to control categorized pr_debugs
logger_types.h defines many DC_LOG_*() categorized debug wrappers.
Many of these use DRM_DEBUG_*, so are controllable using drm.debug,
but others use bare pr_debug()s, each with a different class-prefix
matching "^[\w+]:"
Use DYNDBG_BITMAP_DESC() to create a parameter/debug_dc, and to define
bits to control those pr_debugs by their category.
Signed-off-by: Jim Cromie
---
.../gpu/drm/amd/display/dc/core/dc_debug.c| 42 ++-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_debug.c
b/drivers/gpu/drm/amd/display/dc/core/dc_debug.c
index 21be2a684393..3041e0c3d726 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_debug.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_debug.c
@@ -36,8 +36,48 @@
#include "resource.h"
-#define DC_LOGGER_INIT(logger)
+/* define a drm.debug style dyndbg pr-debug control point */
+unsigned long __debug_dc;
+EXPORT_SYMBOL(__debug_dc);
+
+#define _help(key) "\t\t" key " : help for " key "\n"
+#define cmd_help(key) { .prefix = key, .help = "help for " key }
+
+/* Id like to do these inside DEFINE_DYNDBG_BITMAP, later */
+#define MY_DYNDBG_PARM_DESC(name) \
+ "Enable debug output via /sys/module/amdgpu/parameters/" #name \
+ ", where each bit enables a debug category.\n" \
+ _help("[SURFACE]:") \
+ _help("[CURSOR]:") \
+ _help("[PFLIP]:") \
+ _help("[VBLANK]:") \
+ _help("[HW_LINK_TRAINING]:")\
+ _help("[HW_AUDIO]:")\
+ _help("[SCALER]:") \
+ _help("[BIOS]:")\
+ _help("[BANDWIDTH_CALCS]:") \
+ _help("[DML]:") \
+ _help("[IF_TRACE]:")\
+ _help("[GAMMA]:") \
+ _help("[SMU_MSG]:")
+MODULE_PARM_DESC(debug_dc, MY_DYNDBG_PARM_DESC(name));
+
+DEFINE_DYNDBG_BITMAP(debug_dc, &__debug_dc,
+MY_DYNDBG_PARM_DESC(debug_dc),
+cmd_help("[CURSOR]:"),
+cmd_help("[PFLIP]:"),
+cmd_help("[VBLANK]:"),
+cmd_help("[HW_LINK_TRAINING]:"),
+cmd_help("[HW_AUDIO]:"),
+cmd_help("[SCALER]:"),
+cmd_help("[BIOS]:"),
+cmd_help("[BANDWIDTH_CALCS]:"),
+cmd_help("[DML]:"),
+cmd_help("[IF_TRACE]:"),
+cmd_help("[GAMMA]:"),
+cmd_help("[SMU_MSG]:"));
+#define DC_LOGGER_INIT(logger)
#define SURFACE_TRACE(...) do {\
if (dc->debug.surface_trace) \
--
2.31.1
[PATCH v4 6/7] drm/print: add choice to use dynamic debug in drm-debug
drm's debug system writes 10 distinct categories of messages to syslog
using a small API[1]: drm_dbg*(10 names), DRM_DEBUG*(8 names),
DRM_DEV_DEBUG*(3 names). There are thousands of these callsites, each
categorized by their authors. ~2100 are on my laptop.
These callsites are enabled at runtime by their category, each
controlled by a bit in drm.debug (/sys/modules/drm/parameter/debug)
In the current "basic" implementation, drm_debug_enabled() tests these
bits each time an API[1] call is executed; while cheap individually,
the costs accumulate.
This patch uses dynamic-debug with jump-label to patch enabled calls
onto their respective NOOP slots, avoiding all runtime bit-checks of
__drm_debug.
Dynamic debug has no concept of category, but we can emulate one by
replacing enum categories with a set of prefix-strings; "drm:core:",
"drm:kms:" "drm:driver:" etc, and prepend them (at compile time) to
the given formats.
Then we can use:
`echo module drm format "^drm:core: " +p > control`
to enable every pr_debug("drm:core: ...") with one query.
This conversion yields ~2100 new callsites on my i7/i915 laptop:
dyndbg: 195 debug prints in module drm_kms_helper
dyndbg: 298 debug prints in module drm
dyndbg: 1630 debug prints in module i915
CONFIG_DRM_USE_DYNAMIC_DEBUG enables this, and is available if
CONFIG_DYNAMIC_DEBUG or CONFIG_DYNAMIC_DEBUG_CORE is chosen, and if
CONFIG_JUMP_LABEL is enabled; this because its required to get the
promised optimizations.
The indirection/switchover is layered into the macro scheme:
0. A new callback on drm.debug (in a previous patch)
does: dynamic_debug_exec_queries("format ^drm:kms: +p", "drm*");
DYNDBG_BITMAP_DESC(__drm_debug, "bla blah bitmap yada",
{ "drm:core:", "enable CORE debug messages" },
{ "drm:kms:", "enable KMS debug messages" });
1. A "converted" or "classy" DRM_UT_* map
based on: DRM_UT_* ( symbol => bit-mask )
named it: DRM_DBG_CLASS_* ( symbol => format-class-prefix-string )
So DRM_DBG_CLASS_* is either:
basic: DRM_DBG_CLASS_* <-- DRM_UT_* identity map
enabled:
#define DRM_DBG_CLASS_KMS"drm:kms: "
#define DRM_DBG_CLASS_PRIME "drm:prime: "
#define DRM_DBG_CLASS_ATOMIC "drm:atomic: "
DRM_UT_* are unchanged for now, since theyre used in
drm_debug_enabled() and elsewhere.
2. drm_dev_dbg & drm_debug are renamed (prefixed with '__')
original names are now macros, calling either:
basic: -> to renamed fn
enabled: -> dev_dbg & pr_debug, with DRM_DBG_CLASS_* prefix # format.
this is where drm_debug_enabled() is avoided.
prefix is prepended at compile-time.
3. names in (2) are invoked by API[1]
all these macros now use DRM_DBG_CLASS_* to get right token type
for both !/!! DRM_USE_DYNAMIC_DEBUG cases
NOTES:
dyndbg does require that the prefix be in the compiled-in format
string; run-time prefixing would look like "format ^%s" in a query
(obviously not useful).
dyndbg is completely agnostic wrt the categorization scheme used; but
the query selectivity is completely dependent upon how the scheme fits
with a simple anchored literal substring match (no regex).
ad-hoc categories are implicitly allowed, author discipline and review
is expected.
- "1","2","3" are allowed, 1-9 is effective max. 2 doesnt imply 1.
- "1:","2:","3:" are better, avoiding [1-9]\d: etc
- "todo:", "rfc:" might be handy.
- "drm:*:" is used here.
- "drm:*:*:" is a natural extension.
- "drm:atomic:fail:" has been suggested.
A hierarchical category space is an obvious frontrunner, so it
deserves a little more exploration/scrutiny.
- "drm:kms: " & "drm:kms:" are different
(2nd, w/o trailing space, matches subcats)
- "drm:kms" is also different
(matches "drm:kmsmart", whatever that would be)
- "drm:kms" & "drm:kms*" are different
the latter does not work as you might reasonably expect.
wildcarding not added for format, which was already special
Literal prefixes (as encoded in DRM_DBG_CLASS_* symbols) should
include the trailing space, both to terminate the class prefix, and to
read naturally in logs with plain #catenation.
Prefixes given in args to DYNDBG_BITMAP_DESC() determine the bit-query
map; so to insure the map is stable, new categories or 3rd level
categories must be added to the end. The prefixes may have trailing
spaces, depending upon desired search results.
Since bits are applied 0-N, the later categories can countermand the
earlier ones. IOW, "drm:atomic:fail:" would override "drm:atomic:",
but "drm:atomic: " would never touch "drm:atomic:fail:" callsites.
There are plenty of bikeshed rabbit holes available.
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/Kconfig | 13 +
drivers/gpu/drm/drm_print.c | 15 --
include/drm/drm_print.h | 94 +++--
3 files changed, 93 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index
[PATCH v4 5/7] i915/gvt: control pr_debug("gvt:")s with bits in parameters/debug_gvt
The gvt component of this driver has ~120 pr_debugs, in 9 "classes".
Following the interface model of drm.debug, add a parameter to map
bits to these classes.
If CONFIG_DRM_USE_DYNAMIC_DEBUG=y (and CONFIG_DYNAMIC_DEBUG_CORE), add
-DDYNAMIC_DEBUG_MODULE into Makefile. TBD: maybe add a separate
CONFIG_I915_USE_DYNAMIC_DEBUG to more fully optionalize this.
this is close to our target:
DYNDBG_BITMAP_DESC(__gvt_debug, "dyndbg bitmap desc",
{ "gvt: cmd: ", "command processing" },
{ "gvt: core: ", "core help" },
{ "gvt: dpy: ", "display help" },
{ "gvt: el: ", "help" },
{ "gvt: irq: ", "help" },
{ "gvt: mm: ", "help" },
{ "gvt: mmio: ", "help" },
{ "gvt: render: ", "help" },
{ "gvt: sched: " "help" });
actual patch has a few details different, helper macros mainly.
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/i915/gvt/Makefile | 4
drivers/gpu/drm/i915/i915_params.c | 34 ++
include/drm/drm_print.h| 3 ++-
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gvt/Makefile
b/drivers/gpu/drm/i915/gvt/Makefile
index ea8324abc784..846ba73b8de6 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -7,3 +7,7 @@ GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o
trace_points.o firmware.o \
ccflags-y += -I $(srctree)/$(src) -I
$(srctree)/$(src)/$(GVT_DIR)/
i915-y += $(addprefix $(GVT_DIR)/,
$(GVT_SOURCE))
+
+#ifdef CONFIG_DRM_USE_DYNAMIC_DEBUG
+ccflags-y += -DDYNAMIC_DEBUG_MODULE
+#endif
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index e07f4cfea63a..c951ef76454f 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -265,3 +265,37 @@ void i915_params_free(struct i915_params *params)
I915_PARAMS_FOR_EACH(FREE);
#undef FREE
}
+
+/* POC for callback -> dynamic_debug_exec_queries */
+unsigned long __gvt_debug;
+EXPORT_SYMBOL(__gvt_debug);
+
+#define _help(key) "\t\"" key "\"\t: help for " key "\n"
+#define cmd_help(key) { .prefix = key, .help = key ": help for " key }
+
+#define I915_DYNDBG_PARM_DESC(name) \
+ "Enable debug output via /sys/module/i915/parameters/" #name\
+ ", where each bit enables a debug category.\n" \
+ _help("gvt:cmd:") \
+ _help("gvt:core:") \
+ _help("gvt:dpy:") \
+ _help("gvt:el:")\
+ _help("gvt:irq:") \
+ _help("gvt:mm:")\
+ _help("gvt:mmio:") \
+ _help("gvt:render:")\
+ _help("gvt:sched:")
+
+MODULE_PARM_DESC(debug_gvt, I915_DYNDBG_PARM_DESC(debug_gvt));
+
+DEFINE_DYNDBG_BITMAP(debug_gvt, &__gvt_debug,
+ I915_DYNDBG_PARM_DESC(debug_gvt),
+ cmd_help("gvt:cmd:"),
+ cmd_help("gvt:core:"),
+ cmd_help("gvt:dpy:"),
+ cmd_help("gvt:el:"),
+ cmd_help("gvt:irq:"),
+ cmd_help("gvt:mm:"),
+ cmd_help("gvt:mmio:"),
+ cmd_help("gvt:render:"),
+ cmd_help("gvt:sched:"));
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 15a089a87c22..47803c64b144 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -530,7 +530,8 @@ void __drm_err(const char *format, ...);
const struct drm_device *drm_ = (drm);
\
\
if (drm_debug_enabled(DRM_UT_ ## category) && __ratelimit(_))
\
- drm_dev_printk(drm_ ? drm_->dev : NULL, KERN_DEBUG, fmt, ##
__VA_ARGS__); \
+ drm_dev_dbg((drm_) ? (drm_)->dev : NULL,
\
+ DRM_DBG_CLASS_ ## category, fmt, ##__VA_ARGS__);
\
})
#define drm_dbg_kms_ratelimited(drm, fmt, ...) \
--
2.31.1
[PATCH v4 4/7] i915/gvt: remove spaces in pr_debug "gvt: core:" etc prefixes
Collapsing "gvt: core: " to "gvt:core: " is a better class-prefix;
dropping the internal spaces means a trailing space in a query will
more clearly terminate the prefix.
Consider a generic drm-debug example:
# turn off most ATOMIC reports
echo format "^drm:atomic: " -p > control
# turn off all ATOMIC:* reports
echo format "^drm:atomic:" -p > control
# turn on ATOMIC:FAIL reports
echo format "^drm:atomic:fail: " +p > control
Removing embedded spaces in the class-prefixes simplifies the
corresponding match-prefix. This means that "quoted" match-prefixes
are only needed when the trailing space is desired, in order to
exclude explicitly sub-categorized pr-debugs; in this example,
"drm:atomic:fail:".
RFC: maybe the prefix catenation should paste in the " " class-prefix
terminator explicitly. A pr_debug_() flavor could exclude the " ",
allowing ad-hoc sub-categorization by appending for example, "fail:"
to "drm:atomic:".
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/i915/gvt/debug.h | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/debug.h b/drivers/gpu/drm/i915/gvt/debug.h
index c6027125c1ec..b4021f41c546 100644
--- a/drivers/gpu/drm/i915/gvt/debug.h
+++ b/drivers/gpu/drm/i915/gvt/debug.h
@@ -36,30 +36,30 @@ do {
\
} while (0)
#define gvt_dbg_core(fmt, args...) \
- pr_debug("gvt: core: "fmt, ##args)
+ pr_debug("gvt:core: "fmt, ##args)
#define gvt_dbg_irq(fmt, args...) \
- pr_debug("gvt: irq: "fmt, ##args)
+ pr_debug("gvt:irq: "fmt, ##args)
#define gvt_dbg_mm(fmt, args...) \
- pr_debug("gvt: mm: "fmt, ##args)
+ pr_debug("gvt:mm: "fmt, ##args)
#define gvt_dbg_mmio(fmt, args...) \
- pr_debug("gvt: mmio: "fmt, ##args)
+ pr_debug("gvt:mmio: "fmt, ##args)
#define gvt_dbg_dpy(fmt, args...) \
- pr_debug("gvt: dpy: "fmt, ##args)
+ pr_debug("gvt:dpy: "fmt, ##args)
#define gvt_dbg_el(fmt, args...) \
- pr_debug("gvt: el: "fmt, ##args)
+ pr_debug("gvt:el: "fmt, ##args)
#define gvt_dbg_sched(fmt, args...) \
- pr_debug("gvt: sched: "fmt, ##args)
+ pr_debug("gvt:sched: "fmt, ##args)
#define gvt_dbg_render(fmt, args...) \
- pr_debug("gvt: render: "fmt, ##args)
+ pr_debug("gvt:render: "fmt, ##args)
#define gvt_dbg_cmd(fmt, args...) \
- pr_debug("gvt: cmd: "fmt, ##args)
+ pr_debug("gvt:cmd: "fmt, ##args)
#endif
--
2.31.1
[PATCH v4 3/7] dyndbg: add dyndbg-bitmap definer and callbacks
Add DEFINE_DYNDBG_BITMAP(name, var, bitmap_desc, @bit_descs) to allow
users to define a /sys/module/*/parameter/name, and a mapping from
bits[0-N] to the debug-class-prefixes that the author wishes to
control.
DEFINE_DYNDBG_BITMAP(debug_gvt, __gvt_debug,
"dyndbg bitmap desc",
{ "gvt:cmd: ", "command processing" },
{ "gvt:core: ", "core help" },
{ "gvt:dpy: ", "display help" },
{ "gvt:el: ", "help" },
{ "gvt:irq: ", "help" },
{ "gvt:mm: ", "help" },
{ "gvt:mmio: ", "help" },
{ "gvt:render: ", "help" },
{ "gvt:sched: ", "help" });
dynamic_debug.c: add 3 new elements:
- int param_set_dyndbg() - not working yet, // __gvt_debug
- int param_get_dyndbg()
- struct param_ops_dyndbg
Following the model of kernel/params.c STANDARD_PARAM_DEFS, All 3 are
non-static and exported.
dynamic_debug.h:
extern the struct param_ops_dyndbg prototype. This appears to be
needed to reference the var, forex in i915_params.c
TBD: set_dyndbg() works to enable categories, but fails to disable
them. This is because the code relied on having an old copy of the
param (__gvt_debug) to detect +/- changes. Rewriting the loop is
probably easier than stashing the old state for change detection.
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 36 +++
lib/dynamic_debug.c | 55 +++
2 files changed, 91 insertions(+)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index dce631e678dd..677ad176b167 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -2,6 +2,8 @@
#ifndef _DYNAMIC_DEBUG_H
#define _DYNAMIC_DEBUG_H
+#include
+
#if defined(CONFIG_JUMP_LABEL)
#include
#endif
@@ -227,6 +229,40 @@ static inline int dynamic_debug_exec_queries(const char
*query, const char *modn
return 0;
}
+static int param_set_dyndbg(const char *instr, struct kernel_param *kp)
+{ return 0; }
+static int param_get_dyndbg(char *buffer, struct kernel_param *kp)
+{ return 0; }
+
#endif /* !CONFIG_DYNAMIC_DEBUG_CORE */
+struct dyndbg_bitdesc {
+ /* bitpos is inferred from index in containing array */
+ char *prefix;
+ char *help;
+};
+
+/**
+ * DYNDBG_BITMAP_DESC(name, var, bitmap_desc, @bit_descs)
+ * @name: basename under /sys
+ * @var: C identifier to map
+ * @bitmap_desc: string summarizing dyndbg categories
+ * @bit_descs: list of struct dydbg_bitdesc initializations
+ *
+ * Defines the mapping of bits 0-N to categories/prefixes of
+ * debug-callsites to be controlled.
+ *
+ * Users should also call MODULE_PARM_DESC(name, bitmap_desc).
+ * Maybe we can invoke it on their behalf, but we want MOD-NAME to be
+ * correct, test soon. may also need modname in name - "debug" will
+ * not be unique.
+ */
+#define DEFINE_DYNDBG_BITMAP(name, value, bitmap_desc, ...)\
+ struct dyndbg_bitdesc dyndbg_classes_##name[] = \
+ { __VA_ARGS__, { NULL, NULL } };\
+ module_param_cbd(name, _ops_dyndbg, value, 0644, \
+_classes_##name);
+
+extern const struct kernel_param_ops param_ops_dyndbg;
+
#endif
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index cb5abb42c16a..045e1cf92c44 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1154,3 +1154,58 @@ early_initcall(dynamic_debug_init);
/* Debugfs setup must be done later */
fs_initcall(dynamic_debug_init_control);
+
+#include
+
+#define OUR_QUERY_SIZE 128 /* typically need <40 */
+
+int param_set_dyndbg(const char *instr, const struct kernel_param *kp)
+{
+ unsigned int val;
+ unsigned long changes, result;
+ int rc, chgct = 0, totct = 0, bitpos, bitsmax;
+ char query[OUR_QUERY_SIZE];
+ struct dyndbg_bitdesc *bitmap = (struct dyndbg_bitdesc *) kp->data;
+
+ // pr_info("set_dyndbg: instr: %s curr: %d\n", instr, *kp->arg);
+
+ rc = kstrtouint(instr, 0, );
+ if (rc) {
+ pr_err("set_dyndbg: failed\n");
+ return -EINVAL;
+ }
+ result = val;
+ pr_info("set_dyndbg: result:0x%lx from %s\n", result, instr);
+
+ changes = result; // ^ __gvt_debug;
+
+ for (bitsmax = 0; bitmap[bitsmax].prefix; bitsmax++);
+
+ for_each_set_bit(bitpos, , min(--bitsmax, 64)) {
+
+ sprintf(query, "format '^%s' %cp", bitmap[bitpos].prefix,
+ test_bit(bitpos, ) ? '+' : '-');
+
+ chgct = dynamic_debug_exec_queries(query, "i915");
+
+ pr_info("bit-%d: %d changes on: %s\n", bitpos, chgct, query);
+ totct += chgct;
+ }
+ pr_info("total changes: %d\n", totct);
+ // __gvt_debug = result;
+ return 0;
+}
+EXPORT_SYMBOL(param_set_dyndbg);
+
+int param_get_dyndbg(char *buffer, const struct kernel_param *kp)
+{
+ return scnprintf(buffer, PAGE_SIZE, "%u\n",
+*((unsigned int
[PATCH v4 2/7] moduleparam: add data member to struct kernel_param
Add a void* data member to the struct, to allow attaching private data
that will be used soon by a setter method (via kp->data) to perform
more elaborate actions.
To attach the data at compile time, add new macros:
module_param_cbd() derives from module_param_cb(), adding data param.
It calls __module_param_call_wdata(), which has accepts new data
param and inits .data with it. Re-define __module_param_call() using it.
Use of this new data member will be rare, it might be worth redoing
this as a separate/sub-type to keep the base case.
Signed-off-by: Jim Cromie
---
include/linux/moduleparam.h | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index eed280fae433..e9495b1e794d 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -78,6 +78,7 @@ struct kernel_param {
const struct kparam_string *str;
const struct kparam_array *arr;
};
+ void *data;
};
extern const struct kernel_param __start___param[], __stop___param[];
@@ -175,6 +176,9 @@ struct kparam_array
#define module_param_cb(name, ops, arg, perm)\
__module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1, 0)
+#define module_param_cbd(name, ops, arg, perm, data)
\
+ __module_param_call_wdata(MODULE_PARAM_PREFIX, name, ops, arg, perm,
-1, 0, data)
+
#define module_param_cb_unsafe(name, ops, arg, perm) \
__module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1,\
KERNEL_PARAM_FL_UNSAFE)
@@ -284,14 +288,17 @@ struct kparam_array
/* This is the fundamental function for registering boot/module
parameters. */
-#define __module_param_call(prefix, name, ops, arg, perm, level, flags)
\
+#define __module_param_call(prefix, name, ops, arg, perm, level, flags) \
+ __module_param_call_wdata(prefix, name, ops, arg, perm, level, flags,
NULL)
+
+#define __module_param_call_wdata(prefix, name, ops, arg, perm, level, flags,
data) \
/* Default value instead of permissions? */ \
static const char __param_str_##name[] = prefix #name; \
static struct kernel_param __moduleparam_const __param_##name \
__used __section("__param") \
__aligned(__alignof__(struct kernel_param)) \
= { __param_str_##name, THIS_MODULE, ops, \
- VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg } }
+ VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg }, data }
/* Obsolete - use module_param_cb() */
#define module_param_call(name, _set, _get, arg, perm) \
--
2.31.1
[PATCH v4 1/7] drm/print: fixup spelling in a comment
s/prink/printk/ - no functional changes Signed-off-by: Jim Cromie --- include/drm/drm_print.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 9b66be54dd16..15a089a87c22 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -327,7 +327,7 @@ static inline bool drm_debug_enabled(enum drm_debug_category category) /* * struct device based logging * - * Prefer drm_device based logging over device or prink based logging. + * Prefer drm_device based logging over device or printk based logging. */ __printf(3, 4) -- 2.31.1
[PATCH v4 0/7] drm: use dyndbg in drm_print
hi all,
Apologies for broad --to, but it touches a wide range, if casually.
In order to avoid runtime costs of drm_debug_enabled(), this patchset
re-implements drm.debug to use dyndbg, after extending dyndbg with
kernel_param_ops to implement the bitmap->dydnbg-query and expose it
for use.
To define your pr_debugs categorization (that you want to control):
DEFINE_DYNDBG_BITMAP(debug_gvt, __gvt_debug,
"i915/gvt dyndbg bitmap desc",
/* bits 0-N are initialized with these exprs */
{ "gvt:cmd: ", "command processing" },
{ "gvt:core: ", "core help" },
{ "gvt:dpy: ", "display help" },
{ "gvt:el: ", "help" },
{ "gvt:irq: ", "help" },
{ "gvt:mm: ", "help" },
{ "gvt:mmio: ", "help" },
{ "gvt:render: ", "help" },
{ "gvt:sched: ", "help" });
Then, you can change whole categories of pr_debugs:
echo 0x5A > /sys/modules/i915/parameters/debug_gvt
BIT($i++) is implied by order, can it be autogend with macros and
stuffed into MODULE_PARAM_DESC automagically ?
v4:
- extend kernel_param with void* .data (2/7)
- move v3 kernel_param_ops code into dyndbg (3/7)
- add DEFINE_DYNDBG_BITMAP (3/7)
- use DEFINE_DYNDBG_BITMAP in drm-core, i915 (5/7), amdgpu (7/7)
- lots of changes per @DanVet feedback, thanks!
no doc yet, content more in commit-log refinements
v3: https://lore.kernel.org/lkml/20210714175138.319514-1-jim.cro...@gmail.com/
main element is now patch 6/7,
v2: https://lore.kernel.org/lkml/20210711055003.528167-1-jim.cro...@gmail.com/
v1: https://lore.kernel.org/lkml/20201204035318.332419-1-jim.cro...@gmail.com/
NOTES:
https://github.com/jimc/linux/tree/dd-drm-v4 should be seen and tested
by robots soon.
Using dyndbg to implement categories/classes of pr_debugs requires
that the class-prefix is compiled into the format string, and not
written at runtime with pr_debug("%s %pV", class_str, vaf).
That caveat is reflected in (6/7) where category is catenated with the
format, and ceases to exist as a separate arg for DRM_USE_DD=y builds.
ISTM this also means drm_debug_enabled() isnt going away just yet.
The categories arent atomic in any sense; callsites controlled via
drm.debug can still be tweaked arbitrarily with echo $query > control.
dyndbg's search is global by default; format ^$prefix cuts across
code modules, and anyone can add a pr_debug("drm:foobar: yada")
classification. Rules against this are policy, and belong in the
realm of author taste and code review, not in the tool.
Maybe modname's already available via kp, if we need it. Other query
terms are possible, if struct dyndbg_bitdesc is elaborated.
The dyndbg-queries built by the callback don't include modname. This
allows drm.dbg to control pr-debugs inside i915, despite being done
from a different module. This is ok because i915 used the drm_debug
api for those messages.
i915/gvt uses a different way to prepend its classes, but it shares
the classification space. Like a good namespace citizen, it has
"gvt:" as its top-level name, with ~9 2nd level names.
s...@chromium.org posited "drm:atomic:fail: " as a new category, to
leverage the additional selectivity of dyndbg.
Allowing for N-level namespaces, ' ' terminates the prefix, not ':'.
So "drm:atomic:" & "drm:atomic: " are different searches, 1st
including pr_debug("drm:atomic:fail: ")
DEFINE_DYNDBG_BITMAP() bits and masks are dependent on order of
bit-declarations. Since bits are applied 0-N, later bits could
override (ie sub-categorize) earlier ones. IOW, "drm:atomic:fail: "
just needs to be after "drm:atomic: ".
dyndbg is extremely agnostic about how you use format ^$prefix, your
classification scheme just needs to work within the limitation that
its a simple anchored literal substring match, no regex stuff.
In particular, commit 578b1e0701af ("dynamic_debug: add wildcard
support to filter files/functions/modules") left format out, because
it was already special (floating substr match, no need for '*').
Patch 6/7 has many more "advices" on classification / categorization.
and im restating things.
Much more to say/do instead of what follows, which is v3 prose.
I wanted to send, see how its received by robots and people.
drm_debug_enabled() is called a lot to do unlikely bit-tests to
selectively enable debug printing; this is a good job for
dynamic-debug, IFF it is built with JUMP_LABEL.
This patchset enables the use of dynamic-debug to avoid all those
drm_debug_enabled() overheads, if CONFIG_DRM_USE_DYNAMIC_DEBUG=y.
Doing so creates many new pr_debug callsites,
otherwise i915 has ~120 prdbgs, and drm has just 1;
bash-5.1# modprobe i915
dyndbg: 8 debug prints in module video
dyndbg: 305 debug prints in module drm
dyndbg: 207 debug prints in module drm_kms_helper
dyndbg: 2 debug prints in module ttm
dyndbg: 1720 debug prints in module i915
On amdgpu, enabling it adds ~3200 prdbgs, currently at 56 bytes each.
So CONFIG_DRM_USE_DYNAMIC_DEBUG=y
Re: [PATCH 00/14] drm: Make DRM's IRQ helpers legacy
Hi Thomas, On Tue, Jul 27, 2021 at 08:27:07PM +0200, Thomas Zimmermann wrote: > DRM's IRQ helpers are only helpful for old, non-KMS drivers. Move > the code behind CONFIG_DRM_LEGACY. Convert KMS drivers to Linux > IRQ interfaces. > > DRM provides IRQ helpers for setting up, receiving and removing IRQ > handlers. It's an abstraction over plain Linux functions. The code > is mid-layerish with several callbacks to hook into the rsp drivers. > Old UMS driver have their interrupts enabled via ioctl, so these > abstractions makes some sense. Modern KMS manage all their interrupts > internally. Using the DRM helpers adds indirection without benefits. > > Most KMs drivers already use Linux IRQ functions instead of DRM's > abstraction layer. Patches 1 to 12 convert the remaining ones. > The patches also resolve a bug for devices without assigned interrupt > number. DRM helpers don't test for IRQ_NOTCONNECTED, so drivers do > not detect if the device has no interrupt assigned. > > Patch 13 removes an unused function. > > Patch 14 moves the DRM IRQ helpers behind CONFIG_DRM_LEGACY. Only > the old non-KMS drivers still use the functionality. > > Thomas Zimmermann (14): > drm/amdgpu: Convert to Linux IRQ interfaces > drm/arm/hdlcd: Convert to Linux IRQ interfaces > drm/atmel-hlcdc: Convert to Linux IRQ interfaces > drm/fsl-dcu: Convert to Linux IRQ interfaces > drm/gma500: Convert to Linux IRQ interfaces > drm/kmb: Convert to Linux IRQ interfaces > drm/msm: Convert to Linux IRQ interfaces > drm/mxsfb: Convert to Linux IRQ interfaces > drm/radeon: Convert to Linux IRQ interfaces > drm/tidss: Convert to Linux IRQ interfaces > drm/tilcdc: Convert to Linux IRQ interfaces > drm/vc4: Convert to Linux IRQ interfaces > drm: Remove unused devm_drm_irq_install() > drm: IRQ midlayer is now legacy With the irq_enabled confusion out of the way I want to re-address two issues here that I know you have answered but I am just not convinced. 1) IRQ_NOTCONNECTED We do not have this check in drm_irq today and we should avoid spreading it all over. We are either carrying it forever or we wil lsee patches floating in to drop the check again. The current use in the kernel is minimal: https://elixir.bootlin.com/linux/latest/A/ident/IRQ_NOTCONNECTED So as a minimum drop it from atmel_hlcdc and preferably from the rest as it is really not used. (Speaking as atmel_hlcdc maintainer) 2) devm_request_irq() We are moving towards managed allocation so we do not fail to free resources. And an irq has a lifetime equal the device itself - so an obvious cnadidate for devm_request_irq. If we do not introduce it now we will see a revisit of this later. I can be convinced to wait with this as we will have to do much more in each driver, but I cannot see any good arguments to avoid the more modern way to use devm_request_irq. Sam
Re: Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
On 7/30/21 1:31 AM, Thomas Zimmermann wrote:
> Hi Dan and Sam
>
> Am 29.07.21 um 21:55 schrieb dan.sned...@microchip.com:
>> Hi Thomas and Sam,
>> On 7/29/21 12:48 PM, Sam Ravnborg wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>> know the content is safe
>>>
>>> Hi Thomas,
>>>
Are you sure, you're testing with the latest drm-misc-next or drm-tip?
Because using irq_enabled is deprecated and the flag was recently
replaced
by commit 1e4cd78ed493 ("drm: Don't test for IRQ support in VBLANK
ioctls").
>>
>> Ok, My fault for testing on the wrong branch. When I test this patch on
>> drm-misc-next it works great. Sorry for the confusion!
>>
>>>
>>> I was looking at drm-misc-fixes which did not have this commit :-(
>>> Just my silly excuse why I was convinced this was the issue.
>
> Don't worry.
>
> I'll add Sam's R-b and a Tested-by from Dan to the patch. Is that ok?
The tested-by works for me! Thanks!
>
> Best regards
> Thomas
>
>
>>>
>>> Sam
>>>
>>
>> Best regards,
>> Dan
>>
>
[PATCH] drm/amdgpu: fix possible null-pointer dereference in amdgpu_ttm_tt_populate()
The variable ttm is assigned to the variable gtt, and the variable gtt
is checked in:
if (gtt && gtt->userptr)
This indicates that both ttm and gtt can be NULL.
If so, a null-pointer dereference will occur:
if (ttm->page_flags & TTM_PAGE_FLAG_SG)
Also, some null-pointer dereferences will occur in the function
ttm_pool_alloc() which is called in:
return ttm_pool_alloc(>mman.bdev.pool, ttm, ctx);
To fix these possible null-pointer dereferences, the function returns
-EINVAL when ttm is NULL.
Reported-by: TOTE Robot
Signed-off-by: Tuo Li
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3a55f08e00e1..80440f799c09 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1120,8 +1120,11 @@ static int amdgpu_ttm_tt_populate(struct ttm_device
*bdev,
struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
struct amdgpu_ttm_tt *gtt = (void *)ttm;
+ if (ttm == NULL)
+ return -EINVAL;
+
/* user pages are bound by amdgpu_ttm_tt_pin_userptr() */
- if (gtt && gtt->userptr) {
+ if (gtt->userptr) {
ttm->sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!ttm->sg)
return -ENOMEM;
--
2.25.1
[PATCH] drm/amdgpu: fix possible null-pointer dereference in amdgpu_ttm_tt_unpopulate()
The variable ttm is assigned to the variable gtt, and the variable gtt
is checked in:
if (gtt && gtt->userptr)
This indicates that both ttm and gtt can be NULL.
If so, a null-pointer dereference will occur:
if (ttm->page_flags & TTM_PAGE_FLAG_SG)
Also, some null-pointer dereferences will occur in the function
ttm_pool_free() which is called in:
return ttm_pool_free(>mman.bdev.pool, ttm);
To fix these possible null-pointer dereferences, the function returns
when ttm is NULL.
Reported-by: TOTE Robot
Signed-off-by: Tuo Li
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3a55f08e00e1..0216ca085f11 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1146,7 +1146,10 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_device
*bdev,
struct amdgpu_ttm_tt *gtt = (void *)ttm;
struct amdgpu_device *adev;
- if (gtt && gtt->userptr) {
+ if (ttm == NULL)
+ return;
+
+ if (gtt->userptr) {
amdgpu_ttm_tt_set_user_pages(ttm, NULL);
kfree(ttm->sg);
ttm->sg = NULL;
--
2.25.1
