[PATCH] dynamic_debug: add wildcard support to filter files/functions/modules

[Du, Changbin]

From: "Du, Changbin" <chang...@gmail.com>

This patch add wildcard '*'(matches zero or more characters) and '?'
(matches one character) support when qurying debug flags.

Now we can open debug messages using keywords. eg:
1. open debug logs in all usb drivers
echo "file drivers/usb/* +p" > <debugfs>/dynamic_debug/control
2. open debug logs for usb xhci code
echo "file *xhci* +p" > <debugfs>/dynamic_debug/control

Signed-off-by: Du, Changbin <chang...@gmail.com>
---
lib/dynamic_debug.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 99fec3a..cdcce58 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -127,6 +127,21 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
query->first_lineno, query->last_lineno);
}

+static int match_pattern(char *pat, char *str)
+{
+ switch (*pat) {
+ case '\0':
+ return !*str;
+ case '*':
+ return match_pattern(pat+1, str) ||
+ (*str && match_pattern(pat, str+1));
+ case '?':
+ return *str && match_pattern(pat+1, str+1);
+ default:
+ return *pat == *str && match_pattern(pat+1, str+1);
+ }
+}
+
/*
* Search the tables for _ddebug's which match the given `query' and
* apply the `flags' and `mask' to them. Returns number of matching
@@ -147,7 +162,7 @@ static int ddebug_change(const struct ddebug_query *query,
list_for_each_entry(dt, &ddebug_tables, link) {

/* match against the module name */
- if (query->module && strcmp(query->module, dt->mod_name))
+ if (query->module && !match_pattern(query->module, dt->mod_name))
continue;

for (i = 0; i < dt->num_ddebugs; i++) {
@@ -155,14 +170,16 @@ static int ddebug_change(const struct ddebug_query *query,

/* match against the source filename */
if (query->filename &&
- strcmp(query->filename, dp->filename) &&
- strcmp(query->filename, kbasename(dp->filename)) &&
- strcmp(query->filename, trim_prefix(dp->filename)))
+ !match_pattern(query->filename, dp->filename) &&
+ !match_pattern(query->filename,
+ kbasename(dp->filename)) &&
+ !match_pattern(query->filename,
+ trim_prefix(dp->filename)))
continue;

/* match against the function */
if (query->function &&
- strcmp(query->function, dp->function))
+ !match_pattern(query->function, dp->function))
continue;

/* match against the format */
--
1.8.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Joe Perches]

On Thu, 2013-07-25 at 21:02 +0800, Du, Changbin wrote:
> From: "Du, Changbin" <chang...@gmail.com>
>
> This patch add wildcard '*'(matches zero or more characters) and '?'
> (matches one character) support when qurying debug flags.

Seems very useful. Caveat below.

> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c

[]
> @@ -127,6 +127,21 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
> query->first_lineno, query->last_lineno);
> }
>
> +static int match_pattern(char *pat, char *str)
> +{
> + switch (*pat) {
> + case '\0':
> + return !*str;
> + case '*':
> + return match_pattern(pat+1, str) ||
> + (*str && match_pattern(pat, str+1));
> + case '?':
> + return *str && match_pattern(pat+1, str+1);
> + default:

> + return *pat == *str && match_pattern(pat+1, str 1);
> + }
> +}

What's the maximum length string used today?
On a very long pattern, can this recursion cause stack overflow?

Other than that, I like it.

[Jason Baron]

The recursion here is a concern - especially since the 'pat' pointer is
controlled from userspace. Maybe not at pretty, but this can easily be
done in userspace. For example, to set all driver/usb* one could do
something like:

grep "drivers/usb" control | awk -F ":| " '{ print "file " $1 " line "
$2 }' | xargs -i -n1 echo {} +p > control

Thanks,

-Jason

[Du, Changbin]

From: "Du, Changbin" <chang...@gmail.com>

This patch add wildcard '*'(matches zero or more characters) and '?'
(matches one character) support when qurying debug flags.

Now we can open debug messages using keywords. eg:
1. open debug logs in all usb drivers
echo "file drivers/usb/* +p" > <debugfs>/dynamic_debug/control
2. open debug logs for usb xhci code
echo "file *xhci* +p" > <debugfs>/dynamic_debug/control

Signed-off-by: Du, Changbin <chang...@gmail.com>
---

