Re: [PATCH] rawfbt: new provider

[Nick Alcock <nick.alcock@oracle.com>]

On 9 Dec 2024, Alan Maguire stated:

> On 06/12/2024 16:33, Kris Van Hees wrote:
>> On Fri, Dec 06, 2024 at 10:35:50AM +0000, Alan Maguire wrote:
>>> On 05/12/2024 19:42, Kris Van Hees wrote:
>>>> This provider provides access to all kprobe-based probes that are
>>>> available on the system.  This includes any compiler-generated
>>>> optimized variants of functions, named <func>.<suffix>.
>>>>
>>>
>>> This is great! Having a rawfbt allows users to still trace cases
>>> where fprobe can't currently handle.
>>>
>>> On the .suffix; I get that this is intended to be a raw provider, but
>>> would it be better for stability to expose these functions as "func"
>>> rather than "func.suffix"? It becomes difficult to write portable
>>> scripts when suffixes are included, because wildcarding
>>>
>>> rawfbt::func*:entry
>>>
>>> ...to catch both suffixed and non-suffixed variants may end up tracing
>>> func, func.suffix but also func2 etc.
>> 
>> The trouble is that exposing <func>.<suffix> as <func> will mean that you
>> automatically end up trying to probe *any* <func>.<suffix> whenever you
>> place a probe on <func> using rawfbt, which is not necessarily what you may
>> want.  And there would not be any way to avoid that.
>> 
>> But by not collapsing them, you have the ability to catch them all anyway,
>> if you want, by specifying <func>, <func>.* instead of <func>.
>
> I tried that with the rawfbt code but got:
>
> # dtrace -n 'rawfbt::xfs_cleanup_inode.*:entry {}'
> dtrace: description 'rawfbt::xfs_cleanup_inode.*:entry ' matched 1 probe
> ^C
>
> This matches xfs_cleanup_inode.isra.0, looks good so far. However:
>
> # dtrace -n
> 'rawfbt::xfs_cleanup_inode:entry,rawfbt::xfs_cleanup_inode.*:entry {}'
> dtrace: invalid probe specifier
> rawfbt::xfs_cfs_cleanup_inode:entry,rawfbt::xfs_cleanup_inode.*:entry
> {}: probe description rawfbt::xfs_cfs_cleanup_inode:entry does not match
> any probes
>
> So the absence of the non-suffixed function (which we included for
> portability) triggers the failure. So looks like it will be necessary to
> add -Z:
>
> # /sbin/dtrace -Zn
> 'rawfbt::xfs_cfs_cleanup_inode:entry,rawfbt::xfs_cleanup_inode.*:entry {}'
> dtrace: description
> 'rawfbt::xfs_cfs_cleanup_inode:entry,rawfbt::xfs_cleanup_inode.*:entry '
> matched 1 probe
>
> So I guess as long as we document this clearly for folks wanting to to
> use rawfbt to write portable scripts, it should be okay.

Agreed. Documented behaviour, trivial workaround, maybe can be improved
later -- maybe we shouldn't get a 'no probes' error unless *every probe*
in a probe list is unmatched? There are good reasons for both... maybe
we should add "foo & bar" and "foo | bar" and parens for precedence in
later releases, but this is huge overdesign :P

-- 
NULL && (void)