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