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@xxxxxxxxx>
---
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@xxxxxxxxxxxxxxxxx
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
