Re: [PATCH v2] tracing/filters: Improve subsystem filter

[Tom Zanussi]

On Tue, 2009-07-21 at 09:46 +0800, Li Zefan wrote:
> >> Proposal 2:
> >>
> >>   # cat filter
> >>   irq_handler_entry: irq == 5
> >>   irq_handler_exit: none
> >>   softirq_entry: vec == 1
> >>   softirq_exit: vec == 2
> > 
> > I like proposal 2, it is very intuitive.
> > 
> 
> Me too.

Me three.  It's nice to see all the filters for the subsystem in one
place without having to descend into the event subdirs.  It might also
be nice to see which events are enabled by looking at the subsystem
'enable' file too.  Or maybe the subsystem filter file should show only
filters for enabled events...

> 
> >> Which do you think is better? Or do you have some better idea?
> >>
> >> And in the failure case:
> >>
> >>   # echo 'irq == not_num' > filter
> >>   bash: echo: write error: Invalid argument
> >>
> >> 1:
> >>   # cat filter
> >>   (still shows filters in each event like above)
> >>
> >> or shows error message (the current behavior)
> > 
> > No need to show error messages of failed filter modifications in the 
> > "filter" file.
> > 
> >> 2:
> >>   # cat filter
> >>   irq == not_num
> >>   ^
> >>   parse_error: Couldn't find or set field in one of a subsystem's events
> > 
> > This looks good, BUT, it is too much. If you want to implement an error 
> > message like the above, it should probably be a "pr_info()" thing.
> > 
> 
> Yeah, I think it's too much too, but that's exactly what we have.
> And I posted a patch to remove those error messages, but Tom and
> Frederic didn't seem to like it:
> 
> 	http://lkml.org/lkml/2009/6/17/89

It made sense to me to overload the filter file for individual events
this way since error messages (which I still think are useful) and valid
filters are mutually exclusive.  But that's not the case for the
subsystem filter files, so for those maybe a filter_error file makes
sense.  Maybe for consistency, it also makes sense for the individual
events too - I don't really have a strong opinion either way.

Tom

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