[PATCH] perf: Move fs.* to generic lib/lk/

[PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

Move to generic library and kill magic.h as it is needed only in fs.h.

Signed-off-by: Borislav Petkov <bp@suse.de>
---

Ok, let's try the first one, see how it goes.

 tools/lib/lk/Makefile                                  |  2 ++
 tools/{perf/util => lib/lk}/fs.c                       | 11 ++++++++---
 tools/{perf/util/include/linux/magic.h => lib/lk/fs.h} |  9 ++++++---
 tools/perf/Makefile.perf                               |  3 ---
 tools/perf/tests/parse-events.c                        |  2 +-
 tools/perf/util/cpumap.c                               |  2 +-
 tools/perf/util/fs.h                                   |  7 -------
 tools/perf/util/pmu.c                                  |  2 +-
 tools/perf/util/python-ext-sources                     |  2 +-
 tools/perf/util/record.c                               |  2 +-
 10 files changed, 21 insertions(+), 21 deletions(-)
 rename tools/{perf/util => lib/lk}/fs.c (93%)
 rename tools/{perf/util/include/linux/magic.h => lib/lk/fs.h} (60%)
 delete mode 100644 tools/perf/util/fs.h

diff --git a/tools/lib/lk/Makefile b/tools/lib/lk/Makefile
index 3dba0a4aebbf..261febf6fd09 100644
--- a/tools/lib/lk/Makefile
+++ b/tools/lib/lk/Makefile
@@ -8,8 +8,10 @@ LIB_H=
 LIB_OBJS=
 
 LIB_H += debugfs.h
+LIB_H += fs.h
 
 LIB_OBJS += $(OUTPUT)debugfs.o
+LIB_OBJS += $(OUTPUT)fs.o
 
 LIBFILE = liblk.a
 
diff --git a/tools/perf/util/fs.c b/tools/lib/lk/fs.c
similarity index 93%
rename from tools/perf/util/fs.c
rename to tools/lib/lk/fs.c
index f5be1f26e724..7891fa715d9f 100644
--- a/tools/perf/util/fs.c
+++ b/tools/lib/lk/fs.c
@@ -1,8 +1,13 @@
-
 /* TODO merge/factor into tools/lib/lk/debugfs.c */
 
-#include "util.h"
-#include "util/fs.h"
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/vfs.h>
+
+#include "debugfs.h"
+#include "fs.h"
 
 static const char * const sysfs__fs_known_mountpoints[] = {
 	"/sys",
diff --git a/tools/perf/util/include/linux/magic.h b/tools/lib/lk/fs.h
similarity index 60%
rename from tools/perf/util/include/linux/magic.h
rename to tools/lib/lk/fs.h
index 07d63cf3e0f6..72ddfec739f1 100644
--- a/tools/perf/util/include/linux/magic.h
+++ b/tools/lib/lk/fs.h
@@ -1,5 +1,5 @@
-#ifndef _PERF_LINUX_MAGIC_H_
-#define _PERF_LINUX_MAGIC_H_
+#ifndef __LK_FS_H__
+#define __LK_FS_H__
 
 #ifndef DEBUGFS_MAGIC
 #define DEBUGFS_MAGIC          0x64626720
@@ -13,4 +13,7 @@
 #define PROC_SUPER_MAGIC       0x9fa0
 #endif
 
-#endif
+const char *sysfs__mountpoint(void);
+const char *procfs__mountpoint(void);
+
+#endif /* __LK_FS_H__ */
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index e416ccc7d831..f0c3fa913e98 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -216,7 +216,6 @@ LIB_H += util/include/linux/ctype.h
 LIB_H += util/include/linux/kernel.h
 LIB_H += util/include/linux/list.h
 LIB_H += util/include/linux/export.h
-LIB_H += util/include/linux/magic.h
 LIB_H += util/include/linux/poison.h
 LIB_H += util/include/linux/prefetch.h
 LIB_H += util/include/linux/rbtree.h
@@ -242,7 +241,6 @@ LIB_H += util/cache.h
 LIB_H += util/callchain.h
 LIB_H += util/build-id.h
 LIB_H += util/debug.h
-LIB_H += util/fs.h
 LIB_H += util/pmu.h
 LIB_H += util/event.h
 LIB_H += util/evsel.h
@@ -304,7 +302,6 @@ LIB_OBJS += $(OUTPUT)util/annotate.o
 LIB_OBJS += $(OUTPUT)util/build-id.o
 LIB_OBJS += $(OUTPUT)util/config.o
 LIB_OBJS += $(OUTPUT)util/ctype.o
-LIB_OBJS += $(OUTPUT)util/fs.o
 LIB_OBJS += $(OUTPUT)util/pmu.o
 LIB_OBJS += $(OUTPUT)util/environment.o
 LIB_OBJS += $(OUTPUT)util/event.o
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 3cbd10496087..0bee94ec5cdb 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -2,7 +2,7 @@
 #include "parse-events.h"
 #include "evsel.h"
 #include "evlist.h"
-#include "fs.h"
+#include <lk/fs.h>
 #include <lk/debugfs.h>
 #include "tests.h"
 #include <linux/hw_breakpoint.h>
diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index a9b48c42e81e..f092be9817ce 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -1,5 +1,5 @@
 #include "util.h"
-#include "fs.h"
+#include <lk/fs.h>
 #include "../perf.h"
 #include "cpumap.h"
 #include <assert.h>
diff --git a/tools/perf/util/fs.h b/tools/perf/util/fs.h
deleted file mode 100644
index 5e09ce1bab0e..000000000000
--- a/tools/perf/util/fs.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifndef __PERF_FS
-#define __PERF_FS
-
-const char *sysfs__mountpoint(void);
-const char *procfs__mountpoint(void);
-
-#endif /* __PERF_FS */
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index c232d8dd410b..508264a4929a 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -4,7 +4,7 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <dirent.h>
-#include "fs.h"
+#include <lk/fs.h>
 #include "util.h"
 #include "pmu.h"
 #include "parse-events.h"
diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources
index 239036fb2b2c..32bf22d1dd1b 100644
--- a/tools/perf/util/python-ext-sources
+++ b/tools/perf/util/python-ext-sources
@@ -17,5 +17,5 @@ util/xyarray.c
 util/cgroup.c
 util/rblist.c
 util/strlist.c
-util/fs.c
+../lib/lk/fs.c
 ../../lib/rbtree.c
diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
index c8845b107f60..a6b9e5c3cd45 100644
--- a/tools/perf/util/record.c
+++ b/tools/perf/util/record.c
@@ -2,7 +2,7 @@
 #include "evsel.h"
 #include "cpumap.h"
 #include "parse-events.h"
-#include "fs.h"
+#include <lk/fs.h>
 #include "util.h"
 
 typedef void (*setup_probe_fn_t)(struct perf_evsel *evsel);
-- 
1.8.4

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Ingo Molnar]

* Borislav Petkov <bp@alien8.de> wrote:

> From: Borislav Petkov <bp@suse.de>
> 
> Move to generic library and kill magic.h as it is needed only in fs.h.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>

Ouch, the naming is horrible, what does 'lk' stand for? Please rename 
it to something sensible before it spreads.

libkernel.so and tools/lib/kernel/ would be perfect I think.

Thanks,

	Ingo

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Thu, Nov 21, 2013 at 08:34:10AM +0100, Ingo Molnar wrote:
> Ouch, the naming is horrible, what does 'lk' stand for?

"linux kernel" :-)

> Please rename it to something sensible before it spreads.
> 
> libkernel.so and tools/lib/kernel/ would be perfect I think.

However, now that I think of it, the naming is kinda imprecise having
only "kernel" in it - it only denotes that it is a library of stuff that
has grown in the kernel.

So how about

tools/lib/ktools/
tools/lib/kutils/
tools/lib/kernel-tools/
tools/lib/kernel-utils/
...

and

libktools.so
libkutils.so
...

?

I'd prefer ktools as in the tools/ directory of the kernel repo.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Ingo Molnar]

* Borislav Petkov <bp@alien8.de> wrote:

> On Thu, Nov 21, 2013 at 08:34:10AM +0100, Ingo Molnar wrote:
> > Ouch, the naming is horrible, what does 'lk' stand for?
> 
> "linux kernel" :-)
> 
> > Please rename it to something sensible before it spreads.
> > 
> > libkernel.so and tools/lib/kernel/ would be perfect I think.
> 
> However, now that I think of it, the naming is kinda imprecise 
> having only "kernel" in it - it only denotes that it is a library of 
> stuff that has grown in the kernel.
> 
> So how about
> 
> tools/lib/ktools/
> tools/lib/kutils/
> tools/lib/kernel-tools/
> tools/lib/kernel-utils/
> ...
> 
> and
> 
> libktools.so
> libkutils.so
> ...
> 
> ?
> 
> I'd prefer ktools as in the tools/ directory of the kernel repo.

So what does each name mean, what would they be used for and what's 
the difference between them?

Thanks,

	Ingo

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Thu, Nov 21, 2013 at 12:17:44PM +0100, Ingo Molnar wrote:
> > However, now that I think of it, the naming is kinda imprecise 
> > having only "kernel" in it - it only denotes that it is a library of 
> > stuff that has grown in the kernel.
> > 
> > So how about
> > 
> > tools/lib/ktools/
> > tools/lib/kutils/
> > tools/lib/kernel-tools/
> > tools/lib/kernel-utils/
> > ...
> > 
> > and
> > 
> > libktools.so
> > libkutils.so
> > ...
> > 
> > ?
> > 
> > I'd prefer ktools as in the tools/ directory of the kernel repo.
> 
> So what does each name mean, what would they be used for and what's 
> the difference between them?

Oh no, I'm only proposing multiple libraries but multiple names to
choose *only* *one* from them - the one which we think fits best.

Basically I wanna say: "this is the library of tools which grew in the
tools/ directory of the kernel repo".

Thus I'm proposing

	tools/lib/ktools/

as a path and

	libktools.so

as a library name, because, IMHO, this name is closer to the reality.

tools/lib/kernel/ is also ok - it does not describe the collection of
functionality that precisely, IMHO, that's all.

Am I even making sense here?

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Ingo Molnar]

* Borislav Petkov <bp@alien8.de> wrote:

