[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.00.0906181915350.6890@gandalf.stny.rr.com>
Date: Thu, 18 Jun 2009 19:18:42 -0400 (EDT)
From: Steven Rostedt <rostedt@...dmis.org>
To: Tom Zanussi <tzanussi@...il.com>
cc: Li Zefan <lizf@...fujitsu.com>, Ingo Molnar <mingo@...e.hu>,
Frederic Weisbecker <fweisbec@...il.com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] tracing/filters: restore orignal filter when new
filter isn't applicable
On Thu, 18 Jun 2009, Tom Zanussi wrote:
> Hi,
>
> On Wed, 2009-06-17 at 16:46 +0800, Li Zefan wrote:
> > | commit 7ce7e4249921d5073e764f7ff7ad83cfa9894bd7
> > | Author: Tom Zanussi <tzanussi@...il.com>
> > | Date: Sun Mar 22 03:31:04 2009 -0500
> > |
> > | tracing: add per-event filtering
> > | ...
> > |
> > | Filters can also be set or cleared for a complete subsystem by writing
> > | the same filter as would be written to an individual event to the
> > | filter file at the root of the subsytem. Note however, that if any
> > | event in the subsystem lacks a field specified in the filter being
> > | set, the set will fail and all filters in the subsytem are
> > | automatically cleared. This change from the previous version was made
> > | because using only the fields that happen to exist for a given event
> > | would most likely result in a meaningless filter.
> >
> > I really don't like this change. It is inconvenient, and makes subsystem
> > filter much less useful:
> >
> > # echo 'vec == 1' > irq/softirq_entry/filter
> > # echo 'irq == 5' > irq/filter
> > bash: echo: write error: Invalid argument
> > # cat irq/softirq_entry/filter
> > none
> >
> > I'd expect this will set the filter for irq_handler_entry and
> > irq_handler_exit, and won't touch softirq_entry and softirq_exit.
> >
> > But it just failed, and what's worse, each event's filter was
> > cleared.
> >
>
> The idea behind the change was that after setting a subsystem filter,
> you'd be guaranteed that all or none of the events in the subsystem
> would have the same filter at that point, and not some mix of different
> filters depending on which ones failed or not, which to me seemed
> nonintuitive.
>
> If I set a filter like "vec == 1 && irq == 5", which really has no
> overall meaning, I wouldn't expect softirq_entry to get "vec == 1" and
> irq_handler_entry to get "irq == 5" - I'd rather get an error, but
> that's just me.
>
> So if it makes more sense to users to have subsystem filters propagate
> to whichever events will take them, this patch would be fine with me.
>
I disagree. If a subsystem filter is set, it should be valid for all
filters underneath, if it is not, it should error.
But Li has a point, if you get an error, it should not reset all filters
underneath. That is, if the irq/filter setting took an error, then
irq/softirq_entry/filter should still stay the same.
Perhaps you need to run through it twice. See if the setting of a filter
is valid for all filters underneath, if it is not, then fail. If it is,
then reset all of them, and assign the filter.
-- Steve
--
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/
Powered by blists - more mailing lists