On Mon, 2008-11-17 at 17:23 +0000, Mel Gorman wrote:
> > @@ -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.

My working definition of WARNING was that an event occurred that we can
handle, but with altered semantics.  In this case, we were unable to
create the share directory but we can continue, albeit with sharing
disabled.

> 
> > @@ -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.

They all result in us disabling sharing and continuing with un-shared
remapping.

> > @@ -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?

It should have always been DEBUG.  We never run this function unless
debugging is enabled.

> 
> >  
> > @@ -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.

We seem to disagree on the definition of a warning.  Since all of these
cases result in fallback to no sharing, I'd consider them noteworthy
warning messages.

<snip>
 
> > @@ -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.

I think it's a mixture.  Here we are indicating that we plan to continue
with altered semantics.  That makes it a warning to me.

<snip>

-- 
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center


-------------------------------------------------------------------------
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

Reply via email to