> On Thu, Nov 21, 2013 at 12:17:44PM +0100, Ingo Molnar wrote:
> > > However, now that I think of it, the naming is kinda imprecise 
> > > having only "kernel" in it - it only denotes that it is a library of 
> > > stuff that has grown in the kernel.
> > > 
> > > So how about
> > > 
> > > tools/lib/ktools/
> > > tools/lib/kutils/
> > > tools/lib/kernel-tools/
> > > tools/lib/kernel-utils/
> > > ...
> > > 
> > > and
> > > 
> > > libktools.so
> > > libkutils.so
> > > ...
> > > 
> > > ?
> > > 
> > > I'd prefer ktools as in the tools/ directory of the kernel repo.
> > 
> > So what does each name mean, what would they be used for and what's 
> > the difference between them?
> 
> Oh no, I'm only proposing multiple libraries but multiple names to 
> choose *only* *one* from them - the one which we think fits best.
> 
> Basically I wanna say: "this is the library of tools which grew in 
> the tools/ directory of the kernel repo".
> 
> Thus I'm proposing
> 
> 	tools/lib/ktools/
> 
> as a path and
> 
> 	libktools.so
> 
> as a library name, because, IMHO, this name is closer to the reality.
>
> tools/lib/kernel/ is also ok - it does not describe the collection 
> of functionality that precisely, IMHO, that's all.
> 
> Am I even making sense here?

Ok - so why not call it tools/lib/kernel/ or tools/lib/kernelapi/ or 
tools/lib/kapi/?

That's really what is common about it: it offers various helper 
methods to interface with the Linux kernel: debugfs, procfs, sysfs 
handling routines with no policy, just pure, obvious helpers to use 
kernel functionality.

[ Naming it 'ktools' is not optimal because these are already tools/ 
  ... the secondary meaning of 'tools' in that context is easily lost. 
  Same goes for 'kutils', the secondary meaning is confusing (to me). ]

... and just in case you are wondering why I'm making a fuss about it: 
a naming discussion for something fundamental as this isn't bikeshed 
painting, we really want people to look at that place to readily stuff 
kernel related helpers into, and the name should make that very 
obvious.

Thanks,

	Ingo

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Thu, Nov 21, 2013 at 12:42:24PM +0100, Ingo Molnar wrote:
> Ok - so why not call it tools/lib/kernel/ or tools/lib/kernelapi/ or
> tools/lib/kapi/?

I like short names so "kapi" it is.

> That's really what is common about it: it offers various helper
> methods to interface with the Linux kernel: debugfs, procfs, sysfs
> handling routines with no policy, just pure, obvious helpers to use
> kernel functionality.

Yes.

> [ Naming it 'ktools' is not optimal because these are already tools/
> ... the secondary meaning of 'tools' in that context is easily lost.
> Same goes for 'kutils', the secondary meaning is confusing (to me). ]

Ok.

> ... and just in case you are wondering why I'm making a fuss about it:
> a naming discussion for something fundamental as this isn't bikeshed
> painting, we really want people to look at that place to readily
> stuff kernel related helpers into, and the name should make that very
> obvious.

No, I absolutely don't think it is a bikeshedding exercise too. We
really need that lib because people have started growing their own
facilities in tools/ and outside so having a lib which implements
generic functionality for interfacing with the kernel right is going to
save everyone a lot of time and energy.

So kapi it is, I'm gonna convert what we have to it but leave time for
acme to wake up and smell the coffee ... and the change :-)

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Steven Rostedt]

On Thu, Nov 21, 2013 at 01:06:05PM +0100, Borislav Petkov wrote:
> 
> No, I absolutely don't think it is a bikeshedding exercise too. We
> really need that lib because people have started growing their own
> facilities in tools/ and outside so having a lib which implements
> generic functionality for interfacing with the kernel right is going to
> save everyone a lot of time and energy.

Should we put libtraceevent here too? Have all kapi functionality live here?

As libtraceevent is getting mature, we need to get it out as a real library
soon. There is already three utilities that use it: perf, trace-cmd and 
powertop. And I've heard of others that want that functionatity.

-- Steve

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Thu, Nov 21, 2013 at 07:39:55AM -0500, Steven Rostedt wrote:
> Should we put libtraceevent here too? Have all kapi functionality live
> here?
>
> As libtraceevent is getting mature, we need to get it out as a real
> library soon. There is already three utilities that use it: perf,
> trace-cmd and powertop. And I've heard of others that want that
> functionatity.

Right, my intention was to have topic libraries like libtraceevent,
libkapi and libperfevent (this is the one that's following by exporting
evlist/evsel to other tools).

But I certainly see your angle of lumping *all* kernel-related
functionality together in one lib.

I dunno, I feel like leaving them split for now is better. Why? Not
polluting namespaces unnecessarily and why would tools who don't need
tracing functionality link against it.

Anyway, arguments can be made for both, but I personally prefer leaving
them separate. Unless there's a really compelling reason to have a
single lib - a reason which I cannot think of right now...

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Steven Rostedt]

On Thu, 21 Nov 2013 14:49:49 +0100
Borislav Petkov <bp@alien8.de> wrote:

> Anyway, arguments can be made for both, but I personally prefer leaving
> them separate. Unless there's a really compelling reason to have a
> single lib - a reason which I cannot think of right now...
> 

Honestly, I don't care if they are separate or in a single lib. The
important thing is to get it into a real library that distros ship.

-- Steve

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Thu, Nov 21, 2013 at 08:56:33AM -0500, Steven Rostedt wrote:
> Honestly, I don't care if they are separate or in a single lib. The
> important thing is to get it into a real library that distros ship.

Is there something in the way of libtraceevent to do so already? I mean,
you're building an .so too.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Arnaldo Carvalho de Melo]

Em Thu, Nov 21, 2013 at 01:06:05PM +0100, Borislav Petkov escreveu:
> On Thu, Nov 21, 2013 at 12:42:24PM +0100, Ingo Molnar wrote:
> > ... and just in case you are wondering why I'm making a fuss about it:
> > a naming discussion for something fundamental as this isn't bikeshed
> > painting, we really want people to look at that place to readily
> > stuff kernel related helpers into, and the name should make that very
> > obvious.
 
> No, I absolutely don't think it is a bikeshedding exercise too. We
> really need that lib because people have started growing their own
> facilities in tools/ and outside so having a lib which implements
> generic functionality for interfacing with the kernel right is going to
> save everyone a lot of time and energy.
 
> So kapi it is, I'm gonna convert what we have to it but leave time for
> acme to wake up and smell the coffee ... and the change :-)

I'm ok with having a library that has this mission statement:

"To offers various helper methods to interface with the Linux kernel:
 debugfs, procfs, sysfs handling routines with no policy, just pure,
 obvious helpers to use kernel functionality."

Naming is a bit hard, to keep it small, descriptive, as API can lead
people to think about other kinds of kernel APIs (syscalls?), "fskapi"
to mean "fs based kernel API" would perhaps be more descriptive? A
longer (more descriptive) possibility would be "linux-fskapi".

- Arnaldo

P.S.: "fskapi" also sounds, humm, fun ;-)

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Arnaldo Carvalho de Melo]

Em Thu, Nov 21, 2013 at 07:39:55AM -0500, Steven Rostedt escreveu:
> On Thu, Nov 21, 2013 at 01:06:05PM +0100, Borislav Petkov wrote:
> > No, I absolutely don't think it is a bikeshedding exercise too. We
> > really need that lib because people have started growing their own
> > facilities in tools/ and outside so having a lib which implements
> > generic functionality for interfacing with the kernel right is going to
> > save everyone a lot of time and energy.
 
> Should we put libtraceevent here too? Have all kapi functionality live here?

Don't know, what I'm liking about this discussion now is that we're
focusing on breaking it down into multiple libraries.

So at some point, symbol resolution, that involves parsing multiple "fs
based kernel api" (/proc/{kallsyms,modules,etc), but should not be in
the library being discussed now, will be another tools/perf/util/
"spin-off", that, again, focuses on a clear mission statement and is
useful for multiple /usr/src/linux/tools/ "clients".

And as the first step, only tools/ clients would be targeted, as we
would have some time to do changes that touches the libraries _and_ all
its users, just like we can do with kernel APIs and its users, i.e.
drivers, etc.

Only at a later stage, when those libraries are boringly mature, we
would export them for wider use.
 
> As libtraceevent is getting mature, we need to get it out as a real library
> soon. There is already three utilities that use it: perf, trace-cmd and 
> powertop. And I've heard of others that want that functionatity.
> 
> -- Steve

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Thu, Nov 21, 2013 at 12:05:24PM -0300, Arnaldo Carvalho de Melo wrote:
> "To offers various helper methods to interface with the Linux kernel:
>  debugfs, procfs, sysfs handling routines with no policy, just pure,
>  obvious helpers to use kernel functionality."

Exactly.

> Naming is a bit hard, to keep it small, descriptive, as API can lead
> people to think about other kinds of kernel APIs (syscalls?), "fskapi"
> to mean "fs based kernel API" would perhaps be more descriptive? A
> longer (more descriptive) possibility would be "linux-fskapi".

Yeah, you can't have fskapi because we'll add other stuff to it (see
the diffstat I sent you last week) so not filesystem stuff only. So I
think "kapi" is as generic and as fitting as it gets. We can use the
"kernel-api" variant but I think the "k" is enough.

:-)

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Arnaldo Carvalho de Melo]

Em Thu, Nov 21, 2013 at 04:28:04PM +0100, Borislav Petkov escreveu:
> On Thu, Nov 21, 2013 at 12:05:24PM -0300, Arnaldo Carvalho de Melo wrote:
> > "To offers various helper methods to interface with the Linux kernel:
> >  debugfs, procfs, sysfs handling routines with no policy, just pure,
> >  obvious helpers to use kernel functionality."
 
> Exactly.
 
> > Naming is a bit hard, to keep it small, descriptive, as API can lead
> > people to think about other kinds of kernel APIs (syscalls?), "fskapi"
> > to mean "fs based kernel API" would perhaps be more descriptive? A
> > longer (more descriptive) possibility would be "linux-fskapi".
 
> Yeah, you can't have fskapi because we'll add other stuff to it (see
> the diffstat I sent you last week) so not filesystem stuff only. So I
> think "kapi" is as generic and as fitting as it gets. We can use the
> "kernel-api" variant but I think the "k" is enough.

I think is that it is too generic, the other stuff you mention is not
really "kapi" at all.

The rest, things like util.c, usage.c, rbtree.c, hash, strlist, etc are
all, well, utilities that we got from the kernel, from git, or that were
created for perf, could get a tools/lib/util/ generic name and be
outside the one with the description agreed above.

