linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tracing: Fix event filters and triggers to handle negative numbers
@ 2018-08-23 10:25 Pavel Tikhomirov
  2018-08-24  0:48 ` Steven Rostedt
  0 siblings, 1 reply; 4+ messages in thread
From: Pavel Tikhomirov @ 2018-08-23 10:25 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Ingo Molnar, Al Viro, linux-kernel, Pavel Tikhomirov

Then tracing syscall exit event it is extremely useful to filter exit
codes equal to some negative value, to react only to required errors.
But negative numbers does not work:

[root@snorch sys_exit_read]# echo "ret == -1" > filter
bash: echo: write error: Invalid argument
[root@snorch sys_exit_read]# cat filter
ret == -1
        ^
parse_error: Invalid value (did you forget quotes)?

Similar thing happens when setting triggers.

These is a regression in v4.17 introduced by the commit mentioned below,
testing without these commit shows no problem with negative numbers.

Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 kernel/trace/trace_events_filter.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 84a65173b1e9..2ba449292561 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1299,7 +1299,7 @@ static int parse_pred(const char *str, void *data,
 		/* go past the last quote */
 		i++;
 
-	} else if (isdigit(str[i])) {
+	} else if (isdigit(str[i]) || str[i] == '-') {
 
 		/* Make sure the field is not a string */
 		if (is_string_field(field)) {
@@ -1312,6 +1312,9 @@ static int parse_pred(const char *str, void *data,
 			goto err_free;
 		}
 
+		if (str[i] == '-')
+			i++;
+
 		/* We allow 0xDEADBEEF */
 		while (isalnum(str[i]))
 			i++;
-- 
2.17.1


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

* Re: [PATCH] tracing: Fix event filters and triggers to handle negative numbers
  2018-08-23 10:25 [PATCH] tracing: Fix event filters and triggers to handle negative numbers Pavel Tikhomirov
@ 2018-08-24  0:48 ` Steven Rostedt
  2019-02-28  9:11   ` Pavel Tikhomirov
  0 siblings, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2018-08-24  0:48 UTC (permalink / raw)
  To: Pavel Tikhomirov; +Cc: Ingo Molnar, Al Viro, linux-kernel

On Thu, 23 Aug 2018 13:25:34 +0300
Pavel Tikhomirov <ptikhomirov@virtuozzo.com> wrote:

> Then tracing syscall exit event it is extremely useful to filter exit
> codes equal to some negative value, to react only to required errors.
> But negative numbers does not work:
> 
> [root@snorch sys_exit_read]# echo "ret == -1" > filter
> bash: echo: write error: Invalid argument
> [root@snorch sys_exit_read]# cat filter
> ret == -1
>         ^
> parse_error: Invalid value (did you forget quotes)?

Thanks for the patch. I'll apply it and then start testing it!

-- Steve

> 
> Similar thing happens when setting triggers.
> 
> These is a regression in v4.17 introduced by the commit mentioned below,
> testing without these commit shows no problem with negative numbers.
> 
> Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
>  kernel/trace/trace_events_filter.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 84a65173b1e9..2ba449292561 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1299,7 +1299,7 @@ static int parse_pred(const char *str, void *data,
>  		/* go past the last quote */
>  		i++;
>  
> -	} else if (isdigit(str[i])) {
> +	} else if (isdigit(str[i]) || str[i] == '-') {
>  
>  		/* Make sure the field is not a string */
>  		if (is_string_field(field)) {
> @@ -1312,6 +1312,9 @@ static int parse_pred(const char *str, void *data,
>  			goto err_free;
>  		}
>  
> +		if (str[i] == '-')
> +			i++;
> +
>  		/* We allow 0xDEADBEEF */
>  		while (isalnum(str[i]))
>  			i++;


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

* Re: [PATCH] tracing: Fix event filters and triggers to handle negative numbers
  2018-08-24  0:48 ` Steven Rostedt
@ 2019-02-28  9:11   ` Pavel Tikhomirov
  2019-02-28 16:19     ` Steven Rostedt
  0 siblings, 1 reply; 4+ messages in thread
