On Wed, 13 Apr 2011 12:04:59 -0700 (PDT) David Rientjes <rientjes@xxxxxxxxxx>
wrote:
> On Tue, 22 Mar 2011, Ben Hutchings wrote:
>
> > 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.
> >
> > Cc'd to stable in order to change this before many scripts depend on
> > the current strings.
> >
>
> I agree with this in general, it's certainly the standard way of altering
> a boolean tunable throughout the kernel so it would be nice to use the
> same userspace libraries with THP.
>
> Let's cc Andrew on this since it will go through the -mm tree if it's
> merged.
>
> > Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
> > Cc: stable@xxxxxxxxxx [2.6.38]
> > ---
> > mm/huge_memory.c | 21 +++++++++++----------
> > 1 files changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 113e35c..dc0b3f0 100644
> > --- 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).
Maybe make it "!!test_bit" or even
strcpy(buf, test_bit(...) ? "1\n" : "0\n");
return 2;
NeilBrown
> > }
> > 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;
> > + char *endp;
> > +
> > + value = simple_strtoul(buf, &endp, 0);
> > + if (endp == buf || 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@xxxxxxxxxx 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@xxxxxxxxx";> email@xxxxxxxxx </a>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx 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@xxxxxxxxx";> email@xxxxxxxxx </a>