Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error output

[RaphaÃl Beamonte]

2015-09-11 14:55 GMT-04:00 Arnaldo Carvalho de Melo <arnaldo.melo@xxxxxxxxx>:
> Em Fri, Sep 11, 2015 at 01:50:02PM -0400, RaphaÃl Beamonte escreveu:
>> 2015-09-11 12:16 GMT-04:00 Jiri Olsa <jolsa@xxxxxxxxxx>:
>> > On Sat, Sep 12, 2015 at 01:09:31AM +0900, Namhyung Kim wrote:
>> <SNIP>
>> >> has a problem - if tracefs is mounted under debugfs, the access mode
>> >> of debugfs also affects, so in this case I had to change it both for
>> >> debugfs and tracefs..
>
>> > hum, I wonder the error message needs to be that smart..
>>
>> Hmm... If tracefs is mounted under debugfs, wouldn't remounting
>> debugfs do the trick, as it was done before?
>
> Not necessarily, we may be able to access /a/ but not /a/b/, so, before
> we get to /a/b/ we need to solve access to /a/ to then realize that
> /a/b/ also need permission change so that we can access it.

Well, I kind of had in mind that if we can access /a/, it's that /a/
is not the problem, so why would we remount it? Remounting /a/b/
should do the trick, and there's no reason we couldn't do it if we can
access /a/. Or perhaps I'm missing something?

>> If so, why couldn't we just check the paths with a basic strcmp to
>> verify if tracefs starts by debugfs, and in that case offer to remount
>> debugfs, else offer to remount tracefs?
>
> say it is how it was before tracefs:
>
>  /sys/kernel/debug/tracing/
>
> If we can't access "/sys/kernel/debug/tracing/" because we can't access
> "/sys/kernel/debug/" we need first to change (remount, chmod/grp/own,
> whatever is best in each hypotetical use case) /sys/kernel/debug/ to
> then do the same for /sys/kernel/debug/tracing/, no?

In that case, I'm following: if we can't access /sys/kernel/debug, we
have to remount it anyway, and there's no guarantee that
/sys/kernel/debug/tracing will have the right permissions. In that
case, ok, we perhaps need to remount both. Perhaps because: can
we be sure that if /sys/kernel/debug is inaccessible, it will be the
same for /sys/kernel/debug/tracing ?

> We could of course say something like "Something is wrong with tracefs
> and/or debugfs, figure it out and try again", but we can do better,
> right? 8-P
>
> - Arnaldo

I agree! We were talking in a previous conversation about making perf
the user-friendliest possible. But I don't know if providing two
different remount to the user at the same time is the best thing to do
(even in that second case).
Why not checking first access to debugfs: if it doesn't work, give the
remount line for debugfs then exit. The user will do it then run perf
again. If it works, check tracefs: if it doesn't work, give the
remount line for tracefs. That gives at most two fail-runs of perf
before using it without any problem. It also allows not to give the
user two different remount lines directly when we can't be sure the
second one is useful (perhaps tracefs will be accessible directly?)
Another possibility would be to have another perf command, sort of a
"perf remountfs", to run with sudo and that would make itself both
checks and remount accordingly the two fs.

Thoughts?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/