Re: [PATCH] Allow arbitrary tracefs mount points

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

On 7 Nov 2024, Eugene Loh stated:

> The whole thing with dtp->dt_tracefs_file is...  interesting. We store a temporary string in dtp-> just (if I understand correctly)
> to simplify its free.  It's kind of a cool trick but also kind of ugly.  We'll see what Kris says.

Yep! To massively reduce the amount of noise this patch produces. (This
is what torpedoed every previous attempt of mine to do this.)

I stole the idea from libctf, which does something similar (and did even
in the Solaris days) in ctf_lookup_by_name()...

It works until you think you can call tracefs_file() twice in the same
function call, but honestly if that happens the flames will be immediate
and you'll spot it in no time (and so will the valgrind runs we
sometimes do).

> In dt_prov_dtrace.c attach():
>
> *)  We should convert "len=snprintf(NULL, 0, ...); snprintf(fn, len, ...)" into asprintf() for the format file name.  I realize that
>  in some sense that is "beyond the scope of this patch," but it seems like we've used a couple of patches recently an an opportunity
>  to fix this sort of thing.

Yeah, that niggled at me but I left it explicitly because I thought
people might think it was a bridge too far. Will change.

There was one other place with a [256] static array which I flipped to a
PATH_MAX in lieu of doing something more dynamic.

> *)  Using the variable uprobe_events first for "uprobe_events" and later for "events" seems confusing.  At the very least, how about
>  naming uprobe_events to be something more generic.

Agreed. Renamed to "tmp_path" which at least makes it clear that all
that's really consistent about this variable is that it's a path and
it's temporary.

> On 11/6/24 11:46, Nick Alcock wrote:
>> So as of kernel 6.3 (upstream commit 2455f0e124d317dd08d337a75), the
>> canonical tracefs location has moved to /sys/kernel/tracing.  Unfortunately
>> this requires an explicit mount, and a great many installations have not
>> done this and perhaps never will.  So if the debugfs is mounted on
>> /sys/kernel/debug, it automatically makes /sys/kernel/debug/tracing appear
>> as it used to, as another tracefs mount. And of course there's nothing
>> special about the "canonical location": it's up to the admin, who might
>> choose to remount the thing anywhere at all.
>
> Anywhere at all?  But the scripts in test/ only look in two places.

Yeah, I didn't fix the tests the same way.

> Speaking of which, could runtest.sh look for TRACEFS and then make that available to any test script that needs it?

... in /proc/mounts? I suppose so! I'll have a go.

-- 
NULL && (void)