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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1248155796.6323.46.camel@tropicana>
Date:	Tue, 21 Jul 2009 00:56:36 -0500
From:	Tom Zanussi <tzanussi@...il.com>
To:	Li Zefan <lizf@...fujitsu.com>
Cc:	Steven Rostedt <rostedt@...dmis.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...e.hu>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] tracing/filters: Improve subsystem filter

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/