But they are not "helper methods to interface with the Linux kernel" at
all.

If "libc" is the "library that comes with the C language" we could have
liblk for the "library that came from the Linux kernel".

But "libkapi" seems the wrong name for a library with those utilities :-\

- Arnaldo

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Thu, Nov 21, 2013 at 02:37:14PM -0300, Arnaldo Carvalho de Melo wrote:
> But "libkapi" seems the wrong name for a library with those utilities :-\

Yeah, I know.

However, since we're keeping the library internal, we can take that name
for now and later, when we expose it outside, we can decide what to call
it or to split it.

I have the feeling we will come up with a proper name when we least
expect it. :-)

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:

> Em Thu, Nov 21, 2013 at 04:28:04PM +0100, Borislav Petkov escreveu:
> > On Thu, Nov 21, 2013 at 12:05:24PM -0300, Arnaldo Carvalho de Melo wrote:
> > > "To offers various helper methods to interface with the Linux kernel:
> > >  debugfs, procfs, sysfs handling routines with no policy, just pure,
> > >  obvious helpers to use kernel functionality."
>  
> > Exactly.
>  
> > > Naming is a bit hard, to keep it small, descriptive, as API can lead
> > > people to think about other kinds of kernel APIs (syscalls?), "fskapi"
> > > to mean "fs based kernel API" would perhaps be more descriptive? A
> > > longer (more descriptive) possibility would be "linux-fskapi".
>  
> > Yeah, you can't have fskapi because we'll add other stuff to it 
> > (see the diffstat I sent you last week) so not filesystem stuff 
> > only. So I think "kapi" is as generic and as fitting as it gets. 
> > We can use the "kernel-api" variant but I think the "k" is enough.
> 
> I think is that it is too generic, the other stuff you mention is 
> not really "kapi" at all.
> 
> The rest, things like util.c, usage.c, rbtree.c, hash, strlist, etc 
> are all, well, utilities that we got from the kernel, from git, or 
> that were created for perf, could get a tools/lib/util/ generic name 
> and be outside the one with the description agreed above.
> 
> But they are not "helper methods to interface with the Linux kernel" 
> at all.

I don't think those other bits should go into this library. rbtree 
should go into lib/rbtree/, command-line bits into lib/cmdline/, build 
system helpers into lib/build/, etc.

Merging unrelated things into a single library is a user-space disease 
we need not repeat.

I'd also not expose any of this externally but straight link it into 
the individual utilities - that way it does not matter that it's a 
nice, topical, fine-grained set of functionality.

I don't think we are ready for (nor do we want the overhead of) 
maintaining a library ABI at this stage.

Once things slow down and it's all so robust that we've had at most a 
handful of commits in tools/lib/ in a full year we can think about 
exporting it, maybe ...

Thanks,

	Ingo

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Fri, Nov 22, 2013 at 01:27:01PM +0100, Ingo Molnar wrote:
> I don't think those other bits should go into this library. rbtree
> should go into lib/rbtree/, command-line bits into lib/cmdline/, build
> system helpers into lib/build/, etc.
>
> Merging unrelated things into a single library is a user-space disease
> we need not repeat.

Well, rbtree is basically rblist.c and the rbtree*.h headers which
simply wrap the kernel headers.

cmdline is parse-options.c.

IOW, that's splitting it into too granulary pieces with 1-2 compilation
units ber library.

And what if there are interdependencies between the stuff split this
way? That could become very painful and unnecessary.

So having a simple single library which includes generic stuff needed to
interface with the kernel is much simpler and sane, IMHO.

And, since we're keeping it internal, we can do the split the other way
around instead - first do the single generic library and then carve out
a certain subset of functionality if/when it makes sense.

The same approach we can use for the name - first split and work with it
and change stuff when the need for it arises.

> I'd also not expose any of this externally but straight link it into
> the individual utilities - that way it does not matter that it's a
> nice, topical, fine-grained set of functionality.
>
> I don't think we are ready for (nor do we want the overhead of)
> maintaining a library ABI at this stage.
>
> Once things slow down and it's all so robust that we've had at most
> a handful of commits in tools/lib/ in a full year we can think about
> exporting it, maybe ...

Right.

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Arnaldo Carvalho de Melo]

Em Fri, Nov 22, 2013 at 01:27:01PM +0100, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> > Em Thu, Nov 21, 2013 at 04:28:04PM +0100, Borislav Petkov escreveu:
> > > On Thu, Nov 21, 2013 at 12:05:24PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Naming is a bit hard, to keep it small, descriptive, as API can lead
> > > > people to think about other kinds of kernel APIs (syscalls?), "fskapi"
> > > > to mean "fs based kernel API" would perhaps be more descriptive? A
> > > > longer (more descriptive) possibility would be "linux-fskapi".
 
> > > Yeah, you can't have fskapi because we'll add other stuff to it 
> > > (see the diffstat I sent you last week) so not filesystem stuff 
> > > only. So I think "kapi" is as generic and as fitting as it gets. 
> > > We can use the "kernel-api" variant but I think the "k" is enough.

> > I think is that it is too generic, the other stuff you mention is 
> > not really "kapi" at all.

> > The rest, things like util.c, usage.c, rbtree.c, hash, strlist, etc 
> > are all, well, utilities that we got from the kernel, from git, or 
> > that were created for perf, could get a tools/lib/util/ generic name 
> > and be outside the one with the description agreed above.

> > But they are not "helper methods to interface with the Linux kernel" 
> > at all.
 
> I don't think those other bits should go into this library. rbtree 
> should go into lib/rbtree/, command-line bits into lib/cmdline/, build 
> system helpers into lib/build/, etc.

Agreed.
 
> Merging unrelated things into a single library is a user-space disease 
> we need not repeat.

Agreed.
 
> I'd also not expose any of this externally but straight link it into 
> the individual utilities - that way it does not matter that it's a 
> nice, topical, fine-grained set of functionality.

Agreed.
 
> I don't think we are ready for (nor do we want the overhead of) 
> maintaining a library ABI at this stage.

Agreed.
 
> Once things slow down and it's all so robust that we've had at most a 
> handful of commits in tools/lib/ in a full year we can think about 
> exporting it, maybe ...

Agreed.

:-)

Lets experiment at having things at the right granularity, even if it
involves many, directly linked, like libperf.a, libraries, one at a
time, starting with fskapi (or whatever name ends up being preferred for
this initial one).

- Arnaldo

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Arnaldo Carvalho de Melo]

Em Fri, Nov 22, 2013 at 02:50:34PM +0100, Borislav Petkov escreveu:
> On Fri, Nov 22, 2013 at 01:27:01PM +0100, Ingo Molnar wrote:
> > I don't think those other bits should go into this library. rbtree
> > should go into lib/rbtree/, command-line bits into lib/cmdline/, build
> > system helpers into lib/build/, etc.
> >
> > Merging unrelated things into a single library is a user-space disease
> > we need not repeat.
> 
> Well, rbtree is basically rblist.c and the rbtree*.h headers which
> simply wrap the kernel headers.
> 
> cmdline is parse-options.c.
> 
> IOW, that's splitting it into too granulary pieces with 1-2 compilation
> units ber library.

Lets do one at a time, so far we agreed that the ones that involves
parsing procfs/sysfs etc should go in tools/lib/(fs)?kapi, so lets do
that one.
 
> And what if there are interdependencies between the stuff split this
> way? That could become very painful and unnecessary.
 
> So having a simple single library which includes generic stuff needed to
> interface with the kernel is much simpler and sane, IMHO.
 
> And, since we're keeping it internal, we can do the split the other way
> around instead - first do the single generic library and then carve out
> a certain subset of functionality if/when it makes sense.
 
> The same approach we can use for the name - first split and work with it
> and change stuff when the need for it arises.
 
> > I'd also not expose any of this externally but straight link it into
> > the individual utilities - that way it does not matter that it's a
> > nice, topical, fine-grained set of functionality.
> >
> > I don't think we are ready for (nor do we want the overhead of)
> > maintaining a library ABI at this stage.
> >
> > Once things slow down and it's all so robust that we've had at most
> > a handful of commits in tools/lib/ in a full year we can think about
> > exporting it, maybe ...
> 
> Right.

yeah.

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[David Ahern]

On 11/22/13, 8:00 AM, Arnaldo Carvalho de Melo wrote:
>
> Lets do one at a time, so far we agreed that the ones that involves
> parsing procfs/sysfs etc should go in tools/lib/(fs)?kapi, so lets do
> that one.

libsysfs & libsysfs-devel?

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Ingo Molnar]

* Borislav Petkov <bp@alien8.de> wrote:

> On Fri, Nov 22, 2013 at 01:27:01PM +0100, Ingo Molnar wrote:
> > I don't think those other bits should go into this library. rbtree
> > should go into lib/rbtree/, command-line bits into lib/cmdline/, build
> > system helpers into lib/build/, etc.
> >
> > Merging unrelated things into a single library is a user-space disease
> > we need not repeat.
> 
> Well, rbtree is basically rblist.c and the rbtree*.h headers which 
> simply wrap the kernel headers.

Yes - with some details and a nice, includable .h file that userspace 
tooling can utilize.

> cmdline is parse-options.c.
> 
> IOW, that's splitting it into too granulary pieces with 1-2 
> compilation units ber library.

I see no problem with that - it's basically like util/*.c is, just 
between tools.

> And what if there are interdependencies between the stuff split this 
> way? That could become very painful and unnecessary.

What dependencies do you mean? The only constraint is to not make it 
circular - but that's easy to do if they are nicely separated per 
concept. I don't think rbtree.h ever wants to include cmdline 
processing or debugfs processing.

> So having a simple single library which includes generic stuff 
> needed to interface with the kernel is much simpler and sane, IMHO.

For userspace and for kernel space subsystems a single .h file per 
separate concept works the best. That is why we have a separate 
rbtree.h, list.h, slab.h, etc.

> And, since we're keeping it internal, we can do the split the other 
> way around instead - first do the single generic library and then 
> carve out a certain subset of functionality if/when it makes sense.

Why?

> The same approach we can use for the name - first split and work 
> with it and change stuff when the need for it arises.
> 
> > I'd also not expose any of this externally but straight link it 
> > into the individual utilities - that way it does not matter that 
> > it's a nice, topical, fine-grained set of functionality.
> >
> > I don't think we are ready for (nor do we want the overhead of) 
> > maintaining a library ABI at this stage.
> >
> > Once things slow down and it's all so robust that we've had at 
> > most a handful of commits in tools/lib/ in a full year we can 
> > think about exporting it, maybe ...
> 
> Right.

