2020-08-24 18:55:38

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 0/3] dynamic-debug fixups for 5.9

- fix new export name, with a wrapper for more utility.
- parse format="foo bar" like "format" "foo bar"
- pretty-print

Jim Cromie (3):
dyndbg: give %3u width in pr-format, cosmetic only
dyndbg: refine export, rename to dynamic_debug_exec_queries()
dyndbg: fix problem parsing format="foo bar"

include/linux/dynamic_debug.h | 20 +++++++++---
lib/dynamic_debug.c | 59 ++++++++++++++++++++++-------------
2 files changed, 53 insertions(+), 26 deletions(-)

--
2.26.2


2020-08-24 18:55:45

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 1/3] dyndbg: give %3u width in pr-format, cosmetic only

Specify the print-width so log entries line up nicely.

no functional changes.

Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 1d012e597cc3..01b7d0210412 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -947,7 +947,7 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
list_add(&dt->link, &ddebug_tables);
mutex_unlock(&ddebug_lock);

- v2pr_info("%u debug prints in module %s\n", n, dt->mod_name);
+ v2pr_info("%3u debug prints in module %s\n", n, dt->mod_name);
return 0;
}

--
2.26.2

2020-08-24 18:56:17

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 3/3] dyndbg: fix problem parsing format="foo bar"

14775b049642 dyndbg: accept query terms like file=bar and module=foo

That commit broke on a tokenization modality where a word could start
with a quote, but couldnt continue with one. So the above would
tokenize as 'format="foo' and 'bar"', and fail hard.