changes since v1:
rewrite match_pattern using non-recursion method.
---
lib/dynamic_debug.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 42 insertions(+), 5 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c37aeac..d239207 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -127,6 +127,41 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
query->first_lineno, query->last_lineno);
}

+/* check if the string matches given pattern which includes wildcards */
+static int match_pattern(const char *pattern, const char *string)
+{
+ const char *s, *p;
+ int star = 0;
+
+loop:
+ for (s = string, p = pattern; *s; ++s, ++p) {
+ switch (*p) {
+ case '?':
+ break;
+ case '*':
+ star = 1;
+ string = s;
+ pattern = p;
+ if (!*++pattern)
+ return 1;
+ goto loop;
+ default:
+ if (*s != *p)
+ goto star_check;
+ break;
+ }
+ }
+ if (*p == '*')
+ ++p;
+ return (!*p);
+
+star_check:
+ if (!star)
+ return 0;
+ string++;
+ goto loop;

+}
+
/*
* Search the tables for _ddebug's which match the given `query' and
* apply the `flags' and `mask' to them. Returns number of matching

@@ -147,7 +182,7 @@ static int ddebug_change(const struct ddebug_query *query,

list_for_each_entry(dt, &ddebug_tables, link) {

/* match against the module name */
- if (query->module && strcmp(query->module, dt->mod_name))
+ if (query->module && !match_pattern(query->module, dt->mod_name))
continue;

for (i = 0; i < dt->num_ddebugs; i++) {

@@ -155,14 +190,16 @@ static int ddebug_change(const struct ddebug_query *query,

/* match against the source filename */
if (query->filename &&
- strcmp(query->filename, dp->filename) &&
- strcmp(query->filename, kbasename(dp->filename)) &&
- strcmp(query->filename, trim_prefix(dp->filename)))
+ !match_pattern(query->filename, dp->filename) &&
+ !match_pattern(query->filename,
+ kbasename(dp->filename)) &&
+ !match_pattern(query->filename,
+ trim_prefix(dp->filename)))
continue;

/* match against the function */
if (query->function &&
- strcmp(query->function, dp->function))
+ !match_pattern(query->function, dp->function))
continue;

/* match against the format */
--
1.8.1.2

[Joe Perches]

On Mon, 2013-10-28 at 23:29 +0800, Du, Changbin wrote:
> From: "Du, Changbin" <chang...@gmail.com>

trivial notes:

> This patch add wildcard '*'(matches zero or more characters) and '?'
> (matches one character) support when qurying debug flags.

> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c

[]
> @@ -127,6 +127,41 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
> query->first_lineno, query->last_lineno);
> }
>
> +/* check if the string matches given pattern which includes wildcards */
> +static int match_pattern(const char *pattern, const char *string)

bool match_pattern

> +{
> + const char *s, *p;
> + int star = 0;

bool star = false;

> +
> +loop:
> + for (s = string, p = pattern; *s; ++s, ++p) {
> + switch (*p) {
> + case '?':
> + break;
> + case '*':
> + star = 1;
> + string = s;
> + pattern = p;
> + if (!*++pattern)
> + return 1;
> + goto loop;
> + default:
> + if (*s != *p)
> + goto star_check;

star = false;

> + break;
> + }
> + }
> + if (*p == '*')
> + ++p;
> + return (!*p);

Don't need parentheses.

> +
> +star_check:
> + if (!star)
> + return 0;
> + string++;
> + goto loop;
> +}

The star_check: label block seems like it'd be
better in the switch case as it's only used once.

Maybe the thing would be more readable with a
while loop

[Changbin Du]

Hello, Joe,

Thanks for your comments. I will update the patch. And now I am trying
to remove all the two goto statements. Agree with you, we can use a
while statement instead.

I will send you the new patch for you to review when it's done.


2013/10/29 Joe Perches <j...@perches.com>

[Du, Changbin]

From: "Du, Changbin" <chang...@gmail.com>

This patch add wildcard '*'(matches zero or more characters) and '?'
(matches one character) support when qurying debug flags.

Now we can open debug messages using keywords. eg:
1. open debug logs in all usb drivers
echo "file drivers/usb/* +p" > <debugfs>/dynamic_debug/control
2. open debug logs for usb xhci code
echo "file *xhci* +p" > <debugfs>/dynamic_debug/control

Signed-off-by: Du, Changbin <chang...@gmail.com>
---

