Re: [PATCH] x86: add the debugfs interface for the sysprof tool

From: Ingo Molnar
Date: Sun Feb 24 2008 - 08:45:32 EST



* Theodore Tso <tytso@xxxxxxx> wrote:

> The abdication of responsibility and the lack of trying to solve the
> usability issues is one of the things that really worries me about
> *all* of Linux's RAS tools. We can and should do better! And it's
> really embarassing that the RAS maintainers seem (I assume you are one
> of the oprofile maintainers), seem to be blaming this on the victims,
> the people who are complaining about using *your* tool. Yes, it's a
> shame that Ingo didn't try to fix your tool; open source, and scratch
> your own itch and all of that. To be sure. But at the *same* *time*
> don't you have enough pride to take a look at a tools which so
> obviously has massive lacks in the usability department, and tried to
> fix it years ago? There's more than enough blame to go around twenty
> times over, I would think.

i agree with most of your mail but i beg to differ with what you wrote
about my role :-/ The specific bug/issue i discovered with oprofile i
discovered on the very day i wrote about it to lkml.

In any case i'm not the one to fix oprofile - i cannot fix or report all
itches that i have in ~1 billion lines of userspace - i would have to
spend my whole life complaining ;-)

I'm the one to make sure that patches for useful userspace tools that
get submitted to me eventually go upstream, one way or another. Sysprof
has been around for years, had to rely on a (trivial) external module
and AFAIK the feature even predates oprofile's stack-trace support. The
API is butt-ugly and it's being fixed. So if then it should have been
the oprofile folks porting over sysprof to their new API ... claiming
otherwise would rewrite history i think.

a quick glance at oprofile's stack-trace/callgraph support shows it
being rather buggy: it uses __copy_from_user_inatomic() from NMI
context, which is bad because that can fault and re-enable NMIs, causing
stack recursion/corruption. Fixing it is nontrivial. I'm not sure how
much this feature has been tested - but with a sucky userspace kernel
features _do not get tested_ - it's as simple as that! I'd be happier
with sysprof's pure and simple "we only care about time events" initial
approach and evolve this field via actual interest from users.

Ingo
--
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/