linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dynamic_debug: add wildcard support to filter files/functions/modules
@ 2013-07-25 13:02 Du, Changbin
  2013-07-25 16:47 ` Joe Perches
  0 siblings, 1 reply; 24+ messages in thread
From: Du, Changbin @ 2013-07-25 13:02 UTC (permalink / raw)
  To: jbaron; +Cc: linux-kernel, changbin.du

From: "Du, Changbin" <changbin.du@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 <changbin.du@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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-07-25 13:02 [PATCH] dynamic_debug: add wildcard support to filter files/functions/modules Du, Changbin
@ 2013-07-25 16:47 ` Joe Perches
  2013-07-30  3:59   ` Jason Baron
  0 siblings, 1 reply; 24+ messages in thread
From: Joe Perches @ 2013-07-25 16:47 UTC (permalink / raw)
  To: Du, Changbin; +Cc: jbaron, linux-kernel

On Thu, 2013-07-25 at 21:02 +0800, Du, Changbin wrote:
> From: "Du, Changbin" <changbin.du@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.



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-07-25 16:47 ` Joe Perches
@ 2013-07-30  3:59   ` Jason Baron
  2013-10-28 15:29     ` [PATCH v2] " Du, Changbin
  0 siblings, 1 reply; 24+ messages in thread
From: Jason Baron @ 2013-07-30  3:59 UTC (permalink / raw)
  To: Joe Perches; +Cc: Du, Changbin, linux-kernel

On 07/25/2013 12:47 PM, Joe Perches wrote:
> On Thu, 2013-07-25 at 21:02 +0800, Du, Changbin wrote:
>> From: "Du, Changbin" <changbin.du@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.

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




^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-07-30  3:59   ` Jason Baron
@ 2013-10-28 15:29     ` Du, Changbin
  2013-10-28 16:30       ` Joe Perches
  2013-10-31 22:52       ` [PATCH v2] " Andrew Morton
  0 siblings, 2 replies; 24+ messages in thread
From: Du, Changbin @ 2013-10-28 15:29 UTC (permalink / raw)
  To: jbaron; +Cc: joe, linux-kernel, Du, Changbin

From: "Du, Changbin" <changbin.du@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 <changbin.du@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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-28 15:29     ` [PATCH v2] " Du, Changbin
@ 2013-10-28 16:30       ` Joe Perches
  2013-10-29  8:04         ` Changbin Du
  2013-10-31 22:52       ` [PATCH v2] " Andrew Morton
  1 sibling, 1 reply; 24+ messages in thread
From: Joe Perches @ 2013-10-28 16:30 UTC (permalink / raw)
  To: Du, Changbin; +Cc: jbaron, linux-kernel

