On Mon, Oct 20, 2008 at 12:34:08PM +0100, Andy Whitcroft wrote:
> 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.
>
We do package them but as long as the library is installed correctly
(which appears to be the case), they should get picked up correctly.
> > > 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.
>
Ok, just chuck a comment above it explaining that and we're all good.
> > > +$(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
>
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
-------------------------------------------------------------------------
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
