On Mon, Oct 20, 2008 at 11:36:30AM +0100, Mel Gorman wrote:
> On (17/10/08 17:14), Andy Whitcroft didst pronounce:
> > Currently we have three types of function:
> >
> > file local -- marked static in the normal way,
> > library local -- external but prefixed with __lh_
> > library exported -- external and listed in the library.lds file
> >
> > While the library prefix works, it does not allow functions to trivially
> > move from file local to library local as all references to the function
> > have to be modified to the new name.
> >
> > This patch introduces a new idiom. When a function is intended to be
> > library local it is already necessary to declare that function in the
> > libhugetlbfs_internal.h, if we also add a single define for that function
> > adding the __lh_ prefix (as below) then all other references including the
> > original definition may use the original name unchanged but the function
> > will remain unexported:
> >
> > #define hpool_sizes __lh_hpool_sizes
> > extern int hpool_sizes(struct hpage_pool *, int);
> >
> > This patch converts all current library local functions to this new idiom.
> >
> > Signed-off-by: Andy Whitcroft <[EMAIL PROTECTED]>
>
> There are two spaces vs tabs issues noted below, but it's not a biggie.
> Other than that, I didn't spot anything bad.
>
> Acked-by: Mel Gorman <[EMAIL PROTECTED]>
>
> > ---
> > alloc.c | 2 +-
> > debug.c | 2 +-
> > elflink.c | 6 +++---
> > hugeadm.c | 14 +++++++-------
> > hugetlbfs.h | 3 ++-
> > hugeutils.c | 20 ++++++++++----------
> > init.c | 10 +++++-----
> > kernel-features.c | 2 +-
> > libhugetlbfs_internal.h | 33 ++++++++++++++++++++++++---------
> > morecore.c | 10 +++++-----
> > pagesize.c | 2 +-
> > 11 files changed, 60 insertions(+), 44 deletions(-)
> >
> > diff --git a/alloc.c b/alloc.c
> > index 197c1ce..750b2cb 100644
> > --- a/alloc.c
> > +++ b/alloc.c
> > @@ -103,7 +103,7 @@ void *get_huge_pages(size_t len, ghp_t flags)
> > }
> >
> > /* Fault the region to ensure accesses succeed */
> > - if (__lh_hugetlbfs_prefault(heap_fd, buf, len) != 0) {
> > + if (hugetlbfs_prefault(heap_fd, buf, len) != 0) {
> > munmap(buf, len);
> > close(heap_fd);
> >
> > diff --git a/debug.c b/debug.c
> > index bd54d58..e854dff 100644
> > --- a/debug.c
> > +++ b/debug.c
> > @@ -58,7 +58,7 @@ static void __hugetlbfs_init_debug(void)
> > initialized = 1;
> > }
> >
> > -void __lh_hugetlbfs_setup_debug(void)
> > +void hugetlbfs_setup_debug(void)
> > {
> > __hugetlbfs_init_debug();
> > }
> > diff --git a/elflink.c b/elflink.c
> > index 75a9dbb..f6a7fb5 100644
> > --- a/elflink.c
> > +++ b/elflink.c
> > @@ -809,7 +809,7 @@ static void check_range_empty(void *addr, unsigned long
> > len)
> > WARNING("Unable to verify address range %p - %p. Not empty?\n",
> > addr, addr + len);
> > if (__hugetlbfs_debug)
> > - __lh_dump_proc_pid_maps();
> > + dump_proc_pid_maps();
> > }
> > if (p != MAP_FAILED)
> > munmap(p, len);
> > @@ -1147,7 +1147,7 @@ static int set_hpage_sizes(const char *env)
> > continue;
> >
> > if (*(++pos) == '=') {
> > - size = __lh_parse_page_size(pos + 1);
> > + size = parse_page_size(pos + 1);
> > if (size == -1)
> > return size;
> > } else
> > @@ -1258,7 +1258,7 @@ static int parse_elf()
> > return 0;
> > }
> >
> > -void __lh_hugetlbfs_setup_elflink(void)
> > +void hugetlbfs_setup_elflink(void)
> > {
> > int i, ret;
> >
> > diff --git a/hugeadm.c b/hugeadm.c
> > index 4aef40e..cee6aac 100644
> > --- a/hugeadm.c
> > +++ b/hugeadm.c
> > @@ -89,7 +89,7 @@ void pool_list(void)
> > int pos;
> > int cnt;
> >
> > - cnt = __lh_hpool_sizes(pools, MAX_POOLS);
> > + cnt = hpool_sizes(pools, MAX_POOLS);
> > if (cnt < 0) {
> > ERROR("unable to obtain pools list");
> > exit(EXIT_FAILURE);
> > @@ -165,9 +165,9 @@ void pool_adjust(char *cmd, unsigned int counter)
> > page_size_str, adjust_str, counter);
> >
> > /* Convert and validate the page_size. */
> > - page_size = __lh_parse_page_size(page_size_str);
> > + page_size = parse_page_size(page_size_str);
> >
> > - cnt = __lh_hpool_sizes(pools, MAX_POOLS);
> > + cnt = hpool_sizes(pools, MAX_POOLS);
> > if (cnt < 0) {
> > ERROR("unable to obtain pools list");
> > exit(EXIT_FAILURE);
> > @@ -210,7 +210,7 @@ void pool_adjust(char *cmd, unsigned int counter)
> > * requested should there be insufficient pages. Check the new
> > * value and adjust HUGEPAGES_OC accordingly.
> > */
> > - __lh_get_pool_size(page_size, &pools[pos]);
> > + get_pool_size(page_size, &pools[pos]);
> > if (pools[pos].minimum != min) {
> > ERROR("failed to set pool minimum to %ld became %ld\n",
> > min, pools[pos].minimum);
> > @@ -228,7 +228,7 @@ void page_sizes(int all)
> > int pos;
> > int cnt;
> >
> > - cnt = __lh_hpool_sizes(pools, MAX_POOLS);
> > + cnt = hpool_sizes(pools, MAX_POOLS);
> > if (cnt < 0) {
> > ERROR("unable to obtain pools list");
> > exit(EXIT_FAILURE);
> > @@ -259,8 +259,8 @@ int main(int argc, char** argv)
> > {0},
> > };
> >
> > - __lh_hugetlbfs_setup_debug();
> > - __lh_setup_mounts();
> > + hugetlbfs_setup_debug();
> > + setup_mounts();
> >
>
> Could fix up the whitespace damage here, but no a big issue.
>
> > while (ret != -1) {
> > ret = getopt_long(argc, argv, opts, long_opts, &index);
> > diff --git a/hugetlbfs.h b/hugetlbfs.h
> > index 3f2eb30..c2c88f1 100644
> > --- a/hugetlbfs.h
> > +++ b/hugetlbfs.h
> > @@ -32,7 +32,8 @@ int hugetlbfs_unlinked_fd(void);
> > int hugetlbfs_unlinked_fd_for_size(long page_size);
> >
> > /* Diagnoses/debugging only functions */
> > -long __lh_dump_proc_pid_maps(void);
> > +#define dump_proc_pid_maps __lh_dump_proc_pid_maps
> > +long dump_proc_pid_maps(void);
> >
> > #define PF_LINUX_HUGETLB 0x100000
> >
> > diff --git a/hugeutils.c b/hugeutils.c
> > index 433e6e8..b9e63ad 100644
> > --- a/hugeutils.c
> > +++ b/hugeutils.c
> > @@ -88,7 +88,7 @@ static inline long size_to_smaller_unit(long size)
> > * EINVAL - str could not be parsed or was not greater
> > than zero
> > * EOVERFLOW - Overflow when converting from the specified units
> > */
> > -long __lh_parse_page_size(const char *str)
> > +long parse_page_size(const char *str)
> > {
> > char *pos;
> > long size;
> > @@ -292,7 +292,7 @@ static void probe_default_hpage_size(void)
> > */
> > env = getenv("HUGETLB_DEFAULT_PAGE_SIZE");
> > if (env && strlen(env) > 0)
> > - size = __lh_parse_page_size(env);
> > + size = parse_page_size(env);
> > else {
> > size = file_read_ulong(MEMINFO, "Hugepagesize:");
> > size *= 1024; /* convert from kB to B */
> > @@ -410,7 +410,7 @@ static void find_mounts(void)
> > close(fd);
> > }
> >
> > -void __lh_setup_mounts(void)
> > +void setup_mounts(void)
> > {
> > char *env;
> > int do_scan = 1;
> > @@ -445,7 +445,7 @@ void __lh_setup_mounts(void)
> > debug_show_page_sizes();
> > }
> >
> > -int __lh_get_pool_size(long size, struct hpage_pool *pool)
> > +int get_pool_size(long size, struct hpage_pool *pool)
> > {
> > long nr_over = 0;
> > long nr_used = 0;
> > @@ -493,7 +493,7 @@ int __lh_get_pool_size(long size, struct hpage_pool
> > *pool)
> > return 0;
> > }
> >
> > -int __lh_hpool_sizes(struct hpage_pool *pools, int pcnt)
> > +int hpool_sizes(struct hpage_pool *pools, int pcnt)
> > {
> > long default_size;
> > int which = 0;
> > @@ -503,7 +503,7 @@ int __lh_hpool_sizes(struct hpage_pool *pools, int pcnt)
> > default_size = size_to_smaller_unit(file_read_ulong(MEMINFO,
> > "Hugepagesize:"));
> > if (default_size >= 0 && which < pcnt)
> > - if (__lh_get_pool_size(default_size, &pools[which])) {
> > + if (get_pool_size(default_size, &pools[which])) {
> > pools[which].is_default = 1;
> > which++;
> > }
> > @@ -523,7 +523,7 @@ int __lh_hpool_sizes(struct hpage_pool *pools, int pcnt)
> > if (size < 0 || size == default_size)
> > continue;
> >
> > - if (__lh_get_pool_size(size, &pools[which]))
> > + if (get_pool_size(size, &pools[which]))
> > which++;
> > }
> > closedir(dir);
> > @@ -538,7 +538,7 @@ int __lh_hpool_sizes(struct hpage_pool *pools, int pcnt)
> >
> > /*
> > * NOTE: This function uses data that is initialized by
> > - * __lh_setup_mounts() which is called during libhugetlbfs initialization.
> > + * setup_mounts() which is called during libhugetlbfs initialization.
> > *
> > * returns:
> > * on success, size of a huge page in number of bytes
> > @@ -744,7 +744,7 @@ int hugetlbfs_unlinked_fd(void)
> > }
> >
> > #define IOV_LEN 64
> > -int __lh_hugetlbfs_prefault(int fd, void *addr, size_t length)
> > +int hugetlbfs_prefault(int fd, void *addr, size_t length)
> > {
> > /*
> > * The NUMA users of libhugetlbfs' malloc feature are
> > @@ -827,7 +827,7 @@ int set_nr_overcommit_hugepages(long pagesize, unsigned
> > long val)
> > /********************************************************************/
> >
> > #define MAPS_BUF_SZ 4096
> > -long __lh_dump_proc_pid_maps()
> > +long dump_proc_pid_maps()
> > {
> > FILE *f;
> > char line[MAPS_BUF_SZ];
> > diff --git a/init.c b/init.c
> > index f58d34b..f9cd804 100644
> > --- a/init.c
> > +++ b/init.c
> > @@ -21,11 +21,11 @@
> >
> > static void __attribute__ ((constructor)) setup_libhugetlbfs(void)
> > {
> > - __lh_hugetlbfs_setup_debug();
> > - __lh_setup_mounts();
> > - __lh_setup_features();
> > + hugetlbfs_setup_debug();
> > + setup_mounts();
> > + setup_features();
> > #ifndef NO_ELFLINK
> > - __lh_hugetlbfs_setup_elflink();
> > + hugetlbfs_setup_elflink();
> > #endif
> > - __lh_hugetlbfs_setup_morecore();
> > + hugetlbfs_setup_morecore();
> > }
> > diff --git a/kernel-features.c b/kernel-features.c
> > index 577e560..8f105b4 100644
> > --- a/kernel-features.c
> > +++ b/kernel-features.c
> > @@ -199,7 +199,7 @@ int check_features_env_valid(const char *env)
> > return 0;
> > }
> >
> > -void __lh_setup_features()
> > +void setup_features()
> > {
> > struct utsname u;
> > char *env;
> > diff --git a/libhugetlbfs_internal.h b/libhugetlbfs_internal.h
> > index e07787f..4d7add7 100644
> > --- a/libhugetlbfs_internal.h
> > +++ b/libhugetlbfs_internal.h
> > @@ -41,17 +41,30 @@
> > #define SLICE_HIGH_SHIFT 63
> > #endif
> >
> > +/*
> > + * 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.
> > + */
> > extern int __hugetlbfs_verbose;
> > extern int __hugetlbfs_debug;
> > extern int __hugetlbfs_prefault;
> > -extern void __lh_hugetlbfs_setup_elflink();
> > -extern void __lh_hugetlbfs_setup_morecore();
> > -extern void __lh_hugetlbfs_setup_debug();
> > -extern void __lh_setup_mounts();
> > -extern void __lh_setup_features();
> > +#define hugetlbfs_setup_elflink __lh_hugetlbfs_setup_elflink
> > +extern void hugetlbfs_setup_elflink();
> > +#define hugetlbfs_setup_morecore __lh_hugetlbfs_setup_morecore
> > +extern void hugetlbfs_setup_morecore();
> > +#define hugetlbfs_setup_debug __lh_hugetlbfs_setup_debug
> > +extern void hugetlbfs_setup_debug();
> > +#define setup_mounts __lh_setup_mounts
> > +extern void setup_mounts();
> > +#define setup_features __lh_setup_features
> > +extern void setup_features();
> > extern char __hugetlbfs_hostname[];
> > -extern int __lh_hugetlbfs_prefault(int fd, void *addr, size_t length);
> > -extern long __lh_parse_page_size(const char *str);
> > +#define hugetlbfs_prefault __lh_hugetlbfs_prefault
> > +extern int hugetlbfs_prefault(int fd, void *addr, size_t length);
> > +#define parse_page_size __lh_parse_page_size
> > +extern long parse_page_size(const char *str);
> >
> > #ifndef REPORT_UTIL
> > #define REPORT_UTIL "libhugetlbfs"
> > @@ -97,8 +110,10 @@ struct hpage_pool {
> > int is_default;
> > };
> >
> > -extern int __lh_hpool_sizes(struct hpage_pool *, int);
> > -extern int __lh_get_pool_size(long, struct hpage_pool *);
> > +#define hpool_sizes __lh_hpool_sizes
> > +extern int hpool_sizes(struct hpage_pool *, int);
> > +#define get_pool_size __lh_get_pool_size
> > +extern int get_pool_size(long, struct hpage_pool *);
> >
> > /* Arch-specific callbacks */
> > extern int direct_syscall(int sysnum, ...);
> > diff --git a/morecore.c b/morecore.c
> > index ddcd77d..19d18b8 100644
> > --- a/morecore.c
> > +++ b/morecore.c
> > @@ -108,7 +108,7 @@ static void *hugetlbfs_morecore(ptrdiff_t increment)
> > WARNING("Heap originates at %p instead of %p\n",
> > p, heapbase);
> > if (__hugetlbfs_debug)
> > - __lh_dump_proc_pid_maps();
> > + dump_proc_pid_maps();
> > }
> > /* then setup the heap variables */
> > heapbase = heaptop = p;
> > @@ -118,12 +118,12 @@ static void *hugetlbfs_morecore(ptrdiff_t increment)
> > WARNING("New heap segment mapped at %p instead of %p\n",
> > p, heapbase + mapsize);
> > if (__hugetlbfs_debug)
> > - __lh_dump_proc_pid_maps();
> > + dump_proc_pid_maps();
> > return NULL;
> > }
> >
> > /* Fault the region to ensure accesses succeed */
> > - if (__lh_hugetlbfs_prefault(zero_fd, p, delta) != 0) {
> > + if (hugetlbfs_prefault(zero_fd, p, delta) != 0) {
> > munmap(p, delta);
> > return NULL;
> > }
> > @@ -190,7 +190,7 @@ static void *hugetlbfs_morecore(ptrdiff_t increment)
> > return p;
> > }
> >
> > -void __lh_hugetlbfs_setup_morecore(void)
> > +void hugetlbfs_setup_morecore(void)
> > {
> > char *env, *ep;
> > unsigned long heapaddr;
> > @@ -212,7 +212,7 @@ void __lh_hugetlbfs_setup_morecore(void)
> > if (strncasecmp(env, "y", 1) == 0)
> > hpage_size = gethugepagesize();
> > else
> > - hpage_size = __lh_parse_page_size(env);
> > + hpage_size = parse_page_size(env);
> >
> > if (hpage_size <= 0) {
> > if (errno == ENOSYS)
> > diff --git a/pagesize.c b/pagesize.c
> > index aca4de4..11dd68d 100644
> > --- a/pagesize.c
> > +++ b/pagesize.c
> > @@ -80,7 +80,7 @@ int main(int argc, char** argv)
> > long pagesizes[MAX_PAGESIZES];
> > int i;
> >
> > - __lh_hugetlbfs_setup_debug();
> > + hugetlbfs_setup_debug();
> >
>
> Another spaces vs tabs here, but again barely worth getting into a
> twist over.
>
> > while (ret != -1) {
> > ret = getopt_long(argc, argv, opts, long_opts, &index);
> > --
> > 1.6.0.2.711.gf1ba4
> >
I had not changed the spacing to keep the change purly mechanical. I
guess we should audit things for whitespace generally (as I have noticed
a poor line endings here and there) and sort it out, perhaps after the
next release as a global thing while noone has any patches out standing?
-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