Hey, that's an important point of agreement! :-)

Thanks,

	Ingo

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@ghostprotocols.net> wrote:

> Lets experiment at having things at the right granularity, even if 
> it involves many, directly linked, like libperf.a, libraries, one at 
> a time, starting with fskapi (or whatever name ends up being 
> preferred for this initial one).

That's fine with me - what I stood up against was the idea to merge 
into a single place things like the Git helpers, cmdline/option 
parser, etc.

The naming problem is mostly non-existent if the concepts are kept 
nicely separate: the best name for the debugfs bits is probably 
tools/lib/debugfs/, the best name for command line option parser is 
tools/lib/cmdline/, the best name for trace event parsing is 
tools/lib/traceevents/ [hey, we already have that one in the right 
place! ;-)], etc.

Thanks,

	Ingo

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Ingo Molnar]

* Ingo Molnar <mingo@kernel.org> wrote:

> > cmdline is parse-options.c.
> > 
> > IOW, that's splitting it into too granulary pieces with 1-2 
> > compilation units ber library.
> 
> I see no problem with that - it's basically like util/*.c is, just 
> between tools.

I.e.:

comet:~/tip/tools/perf> ls util/*.h
util/annotate.h   util/data.h       util/fs.h           util/parse-events-bison.h  util/probe-event.h   util/sort.h       util/thread.h       util/values.h
util/build-id.h   util/debug.h      util/header.h       util/parse-events-flex.h   util/probe-finder.h  util/stat.h       util/thread_map.h   util/vdso.h
util/cache.h      util/dso.h        util/help.h         util/parse-events.h        util/pstack.h        util/strbuf.h     util/tool.h         util/xyarray.h
util/callchain.h  util/dwarf-aux.h  util/hist.h         util/parse-options.h       util/quote.h         util/strfilter.h  util/top.h
util/cgroup.h     util/event.h      util/intlist.h      util/perf_regs.h           util/rblist.h        util/strlist.h    util/trace-event.h
util/color.h      util/evlist.h     util/levenshtein.h  util/pmu-bison.h           util/run-command.h   util/svghelper.h  util/types.h
util/comm.h       util/evsel.h      util/machine.h      util/pmu-flex.h            util/session.h       util/symbol.h     util/unwind.h
util/cpumap.h     util/exec_cmd.h   util/map.h          util/pmu.h                 util/sigchain.h      util/target.h     util/util.h

That is pretty healty granularity IMO.

Do we want a separate directory for each one? I don't see a big 
problem with doing that, but it could be kept in tools/lib/util/ or 
tools/lib/core/ as well, _as long as they are not lumped together and 
as long as the individual .h files are kept_.

That also means that these bits shouldn't really be librarized in the 
classical sense into a single .a and linked into whatever tool uses 
it, but should be used individually as singular targets with clean .h 
interfaces to utilize them topically.

That also means that utilities won't run into any dependency problems, 
and the build will be faster as well as it all will be a single 
dependency graph within a single make session.

Thanks,

	Ingo

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Fri, Nov 22, 2013 at 04:39:11PM +0100, Ingo Molnar wrote:
> I see no problem with that - it's basically like util/*.c is, just
> between tools.

But why? Why it is a good thing to have to pay attention to linking to
10 minilibs when you're using 10 utilities for your tool instead of a
small number of topic libraries, 2-3 tops?

What's wrong with the split:

* generic stuff
* trace events
* perf events

?

With "generic stuff" being something like glibc. There's hardly a tool
that needs/links to *all* of glibs's functionality yet glibs doesn't get
split. Do you see what I mean?

> What dependencies do you mean? The only constraint is to not make
> it circular - but that's easy to do if they are nicely separated
> per concept. I don't think rbtree.h ever wants to include cmdline
> processing or debugfs processing.

But if you have a single .a library, you don't care about which
minilibrary to link to what. You basically do take libkapi.a and you're
good to go - no need to hunt every dependency.

With the split above, for example, libkapi.a links to glibc only.
libtraceevent.a and libperfevent.a both link to libkapi.a and glibc. It
is all nice and clean.

If later we decide to carve out certain generic functionality from
libkapi.a, then we do that then instead of doing the splitting now when
we don't exactly know what's going to grow in which direction in the
future...

> For userspace and for kernel space subsystems a single .h file per
> separate concept works the best. That is why we have a separate
> rbtree.h, list.h, slab.h, etc.

That doesn't change - if you've looked at my split diffstat, the headers
remain:

"This is tools/lib/lk/liblk.a which contains only compilation units which
contain generic code. The idea behind this library is to be used by all
tools in tools/.

 tools/{perf/util => lib/lk}/color.c                |   69 +-
 tools/{perf/util => lib/lk}/color.h                |   34 +-
 tools/{perf/util => lib/lk}/cpumap.c               |    9 +-
 tools/{perf/util => lib/lk}/cpumap.h               |    4 +
 tools/{perf/util => lib/lk}/ctype.c                |    2 +-
 tools/{perf/util => lib/lk}/hweight.c              |    1 +
 tools/{perf/util => lib/lk}/include/linux/magic.h  |    0
 tools/{perf/util => lib/lk}/parse-options.c        |    6 +-
 tools/{perf/util => lib/lk}/parse-options.h        |   13 +-
 tools/{perf/util => lib/lk}/rblist.c               |    0
 tools/{perf/util => lib/lk}/rblist.h               |    6 +-
 tools/{perf/util => lib/lk}/strbuf.c               |    3 +-
 tools/{perf/util => lib/lk}/strbuf.h               |   15 +-
 tools/{perf/util => lib/lk}/string.c               |   42 +-
 tools/{perf/util => lib/lk}/strlist.c              |    0
 tools/{perf/util => lib/lk}/strlist.h              |    6 +-
 tools/{perf/util => lib/lk}/sysfs.c                |    4 +
 tools/{perf/util => lib/lk}/thread_map.c           |    0
 tools/{perf/util => lib/lk}/thread_map.h           |    6 +-
 tools/{perf/util => lib/lk}/types.h                |    6 +-
 tools/{perf/util => lib/lk}/usage.c                |    2 +-
 tools/{perf/util => lib/lk}/wrapper.c              |    3 +-
 tools/{perf/util => lib/lk}/xyarray.c              |    0
 tools/{perf/util => lib/lk}/xyarray.h              |    6 +-
 tools/lib/lk/Makefile                              |   62 +-
 tools/lib/lk/config.c                              |  370 ++++++
 tools/lib/lk/config.h                              |   13 +
 tools/lib/lk/ctype.h                               |   37 +
 tools/lib/lk/hweight.h                             |   13 +
 tools/lib/lk/string.h                              |   21 +
 tools/lib/lk/sysfs.h                               |   12 +
 tools/lib/lk/usage.h                               |   34 +
 tools/lib/lk/util.c                                |   18 +
 tools/lib/lk/util.h                                |   38 +
 tools/lib/lk/wrapper.h                             |   27 +
"

> > And, since we're keeping it internal, we can do the split the other 
> > way around instead - first do the single generic library and then 
> > carve out a certain subset of functionality if/when it makes sense.
> 
> Why?

Because it is the only thing that makes sense to me. What I don't
understand is why we need (actually I will have to do it :-\) to
split the generic util/* parts into so many small .a libs? I fail to
understand the reason.

Why isn't better to have a single libkapi.a containing the files above
than having multiple smaller ones?

> Hey, that's an important point of agreement! :-)

Oh, I don't disagree - I just fail to see why you guys need this too
granulary splitup instead of three topic libraries. It just doesn't make
sense to me at all from where I'm standing...

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Fri, Nov 22, 2013 at 04:54:25PM +0100, Ingo Molnar wrote:
> comet:~/tip/tools/perf> ls util/*.h
> util/annotate.h   util/data.h       util/fs.h           util/parse-events-bison.h  util/probe-event.h   util/sort.h       util/thread.h       util/values.h
> util/build-id.h   util/debug.h      util/header.h       util/parse-events-flex.h   util/probe-finder.h  util/stat.h       util/thread_map.h   util/vdso.h
> util/cache.h      util/dso.h        util/help.h         util/parse-events.h        util/pstack.h        util/strbuf.h     util/tool.h         util/xyarray.h
> util/callchain.h  util/dwarf-aux.h  util/hist.h         util/parse-options.h       util/quote.h         util/strfilter.h  util/top.h
> util/cgroup.h     util/event.h      util/intlist.h      util/perf_regs.h           util/rblist.h        util/strlist.h    util/trace-event.h
> util/color.h      util/evlist.h     util/levenshtein.h  util/pmu-bison.h           util/run-command.h   util/svghelper.h  util/types.h
> util/comm.h       util/evsel.h      util/machine.h      util/pmu-flex.h            util/session.h       util/symbol.h     util/unwind.h
> util/cpumap.h     util/exec_cmd.h   util/map.h          util/pmu.h                 util/sigchain.h      util/target.h     util/util.h
> 
> That is pretty healty granularity IMO.
> 
> Do we want a separate directory for each one?

For each single one of them? This would be insane.

> I don't see a big problem with doing that, but it could be kept in
> tools/lib/util/ or tools/lib/core/ as well,

That's much better :)

> _as long as they are not lumped together

Why not a single .a?

> and as long as the individual .h files are kept_.

This has never stood for debate - headers are kept as is.

> That also means that these bits shouldn't really be librarized in the
> classical sense into a single .a and linked into whatever tool uses
> it, but should be used individually as singular targets with clean .h
> interfaces to utilize them topically.

Yeah, but why?

> That also means that utilities won't run into any dependency problems,
> and the build will be faster as well as it all will be a single
> dependency graph within a single make session.

That's maybe the only half-reason for not lumping them together I've
read so far. I say half-reason because the preprocessor already will
include only stuff it needs. And if that were a problem, glibc would've
been multiple libs too.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Ingo Molnar]

* Borislav Petkov <bp@alien8.de> wrote:

> On Fri, Nov 22, 2013 at 04:54:25PM +0100, Ingo Molnar wrote:
> > comet:~/tip/tools/perf> ls util/*.h
> > util/annotate.h   util/data.h       util/fs.h           util/parse-events-bison.h  util/probe-event.h   util/sort.h       util/thread.h       util/values.h
> > util/build-id.h   util/debug.h      util/header.h       util/parse-events-flex.h   util/probe-finder.h  util/stat.h       util/thread_map.h   util/vdso.h
> > util/cache.h      util/dso.h        util/help.h         util/parse-events.h        util/pstack.h        util/strbuf.h     util/tool.h         util/xyarray.h
> > util/callchain.h  util/dwarf-aux.h  util/hist.h         util/parse-options.h       util/quote.h         util/strfilter.h  util/top.h
> > util/cgroup.h     util/event.h      util/intlist.h      util/perf_regs.h           util/rblist.h        util/strlist.h    util/trace-event.h
> > util/color.h      util/evlist.h     util/levenshtein.h  util/pmu-bison.h           util/run-command.h   util/svghelper.h  util/types.h
> > util/comm.h       util/evsel.h      util/machine.h      util/pmu-flex.h            util/session.h       util/symbol.h     util/unwind.h
> > util/cpumap.h     util/exec_cmd.h   util/map.h          util/pmu.h                 util/sigchain.h      util/target.h     util/util.h
> > 
> > That is pretty healty granularity IMO.
> > 
> > Do we want a separate directory for each one?
> 
> For each single one of them? This would be insane.

Not necessarily, if the number goes up - obviously then we'd also want 
to add some second directory structure to organize them into broad 
categories or so.

> > I don't see a big problem with doing that, but it could be kept in 
> > tools/lib/util/ or tools/lib/core/ as well,
> 
> That's much better :)
> 
> > _as long as they are not lumped together
> 
> Why not a single .a?

Unnecessarily longer build time.

> > That also means that these bits shouldn't really be librarized in 
> > the classical sense into a single .a and linked into whatever tool 
> > uses it, but should be used individually as singular targets with 
> > clean .h interfaces to utilize them topically.
> 
> Yeah, but why?

If a tool uses just a handful of these (hopefully quickly increasing 
number of) facilities then it should not be burdened by building it 
all.

> > That also means that utilities won't run into any dependency 
> > problems, and the build will be faster as well as it all will be a 
> > single dependency graph within a single make session.
> 
> That's maybe the only half-reason for not lumping them together I've 
> read so far. I say half-reason because the preprocessor already will 
> include only stuff it needs. And if that were a problem, glibc 
> would've been multiple libs too.

The preprocessor does nothing with the .a - eliminating extra stuff 
from the .a is a link time step, and that means the whole .a will be 
built first ... just to throw away bits of it.

( Also, arguably, from a sw design POV glibc should probably have been
  multiple libs, but that's water down the bridge. No need to 
  repeat/clone such mistakes though. )

Thanks,

	Ingo

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Ingo Molnar]

* Borislav Petkov <bp@alien8.de> wrote:

> On Fri, Nov 22, 2013 at 04:39:11PM +0100, Ingo Molnar wrote:
>
> > I see no problem with that - it's basically like util/*.c is, just 
> > between tools.
> 
> But why? Why it is a good thing to have to pay attention to linking 
> to 10 minilibs when you're using 10 utilities for your tool instead 
> of a small number of topic libraries, 2-3 tops?

It's a single line added to the Makefile, the moment a .h is used for 
the first time. That's not any appreciable overhead.

This would also allow us to farm out most of tools/perf/util/ into 
tools/lib/, without any noticeable changes in build performance or 
build dependencies. Down the line it would (hopefully) result in code 
improvements to these infrastructure bits, sourced from different 
tools.

> What's wrong with the split:
> 
> * generic stuff
> * trace events
> * perf events
> 
> ?

Well, the natural evolution of interfaces ended up with such a split 
up:

comet:~/tip/tools/perf> ls util/*.h
util/annotate.h   util/hist.h           util/strbuf.h
util/build-id.h   util/intlist.h        util/strfilter.h
util/cache.h      util/levenshtein.h    util/strlist.h
util/callchain.h  util/machine.h        util/svghelper.h
util/cgroup.h     util/map.h            util/symbol.h
util/color.h      util/parse-events.h   util/target.h
util/comm.h       util/parse-options.h  util/thread.h
util/cpumap.h     util/perf_regs.h      util/thread_map.h
util/data.h       util/pmu.h            util/tool.h
util/debug.h      util/probe-event.h    util/top.h
util/dso.h        util/probe-finder.h   util/trace-event.h
util/dwarf-aux.h  util/pstack.h         util/types.h
util/event.h      util/quote.h          util/unwind.h
util/evlist.h     util/rblist.h         util/util.h
util/evsel.h      util/run-command.h    util/values.h
util/exec_cmd.h   util/session.h        util/vdso.h
util/fs.h         util/sigchain.h       util/xyarray.h
util/header.h     util/sort.h
util/help.h       util/stat.h

If we want additional structure to it then it should be done via the 
namespace, not by forcing them into bigger .a's. So this kind of extra 
structure makes sense:

  api/types/rbtree.h
  api/types/strbuf.h
  api/formats/dwarf/unwind.h
  api/kernel/pmu.h
  api/kernel/cgroup.h
  api/kernel/debugfs.h

But stuffing them into types.a, formats.a, kernel.a, not so much.

> With "generic stuff" being something like glibc. There's hardly a 
> tool that needs/links to *all* of glibs's functionality yet glibs 
> doesn't get split. Do you see what I mean?

glibc being such a catch-all library is:

 - partly a historic artifact caused by other constraints that don't 
   affect our tooling landscape here

 - partly a political artifact caused by thinking that does not affect 
   our tooling landscape

 - partly a technological mistake.

There's no need for us to repeat that, at least at this stage.

> > What dependencies do you mean? The only constraint is to not make 
> > it circular - but that's easy to do if they are nicely separated 
> > per concept. I don't think rbtree.h ever wants to include cmdline 
> > processing or debugfs processing.
> 
> But if you have a single .a library, you don't care about which 
> minilibrary to link to what. You basically do take libkapi.a and 
> you're good to go - no need to hunt every dependency.

You still need to figure out the .h file - at that point, when you are 
using it for the first time in your tool project, you add the .c file 
to the Makefile - it's not hard and there are real advantages.

> With the split above, for example, libkapi.a links to glibc only. 
> libtraceevent.a and libperfevent.a both link to libkapi.a and glibc. 
> It is all nice and clean.

It does not look that nice and clean once I consider all the nice 
helpers that exist in util/*.[ch] - and which we'd like to share as 
well.

Thanks,

	Ingo

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Tue, Nov 26, 2013 at 07:17:45PM +0100, Ingo Molnar wrote:
> It's a single line added to the Makefile, the moment a .h is used for
> the first time. That's not any appreciable overhead.

Hmm, not quite. It is a bit more jumping through hoops - look at the
variables LK_DIR, LK_PATH and LIBLK for what we're doing right now to
include the library and to enable all the different ways of building
perf.

And we're doing the same for libtraceevent.a too so it has its own set
of variables.

> This would also allow us to farm out most of tools/perf/util/ into
> tools/lib/...

That's the idea. :)

> Well, the natural evolution of interfaces ended up with such a split 
> up:
> 
> comet:~/tip/tools/perf> ls util/*.h
> util/annotate.h   util/hist.h           util/strbuf.h
> util/build-id.h   util/intlist.h        util/strfilter.h
> util/cache.h      util/levenshtein.h    util/strlist.h
> util/callchain.h  util/machine.h        util/svghelper.h
> util/cgroup.h     util/map.h            util/symbol.h
> util/color.h      util/parse-events.h   util/target.h
> util/comm.h       util/parse-options.h  util/thread.h
> util/cpumap.h     util/perf_regs.h      util/thread_map.h
> util/data.h       util/pmu.h            util/tool.h
> util/debug.h      util/probe-event.h    util/top.h
> util/dso.h        util/probe-finder.h   util/trace-event.h
> util/dwarf-aux.h  util/pstack.h         util/types.h
> util/event.h      util/quote.h          util/unwind.h
> util/evlist.h     util/rblist.h         util/util.h
> util/evsel.h      util/run-command.h    util/values.h
> util/exec_cmd.h   util/session.h        util/vdso.h
> util/fs.h         util/sigchain.h       util/xyarray.h
> util/header.h     util/sort.h
> util/help.h       util/stat.h
> 
> If we want additional structure to it then it should be done via the 
> namespace, not by forcing them into bigger .a's. So this kind of extra 
> structure makes sense:
> 
>   api/types/rbtree.h
>   api/types/strbuf.h
>   api/formats/dwarf/unwind.h
>   api/kernel/pmu.h
>   api/kernel/cgroup.h
>   api/kernel/debugfs.h

Ok, splitting them into topics actually makes sense.

> But stuffing them into types.a, formats.a, kernel.a, not so much.

Huh, why not? We take the corresponding .c files and create a single .a
archive per topic from them. This makes a whole lot of sense to me as a
compromise between having a single .a and one .a per compilation unit.

Remember, we still need to do the game with the *LK* variables above
with each new lib and path.

> You still need to figure out the .h file - at that point, when you are
> using it for the first time in your tool project, you add the .c file
> to the Makefile - it's not hard and there are real advantages.

See above.

> It does not look that nice and clean once I consider all the nice
> helpers that exist in util/*.[ch] - and which we'd like to share as
> well.

We will - if not in a single .a at least in a couple of .a's as topic
libraries but please no one .a per a single .c.

For example, it makes a lot of sense to put evsel.* and evlist.* into
one libperfevent.a library which deals solely with handling perf events.

Having libevsel.a and libevlist.a OTOH not so much.

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Tue, Nov 26, 2013 at 07:03:33PM +0100, Ingo Molnar wrote:
> Not necessarily, if the number goes up - obviously then we'd also want
> to add some second directory structure to organize them into broad
> categories or so.

Right, topic libraries. That is starting to make more sense. :)

> Unnecessarily longer build time.

Ok.

> If a tool uses just a handful of these (hopefully quickly increasing
> number of) facilities then it should not be burdened by building it
> all.

Ok. This still hints at topic libraries as being the best alternative.

> The preprocessor does nothing with the .a - eliminating extra stuff
> from the .a is a link time step, and that means the whole .a will be
> built first ...

Ok, this is an argument I can get on board with.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Borislav Petkov]

On Wed, Nov 27, 2013 at 04:39:44PM +0100, Borislav Petkov wrote:
> Ok, splitting them into topics actually makes sense.
> 
> > But stuffing them into types.a, formats.a, kernel.a, not so much.
> 
> Huh, why not? We take the corresponding .c files and create a single .a
> archive per topic from them. This makes a whole lot of sense to me as a
> compromise between having a single .a and one .a per compilation unit.
> 
> Remember, we still need to do the game with the *LK* variables above
> with each new lib and path.

And this is how it could look like below.

We have a common tools/lib/api/ place where the Makefile lives and then
we place the headers in subdirs. For example, all the fs-related stuff
goes to tools/lib/api/fs/ from which we get libapikfs.a (acme, almost
the naming you wanted :-)) and we link it into the tools which need it - in this
case perf and tools/vm/page-types.

Thoughts?

--
From: Borislav Petkov <bp@suse.de>
Subject: [PATCH] tools: Convert to new topic libraries

Move debugfs.* to api/fs/

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 tools/lib/{lk => api}/Makefile     |  6 +++---
 tools/lib/{lk => api/fs}/debugfs.c |  0
 tools/lib/{lk => api/fs}/debugfs.h |  0
 tools/perf/Makefile.perf           | 30 +++++++++++++++---------------
 tools/perf/builtin-kvm.c           |  2 +-
 tools/perf/builtin-probe.c         |  2 +-
 tools/perf/perf.c                  |  2 +-
 tools/perf/tests/parse-events.c    |  2 +-
 tools/perf/util/evlist.c           |  2 +-
 tools/perf/util/evsel.c            |  2 +-
 tools/perf/util/parse-events.c     |  2 +-
 tools/perf/util/probe-event.c      |  2 +-
 tools/perf/util/setup.py           |  4 ++--
 tools/perf/util/trace-event-info.c |  2 +-
 tools/perf/util/util.h             |  2 +-
 tools/vm/Makefile                  | 14 +++++++-------
 tools/vm/page-types.c              |  2 +-
 17 files changed, 38 insertions(+), 38 deletions(-)
 rename tools/lib/{lk => api}/Makefile (90%)
 rename tools/lib/{lk => api/fs}/debugfs.c (100%)
 rename tools/lib/{lk => api/fs}/debugfs.h (100%)

diff --git a/tools/lib/lk/Makefile b/tools/lib/api/Makefile
similarity index 90%
rename from tools/lib/lk/Makefile
rename to tools/lib/api/Makefile
index 3dba0a4aebbf..d749cdc8e1d4 100644
--- a/tools/lib/lk/Makefile
+++ b/tools/lib/api/Makefile
@@ -7,11 +7,11 @@ AR = $(CROSS_COMPILE)ar
 LIB_H=
 LIB_OBJS=
 
-LIB_H += debugfs.h
+LIB_H += fs/debugfs.h
 
-LIB_OBJS += $(OUTPUT)debugfs.o
+LIB_OBJS += $(OUTPUT)fs/debugfs.o
 
-LIBFILE = liblk.a
+LIBFILE = libapikfs.a
 
 CFLAGS = -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS) $(EXTRA_CFLAGS) -fPIC
 EXTLIBS = -lelf -lpthread -lrt -lm
diff --git a/tools/lib/lk/debugfs.c b/tools/lib/api/fs/debugfs.c
similarity index 100%
rename from tools/lib/lk/debugfs.c
rename to tools/lib/api/fs/debugfs.c
diff --git a/tools/lib/lk/debugfs.h b/tools/lib/api/fs/debugfs.h
similarity index 100%
rename from tools/lib/lk/debugfs.h
rename to tools/lib/api/fs/debugfs.h
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index e416ccc7d831..5f8a8de62ddd 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -86,7 +86,7 @@ FLEX    = flex
 BISON   = bison
 STRIP   = strip
 
-LK_DIR          = $(srctree)/tools/lib/lk/
+LIB_DIR          = $(srctree)/tools/lib/api/
 TRACE_EVENT_DIR = $(srctree)/tools/lib/traceevent/
 
 # include config/Makefile by default and rule out
@@ -127,20 +127,20 @@ strip-libs = $(filter-out -l%,$(1))
 ifneq ($(OUTPUT),)
   TE_PATH=$(OUTPUT)
 ifneq ($(subdir),)
-  LK_PATH=$(OUTPUT)/../lib/lk/
+  LIB_PATH=$(OUTPUT)/../lib/api/
 else
-  LK_PATH=$(OUTPUT)
+  LIB_PATH=$(OUTPUT)
 endif
 else
   TE_PATH=$(TRACE_EVENT_DIR)
-  LK_PATH=$(LK_DIR)
+  LIB_PATH=$(LIB_DIR)
 endif
 
 LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
 export LIBTRACEEVENT
 
-LIBLK = $(LK_PATH)liblk.a
-export LIBLK
+LIBAPIKFS = $(LIB_PATH)libapikfs.a
+export LIBAPIKFS
 
 # python extension build directories
 PYTHON_EXTBUILD     := $(OUTPUT)python_ext_build/
@@ -151,7 +151,7 @@ export PYTHON_EXTBUILD_LIB PYTHON_EXTBUILD_TMP
 python-clean := $(call QUIET_CLEAN, python) $(RM) -r $(PYTHON_EXTBUILD) $(OUTPUT)python/perf.so
 
 PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
-PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py $(LIBTRACEEVENT) $(LIBLK)
+PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py $(LIBTRACEEVENT) $(LIBAPIKFS)
 
 $(OUTPUT)python/perf.so: $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS)
 	$(QUIET_GEN)CFLAGS='$(CFLAGS)' $(PYTHON_WORD) util/setup.py \
@@ -438,7 +438,7 @@ BUILTIN_OBJS += $(OUTPUT)builtin-inject.o
 BUILTIN_OBJS += $(OUTPUT)tests/builtin-test.o
 BUILTIN_OBJS += $(OUTPUT)builtin-mem.o
 
-PERFLIBS = $(LIB_FILE) $(LIBLK) $(LIBTRACEEVENT)
+PERFLIBS = $(LIB_FILE) $(LIBAPIKFS) $(LIBTRACEEVENT)
 
 # We choose to avoid "if .. else if .. else .. endif endif"
 # because maintaining the nesting to match is a pain.  If
@@ -717,19 +717,19 @@ $(LIBTRACEEVENT)-clean:
 	$(call QUIET_CLEAN, libtraceevent)
 	@$(MAKE) -C $(TRACE_EVENT_DIR) O=$(OUTPUT) clean >/dev/null
 
-LIBLK_SOURCES = $(wildcard $(LK_PATH)*.[ch])
+LIBAPIKFS_SOURCES = $(wildcard $(LIB_PATH)fs/*.[ch])
 
 # if subdir is set, we've been called from above so target has been built
 # already
-$(LIBLK): $(LIBLK_SOURCES)
+$(LIBAPIKFS): $(LIBAPIKFS_SOURCES)
 ifeq ($(subdir),)
-	$(QUIET_SUBDIR0)$(LK_DIR) $(QUIET_SUBDIR1) O=$(OUTPUT) liblk.a
+	$(QUIET_SUBDIR0)$(LIB_DIR) $(QUIET_SUBDIR1) O=$(OUTPUT) libapikfs.a
 endif
 
-$(LIBLK)-clean:
+$(LIBAPIKFS)-clean:
 ifeq ($(subdir),)
-	$(call QUIET_CLEAN, liblk)
-	@$(MAKE) -C $(LK_DIR) O=$(OUTPUT) clean >/dev/null
+	$(call QUIET_CLEAN, LIBAPIKFS)
+	@$(MAKE) -C $(LIB_DIR) O=$(OUTPUT) clean >/dev/null
 endif
 
 help:
@@ -868,7 +868,7 @@ config-clean:
 	$(call QUIET_CLEAN, config)
 	@$(MAKE) -C config/feature-checks clean >/dev/null
 
-clean: $(LIBTRACEEVENT)-clean $(LIBLK)-clean config-clean
+clean: $(LIBTRACEEVENT)-clean $(LIBAPIKFS)-clean config-clean
 	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIB_OBJS) $(BUILTIN_OBJS) $(LIB_FILE) $(OUTPUT)perf-archive $(OUTPUT)perf.o $(LANG_BINDINGS) $(GTK_OBJS)
 	$(call QUIET_CLEAN, core-progs) $(RM) $(ALL_PROGRAMS) perf
 	$(call QUIET_CLEAN, core-gen)   $(RM)  *.spec *.pyc *.pyo */*.pyc */*.pyo $(OUTPUT)common-cmds.h TAGS tags cscope* $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)PERF-CFLAGS $(OUTPUT)util/*-bison* $(OUTPUT)util/*-flex*
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index f8bf5f244d77..252b6ed507b7 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -13,7 +13,7 @@
 #include "util/parse-options.h"
 #include "util/trace-event.h"
 #include "util/debug.h"
-#include <lk/debugfs.h>
+#include <api/fs/debugfs.h>
 #include "util/tool.h"
 #include "util/stat.h"
 #include "util/top.h"
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 6ea9e85bdc00..c98ccb570509 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -37,7 +37,7 @@
 #include "util/strfilter.h"
 #include "util/symbol.h"
 #include "util/debug.h"
-#include <lk/debugfs.h>
+#include <api/fs/debugfs.h>
 #include "util/parse-options.h"
 #include "util/probe-finder.h"
 #include "util/probe-event.h"
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 8b38b4e80ec2..431798a4110d 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -13,7 +13,7 @@
 #include "util/quote.h"
 #include "util/run-command.h"
 #include "util/parse-events.h"
-#include <lk/debugfs.h>
+#include <api/fs/debugfs.h>
 #include <pthread.h>
 
 const char perf_usage_string[] =
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 3cbd10496087..e4ce8aed29d3 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -3,7 +3,7 @@
 #include "evsel.h"
 #include "evlist.h"
 #include "fs.h"
-#include <lk/debugfs.h>
+#include <api/fs/debugfs.h>
 #include "tests.h"
 #include <linux/hw_breakpoint.h>
 
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 76fa76431329..a07d86397adf 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -7,7 +7,7 @@
  * Released under the GPL v2. (and only v2, not any later version)
  */
 #include "util.h"
