linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jim Cromie <jim.cromie@gmail.com>
To: jbaron@akamai.com, gregkh@linuxfoundation.org,
	linux-kernel@vger.kernel.org
Cc: Jim Cromie <jim.cromie@gmail.com>
Subject: [RFC PATCH v3 14/18] dyndbg: add ddebug_site(_get|_put) abstraction
Date: Mon, 15 Mar 2021 23:07:57 -0600	[thread overview]
Message-ID: <20210316050801.2446401-15-jim.cromie@gmail.com> (raw)
In-Reply-To: <20210316050801.2446401-1-jim.cromie@gmail.com>

Replace direct ->site refs with _get(),_put() internal API.  Right
now, _get() just returns ->site and _put() does nothing.  Later we can
replace that implementation with one using ->module_index to fetch
then forget site data dynamically.

Several approaches are possible:

A: !site -> fill from backing store

1st try at this is/was using zram.  At init, it copied each callsite
into a zs-allocation, and all site-> refs afterward went thru
_get/_put to zs-map on demand, and zs-unmap the site info.  This
worked until I tried to keep callsites mapped while they're enabled,
when it gave lockdep warns/panics.  IIRC theres a zram patchset doing
something with locking; I need to retry this approach, even if other
options are better, this might be a validating use case.

B: block store

Another approach is to compress the new linker section, using some
algorithm thats good at indexed decompression.  I probed this
approach, using objcopy, unsuccessfully:

   objcopy --dump-section __dyndbg=dd \
	   --dump-section __dyndbg_sites=ddsites $IMG

From vmlinux.o dumps were mostly empty (pre-link/reloc data?)
and vmlinux didnt have the section.

C: callsite composed from __dyndbg[N] & __dyndbg_site[N]

We know _ddebug records are in a vector, either in the builtin
__dyndbg linker section, or the same from a modprobed one.  The
builtin section has all builtin module sub-sections catenated
dogether.

At init, we iterate over the section, and "parse it" by creating a
ddebug_table for each module with prdebugs.  ddebug_table.num_debugs
remembers the size of each modules' vector of prdebugs.

We need a few things:

- _ddebug.index field, which knows offset to start of this sub-vector.
  this new field will be "free" because the struct has padding.
  it can be initialized during init, then RO.

- a back-pointer at the beginning of the sub-vector, to the
  ddebug_table "owning" (but not containing) this sub-vector of
  prdebugs.

If we had both, we could get from the ddebug element to its vector
root, back up to the owning ddebug_table, then down to the _callsite
vector, and index to the right element.  While slower than a pointer
deref, this is a cold path, and it allows elimination of the
per-callsite pointer member, thus greater density of the sections, and
still can support sparse site info.

That back-pointer feels tricky.  It needs to be 1st in the sub-vector

D: (C1?) add a header record to each sub-vector

If we can insert a header record into each modules' __dyndbg* section
sub-vectors, we can simplify the cold path above; a single sites*
pointer in the header can give us access to __dyndbg_sites[N]

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

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 5529b17461ae..34329e323ed5 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -144,6 +144,14 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
 		 query->first_lineno, query->last_lineno);
 }
 
+static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
+{
+	return dp->site; /* passthru abstraction */
+}
+static inline void ddebug_site_put(struct _ddebug *dp)
+{
+}
+
 static int ddebug_match_site(const struct ddebug_query *query,
 			     const struct _ddebug *dp,
 			     const struct _ddebug_site *dc)
@@ -239,16 +247,18 @@ static int ddebug_change(const struct ddebug_query *query,
 
 		for (i = 0; i < dt->num_ddebugs; i++) {
 			struct _ddebug *dp = &dt->ddebugs[i];
-			struct _ddebug_site *dc = dp->site;
+			struct _ddebug_site *dc;
+
+			dc = ddebug_site_get(dp);
 
 			if (!ddebug_match_site(query, dp, dc))
-				continue;
+				goto skipsite;
 
 			nfound++;
 
 			newflags = (dp->flags & modifiers->mask) | modifiers->flags;
 			if (newflags == dp->flags)
-				continue;
+				goto skipsite;
 
 			ddebug_alter_site(dp, modifiers);
 
@@ -264,6 +274,9 @@ static int ddebug_change(const struct ddebug_query *query,
 					  dt->mod_name, dp->lineno,
 					  ddebug_describe_flags(dp->flags, &fbuf),
 					  dp->format);
+
+		skipsite:
+			ddebug_site_put(dp);
 		}
 	}
 	mutex_unlock(&ddebug_lock);