changes since v2:
remove the goto statements.
code style issue.
---
lib/dynamic_debug.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 44 insertions(+), 5 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c37aeac..b166d19 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -127,6 +127,43 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)

query->first_lineno, query->last_lineno);
}

+/* check if the string matches given pattern which includes wildcards */

+static bool match_pattern(const char *pattern, const char *string)
+{
+ const char *s = string,
+ *p = pattern;
+ bool star = 0;
+
+ while (*s) {

+ switch (*p) {
+ case '?':

+ ++s, ++p;

+ break;
+ case '*':

+ star = true;
+ string = s;
+ if (!*++p)
+ return true;
+ pattern = p;;
+ break;

+ default:
+ if (*s != *p) {

+ if (!star)
+ return false;
+ string++;
+ s = string;
+ p = pattern;
+ break;
+ }
+ ++s, ++p;
+ }

+ }
+
+ if (*p == '*')
+ ++p;

+ return !*p;

+}
+
/*
* Search the tables for _ddebug's which match the given `query' and
* apply the `flags' and `mask' to them. Returns number of matching

@@ -147,7 +184,7 @@ static int ddebug_change(const struct ddebug_query *query,

list_for_each_entry(dt, &ddebug_tables, link) {

/* match against the module name */
- if (query->module && strcmp(query->module, dt->mod_name))
+ if (query->module && !match_pattern(query->module, dt->mod_name))
continue;

for (i = 0; i < dt->num_ddebugs; i++) {

@@ -155,14 +192,16 @@ static int ddebug_change(const struct ddebug_query *query,

/* match against the source filename */
if (query->filename &&
- strcmp(query->filename, dp->filename) &&
- strcmp(query->filename, kbasename(dp->filename)) &&
- strcmp(query->filename, trim_prefix(dp->filename)))
+ !match_pattern(query->filename, dp->filename) &&
+ !match_pattern(query->filename,
+ kbasename(dp->filename)) &&
+ !match_pattern(query->filename,
+ trim_prefix(dp->filename)))
continue;

/* match against the function */
if (query->function &&
- strcmp(query->function, dp->function))
+ !match_pattern(query->function, dp->function))
continue;

/* match against the format */
--
1.8.1.2

[Joe Perches]

On Tue, 2013-10-29 at 21:33 +0800, Du, Changbin wrote:
> This patch add wildcard '*'(matches zero or more characters) and '?'
> (matches one character) support when qurying debug flags.

Hi again. Some trivial notes and a possible logic error:

> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c

