Re: [patch 1/14] s390: statistics infrastructure.

[Martin Peschke3 <MPESCHKE@de.ibm.com>]

Andrew,

> There is a patch in -mm which moves oprofile and kprobes into a new
> "instrumentation" menu.
>
>
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.14-rc5/2.6.14-rc5-mm1/broken-out/moving-kprobes-and-oprofile-to-instrumentation-support-menu.patch

>
> I held off on merging that because the oprofile guys asked "why bother".
> I guess the statistics infrastructure answers that question.  I'll send
> it on.

sounds reasonable

> > > (If we end up deciding to keep all this in arch/s390 then I
> > > guess we can live with s390 peculiarities though)
> >
> > I will be happy to see some feature like this included outside
> > arch/s390. What is about lib/, or kernel/?
>
> lib/, I guess.

fine

> It could concievably go in fs/debugfs, depending upon how tightly
> coupled it is to debugfs.

No, I don't think so. debufs has been my choice for the user interface.
But it is only one aspect of the statistics infrastructure, not even the
most important one, in my eyes. It's not an enhancement of debugfs,
but an exploitation. And the statistics code could be changed to use
something else than debugfs with moderate effort.

> > +        list_for_each_entry(seg, &rb->seg_lh, list)
> > +                    break;
...
> Yes, we do poke inside list_head a lot and yes, it does feel a bit wrong.
>
> Do whatever you think is best here.  A little explanatory comment would
> help.

Then I will opt for a comment, rather than touching tested code.

> I noticed that there was a /* in there somewhere where a kerneldoc /**
> was intended.

Looked for it without success.
I am not consistent as to */ or **/ at the end of kerneldoc comments, yet.

THanks for your comments,
Martin Peschke

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