Hello Arnd,
On Tue, Apr 1, 2008 at 2:28 AM, Arnd Bergmann <[EMAIL PROTECTED]> wrote:
> stephane eranian wrote:
> > I have spent most of last week restructuring the current perfmon2
> > kernel code base to make it easier to isolate
> > features. That should make it easier for the merge with mainline given
> > that LKML people would like to receive
> > small patches each adding one feature at a time.
>
> Ah, this looks really good. I haven't really followed the perfmon2
> progress over the last few releases. What is your plan for submission
> to upstream inclusion? I guess if you want to have a chance of getting
According to what I have heard from Morton. I need to submit very small
patches each adding one feature. The restructuring I am currently doing
aim at making this easier and will hopefully avoid having to maintain two
seperate code bases. I would like to isolate things a bit more so we would
be able to submit a patch that adds, let's per-thread counting, then system-wide
counting, then per-thread sampling and so on. That is not going to be
necessarily
easy.
> it into 2.6.26, now would be the time to post the patches.
> I can certainly offer some help getting it into shape for inclusion
> if you're interested.
>
I don't think the code is yet ready to start submitting patches. We need
some more work to isolate things more. As I said the area of fous right
now should be the generic to arch-specific interface. There is also another
set of issues with the PMu description modules and how they interface with
the arch-specific code. I would like to see more code pushed into the PMU
description modules.
> At some point I think you'll need a rebased git tree that contains
> the changesets in exactly the way you want them to be merged,
> and then you keep changing those patches, instead of adding new
> changes on top.
Yes, I need to create another GIT tree that tracks mainline and which
has only the pieces we want upstream now. But I am trying to make
the gap between the current GIT tree and the 'to-linus' tree as small
as possible.
>
> > I have created a bunch of new files under perfmon/, each one
> > implementing a feature or logically similar set of
> > features. Similarly, I have also restructured the header files to
> > separate the generic interface definition in
> > perfmon.h from all the stuff needed by the kernel implementation, now
> > in perfmon_kern.h. This model is replicated
> > for each arch. Perfmon_const.h is gone.
>
> I looked at the file structure now, and I think it can be done a little
> bit better still:
>
> * Since you can't build perfmon2 as a module, there is no point prefixing
> the file names, so perfmon/perfmon_*.c can all be renamed to perfmon/*.c.
>
Parts of perfmon2 can be built as modules. Some modules could live
in perfmon/. For instance any generic sampling format would live there
like perfmon_dfl_smpl.c.
> * include/linux/perfmon_{fmt,pmu,dfl_smpl}.h are all included only from
> perfmon/*.c, not from arch code, so they can (and should) be moved to
> perfmon/, to make it clearer what the interface to the arch is.
No, some are needed by code that lives in arch/*/perfmon/. For instance,
perfmon_cell.c. But I will take a closer look to try and isolate some of
the definitions. Maybe some do not belong in a "shared" header file.
> I also got the impression that part of perfmon_kern.h is only needed
> by perfmon/*.c, not by the arch code, so that can be improved further,
> but splitting it into perfmon/perfmon.h and include/linux/perfmon.h
>
Yes, that one I agree. I made the same remark to myself. There is code
in there that is "private" to the generic code, thus it could be in a private
header file in perfmon/. Will do that.
> * include/linux/perfmon.h contains an #ifdef CONFIG_PERFMON, which is
> not valid for a user space header. Config symbols are never set in
> user space, so this doesn't work.
>
User code is never supposed to include kernel header files directly. Perfmon
applications instead use a "user-level" version of perfmon.h which is provided
by libpfm. This is file is totally different from
include/linux/perfmon.h. But it
may be that the CONFIG_PERFMON is superfluous now that I have separated
the kernel-only piece. Will take a look.
> * You need to list in include/*/Kbuild the files that should be
> exported to user space, i.e. perfmon.h. Otherwise, "make headers_install"
> doesn't pick them up
>
Okay, so now I understand your earlier point about perfmon.h I did not know
about the headers_install option. Are those headers supposed to be used by
applications or just kernel modules?
> * asm-ia64/perfmon.h contains #ifdef __KERNEL__ around definitions that
> are exported to user space on other architectures.
>
Will fix that.
\
> > Finally, there is a lot of work in the X86-specific perfmon.c. There
> > is way too much model-specific
> > code in there. I would like to see all of the P4 code outside of
> > perfmon.c and into perfmon_p4.c. Similarly with
> > the PEBS code. Note that the ds.h interface from Markus Metzger from
> > Intel will help in hiding the access to the
> > DS and PEBS information. Markus's patch is likely going to make it
> > into 2.6.25 or .26. That should help us a bit.
>
> I think it would be good to merge the architectures separately, through
> the respective maintainers. If x86 is still behind by a bit, it could
> wait another kernel release.
>
That was my intention. I don't they will accept all arch at once. Obviously,
they care most about X86. But as I said, this one will need some more work.
It simply suffers from too much diversity. The cleanest one so far is
still Itanium.
So maybe start with this one.
thanks for your comments, keep them coming!
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
perfmon2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
