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 2/3] dyndbg: zhandle+1 plus info tweaks, BUG_ONs
Date: Sat, 25 Jul 2020 10:36:01 -0600
Message-ID: <20200725163602.2828969-3-jim.cromie@gmail.com> (raw)
In-Reply-To: <20200725163602.2828969-1-jim.cromie@gmail.com>

In preparation for union between zhandle and callsite, add a +1 offset
to zhandle as its created & stored, and a corresponding -1 in the
get/put helper funcs which access it.

With the +1, the value cannot work as a pointer, so we'll get early
detonation if the union goes poorly.  This relys on the 'fact' uhm
observation that zhandles were always even numbered.  So far so good.

Also add BUG-ONs to track/assert invariants into ddebug_zpool_init
and the get/put inline helpers, and several debug prints.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 72 +++++++++++++++++++++++++++++++--------------
 1 file changed, 50 insertions(+), 22 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index bb23d5d56116..96252ffacb77 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -145,13 +145,15 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
 
 static inline struct _ddebug* ddebug_zrec_get(long unsigned int handle)
 {
+	BUG_ON(!(handle % 2));		/* distinct from pointer */
 	return (struct _ddebug*)
-		zs_map_object(dd_callsite_zpool, handle, ZS_MM_RW);
+		zs_map_object(dd_callsite_zpool, handle - 1, ZS_MM_RW);
 }
 
 static inline void ddebug_zrec_put(long unsigned int handle)
 {
-	zs_unmap_object(dd_callsite_zpool, handle);
+	BUG_ON(!(handle % 2));		/* distinct from pointer */
+	zs_unmap_object(dd_callsite_zpool, handle - 1);
 }
 
 /*
@@ -587,22 +589,31 @@ static int remaining(int wrote)
 	return 0;
 }
 
-static char *dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
+static char *dynamic_emit_prefix( struct _ddebug *dp, char *buf)
 {
 	int pos_after_tid;
 	int pos = 0;
-	struct _ddebug *desc;
+	struct _ddebug *desc = dp;	/* updated if zhandle */
 
 	*buf = '\0';
 
+	BUG_ON(dp->zhandle == 1);
+
 	if (!dp->zhandle) {
-		pr_err("Nul zhandle: %s.%s\n", dp->modname, dp->function);
-		desc = dp;
-	} else {
+		/* without union, happens until late-init */
+		pr_err("nul zhandle: %s.%s\n", dp->modname, dp->function);
+	} else if (dp->zhandle % 2) {
+		/* normal ops, after zpool filled
+		   zhandle is odd to distinguish from pointer
+		*/
 		desc = ddebug_zrec_get(dp->zhandle);
-		v3pr_info("good zhandle: %s.%s\n",
+		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",
+		       desc->modname, desc->function, dp->zhandle);
+
 
 	if (desc->flags & _DPRINTK_FLAGS_INCL_TID) {
 		if (in_interrupt())
@@ -627,9 +638,9 @@ static char *dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
 		buf[PREFIX_SIZE - 1] = '\0';
 
 	if (!dp->zhandle) {
-		pr_err("Nul zhandle\n");
-	} else {
-		v3pr_info("got zhandle\n");
+		pr_err("Nul zhandle: %s.%s\n", desc->modname, desc->function);
+	} else if (dp->zhandle % 2) {
+		v2pr_info("put zhandle: %s.%s\n", desc->modname, desc->function);
 		ddebug_zrec_put(dp->zhandle);
 	}
 
@@ -898,7 +909,6 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 	struct ddebug_iter *iter = m->private;
 	struct _ddebug *dp = p;
 	struct flagsbuf flags;
-	long unsigned int handle;
 
 	if (p == SEQ_START_TOKEN) {
 		seq_puts(m,
@@ -906,16 +916,19 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 		return 0;
 	}
 
-	BUG_ON(!dp->zhandle);
-	handle = dp->zhandle;
+	BUG_ON(!dp->zhandle);		/* must be set by now */
 
-	dp = ddebug_zrec_get(handle);
+	if (dp->zhandle == 1)
+		vpr_info("dp:%p mapping ?? h:%lu %s.%s\n", dp, dp->zhandle,
+			 iter->table->mod_name, dp->function);
+
+	dp = ddebug_zrec_get(dp->zhandle);
 
 	if (!dp) {
-		pr_err("zs-map failed on %lu\n", handle);
+		pr_err("zs-map failed on %lu\n", dp->zhandle);
 		return 0; // bail
 	}
-	v3pr_info("mapped h:%lu %s\n", handle, dp->function);
+	v3pr_info("mapped h:%lu %s\n", dp->zhandle, dp->function);
 
 	seq_printf(m, "%s:%u [%s]%s =%s \"",
 		   trim_prefix(dp->filename), dp->lineno,
@@ -924,7 +937,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 	seq_escape(m, dp->format, "\t\r\n\"");
 	seq_puts(m, "\"\n");
 
-	ddebug_zrec_put(handle);
+	ddebug_zrec_put(dp->zhandle);
 
 	return 0;
 }
@@ -935,6 +948,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
  */
 static void ddebug_proc_stop(struct seq_file *m, void *p)
 {
+	vpr_info("called\n");
 	mutex_unlock(&ddebug_lock);
 }
 
@@ -947,7 +961,6 @@ static const struct seq_operations ddebug_proc_seqops = {
 
 static int ddebug_proc_open(struct inode *inode, struct file *file)
 {
-	vpr_info("called\n");
 	return seq_open_private(file, &ddebug_proc_seqops,
 				sizeof(struct ddebug_iter));
 }
@@ -972,7 +985,7 @@ static const struct proc_ops proc_fops = {
 /*
  * copy struct _ddebug records into the zpool, and remember zhandle in
  * the struct.  This is close to what we'll want to unionize the
- * handle and the struct
+ * handle and the struct.
  */
 static void ddebug_zpool_add(struct _ddebug *dp)
 {
@@ -985,7 +998,21 @@ static void ddebug_zpool_add(struct _ddebug *dp)
 		pr_err("pool malloc failed on %s\n", dp->function);
 		return;
 	}
-	dp->zhandle = handle;
+
+	/* We want !!is_odd(zhandle), to insure crash if used as a
+	   pointer.  Then we can test the unionized value to see if
+	   its a pointer or a zhandle to a zrec.  Observation shows
+	   handle is always even, and the BUG_ON confirms, so
+	   ++zhandle gives us our testable condition.
+	*/
+	BUG_ON(handle % 2);
+
+	/* update zhandle in kmem record before copy to zrec, so its
+	   in both places, to avoid use-after-restore inconsistencies.
+	   This will break once we unionize (the zrec), hopefully all
+	   these comments will help sort it all out.
+	*/
+	dp->zhandle = handle + 1;
 
 	cursor = (struct _ddebug *)
 		zs_map_object(dd_callsite_zpool, handle, ZS_MM_WO);
@@ -999,6 +1026,7 @@ static void ddebug_zpool_add(struct _ddebug *dp)
 	v3pr_info("h:%lu %s\n", handle, cursor->function);
 
 	zs_unmap_object(dd_callsite_zpool, handle);
+
 }
 
 /*
-- 
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 ` Jim Cromie [this message]
2020-07-25 16:36 ` [PATCH 3/3] dyndbg: fixup/correct assumptions re ptr-vals Jim Cromie

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-3-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