On Fri, Nov 14, 2008 at 09:00:38PM +0000, Adam Litke wrote:
>
> ---
>
> elflink.c | 101
> ++++++++++++++++++++++++++++++++-----------------------------
> 1 files changed, 53 insertions(+), 48 deletions(-)
>
> diff --git a/elflink.c b/elflink.c
> index 63229f6..e38e796 100644
> --- a/elflink.c
> +++ b/elflink.c
> @@ -252,14 +252,14 @@ static int find_or_create_share_path(long page_size)
> if (env) {
> /* Given an explicit path */
> if (hugetlbfs_test_path(env) != 1) {
> - ERROR("HUGETLB_SHARE_PATH %s is not on a hugetlbfs"
> + WARNING("HUGETLB_SHARE_PATH %s is not on a hugetlbfs"
> " filesystem\n", env);
> return -1;
> }
>
> /* Make sure the page size matches */
> if (page_size != hugetlbfs_test_pagesize(env)) {
> - ERROR("HUGETLB_SHARE_PATH %s is not valid for a %li "
> + WARNING("HUGETLB_SHARE_PATH %s is not valid for a %li "
> "kB page size\n", env, page_size / 1024);
> return -1;
> }
Ok.
> @@ -276,7 +276,7 @@ static int find_or_create_share_path(long page_size)
>
> ret = mkdir(share_readonly_path, 0700);
> if ((ret != 0) && (errno != EEXIST)) {
> - ERROR("Error creating share directory %s\n",
> + WARNING("Error creating share directory %s\n",
> share_readonly_path);
> return -1;
> }
hmm, I'm 50/50 on this one. It implies to me that sharing is disabled so
if this is now a warning, can we tell the user that sharing is disabled in
it? I think if I saw "ERROR", I would automatically assume it was busted
but not so sure about a warning.
> @@ -284,24 +284,24 @@ static int find_or_create_share_path(long page_size)
> /* Check the share directory is sane */
> ret = lstat(share_readonly_path, &sb);
> if (ret != 0) {
> - ERROR("Couldn't stat() %s: %s\n", share_readonly_path,
> + WARNING("Couldn't stat() %s: %s\n", share_readonly_path,
> strerror(errno));
> return -1;
> }
>
> if (! S_ISDIR(sb.st_mode)) {
> - ERROR("%s is not a directory\n", share_readonly_path);
> + WARNING("%s is not a directory\n", share_readonly_path);
> return -1;
> }
>
> if (sb.st_uid != getuid()) {
> - ERROR("%s has wrong owner (uid=%d instead of %d)\n",
> + WARNING("%s has wrong owner (uid=%d instead of %d)\n",
> share_readonly_path, sb.st_uid, getuid());
> return -1;
> }
>
> if (sb.st_mode & (S_IWGRP | S_IWOTH)) {
> - ERROR("%s has bad permissions 0%03o\n",
> + WARNING("%s has bad permissions 0%03o\n",
> share_readonly_path, sb.st_mode);
> return -1;
> }
Similarly, all these look like errors to me.
> @@ -319,7 +319,7 @@ static void check_bss(unsigned long *start, unsigned long
> *end)
>
> for (addr = start; addr < end; addr++) {
> if (*addr != 0)
> - WARNING("Non-zero BSS data @ %p: %lx\n", addr, *addr);
> + DEBUG("Non-zero BSS data @ %p: %lx\n", addr, *addr);
> }
> }
What changed that this is now DEBUG instead of WARNING?
>
> @@ -347,14 +347,14 @@ static int get_shared_file_name(struct seg_info
> *htlb_seg_info, char *file_path)
> memset(binary, 0, sizeof(binary));
> ret = readlink("/proc/self/exe", binary, PATH_MAX);
> if (ret < 0) {
> - ERROR("shared_file: readlink() on /proc/self/exe "
> + WARNING("shared_file: readlink() on /proc/self/exe "
> "failed: %s\n", strerror(errno));
> return -1;
> }
>
> binary2 = basename(binary);
> if (!binary2) {
> - ERROR("shared_file: basename() on %s failed: %s\n",
> + WARNING("shared_file: basename() on %s failed: %s\n",
> binary, strerror(errno));
> return -1;
> }
They all look like errors to me that result in broken sharing.
> @@ -605,7 +605,7 @@ static int save_phdr(int table_idx, int phnum, const
> ElfW(Phdr) *phdr)
> int prot = 0;
>
> if (table_idx >= MAX_HTLB_SEGS) {
> - ERROR("Executable has too many segments (max %d)\n",
> + WARNING("Executable has too many segments (max %d)\n",
> MAX_HTLB_SEGS);
> htlb_num_segs = 0;
> return -1;
Ok, because I think we can still continue in this case.
> @@ -624,7 +624,7 @@ static int save_phdr(int table_idx, int phnum, const
> ElfW(Phdr) *phdr)
> htlb_seg_table[table_idx].prot = prot;
> htlb_seg_table[table_idx].index = phnum;
>
> - DEBUG("Segment %d (phdr %d): %#0lx-%#0lx (filesz=%#0lx) "
> + INFO("Segment %d (phdr %d): %#0lx-%#0lx (filesz=%#0lx) "
> "(prot = %#0x)\n", table_idx, phnum,
> (unsigned long) phdr->p_vaddr,
> (unsigned long) phdr->p_vaddr + phdr->p_memsz,
Ok.
> @@ -694,7 +694,8 @@ int parse_elf_normal(struct dl_phdr_info *info, size_t
> size, void *data)
> continue;
>
> if (i >= MAX_SEGS) {
> - ERROR("Maximum number of PT_LOAD segments exceeded\n");
> + WARNING("Maximum number of PT_LOAD segments"
> + "exceeded\n");
> return 1;
> }
>
Ok as again we can continue.
> @@ -763,14 +764,14 @@ int parse_elf_partial(struct dl_phdr_info *info, size_t
> size, void *data)
> */
> memsz = info->dlpi_phdr[i].p_memsz;
> if (memsz < gap) {
> - DEBUG("Segment %d's unaligned memsz is too small: "
> + INFO("Segment %d's unaligned memsz is too small: "
> "%#0lx < %#0lx\n",
> i, memsz, gap);
> continue;
> }
> memsz -= gap;
> if (memsz < (slice_end - vaddr)) {
> - DEBUG("Segment %d's aligned memsz is too small: "
> + INFO("Segment %d's aligned memsz is too small: "
> "%#0lx < %#0lx\n",
> i, memsz, slice_end - vaddr);
> continue;
Ok.
> @@ -861,7 +862,7 @@ static int prepare_segment(struct seg_info *seg)
> /* Create the temporary huge page mmap */
> p = mmap(NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED, seg->fd, 0);
> if (p == MAP_FAILED) {
> - ERROR("Couldn't map hugepage segment to copy data: %s\n",
> + WARNING("Couldn't map hugepage segment to copy data: %s\n",
> strerror(errno));
> return -1;
> }
Ok, because we can still continue with small pages.
> @@ -875,10 +876,10 @@ static int prepare_segment(struct seg_info *seg)
> * memsz region. The rest of the memsz is understood to be zeroes and
> * need not be copied.
> */
> - DEBUG("Mapped hugeseg at %p. Copying %#0lx bytes and %#0lx extra bytes"
> + INFO("Mapped hugeseg at %p. Copying %#0lx bytes and %#0lx extra bytes"
> " from %p...", p, seg->filesz, seg->extrasz, seg->vaddr);
> memcpy(p + offset, seg->vaddr, seg->filesz + seg->extrasz);
> - DEBUG_CONT("done\n");
> + INFO_CONT("done\n");
>
> munmap(p, size);
>
> @@ -899,13 +900,13 @@ static int fork_and_prepare_segment(struct seg_info
> *htlb_seg_info)
> int pid, ret, status;
>
> if ((pid = fork()) < 0) {
> - DEBUG("fork failed");
> + WARNING("fork failed");
> return -1;
> }
> if (pid == 0) {
> ret = prepare_segment(htlb_seg_info);
> if (ret < 0) {
> - DEBUG("Failed to prepare segment\n");
> + WARNING("Failed to prepare segment\n");
> exit(1);
> }
> else
> @@ -913,14 +914,14 @@ static int fork_and_prepare_segment(struct seg_info
> *htlb_seg_info)
> }
> ret = waitpid(pid, &status, 0);
> if (ret == -1) {
> - DEBUG("waitpid failed");
> + WARNING("waitpid failed");
> return -1;
> }
>
> if (WEXITSTATUS(status) != 0)
> return -1;
>
> - DEBUG("Prepare succeeded\n");
> + INFO("Prepare succeeded\n");
> return 0;
> }
>
> @@ -983,8 +984,8 @@ static int find_or_prepare_shared_file(struct seg_info
> *htlb_seg_info)
> /* Also got an exclusive file -> clean up */
> ret = unlink(tmp_path);
> if (ret != 0)
> - ERROR("shared_file: unable to clean up"
> - " unneeded file %s: %s\n",
> + WARNING("shared_file: unable to clean "
> + "up unneeded file %s: %s\n",
> tmp_path, strerror(errno));
> close(fdx);
> } else if (errnox != EEXIST) {
> @@ -1005,16 +1006,16 @@ static int find_or_prepare_shared_file(struct
> seg_info *htlb_seg_info)
>
> htlb_seg_info->fd = fdx;
>
> - DEBUG("Got unpopulated shared fd -- Preparing\n");
> + INFO("Got unpopulated shared fd -- Preparing\n");
> ret = fork_and_prepare_segment(htlb_seg_info);
> if (ret < 0)
> goto fail;
>
> - DEBUG("Prepare succeeded\n");
> + INFO("Prepare succeeded\n");
> /* move to permanent location */
> ret = rename(tmp_path, final_path);
> if (ret != 0) {
> - ERROR("shared_file: unable to rename %s"
> + WARNING("shared_file: unable to rename %s"
> " to %s: %s\n", tmp_path, final_path,
> strerror(errno));
> goto fail;
> @@ -1033,8 +1034,8 @@ static int find_or_prepare_shared_file(struct seg_info
> *htlb_seg_info)
> if (fdx > 0) {
> ret = unlink(tmp_path);
> if (ret != 0)
> - ERROR("shared_file: Unable to clean up temp file %s on"
> - " failure: %s\n", tmp_path, strerror(errno));
> + WARNING("shared_file: Unable to clean up temp file %s "
> + "on failure: %s\n", tmp_path, strerror(errno));
> close(fdx);
> }
>
> @@ -1063,7 +1064,7 @@ static int obtain_prepared_file(struct seg_info
> *htlb_seg_info)
> if (ret == 0)
> return 0;
> /* but, fall through to unlinked files, if sharing fails */
> - DEBUG("Falling back to unlinked files\n");
> + WARNING("Falling back to unlinked files\n");
INFO? I think the other messages that would lead to this are INFO at the
moment.
> }
> fd = hugetlbfs_unlinked_fd_for_size(hpage_size);
> if (fd < 0)
> @@ -1163,14 +1164,15 @@ static int set_hpage_sizes(const char *env)
>
> if (size <= 0) {
> if (errno == ENOSYS)
> - ERROR("Hugepages unavailable\n");
> + WARNING("Hugepages unavailable\n");
> else if (errno == EOVERFLOW)
> - ERROR("Hugepage size too large\n");
> + WARNING("Hugepage size too large\n");
> else
> - ERROR("Hugepage size (%s)\n", strerror(errno));
> + WARNING("Hugepage size (%s)\n",
> + strerror(errno));
> size = 0;
> } else if (!hugetlbfs_find_path_for_size(size)) {
> - ERROR("Hugepage size %li unavailable", size);
> + WARNING("Hugepage size %li unavailable", size);
> size = 0;
> }
>
> @@ -1189,12 +1191,12 @@ static int check_env(void)
>
> env = getenv("HUGETLB_ELFMAP");
> if (env && (strcasecmp(env, "no") == 0)) {
> - DEBUG("HUGETLB_ELFMAP=%s, not attempting to remap program "
> + INFO("HUGETLB_ELFMAP=%s, not attempting to remap program "
> "segments\n", env);
> return -1;
> }
> if (env && set_hpage_sizes(env)) {
> - ERROR("Cannot set elfmap page sizes: %s", strerror(errno));
> + WARNING("Cannot set elfmap page sizes: %s", strerror(errno));
> return -1;
> }
>
> @@ -1203,17 +1205,19 @@ static int check_env(void)
> env2 = getenv("HUGETLB_FORCE_ELFMAP");
> if (env2 && (strcasecmp(env2, "yes") == 0)) {
> force_remap = 1;
> - DEBUG("HUGETLB_FORCE_ELFMAP=%s, "
> + INFO("HUGETLB_FORCE_ELFMAP=%s, "
> "enabling partial segment "
> "remapping for non-relinked "
> "binaries\n",
> env2);
> - DEBUG("Disabling filesz copy optimization\n");
> + INFO("Disabling filesz copy optimization\n");
> minimal_copy = 0;
> } else {
> if (&__executable_start) {
> - ERROR("LD_PRELOAD is incompatible with segment
> remapping\n");
> - ERROR("Segment remapping has been DISABLED\n");
> + WARNING("LD_PRELOAD is incompatible with "
> + "segment remapping\n");
> + WARNING("Segment remapping has been "
> + "DISABLED\n");
> return -1;
> }
> }
> @@ -1221,7 +1225,7 @@ static int check_env(void)
>
> env = getenv("HUGETLB_MINIMAL_COPY");
> if (minimal_copy && env && (strcasecmp(env, "no") == 0)) {
> - DEBUG("HUGETLB_MINIMAL_COPY=%s, disabling filesz copy "
> + INFO("HUGETLB_MINIMAL_COPY=%s, disabling filesz copy "
> "optimization\n", env);
> minimal_copy = 0;
> }
> @@ -1230,16 +1234,16 @@ static int check_env(void)
> if (env)
> sharing = atoi(env);
> if (sharing == 2) {
> - ERROR("HUGETLB_SHARE=%d, however sharing of writable\n"
> + WARNING("HUGETLB_SHARE=%d, however sharing of writable\n"
> "segments has been deprecated and is now disabled\n",
> sharing);
> sharing = 0;
> } else {
> - DEBUG("HUGETLB_SHARE=%d, sharing ", sharing);
> + INFO("HUGETLB_SHARE=%d, sharing ", sharing);
> if (sharing == 1) {
> - DEBUG_CONT("enabled for only read-only segments\n");
> + INFO_CONT("enabled for only read-only segments\n");
> } else {
> - DEBUG_CONT("disabled\n");
> + INFO_CONT("disabled\n");
> sharing = 0;
> }
> }
> @@ -1259,7 +1263,7 @@ static int parse_elf()
> dl_iterate_phdr(parse_elf_normal, NULL);
>
> if (htlb_num_segs == 0) {
> - DEBUG("No segments were appropriate for remapping\n");
> + INFO("No segments were appropriate for remapping\n");
> return -1;
> }
>
> @@ -1276,7 +1280,7 @@ void hugetlbfs_setup_elflink(void)
> if (parse_elf())
> return;
>
> - DEBUG("libhugetlbfs version: %s\n", VERSION);
> + INFO("libhugetlbfs version: %s\n", VERSION);
>
> /* Do we need to find a share directory */
> if (sharing) {
> @@ -1297,7 +1301,8 @@ void hugetlbfs_setup_elflink(void)
> for (i = 0; i < htlb_num_segs; i++) {
> ret = obtain_prepared_file(&htlb_seg_table[i]);
> if (ret < 0) {
> - DEBUG("Failed to setup hugetlbfs file for segment
> %d\n", i);
> + WARNING("Failed to setup hugetlbfs file for segment "
> + "%d\n", i);
>
> /* Close files we have already prepared */
> for (; i >= 0; i--)
>
--
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