-#include <lk/debugfs.h>
+#include <api/fs/debugfs.h>
 #include <poll.h>
 #include "cpumap.h"
 #include "thread_map.h"
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index b5fe7f9b2e15..2268c80ba0d0 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -9,7 +9,7 @@
 
 #include <byteswap.h>
 #include <linux/bitops.h>
-#include <lk/debugfs.h>
+#include <api/fs/debugfs.h>
 #include <traceevent/event-parse.h>
 #include <linux/hw_breakpoint.h>
 #include <linux/perf_event.h>
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 969cb8f0d88d..094c28ba2fae 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -10,7 +10,7 @@
 #include "symbol.h"
 #include "cache.h"
 #include "header.h"
-#include <lk/debugfs.h>
+#include <api/fs/debugfs.h>
 #include "parse-events-bison.h"
 #define YY_EXTRA_TYPE int
 #include "parse-events-flex.h"
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 9c6989ca2bea..8cd98ef628be 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -40,7 +40,7 @@
 #include "color.h"
 #include "symbol.h"
 #include "thread.h"
-#include <lk/debugfs.h>
+#include <api/fs/debugfs.h>
 #include "trace-event.h"	/* For __maybe_unused */
 #include "probe-event.h"
 #include "probe-finder.h"
diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
index 58ea5ca6c255..d0aee4b9dfd4 100644
--- a/tools/perf/util/setup.py
+++ b/tools/perf/util/setup.py
@@ -25,7 +25,7 @@ cflags += ['-fno-strict-aliasing', '-Wno-write-strings', '-Wno-unused-parameter'
 build_lib = getenv('PYTHON_EXTBUILD_LIB')
 build_tmp = getenv('PYTHON_EXTBUILD_TMP')
 libtraceevent = getenv('LIBTRACEEVENT')
-liblk = getenv('LIBLK')
+libapikfs = getenv('LIBAPIKFS')
 
 ext_sources = [f.strip() for f in file('util/python-ext-sources')
 				if len(f.strip()) > 0 and f[0] != '#']
@@ -34,7 +34,7 @@ perf = Extension('perf',
 		  sources = ext_sources,
 		  include_dirs = ['util/include'],
 		  extra_compile_args = cflags,
-		  extra_objects = [libtraceevent, liblk],
+		  extra_objects = [libtraceevent, libapikfs],
                  )
 
 setup(name='perf',
diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
index f3c9e551bd35..c354b95a2348 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -38,7 +38,7 @@
 
 #include "../perf.h"
 #include "trace-event.h"
-#include <lk/debugfs.h>
+#include <api/fs/debugfs.h>
 #include "evsel.h"
 
 #define VERSION "0.5"
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index c8f362daba87..eb75bbb106e9 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -71,7 +71,7 @@
 #include <linux/magic.h>
 #include "types.h"
 #include <sys/ttydefaults.h>
-#include <lk/debugfs.h>
+#include <api/fs/debugfs.h>
 #include <termios.h>
 
 extern const char *graph_line;
diff --git a/tools/vm/Makefile b/tools/vm/Makefile
index 24e9ddd93fa4..3d907dacf2ac 100644
--- a/tools/vm/Makefile
+++ b/tools/vm/Makefile
@@ -2,21 +2,21 @@
 #
 TARGETS=page-types slabinfo
 
-LK_DIR = ../lib/lk
-LIBLK = $(LK_DIR)/liblk.a
+LIB_DIR = ../lib/api
+LIBS = $(LIB_DIR)/libapikfs.a
 
 CC = $(CROSS_COMPILE)gcc
 CFLAGS = -Wall -Wextra -I../lib/
-LDFLAGS = $(LIBLK)
+LDFLAGS = $(LIBS)
 
-$(TARGETS): liblk
+$(TARGETS): $(LIBS)
 
-liblk:
-	make -C $(LK_DIR)
+$(LIBS):
+	make -C $(LIB_DIR)
 
 %: %.c
 	$(CC) $(CFLAGS) -o $@ $< $(LDFLAGS)
 
 clean:
 	$(RM) page-types slabinfo
-	make -C ../lib/lk clean
+	make -C $(LIB_DIR) clean
diff --git a/tools/vm/page-types.c b/tools/vm/page-types.c
index d5e9d6d185c8..f9be24d9efac 100644
--- a/tools/vm/page-types.c
+++ b/tools/vm/page-types.c
@@ -36,7 +36,7 @@
 #include <sys/statfs.h>
 #include "../../include/uapi/linux/magic.h"
 #include "../../include/uapi/linux/kernel-page-flags.h"
-#include <lk/debugfs.h>
+#include <api/fs/debugfs.h>
 
 #ifndef MAX_PATH
 # define MAX_PATH 256
-- 
1.8.4


-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] perf: Move fs.* to generic lib/lk/

[Arnaldo Carvalho de Melo]

Em Thu, Nov 28, 2013 at 01:16:57PM +0100, Borislav Petkov escreveu:
> On Wed, Nov 27, 2013 at 04:39:44PM +0100, Borislav Petkov wrote:
> > Ok, splitting them into topics actually makes sense.
> > 
> > > But stuffing them into types.a, formats.a, kernel.a, not so much.
> > 
> > Huh, why not? We take the corresponding .c files and create a single .a
> > archive per topic from them. This makes a whole lot of sense to me as a
> > compromise between having a single .a and one .a per compilation unit.
> > 
> > Remember, we still need to do the game with the *LK* variables above
> > with each new lib and path.
> 
> And this is how it could look like below.
> 
> We have a common tools/lib/api/ place where the Makefile lives and then
> we place the headers in subdirs. For example, all the fs-related stuff
> goes to tools/lib/api/fs/ from which we get libapikfs.a (acme, almost
> the naming you wanted :-)) and we link it into the tools which need it - in this
> case perf and tools/vm/page-types.