[]
> @@ -127,6 +127,43 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
> query->first_lineno, query->last_lineno);
> }
>
> +/* check if the string matches given pattern which includes wildcards */
> +static bool match_pattern(const char *pattern, const char *string)
> +{
> + const char *s = string,
> + *p = pattern;

This sort of alignment is pretty unusual.
Most kernel uses just repeat the type like:

const char *s = string;
const char *p = pattern;

> + bool star = 0;

bool star = false;

> +
> + while (*s) {
> + switch (*p) {
> + case '?':
> + ++s, ++p;
> + break;
> + case '*':
> + star = true;
> + string = s;
> + if (!*++p)
> + return true;
> + pattern = p;;

repeated ;
Running your patches through checkpatch should find
this sort of defect.

> + break;
> + default:
> + if (*s != *p) {
> + if (!star)
> + return false;
> + string++;
> + s = string;
> + p = pattern;
> + break;
> + }
> + ++s, ++p;

Maybe nicer with an if/else, I think you're still
missing a reset of "star = false;" and I also think
it's better to use a break here too.

if (*s == *p) {
s++;
p++;
star = false;
} else {
if (!star)
return false;
string++;
s = string;
p = pattern;
}
break;

[Marcel Holtmann]

Hi Changbin,

> This patch add wildcard '*'(matches zero or more characters) and '?'
> (matches one character) support when qurying debug flags.
>
> Now we can open debug messages using keywords. eg:
> 1. open debug logs in all usb drivers
> echo "file drivers/usb/* +p" > <debugfs>/dynamic_debug/control
> 2. open debug logs for usb xhci code
> echo "file *xhci* +p" > <debugfs>/dynamic_debug/control

and this should have a section in Documentation/dynamic-debug-howto.txt.

Regards

Marcel

[Changbin Du]

2013/10/30 Marcel Holtmann <mar...@holtmann.org>:

> Hi Changbin,
>
>> This patch add wildcard '*'(matches zero or more characters) and '?'
>> (matches one character) support when qurying debug flags.
>>
>> Now we can open debug messages using keywords. eg:
>> 1. open debug logs in all usb drivers
>> echo "file drivers/usb/* +p" > <debugfs>/dynamic_debug/control
>> 2. open debug logs for usb xhci code
>> echo "file *xhci* +p" > <debugfs>/dynamic_debug/control
>
> and this should have a section in Documentation/dynamic-debug-howto.txt.

Yes, I will add a section to update the documentation also. Thanks for
reminding.

[Changbin Du]

2013/10/30 Joe Perches <j...@perches.com>:

> On Tue, 2013-10-29 at 21:33 +0800, Du, Changbin wrote:
>> This patch add wildcard '*'(matches zero or more characters) and '?'
>> (matches one character) support when qurying debug flags.
>
> Hi again. Some trivial notes and a possible logic error:
>

>> +/* check if the string matches given pattern which includes wildcards */
>> +static bool match_pattern(const char *pattern, const char *string)
>> +{
>> + const char *s = string,
>> + *p = pattern;
>
> This sort of alignment is pretty unusual.
> Most kernel uses just repeat the type like:
>
> const char *s = string;
> const char *p = pattern;
>

I think so.

>> + bool star = 0;
>
> bool star = false;
>
>> +
>> + while (*s) {
>> + switch (*p) {
>> + case '?':
>> + ++s, ++p;
>> + break;
>> + case '*':
>> + star = true;
>> + string = s;
>> + if (!*++p)
>> + return true;
>> + pattern = p;;
>
> repeated ;
> Running your patches through checkpatch should find
> this sort of defect.
>

Sorry, it's may fault. I forgot to check it using the tool.

I have run loss of test before sending patch. all case passed. But I
will double check if need reset star flag. really thanks!

[Changbin Du]

2013/10/30 Changbin Du <chang...@gmail.com>:

> 2013/10/30 Joe Perches <j...@perches.com>:
>> On Tue, 2013-10-29 at 21:33 +0800, Du, Changbin wrote:
>>> This patch add wildcard '*'(matches zero or more characters) and '?'
>>> (matches one character) support when qurying debug flags.
>>
>> Hi again. Some trivial notes and a possible logic error:
>>

>> Maybe nicer with an if/else, I think you're still
>> missing a reset of "star = false;" and I also think
>> it's better to use a break here too.
>>
>> if (*s == *p) {
>> s++;
>> p++;
>> star = false;
>> } else {
>> if (!star)
>> return false;
>> string++;
>> s = string;
>> p = pattern;
>> }
>> break;
>
> I have run loss of test before sending patch. all case passed. But I
> will double check if need reset star flag. really thanks!

Hi, Joe. I checked this. The "star = false;" can not have here.
Attachment is a test program that I use it to test the algorithm. it will
compare this non-recursion and old recursion if they are equal.

Now I will send the v3 patch, please help to review. Thanks!

[Du, Changbin]

From: "Du, Changbin" <chang...@gmail.com>

This patch add wildcard '*'(matches zero or more characters) and '?'
(matches one character) support when qurying debug flags.

Now we can open debug messages using keywords. eg:
1. open debug logs in all usb drivers
echo "file drivers/usb/* +p" > <debugfs>/dynamic_debug/control
2. open debug logs for usb xhci code
echo "file *xhci* +p" > <debugfs>/dynamic_debug/control

Signed-off-by: Du, Changbin <chang...@gmail.com>
---

lib/dynamic_debug.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 49 insertions(+), 5 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c37aeac..b953780 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -8,6 +8,7 @@
* By Greg Banks <g...@melbourne.sgi.com>
* Copyright (c) 2008 Silicon Graphics Inc. All Rights Reserved.
* Copyright (C) 2011 Bart Van Assche. All Rights Reserved.
+ * Copyright (C) 2013 Du, Changbin <chang...@gmail.com>
*/

#define pr_fmt(fmt) KBUILD_MODNAME ":%s: " fmt, __func__
@@ -127,6 +128,46 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
query->first_lineno, query->last_lineno);
}

+/* check if the string matches given pattern which includes wildcards */
+static bool match_pattern(const char *pattern, const char *string)
+{

+ const char *s = string;
+ const char *p = pattern;
+ bool star = false;

+
+ while (*s) {
+ switch (*p) {
+ case '?':

+ s++;
+ p++;

+ break;
+ case '*':
+ star = true;
+ string = s;
+ if (!*++p)
+ return true;
+ pattern = p;

+ break;
+ default:
+ if (*s == *p) {
+ s++;
+ p++;
+ } else {

+ if (!star)
+ return false;
+ string++;
+ s = string;
+ p = pattern;
+ }

+ break;

+ }
+ }
+
+ if (*p == '*')
+ ++p;
+ return !*p;
+}
+
/*
* Search the tables for _ddebug's which match the given `query' and
* apply the `flags' and `mask' to them. Returns number of matching

@@ -147,7 +188,8 @@ static int ddebug_change(const struct ddebug_query *query,

list_for_each_entry(dt, &ddebug_tables, link) {

/* match against the module name */
- if (query->module && strcmp(query->module, dt->mod_name))
+ if (query->module &&

+ !match_pattern(query->module, dt->mod_name))

continue;

for (i = 0; i < dt->num_ddebugs; i++) {

@@ -155,14 +197,16 @@ static int ddebug_change(const struct ddebug_query *query,

/* match against the source filename */
if (query->filename &&
- strcmp(query->filename, dp->filename) &&
- strcmp(query->filename, kbasename(dp->filename)) &&
- strcmp(query->filename, trim_prefix(dp->filename)))
+ !match_pattern(query->filename, dp->filename) &&
+ !match_pattern(query->filename,
+ kbasename(dp->filename)) &&
+ !match_pattern(query->filename,
+ trim_prefix(dp->filename)))
continue;

/* match against the function */
if (query->function &&
- strcmp(query->function, dp->function))
+ !match_pattern(query->function, dp->function))
continue;

/* match against the format */
--
1.8.1.2

[Andrew Morton]

On Mon, 28 Oct 2013 23:29:10 +0800 "Du, Changbin" <chang...@gmail.com> wrote:

> This patch add wildcard '*'(matches zero or more characters) and '?'
> (matches one character) support when qurying debug flags.
>
> Now we can open debug messages using keywords. eg:
> 1. open debug logs in all usb drivers
> echo "file drivers/usb/* +p" > <debugfs>/dynamic_debug/control
> 2. open debug logs for usb xhci code
> echo "file *xhci* +p" > <debugfs>/dynamic_debug/control

This patch would be a lot easier to understand (and hence review) if
you had remembered to update Documentation/dynamic-debug-howto.txt!

> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -127,6 +127,41 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
> query->first_lineno, query->last_lineno);
> }
>
> +/* check if the string matches given pattern which includes wildcards */
> +static int match_pattern(const char *pattern, const char *string)

No, something like this should be in lib/ so that other callers can use
it. We already have at least one copy handy in
drivers/ata/libata-core.c:glob_match(). A better approach would be to
move that glob_match() into lib/glob_match.c then teach dynamic_debug
to use it.

There are probably other private globbing functions lying around the
kernel, but it's rather a hard thing to grep for...

[Joe Perches]

On Thu, 2013-10-31 at 15:52 -0700, Andrew Morton wrote:
> On Mon, 28 Oct 2013 23:29:10 +0800 "Du, Changbin" <chang...@gmail.com> wrote:

[]

> > +/* check if the string matches given pattern which includes wildcards */
> > +static int match_pattern(const char *pattern, const char *string)
[]
> No, something like this should be in lib/ so that other callers can use
> it. We already have at least one copy handy in
> drivers/ata/libata-core.c:glob_match(). A better approach would be to
> move that glob_match() into lib/glob_match.c then teach dynamic_debug
> to use it.
>
> There are probably other private globbing functions lying around the
> kernel, but it's rather a hard thing to grep for...

Maybe use lib/parser.c where the other match_<foo> functions
are already.

match_glob has the disadvantage that it's recursive too.

trace has:

kernel/trace/trace_events_filter.c:static int regex_match_full(char *str, struct regex *r, int len)
kernel/trace/trace_events_filter.c:static int regex_match_front(char *str, struct regex *r, int len)
kernel/trace/trace_events_filter.c:static int regex_match_middle(char *str, struct regex *r, int len)
kernel/trace/trace_events_filter.c:static int regex_match_end(char *str, struct regex *r, int len)

but there probably aren't many more that could be converted.

[Changbin Du]

2013/11/1 Joe Perches <j...@perches.com>:

> On Thu, 2013-10-31 at 15:52 -0700, Andrew Morton wrote:
>> On Mon, 28 Oct 2013 23:29:10 +0800 "Du, Changbin" <chang...@gmail.com> wrote:
> []
>> > +/* check if the string matches given pattern which includes wildcards */
>> > +static int match_pattern(const char *pattern, const char *string)
> []
>> No, something like this should be in lib/ so that other callers can use
>> it. We already have at least one copy handy in
>> drivers/ata/libata-core.c:glob_match(). A better approach would be to
>> move that glob_match() into lib/glob_match.c then teach dynamic_debug
>> to use it.
>>
>> There are probably other private globbing functions lying around the
>> kernel, but it's rather a hard thing to grep for...
>
> Maybe use lib/parser.c where the other match_<foo> functions
> are already.

match_<foo> functions in lib/parser.c just do simple match, they
doesn't support wildcards.
So it's not useful for us.

>
> match_glob has the disadvantage that it's recursive too.
>

As you mentioned before, we cannot use it since strings are given from
userspace. :)

> trace has:
>
> kernel/trace/trace_events_filter.c:static int regex_match_full(char *str, struct regex *r, int len)
> kernel/trace/trace_events_filter.c:static int regex_match_front(char *str, struct regex *r, int len)
> kernel/trace/trace_events_filter.c:static int regex_match_middle(char *str, struct regex *r, int len)
> kernel/trace/trace_events_filter.c:static int regex_match_end(char *str, struct regex *r, int len)
>
> but there probably aren't many more that could be converted.

Trace filter support wildcards, but it seems the match algorithm
couples with trace code. I'll try to see
if it can be split.

[Changbin Du]

2013/11/1 Joe Perches <j...@perches.com>:

> On Thu, 2013-10-31 at 15:52 -0700, Andrew Morton wrote:
>> On Mon, 28 Oct 2013 23:29:10 +0800 "Du, Changbin" <chang...@gmail.com> wrote:
> []
>> > +/* check if the string matches given pattern which includes wildcards */
>> > +static int match_pattern(const char *pattern, const char *string)
> []
>> No, something like this should be in lib/ so that other callers can use
>> it. We already have at least one copy handy in
>> drivers/ata/libata-core.c:glob_match(). A better approach would be to
>> move that glob_match() into lib/glob_match.c then teach dynamic_debug
>> to use it.
>>
>> There are probably other private globbing functions lying around the
>> kernel, but it's rather a hard thing to grep for...
>
> Maybe use lib/parser.c where the other match_<foo> functions
> are already.

match_<foo> functions in lib/parser.c just do simple match, they
doesn't support wildcards.
So it's not useful for us.

>

> match_glob has the disadvantage that it's recursive too.
>

As you mentioned before, we cannot use it since strings are given from
userspace. :)

