[PATCH 2/3] dyndbg: zhandle+1 plus info tweaks, BUG_ONs

[Jim Cromie]

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