On Tuesday 01 April 2008, stephane eranian wrote:
> On Tue, Apr 1, 2008 at 2:28 AM, Arnd Bergmann <[EMAIL PROTECTED]> wrote:
> > stephane eranian wrote:
> > 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.
Note that splitting the patches for review doesn't mean that you have
to split the implementation into smaller files, although often these
go hand in hand.
Also, not every patch has to be meaningful by itself, the strict requirement
is just that adding one aspect of the code doesn't break anything.
E.g. one patch can add a file that can't be used by itself, as long as
it doesn't get enabled in the Makefile.
If it's possible to add functionality in separate units, that's much
better than just adding all files as separate patches and then adding
the Makefile, but OTOH the main purpose of the splitting is to make
reviewing the different aspects easier.
> > 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.
Ok, however I think it would be good to post the patches for review anyway,
as soon as they are reasonably split up. It's important to let people know
that someone is still working on them, and give them a chance to complain
now, rather than waiting for the time of submission for people to give
comments.
> 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.
Ok, those parts should then retain these names, at least for the module.
The current Makefile doesn't seem to allow modules, but one option
for you would be something like
obj-$(CONFIG_PERFMON_DFL_SMPL) += perfmon_dfl_smpl.o
perfmon_dfl_smpl-y += dfl_smpl.o
This way, the source file has the short name, but the module gets
the long name.
> 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.
ok.
> > 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.
I've started looking into that myself. Once I have something that looks
reasonable, I can send you a patch.
> > * 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.
Since the perfmon.h file defines the ABI between kernel and user space, it
would be better to have them identical, even if libpfm uses a copy instead
of the version from /usr/include/linux.
> > * 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?
These exported headers are only meant for user space, not for modules. In
fact, all parts hidden in #ifdef __KERNEL__ get removed in the headers_install
stage, so you can't use them.
Arnd <><
-------------------------------------------------------------------------
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