> trace has:
>
> kernel/trace/trace_events_filter.c:static int regex_match_full(char *str, struct regex *r, int len)
> kernel/trace/trace_events_filter.c:static int regex_match_front(char *str, struct regex *r, int len)
> kernel/trace/trace_events_filter.c:static int regex_match_middle(char *str, struct regex *r, int len)
> kernel/trace/trace_events_filter.c:static int regex_match_end(char *str, struct regex *r, int len)
>
> but there probably aren't many more that could be converted.

Trace filter support wildcards, but it seems the match algorithm
couples with trace code. I'll try to see
if it can be split.

[Joe Perches]

On Thu, 2013-11-07 at 11:11 +0800, Changbin Du wrote:
> 2013/11/1 Joe Perches <j...@perches.com>:
> > On Thu, 2013-10-31 at 15:52 -0700, Andrew Morton wrote:
> >> On Mon, 28 Oct 2013 23:29:10 +0800 "Du, Changbin" <chang...@gmail.com> wrote:
> > []
> >> > +/* check if the string matches given pattern which includes wildcards */
> >> > +static int match_pattern(const char *pattern, const char *string)
> > []
> >> No, something like this should be in lib/ so that other callers can use
> >> it. We already have at least one copy handy in
> >> drivers/ata/libata-core.c:glob_match(). A better approach would be to
> >> move that glob_match() into lib/glob_match.c then teach dynamic_debug
> >> to use it.
> >>
> >> There are probably other private globbing functions lying around the
> >> kernel, but it's rather a hard thing to grep for...
> >
> > Maybe use lib/parser.c where the other match_<foo> functions
> > are already.
>
> match_<foo> functions in lib/parser.c just do simple match, they
> doesn't support wildcards.
> So it's not useful for us.

