Re: [PATCH 1/3] perf, pt, coresight: Clean up address filter structure

[Alexander Shishkin]

Mathieu Poirier <mathieu.poirier@xxxxxxxxxx> writes:

> If this is what you want to convey then
>
> + * @action:    filter/start/stop
>
> needs to be fixed.  This can be interpreted as "use range filter,
> start filter or stop filter" - which is exactly what I did.  Something
> like

I was beginning to think that the 'correct' way only existed in my head,
but then I noticed elsewhere there's a comment:

| where ACTION is one of the
|  * "filter": limit the trace to this region
|  * "start": start tracing from this address
|  * "stop": stop tracing at this address/region;

so it is sort of explained. Maybe I also need to spell it out in the
structure. And the man page, of course, when I get to it.

>> The fact that the CS driver gets it wrong
>> just proves the point that "filter->filter" is confusing and misleading
>> and needs to be replaced.
>>
>
> I could not agree more.
>
> On the flip side it doesn't change anything to my original argument:
> the code should not be made to be smart.  If a range filter is used
> then a size of zero should be treated as an error.

It actually does, because when you say 'range filter' you really mean
'filter filter' and we need to get this difference straight as I also
mentioned in the other email. ACTION=="filter" does not make sense with
size==0, this much is true. The other two are fine, though.

So considering all of the above, I have amended the patch to something
like the following. I'm not 100% sure that the CS side is accurate, but
I'm hoping you can have a look.

Side note: while looking through address comparator code, I noticed your
catch-all filter gets the end address wrong if compiled in 64bit mode.