Looking just at the description above: I like it, does what I think Ingo
suggested and that is how I think it should be done.

Looking at the implementation, I think some tools can even link directly
to the .o files, avoiding the .a file altogether.

But that is just an optimization/finer granularity tools/lib/
cherrypicking that toolers can make use of.

- Arnaldo

> Thoughts?
> 
> --
> From: Borislav Petkov <bp@suse.de>
> Subject: [PATCH] tools: Convert to new topic libraries
> 
> Move debugfs.* to api/fs/
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  tools/lib/{lk => api}/Makefile     |  6 +++---
>  tools/lib/{lk => api/fs}/debugfs.c |  0
>  tools/lib/{lk => api/fs}/debugfs.h |  0
>  tools/perf/Makefile.perf           | 30 +++++++++++++++---------------
>  tools/perf/builtin-kvm.c           |  2 +-
>  tools/perf/builtin-probe.c         |  2 +-
>  tools/perf/perf.c                  |  2 +-
>  tools/perf/tests/parse-events.c    |  2 +-
>  tools/perf/util/evlist.c           |  2 +-
>  tools/perf/util/evsel.c            |  2 +-
>  tools/perf/util/parse-events.c     |  2 +-
>  tools/perf/util/probe-event.c      |  2 +-
>  tools/perf/util/setup.py           |  4 ++--
>  tools/perf/util/trace-event-info.c |  2 +-
>  tools/perf/util/util.h             |  2 +-
>  tools/vm/Makefile                  | 14 +++++++-------
>  tools/vm/page-types.c              |  2 +-
>  17 files changed, 38 insertions(+), 38 deletions(-)
>  rename tools/lib/{lk => api}/Makefile (90%)
>  rename tools/lib/{lk => api/fs}/debugfs.c (100%)
>  rename tools/lib/{lk => api/fs}/debugfs.h (100%)
> 
> diff --git a/tools/lib/lk/Makefile b/tools/lib/api/Makefile
> similarity index 90%
> rename from tools/lib/lk/Makefile
> rename to tools/lib/api/Makefile
> index 3dba0a4aebbf..d749cdc8e1d4 100644
> --- a/tools/lib/lk/Makefile
> +++ b/tools/lib/api/Makefile
> @@ -7,11 +7,11 @@ AR = $(CROSS_COMPILE)ar
>  LIB_H=
>  LIB_OBJS=
>  
> -LIB_H += debugfs.h
> +LIB_H += fs/debugfs.h
>  
> -LIB_OBJS += $(OUTPUT)debugfs.o
> +LIB_OBJS += $(OUTPUT)fs/debugfs.o
>  
> -LIBFILE = liblk.a
> +LIBFILE = libapikfs.a
>  
>  CFLAGS = -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS) $(EXTRA_CFLAGS) -fPIC
>  EXTLIBS = -lelf -lpthread -lrt -lm
> diff --git a/tools/lib/lk/debugfs.c b/tools/lib/api/fs/debugfs.c
> similarity index 100%
> rename from tools/lib/lk/debugfs.c
> rename to tools/lib/api/fs/debugfs.c
> diff --git a/tools/lib/lk/debugfs.h b/tools/lib/api/fs/debugfs.h
> similarity index 100%
> rename from tools/lib/lk/debugfs.h
> rename to tools/lib/api/fs/debugfs.h
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index e416ccc7d831..5f8a8de62ddd 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -86,7 +86,7 @@ FLEX    = flex
>  BISON   = bison
>  STRIP   = strip
>  
> -LK_DIR          = $(srctree)/tools/lib/lk/
> +LIB_DIR          = $(srctree)/tools/lib/api/
>  TRACE_EVENT_DIR = $(srctree)/tools/lib/traceevent/
>  
>  # include config/Makefile by default and rule out
> @@ -127,20 +127,20 @@ strip-libs = $(filter-out -l%,$(1))
>  ifneq ($(OUTPUT),)
>    TE_PATH=$(OUTPUT)
>  ifneq ($(subdir),)
> -  LK_PATH=$(OUTPUT)/../lib/lk/
> +  LIB_PATH=$(OUTPUT)/../lib/api/
>  else
> -  LK_PATH=$(OUTPUT)
> +  LIB_PATH=$(OUTPUT)
>  endif
>  else
>    TE_PATH=$(TRACE_EVENT_DIR)
> -  LK_PATH=$(LK_DIR)
> +  LIB_PATH=$(LIB_DIR)
>  endif
>  
>  LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
>  export LIBTRACEEVENT
>  
> -LIBLK = $(LK_PATH)liblk.a
> -export LIBLK
> +LIBAPIKFS = $(LIB_PATH)libapikfs.a
> +export LIBAPIKFS
>  
>  # python extension build directories
>  PYTHON_EXTBUILD     := $(OUTPUT)python_ext_build/
> @@ -151,7 +151,7 @@ export PYTHON_EXTBUILD_LIB PYTHON_EXTBUILD_TMP
>  python-clean := $(call QUIET_CLEAN, python) $(RM) -r $(PYTHON_EXTBUILD) $(OUTPUT)python/perf.so
>  
>  PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
> -PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py $(LIBTRACEEVENT) $(LIBLK)
> +PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py $(LIBTRACEEVENT) $(LIBAPIKFS)
>  
>  $(OUTPUT)python/perf.so: $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS)
>  	$(QUIET_GEN)CFLAGS='$(CFLAGS)' $(PYTHON_WORD) util/setup.py \
> @@ -438,7 +438,7 @@ BUILTIN_OBJS += $(OUTPUT)builtin-inject.o
>  BUILTIN_OBJS += $(OUTPUT)tests/builtin-test.o
>  BUILTIN_OBJS += $(OUTPUT)builtin-mem.o
>  
> -PERFLIBS = $(LIB_FILE) $(LIBLK) $(LIBTRACEEVENT)
> +PERFLIBS = $(LIB_FILE) $(LIBAPIKFS) $(LIBTRACEEVENT)
>  
>  # We choose to avoid "if .. else if .. else .. endif endif"
>  # because maintaining the nesting to match is a pain.  If
> @@ -717,19 +717,19 @@ $(LIBTRACEEVENT)-clean:
>  	$(call QUIET_CLEAN, libtraceevent)
>  	@$(MAKE) -C $(TRACE_EVENT_DIR) O=$(OUTPUT) clean >/dev/null
>  
> -LIBLK_SOURCES = $(wildcard $(LK_PATH)*.[ch])
> +LIBAPIKFS_SOURCES = $(wildcard $(LIB_PATH)fs/*.[ch])
>  
>  # if subdir is set, we've been called from above so target has been built
>  # already
> -$(LIBLK): $(LIBLK_SOURCES)
> +$(LIBAPIKFS): $(LIBAPIKFS_SOURCES)
>  ifeq ($(subdir),)
> -	$(QUIET_SUBDIR0)$(LK_DIR) $(QUIET_SUBDIR1) O=$(OUTPUT) liblk.a
> +	$(QUIET_SUBDIR0)$(LIB_DIR) $(QUIET_SUBDIR1) O=$(OUTPUT) libapikfs.a
>  endif
>  
> -$(LIBLK)-clean:
> +$(LIBAPIKFS)-clean:
>  ifeq ($(subdir),)
> -	$(call QUIET_CLEAN, liblk)
> -	@$(MAKE) -C $(LK_DIR) O=$(OUTPUT) clean >/dev/null
> +	$(call QUIET_CLEAN, LIBAPIKFS)
> +	@$(MAKE) -C $(LIB_DIR) O=$(OUTPUT) clean >/dev/null
>  endif
>  
>  help:
> @@ -868,7 +868,7 @@ config-clean:
>  	$(call QUIET_CLEAN, config)
>  	@$(MAKE) -C config/feature-checks clean >/dev/null
>  
> -clean: $(LIBTRACEEVENT)-clean $(LIBLK)-clean config-clean
> +clean: $(LIBTRACEEVENT)-clean $(LIBAPIKFS)-clean config-clean
>  	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIB_OBJS) $(BUILTIN_OBJS) $(LIB_FILE) $(OUTPUT)perf-archive $(OUTPUT)perf.o $(LANG_BINDINGS) $(GTK_OBJS)
>  	$(call QUIET_CLEAN, core-progs) $(RM) $(ALL_PROGRAMS) perf
>  	$(call QUIET_CLEAN, core-gen)   $(RM)  *.spec *.pyc *.pyo */*.pyc */*.pyo $(OUTPUT)common-cmds.h TAGS tags cscope* $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)PERF-CFLAGS $(OUTPUT)util/*-bison* $(OUTPUT)util/*-flex*
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index f8bf5f244d77..252b6ed507b7 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -13,7 +13,7 @@
>  #include "util/parse-options.h"
>  #include "util/trace-event.h"
>  #include "util/debug.h"
> -#include <lk/debugfs.h>
> +#include <api/fs/debugfs.h>
>  #include "util/tool.h"
>  #include "util/stat.h"
>  #include "util/top.h"
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 6ea9e85bdc00..c98ccb570509 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -37,7 +37,7 @@
>  #include "util/strfilter.h"
>  #include "util/symbol.h"
>  #include "util/debug.h"
> -#include <lk/debugfs.h>
> +#include <api/fs/debugfs.h>
>  #include "util/parse-options.h"
>  #include "util/probe-finder.h"
>  #include "util/probe-event.h"
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index 8b38b4e80ec2..431798a4110d 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -13,7 +13,7 @@
>  #include "util/quote.h"
>  #include "util/run-command.h"
>  #include "util/parse-events.h"
> -#include <lk/debugfs.h>
> +#include <api/fs/debugfs.h>
>  #include <pthread.h>
>  
>  const char perf_usage_string[] =
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index 3cbd10496087..e4ce8aed29d3 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -3,7 +3,7 @@
>  #include "evsel.h"
>  #include "evlist.h"
>  #include "fs.h"
> -#include <lk/debugfs.h>
> +#include <api/fs/debugfs.h>
>  #include "tests.h"
>  #include <linux/hw_breakpoint.h>
>  
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 76fa76431329..a07d86397adf 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -7,7 +7,7 @@
>   * Released under the GPL v2. (and only v2, not any later version)
>   */
>  #include "util.h"
> -#include <lk/debugfs.h>
> +#include <api/fs/debugfs.h>
>  #include <poll.h>
>  #include "cpumap.h"
>  #include "thread_map.h"
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index b5fe7f9b2e15..2268c80ba0d0 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -9,7 +9,7 @@
>  
>  #include <byteswap.h>
>  #include <linux/bitops.h>
> -#include <lk/debugfs.h>
> +#include <api/fs/debugfs.h>
>  #include <traceevent/event-parse.h>
>  #include <linux/hw_breakpoint.h>
>  #include <linux/perf_event.h>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 969cb8f0d88d..094c28ba2fae 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -10,7 +10,7 @@
>  #include "symbol.h"
>  #include "cache.h"
>  #include "header.h"
> -#include <lk/debugfs.h>
> +#include <api/fs/debugfs.h>
>  #include "parse-events-bison.h"
>  #define YY_EXTRA_TYPE int
>  #include "parse-events-flex.h"
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 9c6989ca2bea..8cd98ef628be 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -40,7 +40,7 @@
>  #include "color.h"
>  #include "symbol.h"
>  #include "thread.h"
> -#include <lk/debugfs.h>
> +#include <api/fs/debugfs.h>
>  #include "trace-event.h"	/* For __maybe_unused */
>  #include "probe-event.h"
>  #include "probe-finder.h"
> diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
> index 58ea5ca6c255..d0aee4b9dfd4 100644
> --- a/tools/perf/util/setup.py
> +++ b/tools/perf/util/setup.py
> @@ -25,7 +25,7 @@ cflags += ['-fno-strict-aliasing', '-Wno-write-strings', '-Wno-unused-parameter'
>  build_lib = getenv('PYTHON_EXTBUILD_LIB')
>  build_tmp = getenv('PYTHON_EXTBUILD_TMP')
>  libtraceevent = getenv('LIBTRACEEVENT')
> -liblk = getenv('LIBLK')
> +libapikfs = getenv('LIBAPIKFS')
>  
>  ext_sources = [f.strip() for f in file('util/python-ext-sources')
>  				if len(f.strip()) > 0 and f[0] != '#']
> @@ -34,7 +34,7 @@ perf = Extension('perf',
>  		  sources = ext_sources,
>  		  include_dirs = ['util/include'],
>  		  extra_compile_args = cflags,
> -		  extra_objects = [libtraceevent, liblk],
> +		  extra_objects = [libtraceevent, libapikfs],
>                   )
>  
>  setup(name='perf',
> diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
> index f3c9e551bd35..c354b95a2348 100644
> --- a/tools/perf/util/trace-event-info.c
> +++ b/tools/perf/util/trace-event-info.c
> @@ -38,7 +38,7 @@
>  
>  #include "../perf.h"
>  #include "trace-event.h"
> -#include <lk/debugfs.h>
> +#include <api/fs/debugfs.h>
>  #include "evsel.h"
>  
>  #define VERSION "0.5"
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index c8f362daba87..eb75bbb106e9 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -71,7 +71,7 @@
>  #include <linux/magic.h>
>  #include "types.h"
>  #include <sys/ttydefaults.h>
> -#include <lk/debugfs.h>
> +#include <api/fs/debugfs.h>
>  #include <termios.h>
>  
>  extern const char *graph_line;
> diff --git a/tools/vm/Makefile b/tools/vm/Makefile
> index 24e9ddd93fa4..3d907dacf2ac 100644
> --- a/tools/vm/Makefile
> +++ b/tools/vm/Makefile
> @@ -2,21 +2,21 @@
>  #
>  TARGETS=page-types slabinfo
>  
> -LK_DIR = ../lib/lk
> -LIBLK = $(LK_DIR)/liblk.a
> +LIB_DIR = ../lib/api
> +LIBS = $(LIB_DIR)/libapikfs.a
>  
>  CC = $(CROSS_COMPILE)gcc
>  CFLAGS = -Wall -Wextra -I../lib/
> -LDFLAGS = $(LIBLK)
> +LDFLAGS = $(LIBS)
>  
> -$(TARGETS): liblk
> +$(TARGETS): $(LIBS)
>  
> -liblk:
> -	make -C $(LK_DIR)
> +$(LIBS):
> +	make -C $(LIB_DIR)
>  
>  %: %.c
>  	$(CC) $(CFLAGS) -o $@ $< $(LDFLAGS)
>  
>  clean:
>  	$(RM) page-types slabinfo
> -	make -C ../lib/lk clean
> +	make -C $(LIB_DIR) clean
> diff --git a/tools/vm/page-types.c b/tools/vm/page-types.c
> index d5e9d6d185c8..f9be24d9efac 100644
> --- a/tools/vm/page-types.c
> +++ b/tools/vm/page-types.c
> @@ -36,7 +36,7 @@
>  #include <sys/statfs.h>
>  #include "../../include/uapi/linux/magic.h"
>  #include "../../include/uapi/linux/kernel-page-flags.h"
> -#include <lk/debugfs.h>
> +#include <api/fs/debugfs.h>
>  
>  #ifndef MAX_PATH
>  # define MAX_PATH 256
> -- 
> 1.8.4
> 
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> Sent from a fat crate under my desk. Formatting is fine.
> --