- fix new export name, with a wrapper for more utility.
now with fixes per Joe Perches
- 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
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
commit 59cf47e7df31 dyndbg: export ddebug_exec_queries
left a few configs broken, fix them with ifdef stub-fns.
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.
--
v2- fixes per Joe Perches
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 20 ++++++++++++++++----
lib/dynamic_debug.c | 26 ++++++++++++++++++++++++--
2 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index aa9ff9e1c0b3..8aa0c7c2608c 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)
+{
+ pr_warn("kernel not built with 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..a23b5d153153 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,29 @@ int ddebug_exec_queries(char *query, const char *modname)
return exitcode;
return nfound;
}
-EXPORT_SYMBOL_GPL(ddebug_exec_queries);
+
+/**
+ * dynamic_debug_exec_queries - select and change dynamic-debug prints
+ * @query: query-string described in admin-guide/dynamic-debug-howto
+ * @modname: string containing module name, usually &module.mod_name
+ *
+ * This uses 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)
+{
+ int rc;
+ char *qry = kstrndup(query, PAGE_SIZE, GFP_KERNEL);
+ if (!query)
+ return -ENOMEM;
+
+ rc = ddebug_exec_queries(qry, modname);
+ kfree(qry);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(dynamic_debug_exec_queries);
#define PREFIX_SIZE 64
--
2.26.2
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 a23b5d153153..04b851117eeb 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
Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: 738b8a92befe6011b8c766e40aa1fbd26ada2695 ("[PATCH v2 3/3] dyndbg: fix problem parsing format="foo bar"")
url: https://github.com/0day-ci/linux/commits/Jim-Cromie/dynamic-debug-fixups-for-5-9/20200826-022923
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 6a9dc5fd6170d0a41c8a14eb19e63d94bea5705a
in testcase: kernel-selftests
with following parameters:
group: kselftests-livepatch
test-description: The kernel contains a set of "self tests" under the tools/testing/selftests/ directory. These are intended to be small unit tests to exercise individual code paths in the kernel.
test-url: https://www.kernel.org/doc/Documentation/kselftest.txt
on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
+------------------------------------------+------------+------------+
| | 6ed59ab9d5 | 738b8a92be |
+------------------------------------------+------------+------------+
| boot_successes | 4 | 0 |
| boot_failures | 0 | 4 |
| kernel_BUG_at_lib/dynamic_debug.c | 0 | 4 |
| invalid_opcode:#[##] | 0 | 4 |
| RIP:ddebug_exec_query | 0 | 4 |
| Kernel_panic-not_syncing:Fatal_exception | 0 | 4 |
+------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>
[ 53.159074] kernel BUG at lib/dynamic_debug.c:267!
[ 53.163121] invalid opcode: 0000 [#1] PREEMPT SMP PTI
[ 53.164809] CPU: 1 PID: 1077 Comm: test-livepatch. Tainted: G K 5.9.0-rc2-00045-g738b8a92befe6 #1
[ 53.167382] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
[ 53.168793] RIP: 0010:ddebug_exec_query+0x676/0x760
[ 53.169904] Code: 0f 85 73 08 00 00 4c 89 7c 24 28 e9 2d fc ff ff 4c 89 fe 48 8d 7c 24 10 e8 b7 f8 ff ff 85 c0 0f 84 18 fc ff ff e9 cf 07 00 00 <0f> 0b 8b 15 3a 7b e2 02 85 d2 0f 85 c6 00 00 00 45 31 e4 48 c7 c6
[ 53.172910] RSP: 0018:ffffaac6c0e7bd78 EFLAGS: 00010246
[ 53.174112] RAX: 000000000000003d RBX: ffff8d1c51300c2c RCX: 0000000000000000
[ 53.175451] RDX: 000000000000003d RSI: ffff8d1c51300c28 RDI: ffff8d1c51300c2c
[ 53.176814] RBP: 0000000000000004 R08: ffff8d1c51300c2e R09: 0000000000000000
[ 53.178156] R10: ffff8d1c488530c0 R11: ffff8d1c48853e08 R12: 0000000000000004
[ 53.179496] R13: 0000000000000004 R14: 0000000000000000 R15: 0000000000000000
[ 53.180877] FS: 00007f1c428ac740(0000) GS:ffff8d1d37d00000(0000) knlGS:0000000000000000
[ 53.182310] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 53.183559] CR2: 0000000000442d70 CR3: 0000000165d2a000 CR4: 00000000000406e0
[ 53.186445] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 53.187822] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 53.189151] Call Trace:
[ 53.190147] ddebug_exec_queries+0x6a/0x100
[ 53.191251] ddebug_proc_write+0x4e/0x80
[ 53.192355] full_proxy_write+0x56/0x80
[ 53.193421] vfs_write+0xec/0x240
[ 53.194431] ksys_write+0x68/0xe0
[ 53.195431] do_syscall_64+0x33/0x40
[ 53.196462] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 53.197592] RIP: 0033:0x7f1c42999504
[ 53.198621] Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b3 0f 1f 80 00 00 00 00 48 8d 05 f9 61 0d 00 8b 00 85 c0 75 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 41 54 49 89 d4 55 48 89 f5 53
[ 53.201660] RSP: 002b:00007fff0d6b6208 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[ 53.203057] RAX: ffffffffffffffda RBX: 00000000000000bc RCX: 00007f1c42999504
[ 53.204444] RDX: 00000000000000bc RSI: 0000564b7e8061d0 RDI: 0000000000000001
[ 53.205818] RBP: 0000564b7e8061d0 R08: fffffffffffffff0 R09: 00007f1c42a29e80
[ 53.207197] R10: 0000564b7e80628c R11: 0000000000000246 R12: 00007f1c42a6b760
[ 53.208575] R13: 00000000000000bc R14: 00007f1c42a66760 R15: 00000000000000bc
[ 53.209950] Modules linked in: sr_mod cdrom sg snd_pcm intel_rapl_msr ata_generic pata_acpi ppdev ata_piix libata intel_rapl_common crct10dif_pclmul snd_timer crc32_pclmul snd crc32c_intel ghash_clmulni_intel aesni_intel crypto_simd bochs_drm drm_vram_helper cryptd drm_ttm_helper glue_helper joydev soundcore ttm pcspkr serio_raw i2c_piix4 parport_pc ipmi_devintf ipmi_msghandler floppy parport ip_tables [last unloaded: test_klp_atomic_replace]
[ 53.217827] ---[ end trace 32d22997ab7e3c2a ]---
To reproduce:
# build kernel
cd linux
cp config-5.9.0-rc2-00045-g738b8a92befe6 .config
make HOSTCC=gcc-9 CC=gcc-9 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email
Thanks,
lkp
On Tue, Aug 25, 2020 at 11:33:39AM -0600, Jim Cromie wrote:
> 14775b049642 dyndbg: accept query terms like file=bar and module=foo
>
What does that above line mean???
> 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]>
Again, a Fixes: tag, and fix up your text above please.
Can you do this and send a v3 of this series and any other pending
dyndbg fixes for 5.9-final?
thanks,
greg k-h
On Tue, Aug 25, 2020 at 11:33:38AM -0600, Jim Cromie wrote:
> commit 59cf47e7df31 dyndbg: export ddebug_exec_queries
> left a few configs broken, fix them with ifdef stub-fns.
Please use the standard way of printing commit ids, it's in the
submitting patches document.
>
> 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.
>
> --
> v2- fixes per Joe Perches
Shouldn't that go below the --- line?
>
> Signed-off-by: Jim Cromie <[email protected]>
Can you put a "Fixes:" tag in here?
thanks,
greg k-h