This commit fixes the tokenizer by terminating an unquoted token on
the '=', avoiding that problem. And since ddebug-parse-query will
never see a combined 'keyword=value', revert those parts of the
previous commit.

Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index b6ab2c643116..4af686e6ef59 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -237,6 +237,7 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
{
int nwords = 0;

+ vpr_info("entry, buf:'%s'\n", buf);
while (*buf) {
char *end;

@@ -247,6 +248,8 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
if (*buf == '#')
break; /* token starts comment, skip rest of line */

+ vpr_info("start-of-word:%d '%s'\n", nwords, buf);
+
/* find `end' of word, whitespace separated or quoted */
if (*buf == '"' || *buf == '\'') {
int quote = *buf++;
@@ -257,7 +260,9 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
return -EINVAL; /* unclosed quote */
}
} else {
- for (end = buf; *end && !isspace(*end); end++)
+ for (end = buf;
+ *end && *end != '=' && !isspace(*end);
+ end++)
;
BUG_ON(end == buf);
}
@@ -373,30 +378,20 @@ static int ddebug_parse_query(char *words[], int nwords,
unsigned int i;
int rc = 0;
char *fline;
- char *keyword, *arg;

+ if (nwords % 2 != 0) {
+ pr_err("expecting pairs of match-spec <value>\n");
+ return -EINVAL;
+ }
if (modname)
/* support $modname.dyndbg=<multiple queries> */
query->module = modname;

- for (i = 0; i < nwords; i++) {
- /* accept keyword=arg */
- vpr_info("%d w:%s\n", i, words[i]);
-
- keyword = words[i];
- arg = strchr(keyword, '=');
- if (arg) {
- *arg++ = '\0';
- } else {
- i++; /* next word is arg */
- if (!(i < nwords)) {
- pr_err("missing arg to keyword: %s\n", keyword);
- return -EINVAL;
- }
- arg = words[i];
- }
- vpr_info("%d key:%s arg:%s\n", i, keyword, arg);
+ for (i = 0; i < nwords; i+=2) {
+ char *keyword = words[i];
+ char *arg = words[i+1];

+ vpr_info("key:'%s' arg:'%s'\n", keyword, arg);
if (!strcmp(keyword, "func")) {
rc = check_set(&query->function, arg, "func");
} else if (!strcmp(keyword, "file")) {
--
2.26.2

2020-08-24 18:58:26

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 2/3] dyndbg: refine export, rename to dynamic_debug_exec_queries()

commit 59cf47e7df31 dyndbg: export ddebug_exec_queries
left a few configs broken, fix them with ifdef-stubs.

Rename the export to dynamic_debug_exec_queries(). This is a more
canonical function name, instead of exposing the 'ddebug' internal
name prefix. Do this now, before export hits v5.9.0

Implement as new function wrapping ddebug_exec_queries(now static
again), which copies the query-string, preserving ddebug_exec_queries'
in-place parsing, while allowing users to pass "const strings".

Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 20 ++++++++++++++++----
lib/dynamic_debug.c | 24 ++++++++++++++++++++++--
2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index aa9ff9e1c0b3..b0191d3aff26 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -49,6 +49,10 @@ struct _ddebug {


#if defined(CONFIG_DYNAMIC_DEBUG_CORE)
+
+/* exported for module authors to exercise >control */
+int dynamic_debug_exec_queries(const char *query, const char *modname);
+
int ddebug_add_module(struct _ddebug *tab, unsigned int n,
const char *modname);
extern int ddebug_remove_module(const char *mod_name);
@@ -105,7 +109,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
static_branch_unlikely(&descriptor.key.dd_key_false)
#endif

-#else /* !HAVE_JUMP_LABEL */
+#else /* !CONFIG_JUMP_LABEL */

#define _DPRINTK_KEY_INIT

@@ -117,7 +121,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)
#endif

-#endif
+#endif /* CONFIG_JUMP_LABEL */

#define __dynamic_func_call(id, fmt, func, ...) do { \
DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt); \
@@ -172,10 +176,11 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
KERN_DEBUG, prefix_str, prefix_type, \
rowsize, groupsize, buf, len, ascii)

-#else
+#else /* !CONFIG_DYNAMIC_DEBUG_CORE */

#include <linux/string.h>
#include <linux/errno.h>
+#include <linux/printk.h>

static inline int ddebug_add_module(struct _ddebug *tab, unsigned int n,
const char *modname)
@@ -210,6 +215,13 @@ static inline int ddebug_dyndbg_module_param_cb(char *param, char *val,
print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, \
rowsize, groupsize, buf, len, ascii); \
} while (0)
-#endif
+
+static inline int dynamic_debug_exec_queries(const char *query, const char *modname)
+{
+ printk(KERN_WARNING "kernel not built w CONFIG_DYNAMIC_DEBUG_CORE\n");
+ return 0;
+}
+
+#endif /* !CONFIG_DYNAMIC_DEBUG_CORE */

#endif
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 01b7d0210412..b6ab2c643116 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -525,7 +525,7 @@ static int ddebug_exec_query(char *query_string, const char *modname)
last error or number of matching callsites. Module name is either
in param (for boot arg) or perhaps in query string.
*/
-int ddebug_exec_queries(char *query, const char *modname)
+static int ddebug_exec_queries(char *query, const char *modname)
{
char *split;
int i, errs = 0, exitcode = 0, rc, nfound = 0;
@@ -557,7 +557,27 @@ int ddebug_exec_queries(char *query, const char *modname)
return exitcode;
return nfound;
}
-EXPORT_SYMBOL_GPL(ddebug_exec_queries);
+
+/**
+ * dynamic_debug_exec_queries - apply changes to selected dynamic-debug prints
+ * @query: string with callsite-selectors +enablement+decorations
+ * @modname: string containing module name
+ *
+ * This implements the >/proc/dynamic_debug/control reader, allowing
+ * module authors to modify their dynamic-debug callsites. The modname
+ * is canonically struct module.mod_name, but can also be null or a
+ * module-wildcard, for example: "drm*".
+ */
+int dynamic_debug_exec_queries(const char *query, const char *modname)
+{
+ char *qry = kmalloc(PAGE_SIZE, GFP_KERNEL);
+ int rc;
+ strncpy(qry, query, PAGE_SIZE);
+ rc = ddebug_exec_queries(qry, modname);
+ kfree(qry);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(dynamic_debug_exec_queries);

#define PREFIX_SIZE 64

--
2.26.2

2020-08-24 19:10:56

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 2/3] dyndbg: refine export, rename to dynamic_debug_exec_queries()

On Mon, 2020-08-24 at 12:54 -0600, Jim Cromie wrote:
> commit 59cf47e7df31 dyndbg: export ddebug_exec_queries
> left a few configs broken, fix them with ifdef-stubs.
>
> Rename the export to dynamic_debug_exec_queries(). This is a more
> canonical function name, instead of exposing the 'ddebug' internal
> name prefix. Do this now, before export hits v5.9.0
>
> Implement as new function wrapping ddebug_exec_queries(now static
> again), which copies the query-string, preserving ddebug_exec_queries'
> in-place parsing, while allowing users to pass "const strings".
[]
> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
[]
> @@ -210,6 +215,13 @@ static inline int ddebug_dyndbg_module_param_cb(char *param, char *val,
> print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, \
> rowsize, groupsize, buf, len, ascii); \
> } while (0)
> -#endif
> +
> +static inline int dynamic_debug_exec_queries(const char *query, const char *modname)
> +{
> + printk(KERN_WARNING "kernel not built w CONFIG_DYNAMIC_DEBUG_CORE\n");

pr_warn and w should be with

> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
[]
> @@ -557,7 +557,27 @@ int ddebug_exec_queries(char *query, const char *modname)
> return exitcode;
> return nfound;
> }
> -EXPORT_SYMBOL_GPL(ddebug_exec_queries);
> +
> +/**
> + * dynamic_debug_exec_queries - apply changes to selected dynamic-debug prints
> + * @query: string with callsite-selectors +enablement+decorations
> + * @modname: string containing module name
> + *
> + * This implements the >/proc/dynamic_debug/control reader, allowing
> + * module authors to modify their dynamic-debug callsites. The modname
> + * is canonically struct module.mod_name, but can also be null or a
> + * module-wildcard, for example: "drm*".
> + */
> +int dynamic_debug_exec_queries(const char *query, const char *modname)
> +{
> + char *qry = kmalloc(PAGE_SIZE, GFP_KERNEL);
> + int rc;
> + strncpy(qry, query, PAGE_SIZE);

kstrndup?