On Mon, 2013-10-28 at 23:29 +0800, Du, Changbin wrote:
> From: "Du, Changbin" <changbin.du@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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-28 16:30       ` Joe Perches
@ 2013-10-29  8:04         ` Changbin Du
  2013-10-29 13:33           ` [PATCH v3] " Du, Changbin
  0 siblings, 1 reply; 24+ messages in thread
From: Changbin Du @ 2013-10-29  8:04 UTC (permalink / raw)
  To: Joe Perches; +Cc: Jason Baron, linux-kernel

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 <joe@perches.com>
>
> On Mon, 2013-10-28 at 23:29 +0800, Du, Changbin wrote:
> > From: "Du, Changbin" <changbin.du@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
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-29  8:04         ` Changbin Du
@ 2013-10-29 13:33           ` Du, Changbin
  2013-10-29 16:20             ` Joe Perches
  2013-10-29 20:21             ` [PATCH v3] dynamic_debug: add wildcard support to filter files/functions/modules Marcel Holtmann
  0 siblings, 2 replies; 24+ messages in thread
From: Du, Changbin @ 2013-10-29 13:33 UTC (permalink / raw)
  To: jbaron, joe; +Cc: linux-kernel, Du, Changbin

From: "Du, Changbin" <changbin.du@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 <changbin.du@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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v3] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-29 13:33           ` [PATCH v3] " Du, Changbin
@ 2013-10-29 16:20             ` Joe Perches
  2013-10-30  6:58               ` Changbin Du
  2013-10-29 20:21             ` [PATCH v3] dynamic_debug: add wildcard support to filter files/functions/modules Marcel Holtmann
  1 sibling, 1 reply; 24+ messages in thread
From: Joe Perches @ 2013-10-29 16:20 UTC (permalink / raw)
  To: Du, Changbin; +Cc: jbaron, linux-kernel

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;




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-29 13:33           ` [PATCH v3] " Du, Changbin
  2013-10-29 16:20             ` Joe Perches
@ 2013-10-29 20:21             ` Marcel Holtmann
  2013-10-30  6:24               ` Changbin Du
  1 sibling, 1 reply; 24+ messages in thread
From: Marcel Holtmann @ 2013-10-29 20:21 UTC (permalink / raw)
  To: Du, Changbin; +Cc: jbaron, Joe Perches, linux-kernel@vger.kernel.org list

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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-29 20:21             ` [PATCH v3] dynamic_debug: add wildcard support to filter files/functions/modules Marcel Holtmann
@ 2013-10-30  6:24               ` Changbin Du
  0 siblings, 0 replies; 24+ messages in thread
From: Changbin Du @ 2013-10-30  6:24 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Jason Baron, Joe Perches, linux-kernel@vger.kernel.org list

2013/10/30 Marcel Holtmann <marcel@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.

>
> Regards
>
> Marcel
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-29 16:20             ` Joe Perches
@ 2013-10-30  6:58               ` Changbin Du
  2013-10-30 13:57                 ` Changbin Du
  0 siblings, 1 reply; 24+ messages in thread
From: Changbin Du @ 2013-10-30  6:58 UTC (permalink / raw)
  To: Joe Perches; +Cc: Jason Baron, linux-kernel@vger.kernel.org list

2013/10/30 Joe Perches <joe@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.

>> +                     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;

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-30  6:58               ` Changbin Du
@ 2013-10-30 13:57                 ` Changbin Du
  2013-10-30 14:21                   ` [PATCH v4 1/2] " Du, Changbin
  2013-10-30 14:21                   ` [PATCH v4 2/2] dynamic-debug-howto.txt: update since new wildcard support Du, Changbin
  0 siblings, 2 replies; 24+ messages in thread
From: Changbin Du @ 2013-10-30 13:57 UTC (permalink / raw)
  To: Joe Perches; +Cc: Jason Baron, linux-kernel@vger.kernel.org list

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

2013/10/30 Changbin Du <changbin.du@gmail.com>:
> 2013/10/30 Joe Perches <joe@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!