It's not meant to be useful so much as be a possible
generic location for your match_regex function.

[Changbin Du]

2013/11/7 Joe Perches <j...@perches.com>:

> On Thu, 2013-11-07 at 11:11 +0800, Changbin Du wrote:
>> 2013/11/1 Joe Perches <j...@perches.com>:

>> match_<foo> functions in lib/parser.c just do simple match, they
>> doesn't support wildcards.
>> So it's not useful for us.
>
> It's not meant to be useful so much as be a possible
> generic location for your match_regex function.
>

I misunderstood you. This is a appropriate file to place the matching function.
I have moved it to this file.

I checked the regex_match_foo functions in trace_events_filter.c. Per my
reading, I think it's a little tedious, and only support '*' character. They are
glued with tracing framework. So i gave up to re-used them.

I will send new version patch set to you later.

Thanks!
Du, Changbin

[Du, Changbin]

From: "Du, Changbin" <chang...@gmail.com>

Add wildcard '*'(matches zero or more characters) and '?'

(matches one character) support when qurying debug flags.

Now we can open debug messages using keywords. eg:
1. open debug logs in all usb drivers
echo "file drivers/usb/* +p" > <debugfs>/dynamic_debug/control
2. open debug logs for usb xhci code
echo "file *xhci* +p" > <debugfs>/dynamic_debug/control

