Kernel Newbies archive on lore.kernel.org
 help / color / Atom feed
From: Jim Cromie <jim.cromie@gmail.com>
To: jbaron@akamai.com
Cc: Jim Cromie <jim.cromie@gmail.com>, kernelnewbies@kernelnewbies.org
Subject: [PATCH 3/3] dyndbg: fixup/correct assumptions re ptr-vals
Date: Sat, 25 Jul 2020 10:36:02 -0600
Message-ID: <20200725163602.2828969-4-jim.cromie@gmail.com> (raw)
In-Reply-To: <20200725163602.2828969-1-jim.cromie@gmail.com>

The is_odd(dp->zhandle) property is apparently reliable;
BUG_ONs in HEAD~1 demonstrate it.

But is_odd(dp->modname) is also sometimes true, at least when it
points into the __dyndbg section, which has been packed by the linker.

This means the (dp->zhandle % 2) test couldnt distinguish between the
2 values shared in the union.  Happily, it blew up, challenging
assumptions. 'gdb> p *dp' confirmed the odd pointer.

Breakpoint 1, dynamic_emit_prefix (dp=0xffffffff8276e190 <__UNIQUE_ID_ddebug429.11>,
    buf=0xffffc90000013d48 "߇]\201\377\377\377\377") at ../lib/dynamic_debug.c:598
598		*buf = '\0';
1: dp = (struct _ddebug *) 0xffffffff8276e190 <__UNIQUE_ID_ddebug429.11>
2: *dp = {{is_zhandle = 0, {zhandle = 18446744071599650201, {modname = 0xffffffff823d7599 "intel_idle",
        function = 0xffffffff820d8b20 <__func__.7> "intel_idle_init",
        filename = 0xffffffff823d75a4 "drivers/idle/intel_idle.c",
        format = 0xffffffff823d74e8 "Please enable MWAIT in BIOS SETUP\n", lineno = 1609, flags = 1, key = {
          dd_key_true = {key = {enabled = {counter = 1}, {type = 18446744071600203817,
                entries = 0xffffffff8245e829, next = 0xffffffff8245e829}}}, dd_key_false = {key = {enabled = {
                counter = 1}, {type = 18446744071600203817, entries = 0xffffffff8245e829,
                next = 0xffffffff8245e829}}}}}}}}

So instead, add an explicit flag-int: is_zhandle, to remember when the
union is changed to zhandle.

Again, we abuse indenting to minimize whitespace, and add an anonymous
outer struct to contain is_zhandle, and previous contents.

Maybe this flag can be hidden somewhere else; splitting struct _ddebug
into parts is still needed (flags cant be in zram for non-JUMP_LABEL
builds), so maybe it ends up there.

Or perhaps the linker can be convinced to be slightly less
parsimonious with the ram, making this is_odd() test viable.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 include/linux/dynamic_debug.h |  5 +++++
 lib/dynamic_debug.c           | 17 ++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index d23f283fff70..345e86e23bb9 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -13,6 +13,10 @@
  */
 struct _ddebug {
 	struct {
+		int is_zhandle;
+		// 
+	union {
+	//struct {
 		long unsigned int zhandle;
 		struct {
 	/*
@@ -48,6 +52,7 @@ struct _ddebug {
 	} key;
 #endif
 		};}; // struct union
+	}; // struct
 } __attribute__((aligned(8)));
 
 
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 96252ffacb77..6e93b19bf141 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -602,13 +602,22 @@ static char *dynamic_emit_prefix( struct _ddebug *dp, char *buf)
 	if (!dp->zhandle) {
 		/* without union, happens until late-init */
 		pr_err("nul zhandle: %s.%s\n", dp->modname, dp->function);
-	} else if (dp->zhandle % 2) {
+	}
+	else if (dp->is_zhandle) {
+		pr_err("is-zhandle:%d zhandle.mod2:%d\n", dp->is_zhandle, (int)dp->zhandle % 2);
+		v3pr_info("get zhandle: %s.%s\n", dp->modname, dp->function);
+		desc = ddebug_zrec_get(dp->zhandle);
+		v3pr_info("got zhandle: %s.%s\n", desc->modname, desc->function);
+	}
+	else if (dp->zhandle % 2) {
+		pr_err("odd zhandle get %lu %p\n", dp->zhandle, (void*)dp->zhandle);
 		/* normal ops, after zpool filled
 		   zhandle is odd to distinguish from pointer
-		*/
+
 		desc = ddebug_zrec_get(dp->zhandle);
 		v3pr_info("get zhandle: %s.%s\n",
 			  desc->modname, desc->function);
+		*/
 	} else
 		/* with union, happens until late-init */
 		pr_err("some transitional state: %s.%s %lu\n",
@@ -639,7 +648,7 @@ static char *dynamic_emit_prefix( struct _ddebug *dp, char *buf)
 
 	if (!dp->zhandle) {
 		pr_err("Nul zhandle: %s.%s\n", desc->modname, desc->function);
-	} else if (dp->zhandle % 2) {
+	} else if (dp->is_zhandle) {
 		v2pr_info("put zhandle: %s.%s\n", desc->modname, desc->function);
 		ddebug_zrec_put(dp->zhandle);
 	}
@@ -1014,6 +1023,8 @@ static void ddebug_zpool_add(struct _ddebug *dp)
 	*/
 	dp->zhandle = handle + 1;
 
+	dp->is_zhandle = 1; /* sanity check on everything else */
+
 	cursor = (struct _ddebug *)
 		zs_map_object(dd_callsite_zpool, handle, ZS_MM_WO);
 
-- 
2.26.2


_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

      parent reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-25 16:35 [PATCH 0/3] dyndbg: WIP semi-viable diet plan ? Jim Cromie
2020-07-25 16:36 ` [PATCH 1/3] dyndbg: WIP replace __dyndbg section with a zs-pool copy Jim Cromie
2020-07-25 16:36 ` [PATCH 2/3] dyndbg: zhandle+1 plus info tweaks, BUG_ONs Jim Cromie
2020-07-25 16:36 ` Jim Cromie [this message]

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=20200725163602.2828969-4-jim.cromie@gmail.com \
    --to=jim.cromie@gmail.com \
    --cc=jbaron@akamai.com \
    --cc=kernelnewbies@kernelnewbies.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

Kernel Newbies archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernelnewbies/0 kernelnewbies/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kernelnewbies kernelnewbies/ https://lore.kernel.org/kernelnewbies \
		kernelnewbies@kernelnewbies.org
	public-inbox-index kernelnewbies

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernelnewbies.kernelnewbies


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git