@@ -633,11 +646,11 @@ 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;
-	const struct _ddebug_site *desc = dp->site;
+	const struct _ddebug_site *desc;
 
 	*buf = '\0';
 
@@ -653,6 +666,7 @@ static char *__dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
 	if (!(dp->flags & _DPRINTK_FLAGS_INCL_ANYSITE))
 		return buf;
 
+	desc = ddebug_site_get(dp);
 	if (desc) {
 		if (dp->flags & _DPRINTK_FLAGS_INCL_MODNAME)
 			pos += snprintf(buf + pos, remaining(pos), "%s:",
@@ -670,6 +684,8 @@ static char *__dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
 	if (pos >= PREFIX_SIZE)
 		buf[PREFIX_SIZE - 1] = '\0';
 
+	ddebug_site_put(dp);
+
 	return buf;
 }
 
@@ -952,7 +968,8 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 		return 0;
 	}
 
-	dc = dp->site;
+	dc = ddebug_site_get(dp);
+
 	if (dc) {
 		seq_printf(m, "%s:%u [%s]%s =%s \"",
 			   trim_prefix(dc->filename), dp->lineno,
@@ -968,6 +985,8 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 		seq_puts(m, "\"\n");
 	}
 
+	ddebug_site_put(dp);
+
 	return 0;
 }
 
-- 
2.29.2


  parent reply	other threads:[~2021-03-16  5:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-16  5:07 [RFC PATCH v3 00/18] dynamic debug diet plan Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 01/18] dyndbg: split struct _ddebug, move display fields to new _ddebug_site Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 02/18] dyndbg: __init iterate over __dyndbg & __dyndbg_site in parallel Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 03/18] dyndbg: refactor part of ddebug_change to ddebug_match_site Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 04/18] dyndbg: accept null site in ddebug_match_site Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 05/18] dyndbg: hoist ->site out of ddebug_match_site Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 06/18] dyndbg: accept null site in ddebug_change Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 07/18] dyndbg: accept null site in dynamic_emit_prefix Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 08/18] dyndbg: accept null site in ddebug_proc_show Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 09/18] dyndbg: optimize ddebug_emit_prefix Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 10/18] dyndbg: avoid calling dyndbg_emit_prefix when it has no work Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 11/18] dyndbg: refactor ddebug_alter_site out of ddebug_change Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 12/18] dyndbg: allow deleting site info via control interface Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 13/18] dyndbg+module: expose ddebug_sites to modules Jim Cromie
2021-03-16  5:07 ` Jim Cromie [this message]
2021-03-16  5:07 ` [RFC PATCH v3 15/18] dyndbg: add _index to struct _ddebug Jim Cromie
2021-03-16  5:07 ` [RFC PATCH v3 16/18] dyndbg: prevent build bugs via -DNO_DYNAMIC_DEBUG_TABLE Jim Cromie
2021-03-16  5:08 ` [RFC PATCH v3 17/18] dyndbg: RFC - DECLARE/DEFINE_DYNAMIC_DEBUG_TABLE Jim Cromie
2021-03-16  5:08 ` [RFC PATCH v3 18/18] dyndbg: shuffle ddebug_table fields Jim Cromie
2021-03-19  4:12 ` [RFC PATCH v3 00/18] dynamic debug diet plan Andi Kleen
2021-05-02  2:16   ` 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=20210316050801.2446401-15-jim.cromie@gmail.com \
    --to=jim.cromie@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jbaron@akamai.com \
    --cc=linux-kernel@vger.kernel.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).