From: Pavel Tikhomirov @ 2019-02-28  9:11 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Ingo Molnar, Al Viro, linux-kernel

ping, looks like the patch was lost

On 8/24/18 3:48 AM, Steven Rostedt wrote:
> On Thu, 23 Aug 2018 13:25:34 +0300
> Pavel Tikhomirov <ptikhomirov@virtuozzo.com> wrote:
> 
>> Then tracing syscall exit event it is extremely useful to filter exit
>> codes equal to some negative value, to react only to required errors.
>> But negative numbers does not work:
>>
>> [root@snorch sys_exit_read]# echo "ret == -1" > filter
>> bash: echo: write error: Invalid argument
>> [root@snorch sys_exit_read]# cat filter
>> ret == -1
>>          ^
>> parse_error: Invalid value (did you forget quotes)?
> 
> Thanks for the patch. I'll apply it and then start testing it!
> 
> -- Steve
> 
>>
>> Similar thing happens when setting triggers.
>>
>> These is a regression in v4.17 introduced by the commit mentioned below,
>> testing without these commit shows no problem with negative numbers.
>>
>> Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
>> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>> ---
>>   kernel/trace/trace_events_filter.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
>> index 84a65173b1e9..2ba449292561 100644
>> --- a/kernel/trace/trace_events_filter.c
>> +++ b/kernel/trace/trace_events_filter.c
>> @@ -1299,7 +1299,7 @@ static int parse_pred(const char *str, void *data,
>>   		/* go past the last quote */
>>   		i++;
>>   
>> -	} else if (isdigit(str[i])) {
>> +	} else if (isdigit(str[i]) || str[i] == '-') {
>>   
>>   		/* Make sure the field is not a string */
>>   		if (is_string_field(field)) {
>> @@ -1312,6 +1312,9 @@ static int parse_pred(const char *str, void *data,
>>   			goto err_free;
>>   		}
>>   
>> +		if (str[i] == '-')
>> +			i++;
>> +
>>   		/* We allow 0xDEADBEEF */
>>   		while (isalnum(str[i]))
>>   			i++;
> 

-- 
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.

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

* Re: [PATCH] tracing: Fix event filters and triggers to handle negative numbers
  2019-02-28  9:11   ` Pavel Tikhomirov
@ 2019-02-28 16:19     ` Steven Rostedt
  0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2019-02-28 16:19 UTC (permalink / raw)
  To: Pavel Tikhomirov; +Cc: Ingo Molnar, Al Viro, linux-kernel

On Thu, 28 Feb 2019 09:11:37 +0000
Pavel Tikhomirov <ptikhomirov@virtuozzo.com> wrote:

> ping, looks like the patch was lost
> 
> On 8/24/18 3:48 AM, Steven Rostedt wrote:
> > On Thu, 23 Aug 2018 13:25:34 +0300
> > Pavel Tikhomirov <ptikhomirov@virtuozzo.com> wrote:
> >   
> >> Then tracing syscall exit event it is extremely useful to filter exit
> >> codes equal to some negative value, to react only to required errors.
> >> But negative numbers does not work:
> >>
> >> [root@snorch sys_exit_read]# echo "ret == -1" > filter
> >> bash: echo: write error: Invalid argument
> >> [root@snorch sys_exit_read]# cat filter
> >> ret == -1
> >>          ^
> >> parse_error: Invalid value (did you forget quotes)?  
> > 
> > Thanks for the patch. I'll apply it and then start testing it!

?!?

I remember applying this to my git queue (I don't reply like this
before doing so). Not sure how it got lost. All I can think of is that
it might have been added with commits that ended up failing the tests,
and got reverted with that code :-/

I'm currently traveling. Hopefully I remember tomorrow to add this.
Thanks for pointing this out, and I need to be a bit more careful when
reverting failed commits :-p

-- Steve

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

end of thread, other threads:[~2019-02-28 16:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-23 10:25 [PATCH] tracing: Fix event filters and triggers to handle negative numbers Pavel Tikhomirov
2018-08-24  0:48 ` Steven Rostedt
2019-02-28  9:11   ` Pavel Tikhomirov
2019-02-28 16:19     ` Steven Rostedt

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