[-- Attachment #2: wildcard_alg.c --]
[-- Type: text/x-csrc, Size: 3230 bytes --]

#include <stdio.h>
#include <stdlib.h>

#define bool int
#define false 0
#define true  1

static int match_pattern1(char *pat, char *str)
{
	switch (*pat) {
	case '\0':
		return !*str;
	case '*':
		return  match_pattern1(pat+1, str) ||
		        (*str && match_pattern1(pat, str+1));
	case '?':
		return *str && match_pattern1(pat+1, str+1);
	default:
		return *pat == *str && match_pattern1(pat+1, str+1);
	}
}

static bool match_pattern2(const char *pattern, const char *string)
{
	const char *s;
	const char *p;
	bool star = false;

loop:
	for (s = string, p = pattern; *s; ++s, ++p) {
		switch (*p) {
		case '?':
			break;
		case '*':
			star = true;
			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;
}

static bool match_pattern3(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;
}



/* return 0 if all the result of three function is same, else return 1. */
static int test_match()
{
	char *Str[] = { "ZIP",
	                ".ZIP",
	                "ZIP.",
	                "afdZfdaZIP"
	                "adfsafZfdsadfIfdafdsPfdasdf",
	                "accbddcrrfddhuulggnffphhqggyyyrnnvhgflllmmnnnnkpi.iuuuiyt",
	                "A.bkdfadfasfa.faskfa.sfaf?kl.A.ZIP",
	                "Asdhgerlthwegjsdklgjsdgjsdkgjsdgjsdg.ZIP",
	                "AAgsdjtweoruterjtertweiutwejtwejtwetwoejtwejtrwleAA.ZIP",
	                "Agjsdgjdsjgsdkjgsdjgsjd?gjsd?gjsd?gj?sdgj.A.ZIdgjsdkjglsdjgPPO",
	                "fasdfe323rerteter55rtrewtrwwe"
	              };
	char *Patt[] = { "?ZIP", "*ZIP", "ZIP.", "*ZIP", "*?ZIP", "?*?ZIP", "*?*?ZIP", "*Z*I*P*",
	                 "*?*?*ZIP", "a*?*?*ZIP", "*ZI?", "*ZI?", "*ZI*", "*ZI*",
	                 "a*?*?", "?*?*","ZIP", "ZIP", "AAZIP", "AZIP", "AAAAZIP", "AAZIPPO"
	               };
	int i = 0, j = 0, ret = 0;

	for (i = 0 ; i < sizeof(Patt)/sizeof(Patt[0]); i++) {
		for (j = 0; j < sizeof(Str)/sizeof(Str[0]); j++) {
			char *pat = Patt[i];
			char *str = Str[j];
			bool match1 = match_pattern1(pat, str);
			bool match2 = match_pattern2(pat, str);
			bool match3 = match_pattern3(pat, str);

			printf("\"%s\" =?= \"%s\"\n", pat, str);
			if (match1 == match2 && match2 == match3)
				printf("All three result is same: %d\n", match1);
			else {
				printf("****\n"
				       "****Someone is wrong: match1=%d match2=%d match3=%d\n"
				       "****\n",
				       match1, match2, match3);
				ret = 1;
			}
			printf("\n");
		}
		printf("\n");
	}
	return ret;
}

int main()
{
	return test_match();
}

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v4 1/2] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-30 13:57                 ` Changbin Du
@ 2013-10-30 14:21                   ` Du, Changbin
  2013-10-30 14:21                   ` [PATCH v4 2/2] dynamic-debug-howto.txt: update since new wildcard support Du, Changbin
  1 sibling, 0 replies; 24+ messages in thread
From: Du, Changbin @ 2013-10-30 14:21 UTC (permalink / raw)
  To: jbaron, joe; +Cc: linux-kernel, marcel, Du, Changbin

From: "Du, Changbin" <changbin.du@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 <changbin.du@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 <gnb@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 <changbin.du@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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v4 2/2] dynamic-debug-howto.txt: update since new wildcard support
  2013-10-30 13:57                 ` Changbin Du
  2013-10-30 14:21                   ` [PATCH v4 1/2] " Du, Changbin
@ 2013-10-30 14:21                   ` Du, Changbin
  1 sibling, 0 replies; 24+ messages in thread
From: Du, Changbin @ 2013-10-30 14:21 UTC (permalink / raw)
  To: jbaron, joe; +Cc: linux-kernel, marcel, Du, Changbin

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

Add the usage of using new feature wildcard support.

Signed-off-by: Du, Changbin <changbin.du@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
 
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-28 15:29     ` [PATCH v2] " Du, Changbin
  2013-10-28 16:30       ` Joe Perches
@ 2013-10-31 22:52       ` Andrew Morton
  2013-10-31 23:30         ` Joe Perches
  1 sibling, 1 reply; 24+ messages in thread
From: Andrew Morton @ 2013-10-31 22:52 UTC (permalink / raw)
  To: Du, Changbin; +Cc: jbaron, joe, linux-kernel

On Mon, 28 Oct 2013 23:29:10 +0800 "Du, Changbin" <changbin.du@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...

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-31 22:52       ` [PATCH v2] " Andrew Morton
@ 2013-10-31 23:30         ` Joe Perches
  2013-11-07  3:04           ` Changbin Du
  2013-11-07  3:11           ` Changbin Du
  0 siblings, 2 replies; 24+ messages in thread
From: Joe Perches @ 2013-10-31 23:30 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Du, Changbin, jbaron, linux-kernel

On Thu, 2013-10-31 at 15:52 -0700, Andrew Morton wrote:
> On Mon, 28 Oct 2013 23:29:10 +0800 "Du, Changbin" <changbin.du@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.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-31 23:30         ` Joe Perches
@ 2013-11-07  3:04           ` Changbin Du
  2013-11-07  3:11           ` Changbin Du
  1 sibling, 0 replies; 24+ messages in thread
From: Changbin Du @ 2013-11-07  3:04 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, Jason Baron, linux-kernel@vger.kernel.org list

2013/11/1 Joe Perches <joe@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" <changbin.du@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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-10-31 23:30         ` Joe Perches
  2013-11-07  3:04           ` Changbin Du
@ 2013-11-07  3:11           ` Changbin Du
  2013-11-07  6:12             ` Joe Perches
  1 sibling, 1 reply; 24+ messages in thread
From: Changbin Du @ 2013-11-07  3:11 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, Jason Baron, linux-kernel@vger.kernel.org list

2013/11/1 Joe Perches <joe@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" <changbin.du@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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-11-07  3:11           ` Changbin Du
@ 2013-11-07  6:12             ` Joe Perches
  2013-11-15  4:01               ` Changbin Du
  0 siblings, 1 reply; 24+ messages in thread
From: Joe Perches @ 2013-11-07  6:12 UTC (permalink / raw)
  To: Changbin Du; +Cc: Andrew Morton, Jason Baron, linux-kernel@vger.kernel.org list

On Thu, 2013-11-07 at 11:11 +0800, Changbin Du wrote:
> 2013/11/1 Joe Perches <joe@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" <changbin.du@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.



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-11-07  6:12             ` Joe Perches
@ 2013-11-15  4:01               ` Changbin Du
  2013-11-16  8:24                 ` [PATCH v5 0/3] add wildcard support for dynamic debug Du, Changbin
                                   ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: Changbin Du @ 2013-11-15  4:01 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, Jason Baron, linux-kernel@vger.kernel.org list

2013/11/7 Joe Perches <joe@perches.com>:
> On Thu, 2013-11-07 at 11:11 +0800, Changbin Du wrote:
>> 2013/11/1 Joe Perches <joe@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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v5 0/3] add wildcard support for dynamic debug
  2013-11-15  4:01               ` Changbin Du
@ 2013-11-16  8:24                 ` Du, Changbin
  2013-11-16  8:24                 ` [PATCH v5 1/3] lib/parser.c: add match_wildcard function Du, Changbin
                                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 24+ messages in thread
From: Du, Changbin @ 2013-11-16  8:24 UTC (permalink / raw)
  To: jbaron, joe; +Cc: linux-kernel, marcel, akpm, Du, Changbin

From: "Du, Changbin" <changbin.du@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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v5 1/3] lib/parser.c: add match_wildcard function
  2013-11-15  4:01               ` Changbin Du
  2013-11-16  8:24                 ` [PATCH v5 0/3] add wildcard support for dynamic debug Du, Changbin
@ 2013-11-16  8:24                 ` Du, Changbin
  2013-11-16  8:24                 ` [PATCH v5 2/3] dynamic_debug: add wildcard support to filter files/functions/modules Du, Changbin
  2013-11-16  8:24                 ` [PATCH v5 3/3] dynamic-debug-howto.txt: update since new wildcard support Du, Changbin
  3 siblings, 0 replies; 24+ messages in thread
From: Du, Changbin @ 2013-11-16  8:24 UTC (permalink / raw)
  To: jbaron, joe; +Cc: linux-kernel, marcel, akpm, Du, Changbin

From: "Du, Changbin" <changbin.du@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 <changbin.du@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);
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v5 2/3] dynamic_debug: add wildcard support to filter files/functions/modules
  2013-11-15  4:01               ` Changbin Du
  2013-11-16  8:24                 ` [PATCH v5 0/3] add wildcard support for dynamic debug Du, Changbin
  2013-11-16  8:24                 ` [PATCH v5 1/3] lib/parser.c: add match_wildcard function Du, Changbin
@ 2013-11-16  8:24                 ` Du, Changbin
  2013-11-16  8:24                 ` [PATCH v5 3/3] dynamic-debug-howto.txt: update since new wildcard support Du, Changbin
  3 siblings, 0 replies; 24+ messages in thread
From: Du, Changbin @ 2013-11-16  8:24 UTC (permalink / raw)
  To: jbaron, joe; +Cc: linux-kernel, marcel, akpm, Du, Changbin

From: "Du, Changbin" <changbin.du@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 <changbin.du@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 <gnb@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 <changbin.du@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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v5 3/3] dynamic-debug-howto.txt: update since new wildcard support
  2013-11-15  4:01               ` Changbin Du
                                   ` (2 preceding siblings ...)
  2013-11-16  8:24                 ` [PATCH v5 2/3] dynamic_debug: add wildcard support to filter files/functions/modules Du, Changbin
@ 2013-11-16  8:24                 ` Du, Changbin
  3 siblings, 0 replies; 24+ messages in thread
