Re: [PATCH] mm/thp: Use conventional format for boolean attributes

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 14 Apr 2011 14:48:07 +1000
NeilBrown <neilb@suse.de> wrote:

> > > --- a/mm/huge_memory.c
> > > +++ b/mm/huge_memory.c
> > > @@ -244,24 +244,25 @@ static ssize_t single_flag_show(struct kobject *kobj,
> > >  				struct kobj_attribute *attr, char *buf,
> > >  				enum transparent_hugepage_flag flag)
> > >  {
> > > -	if (test_bit(flag, &transparent_hugepage_flags))
> > > -		return sprintf(buf, "[yes] no\n");
> > > -	else
> > > -		return sprintf(buf, "yes [no]\n");
> > > +	return sprintf(buf, "%d\n",
> > > +		       test_bit(flag, &transparent_hugepage_flags));
> 
> It test bit guaranteed to return 0 or 1?
> 
> I think the x86 version returns 0 or -1 (that is from reading the code and
> using google to check what 'sbb' does).

Thanks for catching that.  One wonders how well-tested the patch was!

Speaking of which...

Here's the current status.  Ben, can you please test this soon?

From: Ben Hutchings <ben@decadent.org.uk>

The conventional format for boolean attributes in sysfs is numeric ("0" or
"1" followed by new-line).  Any boolean attribute can then be read and
written using a generic function.  Using the strings "yes [no]", "[yes]
no" (read), "yes" and "no" (write) will frustrate this.

[akpm@linux-foundation.org: use kstrtoul()]
[akpm@linux-foundation.org: test_bit() doesn't return 1/0, per Neil]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Johannes Weiner <jweiner@redhat.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: David Rientjes <rientjes@google.com>
Cc: NeilBrown <neilb@suse.de>
Cc: <stable@kernel.org> 	[2.6.38.x]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/huge_memory.c |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff -puN mm/huge_memory.c~mm-thp-use-conventional-format-for-boolean-attributes mm/huge_memory.c
--- a/mm/huge_memory.c~mm-thp-use-conventional-format-for-boolean-attributes
+++ a/mm/huge_memory.c
@@ -244,24 +244,28 @@ static ssize_t single_flag_show(struct k
 				struct kobj_attribute *attr, char *buf,
 				enum transparent_hugepage_flag flag)
 {
-	if (test_bit(flag, &transparent_hugepage_flags))
-		return sprintf(buf, "[yes] no\n");
-	else
-		return sprintf(buf, "yes [no]\n");
+	return sprintf(buf, "%d\n",
+		       !!test_bit(flag, &transparent_hugepage_flags));
 }
+
 static ssize_t single_flag_store(struct kobject *kobj,
 				 struct kobj_attribute *attr,
 				 const char *buf, size_t count,
 				 enum transparent_hugepage_flag flag)
 {
-	if (!memcmp("yes", buf,
-		    min(sizeof("yes")-1, count))) {
+	unsigned long value;
+	int ret;
+
+	ret = kstrtoul(buf, 10, &value);
+	if (ret < 0)
+		return ret;
+	if (value > 1)
+		return -EINVAL;
+
+	if (value)
 		set_bit(flag, &transparent_hugepage_flags);
-	} else if (!memcmp("no", buf,
-			   min(sizeof("no")-1, count))) {
+	else
 		clear_bit(flag, &transparent_hugepage_flags);
-	} else
-		return -EINVAL;
 
 	return count;
 }
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>