On Mon, Oct 20, 2008 at 11:47:10AM +0100, Mel Gorman wrote:
> On (17/10/08 17:14), Andy Whitcroft didst pronounce:
> > Add a new private utilities library consisting of various useful helpers
> > normally hidden within libhugetlbfs. Extend the library local marker idiom
> > to include a private utilities marker __pu_. When we build libhugetlbfs
> > these are forced local when building libhugetlbfs_privutils these are the
> > only routines exported. This makes it very hard for the two libraries to
> > interfere with each other when both are linked to the same binary, which is
> > particularly important when testing the library; we must test the real one.
> >
> > Signed-off-by: Andy Whitcroft <[EMAIL PROTECTED]>
> > ---
> > Makefile | 31 ++++++++++++++++++++++++++-----
> > init_privutils.c | 25 +++++++++++++++++++++++++
> > libhugetlbfs_internal.h | 7 ++++++-
> > libhugetlbfs_privutils.h | 22 ++++++++++++++++++++++
> > privutils.lds | 6 ++++++
> > tests/Makefile | 2 +-
> > tests/counters.c | 1 +
> > tests/hugetests.h | 3 +++
> > tests/libtestutils.c | 1 +
> > version.lds | 1 +
> > 10 files changed, 92 insertions(+), 7 deletions(-)
> > create mode 100644 init_privutils.c
> > create mode 100644 libhugetlbfs_privutils.h
> > create mode 100644 privutils.lds
> >
> > diff --git a/Makefile b/Makefile
> > index 2d5a120..1bb4589 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -2,7 +2,8 @@ PREFIX = /usr/local
> > EXEDIR = /bin
> >
> > LIBOBJS = hugeutils.o version.o init.o morecore.o debug.o alloc.o shm.o
> > kernel-features.o
> > -INSTALL_OBJ_LIBS = libhugetlbfs.so libhugetlbfs.a
> > +LIBPUOBJS = init_privutils.o debug.o
> > +INSTALL_OBJ_LIBS = libhugetlbfs.so libhugetlbfs.a libhugetlbfs_privutils.so
>
> Ok, so distribution SPEC files and the like will need to be aware there
> is another library being installed. This is not a problem as such, but
> it's worth noting.
Yes, that is a concern. It is only used actually for the tests, and I
don't know if we package those or not. For the utilities we deliberatly
link against the libhugetlbfs_privutils.a to ensure we have no
dependancies for those utilities.
> > BIN_OBJ_DIR=obj
> > INSTALL_BIN = hugectl hugeedit hugeadm pagesize
> > INSTALL_HEADERS = hugetlbfs.h
> > @@ -20,7 +21,7 @@ NODEPTARGETS=<version.h> <clean>
> >
> > INSTALL = install
> >
> > -LDFLAGS += --no-undefined-version -Wl,--version-script=version.lds -ldl
> > +LDFLAGS += --no-undefined-version -ldl
>
> Ok, this is clear as well as we now have two lds files and we need to
> distinguish between them.
>
> > CFLAGS ?= -O2 -g
> > CFLAGS += -Wall -fPIC
> > CPPFLAGS += -D__LIBHUGETLBFS__
> > @@ -154,7 +155,7 @@ all: libs tests tools
> >
> > .PHONY: tests libs
> >
> > -libs: $(foreach file,$(INSTALL_OBJ_LIBS),$(OBJDIRS:%=%/$(file)))
> > +libs: $(foreach file,$(INSTALL_OBJ_LIBS),$(OBJDIRS:%=%/$(file)))
> > $(BIN_OBJ_DIR)/libhugetlbfs_privutils.a
> >
> > tests: libs # Force make to build the library first
> > tests: tests/all
> > @@ -227,11 +228,31 @@ obj64/libhugetlbfs.a: $(LIBOBJS64)
> >
> > obj32/libhugetlbfs.so: $(LIBOBJS32)
> > @$(VECHO) LD32 "(shared)" $@
> > - $(CC32) $(LDFLAGS) -Wl,-soname,$(notdir $@) -shared -o $@ $^ $(LDLIBS)
> > + $(CC32) $(LDFLAGS) -Wl,--version-script=version.lds
> > -Wl,-soname,$(notdir $@) -shared -o $@ $^ $(LDLIBS)
> >
> > obj64/libhugetlbfs.so: $(LIBOBJS64)
> > @$(VECHO) LD64 "(shared)" $@
> > - $(CC64) $(LDFLAGS) -Wl,-soname,$(notdir $@) -shared -o $@ $^ $(LDLIBS)
> > + $(CC64) $(LDFLAGS) -Wl,--version-script=version.lds
> > -Wl,-soname,$(notdir $@) -shared -o $@ $^ $(LDLIBS)
> > +
> > +#obj32/libhugetlbfs_privutils.a: $(LIBPUOBJS:%=obj32/%)
> > +# @$(VECHO) AR32 $@
> > +# $(AR) $(ARFLAGS) $@ $^
> > +#
> > +#obj64/libhugetlbfs_privutils.a: $(LIBPUOBJS:%=obj64/%)
> > +# @$(VECHO) AR64 $@
> > +# $(AR) $(ARFLAGS) $@ $^
> > +
>
> What's this comment stuff in the Makefile about?
>
> (suspect it's an oversight that it still exists)
Its something I would generally deliberatly do. It records how one
would make the dynamic version of the library though we never need it so
currently it is not built.
> > +$(BIN_OBJ_DIR)/libhugetlbfs_privutils.a: $(LIBPUOBJS:%=$(BIN_OBJ_DIR)/%)
> > + @$(VECHO) ARHOST $@
> > + $(AR) $(ARFLAGS) $@ $^
> > +
> > +obj32/libhugetlbfs_privutils.so: $(LIBPUOBJS:%=obj32/%)
> > + @$(VECHO) LD32 "(shared)" $@
> > + $(CC32) $(LDFLAGS) -Wl,--version-script=privutils.lds
> > -Wl,-soname,$(notdir $@) -shared -o $@ $^ $(LDLIBS)
> > +
> > +obj64/libhugetlbfs_privutils.so: $(LIBPUOBJS:%=obj64/%)
> > + @$(VECHO) LD64 "(shared)" $@
> > + $(CC64) $(LDFLAGS) -Wl,--version-script=privutils.lds
> > -Wl,-soname,$(notdir $@) -shared -o $@ $^ $(LDLIBS)
> >
> > obj32/%.i: %.c
> > @$(VECHO) CPP $@
> > diff --git a/init_privutils.c b/init_privutils.c
> > new file mode 100644
> > index 0000000..6c9243d
> > --- /dev/null
> > +++ b/init_privutils.c
> > @@ -0,0 +1,25 @@
> > +/*
> > + * libhugetlbfs - Easy use of Linux hugepages
> > + * Copyright (C) 2008 Nishanth Aravamudan, IBM Corporation
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public License
> > + * as published by the Free Software Foundation; either version 2.1 of
> > + * the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +#include "libhugetlbfs_internal.h"
> > +
> > +static void __attribute__ ((constructor)) setup_libhugetlbfs(void)
> > +{
> > + hugetlbfs_setup_debug();
> > +}
> > diff --git a/libhugetlbfs_internal.h b/libhugetlbfs_internal.h
> > index 4d7add7..4e20885 100644
> > --- a/libhugetlbfs_internal.h
> > +++ b/libhugetlbfs_internal.h
> > @@ -27,6 +27,8 @@
> > #error This header should not be included by library users.
> > #endif /* __LIBHUGETLBFS__ */
> >
> > +#include "libhugetlbfs_privutils.h"
> > +
> > #define stringify_1(x) #x
> > #define stringify(x) stringify_1(x)
> >
> > @@ -45,7 +47,10 @@
> > * When adding a library local variable externalise the symbol as
> > * normal, plus add a #define of the form below. This define effectively
> > * renames the routine into the local namespace __lh_* which is forced
> > - * local in the linker script version.lds.
> > + * local in the linker script version.lds. Some routines may need to be
> > + * exported in the utilities library these are marked __pu_* which marks
> > + * them for export in libhugetlbfs_privutils; their definitions should
> > + * appear in libhugetlbfs_privutils.h rather than here.
> > */
> > extern int __hugetlbfs_verbose;
> > extern int __hugetlbfs_debug;
> > diff --git a/libhugetlbfs_privutils.h b/libhugetlbfs_privutils.h
> > new file mode 100644
> > index 0000000..1a45ed0
> > --- /dev/null
> > +++ b/libhugetlbfs_privutils.h
> > @@ -0,0 +1,22 @@
> > +/*
> > + * libhugetlbfs - Easy use of Linux hugepages
> > + * Copyright (C) 2005-2006 David Gibson & Adam Litke, IBM Corporation.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public License
> > + * as published by the Free Software Foundation; either version 2.1 of
> > + * the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +#ifndef _LIBHUGETLBFS_PRIVUTILS_H
> > +#define _LIBHUGETLBFS_PRIVUTILS_H
> > +
> > +#endif /* _LIBHUGETLBFS_PRIVUTILS_H */
> > diff --git a/privutils.lds b/privutils.lds
> > new file mode 100644
> > index 0000000..5d481e2
> > --- /dev/null
> > +++ b/privutils.lds
> > @@ -0,0 +1,6 @@
> > +VERS_1.0 {
> > + global:
> > + __pu_*;
> > + local:
> > + *;
> > +};
> > diff --git a/tests/Makefile b/tests/Makefile
> > index e4e1ce2..f03ec68 100644
> > --- a/tests/Makefile
> > +++ b/tests/Makefile
> > @@ -22,7 +22,7 @@ BADTOOLCHAIN = bad-toolchain.sh
> >
> > CFLAGS = -O2 -Wall -g
> > CPPFLAGS = -I..
> > -LDLIBS = -ldl -lpthread
> > +LDLIBS = -ldl -lpthread -lhugetlbfs_privutils
> > LDFLAGS32 = -L../obj32
> > LDFLAGS64 = -L../obj64
> > INSTALL = install
> > diff --git a/tests/counters.c b/tests/counters.c
> > index 522a00d..0284809 100644
> > --- a/tests/counters.c
> > +++ b/tests/counters.c
> > @@ -181,6 +181,7 @@ void _set_nr_hugepages(unsigned long count, int line)
> > out:
> > verify_counters(line, et, ef, er, es);
> > }
> > +#undef set_nr_hugepages
> > #define set_nr_hugepages(c) _set_nr_hugepages(c, __LINE__)
> >
> > void _map(int s, int hpages, int flags, int line)
> > diff --git a/tests/hugetests.h b/tests/hugetests.h
> > index bb1146e..270923b 100644
> > --- a/tests/hugetests.h
> > +++ b/tests/hugetests.h
> > @@ -22,6 +22,9 @@
> >
> > #include <errno.h>
> > #include <string.h>
> > +
> > +#include "libhugetlbfs_privutils.h"
> > +
> > #define DEBUG
> >
> > /* Test return codes */
> > diff --git a/tests/libtestutils.c b/tests/libtestutils.c
> > index 93388a9..4eeb880 100644
> > --- a/tests/libtestutils.c
> > +++ b/tests/libtestutils.c
> > @@ -37,6 +37,7 @@
> > #include <fcntl.h>
> >
> > #include "hugetlbfs.h"
> > +#include "libhugetlbfs_privutils.h"
> > #include "hugetests.h"
> >
> > void check_free_huge_pages(int nr_pages_needed)
> > diff --git a/version.lds b/version.lds
> > index f5c480a..ba06c82 100644
> > --- a/version.lds
> > +++ b/version.lds
> > @@ -6,6 +6,7 @@ VERS_1.0 {
> > hugetlbfs_unlinked_fd;
> > local:
> > __lh_*;
> > + __pu_*;
> > };
> >
> > HTLBFS_2.0 {
>
> As far as I can see, this makes no change to libhugetlbfs itself and
> applications continue to use it as normal. This is good and baring the
> strange commented out stuff in the Makefile
>
> Acked-by: Mel Gorman <[EMAIL PROTECTED]>
-apw
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Libhugetlbfs-devel mailing list
Libhugetlbfs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libhugetlbfs-devel