From: Du, Changbin @ 2013-11-16  8:24 UTC (permalink / raw)
  To: jbaron, joe; +Cc: linux-kernel, marcel, akpm, Du, Changbin

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

Add the usage of using new feature wildcard support.

Signed-off-by: Du, Changbin <changbin.du@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
 
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2013-11-16  8:25 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-25 13:02 [PATCH] dynamic_debug: add wildcard support to filter files/functions/modules Du, Changbin
2013-07-25 16:47 ` Joe Perches
2013-07-30  3:59   ` Jason Baron
2013-10-28 15:29     ` [PATCH v2] " Du, Changbin
2013-10-28 16:30       ` Joe Perches
2013-10-29  8:04         ` Changbin Du
2013-10-29 13:33           ` [PATCH v3] " Du, Changbin
2013-10-29 16:20             ` Joe Perches
2013-10-30  6:58               ` Changbin Du
2013-10-30 13:57                 ` Changbin Du
2013-10-30 14:21                   ` [PATCH v4 1/2] " Du, Changbin
2013-10-30 14:21                   ` [PATCH v4 2/2] dynamic-debug-howto.txt: update since new wildcard support Du, Changbin
2013-10-29 20:21             ` [PATCH v3] dynamic_debug: add wildcard support to filter files/functions/modules Marcel Holtmann
2013-10-30  6:24               ` Changbin Du
2013-10-31 22:52       ` [PATCH v2] " Andrew Morton
2013-10-31 23:30         ` Joe Perches
2013-11-07  3:04           ` Changbin Du
2013-11-07  3:11           ` Changbin Du
2013-11-07  6:12             ` Joe Perches
2013-11-15  4:01               ` Changbin Du
2013-11-16  8:24                 ` [PATCH v5 0/3] add wildcard support for dynamic debug Du, Changbin
2013-11-16  8:24                 ` [PATCH v5 1/3] lib/parser.c: add match_wildcard function Du, Changbin
2013-11-16  8:24                 ` [PATCH v5 2/3] dynamic_debug: add wildcard support to filter files/functions/modules Du, Changbin
2013-11-16  8:24                 ` [PATCH v5 3/3] dynamic-debug-howto.txt: update since new wildcard support Du, Changbin

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).