Heres 2 maintenance patches;
- a minor short-circuit optimization
- a cleanup of old cruft
Jim Cromie (2):
dyndbg: avoid calling dyndbg_emit_prefix when it has no work
dyndbg: drop uninformative vpr_info
include/linux/dynamic_debug.h | 9 +++++++++
lib/dynamic_debug.c | 10 ++++++++--
2 files changed, 17 insertions(+), 2 deletions(-)
--
2.30.2
Wrap function in a static-inline one, which checks flags to avoid
calling the function unnecessarily.
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 9 +++++++++
lib/dynamic_debug.c | 9 ++++++++-
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index a57ee75342cf..173535e725f7 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -32,6 +32,15 @@ struct _ddebug {
#define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
#define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
#define _DPRINTK_FLAGS_INCL_TID (1<<4)
+
+#define _DPRINTK_FLAGS_INCL_ANYSITE \
+ (_DPRINTK_FLAGS_INCL_MODNAME \
+ | _DPRINTK_FLAGS_INCL_FUNCNAME \
+ | _DPRINTK_FLAGS_INCL_LINENO)
+#define _DPRINTK_FLAGS_INCL_ANY \
+ (_DPRINTK_FLAGS_INCL_ANYSITE \
+ | _DPRINTK_FLAGS_INCL_TID)
+
#if defined DEBUG
#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
#else
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c70d6347afa2..613293896e47 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -586,7 +586,7 @@ static int remaining(int wrote)
return 0;
}
-static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
+static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
{
int pos_after_tid;
int pos = 0;
@@ -618,6 +618,13 @@ static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
return buf;
}
+static inline char *dynamic_emit_prefix(struct _ddebug *desc, char *buf)
+{
+ if (unlikely(desc->flags & _DPRINTK_FLAGS_INCL_ANY))
+ return __dynamic_emit_prefix(desc, buf);
+ return buf;
+}
+
void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
{
va_list args;
--
2.30.2
Remove a vpr_info which I added in 2012, when I knew even less than now.
In 2019, a simpler pr_fmt stripped it of context, and any remaining value.
no functional change.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 613293896e47..b260d8218628 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -922,7 +922,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));
}
--
2.30.2
Hi Jim,
On 4/27/21 9:00 PM, Jim Cromie wrote:
> Wrap function in a static-inline one, which checks flags to avoid
> calling the function unnecessarily.
>
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> include/linux/dynamic_debug.h | 9 +++++++++
> lib/dynamic_debug.c | 9 ++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> index a57ee75342cf..173535e725f7 100644
> --- a/include/linux/dynamic_debug.h
> +++ b/include/linux/dynamic_debug.h
> @@ -32,6 +32,15 @@ struct _ddebug {
> #define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
> #define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
> #define _DPRINTK_FLAGS_INCL_TID (1<<4)
> +
> +#define _DPRINTK_FLAGS_INCL_ANYSITE \
> + (_DPRINTK_FLAGS_INCL_MODNAME \
> + | _DPRINTK_FLAGS_INCL_FUNCNAME \
> + | _DPRINTK_FLAGS_INCL_LINENO)
> +#define _DPRINTK_FLAGS_INCL_ANY \
> + (_DPRINTK_FLAGS_INCL_ANYSITE \
> + | _DPRINTK_FLAGS_INCL_TID)
> +
I'm not sure it's worth having an unused define here by dynamic_debug.c.
I would prefer to just have _DPRINTK_FLAGS_INCL_ANY that has all the flags
in a single define.
> #if defined DEBUG
> #define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
> #else
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index c70d6347afa2..613293896e47 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -586,7 +586,7 @@ static int remaining(int wrote)
> return 0;
> }
>
> -static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
> +static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
> {
> int pos_after_tid;
> int pos = 0;
> @@ -618,6 +618,13 @@ static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
> return buf;
> }
>
> +static inline char *dynamic_emit_prefix(struct _ddebug *desc, char *buf)
> +{
> + if (unlikely(desc->flags & _DPRINTK_FLAGS_INCL_ANY))
> + return __dynamic_emit_prefix(desc, buf);
> + return buf;
> +}
> +
hmmm - looking at __dynamic_emit_prefix() it starts by doing:
589 static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
590 {
591 int pos_after_tid;
592 int pos = 0;
593
594 *buf = '\0';
So now we are missing the string termination if no flags are set...
Thanks,
-Jason
> void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
> {
> va_list args;
>
On Thu, Apr 29, 2021 at 3:03 PM Jason Baron <[email protected]> wrote:
>
> Hi Jim,
>
> On 4/27/21 9:00 PM, Jim Cromie wrote:
> > Wrap function in a static-inline one, which checks flags to avoid
> > calling the function unnecessarily.
> >
> > Signed-off-by: Jim Cromie <[email protected]>
> > ---
> > include/linux/dynamic_debug.h | 9 +++++++++
> > lib/dynamic_debug.c | 9 ++++++++-
> > 2 files changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> > index a57ee75342cf..173535e725f7 100644
> > --- a/include/linux/dynamic_debug.h
> > +++ b/include/linux/dynamic_debug.h
> > @@ -32,6 +32,15 @@ struct _ddebug {
> > #define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
> > #define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
> > #define _DPRINTK_FLAGS_INCL_TID (1<<4)
> > +
> > +#define _DPRINTK_FLAGS_INCL_ANYSITE \
> > + (_DPRINTK_FLAGS_INCL_MODNAME \
> > + | _DPRINTK_FLAGS_INCL_FUNCNAME \
> > + | _DPRINTK_FLAGS_INCL_LINENO)
> > +#define _DPRINTK_FLAGS_INCL_ANY \
> > + (_DPRINTK_FLAGS_INCL_ANYSITE \
> > + | _DPRINTK_FLAGS_INCL_TID)
> > +
>
> I'm not sure it's worth having an unused define here by dynamic_debug.c.
>
> I would prefer to just have _DPRINTK_FLAGS_INCL_ANY that has all the flags
> in a single define.
I have another patch to emit_prefix that uses !ANYSITE to return early
if only TID is wanted
but I can split that out,
or maybe I can pull the other patch forward out of the dd-diet-plan
set Im working.
>
> hmmm - looking at __dynamic_emit_prefix() it starts by doing:
>
>
> 589 static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
> 590 {
> 591 int pos_after_tid;
> 592 int pos = 0;
> 593
> 594 *buf = '\0';
>
>
> So now we are missing the string termination if no flags are set...
yes thats wrong. looks like I can hoist that init into the caller
> Thanks,
>
> -Jason
>
thanks
Jim