Signed-off-by: Du, Changbin <chang...@gmail.com>
---

lib/dynamic_debug.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c37aeac..600ac57 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c

@@ -8,6 +8,7 @@
* By Greg Banks <g...@melbourne.sgi.com>
* Copyright (c) 2008 Silicon Graphics Inc. All Rights Reserved.
* Copyright (C) 2011 Bart Van Assche. All Rights Reserved.
+ * Copyright (C) 2013 Du, Changbin <chang...@gmail.com>
*/

#define pr_fmt(fmt) KBUILD_MODNAME ":%s: " fmt, __func__

@@ -24,6 +25,7 @@
#include <linux/sysctl.h>
#include <linux/ctype.h>
#include <linux/string.h>
+#include <linux/parser.h>
#include <linux/string_helpers.h>
#include <linux/uaccess.h>
#include <linux/dynamic_debug.h>
@@ -147,7 +149,8 @@ static int ddebug_change(const struct ddebug_query *query,

list_for_each_entry(dt, &ddebug_tables, link) {

/* match against the module name */
- if (query->module && strcmp(query->module, dt->mod_name))
+ if (query->module &&

+ !match_wildcard(query->module, dt->mod_name))

continue;

for (i = 0; i < dt->num_ddebugs; i++) {

@@ -155,14 +158,16 @@ static int ddebug_change(const struct ddebug_query *query,

/* match against the source filename */
if (query->filename &&
- strcmp(query->filename, dp->filename) &&
- strcmp(query->filename, kbasename(dp->filename)) &&
- strcmp(query->filename, trim_prefix(dp->filename)))

+ !match_wildcard(query->filename, dp->filename) &&
+ !match_wildcard(query->filename,
+ kbasename(dp->filename)) &&
+ !match_wildcard(query->filename,

+ trim_prefix(dp->filename)))
continue;

/* match against the function */
if (query->function &&
- strcmp(query->function, dp->function))

+ !match_wildcard(query->function, dp->function))

continue;

/* match against the format */
--

1.8.3.2

[Du, Changbin]

From: "Du, Changbin" <chang...@gmail.com>

These patches are to make it easier to filter kernel debug logs which
we want.
Whith wildcard support, below command can enable all usb debug logs:
#echo "file drivers/usb/* +p" > <debugfs>/dynamic_debug/control
This patch only enables two wildcard:
'*' - matches zero or more characters
'?' - matches one character

Changes since v4:
moved matching function to lib/parser.c

Du, Changbin (3):
lib/parser.c: add match_wildcard function
dynamic_debug: add wildcard support to filter files/functions/modules
dynamic-debug-howto.txt: update since new wildcard support

Documentation/dynamic-debug-howto.txt | 9 +++++++
include/linux/parser.h | 1 +
lib/dynamic_debug.c | 15 +++++++----
lib/parser.c | 51 +++++++++++++++++++++++++++++++++++
4 files changed, 71 insertions(+), 5 deletions(-)

--
1.8.3.2

[Du, Changbin]

From: "Du, Changbin" <chang...@gmail.com>

match_wildcard function is a simple implementation of wildcard
matching algorithm. It only supports two usual wildcardes:

'*' - matches zero or more characters
'?' - matches one character

This algorithm is safe since it's of non-recursion.

Signed-off-by: Du, Changbin <chang...@gmail.com>
---

include/linux/parser.h | 1 +
lib/parser.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)

diff --git a/include/linux/parser.h b/include/linux/parser.h
index ea2281e..39d5b79 100644
--- a/include/linux/parser.h
+++ b/include/linux/parser.h
@@ -29,5 +29,6 @@ int match_token(char *, const match_table_t table, substring_t args[]);
int match_int(substring_t *, int *result);
int match_octal(substring_t *, int *result);
int match_hex(substring_t *, int *result);
+bool match_wildcard(const char *pattern, const char *str);
size_t match_strlcpy(char *, const substring_t *, size_t);
char *match_strdup(const substring_t *);
diff --git a/lib/parser.c b/lib/parser.c
index 807b2aa..ee52955 100644
--- a/lib/parser.c
+++ b/lib/parser.c
@@ -193,6 +193,56 @@ int match_hex(substring_t *s, int *result)
}

/**
+ * match_wildcard: - parse if a string matches given wildcard pattern
+ * @pattern: wildcard pattern
+ * @str: the string to be parsed
+ *
+ * Description: Parse the string @str to check if matches wildcard
+ * pattern @pattern. The pattern may contain two type wildcardes:
+ * '*' - matches zero or more characters
+ * '?' - matches one character
+ * If it's matched, return true, else return false.
+ */
+bool match_wildcard(const char *pattern, const char *str)
+{
+ const char *s = str;

+ const char *p = pattern;
+ bool star = false;
+
+ while (*s) {
+ switch (*p) {
+ case '?':
+ s++;
+ p++;
+ break;
+ case '*':
+ star = true;

+ str = s;

+ if (!*++p)
+ return true;
+ pattern = p;
+ break;
+ default:
+ if (*s == *p) {
+ s++;
+ p++;
+ } else {
+ if (!star)
+ return false;

+ str++;
+ s = str;

+ p = pattern;
+ }
+ break;
+ }
+ }
+
+ if (*p == '*')
+ ++p;
+ return !*p;
+}
+

+/**
* match_strlcpy: - Copy the characters from a substring_t to a sized buffer
* @dest: where to copy to
* @src: &substring_t to copy
@@ -235,5 +285,6 @@ EXPORT_SYMBOL(match_token);
EXPORT_SYMBOL(match_int);
EXPORT_SYMBOL(match_octal);
EXPORT_SYMBOL(match_hex);
+EXPORT_SYMBOL(match_wildcard);
EXPORT_SYMBOL(match_strlcpy);
EXPORT_SYMBOL(match_strdup);

[Du, Changbin]

From: "Du, Changbin" <chang...@gmail.com>

Add the usage of using new feature wildcard support.

Signed-off-by: Du, Changbin <chang...@gmail.com>
---

Documentation/dynamic-debug-howto.txt | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/Documentation/dynamic-debug-howto.txt b/Documentation/dynamic-debug-howto.txt
index 1bbdcfc..46325eb 100644
--- a/Documentation/dynamic-debug-howto.txt
+++ b/Documentation/dynamic-debug-howto.txt
@@ -108,6 +108,12 @@ If your query set is big, you can batch them too:

~# cat query-batch-file > <debugfs>/dynamic_debug/control

+A another way is to use wildcard. The match rule support '*' (matches
+zero or more characters) and '?' (matches exactly one character).For
+example, you can match all usb drivers:
+
+ ~# echo "file drivers/usb/* +p" > <debugfs>/dynamic_debug/control
+
At the syntactical level, a command comprises a sequence of match
specifications, followed by a flags change specification.

@@ -315,6 +321,9 @@ nullarbor:~ # echo -n 'func svc_process -p' >
nullarbor:~ # echo -n 'format "nfsd: READ" +p' >
<debugfs>/dynamic_debug/control

+// enable messages in files of which the pathes include string "usb"
+nullarbor:~ # echo -n '*usb* +p' > <debugfs>/dynamic_debug/control
+
// enable all messages
nullarbor:~ # echo -n '+p' > <debugfs>/dynamic_debug/control