On Mon, Mar 21, 2011 at 03:00:31AM +0000, Ben Hutchings wrote:
> This kind of cute format:
>
> if (test_bit(enabled, &transparent_hugepage_flags)) {
> VM_BUG_ON(test_bit(req_madv, &transparent_hugepage_flags));
> return sprintf(buf, "[always] madvise never\n");
> } else if (test_bit(req_madv, &transparent_hugepage_flags))
> return sprintf(buf, "always [madvise] never\n");
> else
> return sprintf(buf, "always madvise [never]\n");
>
> is probably nice for a kernel developer or experimental user poking
> around in sysfs. But sysfs is mostly meant for programs to read and
> write, and this format is unnecessarily complex for a program to parse.
>
> Please use separate attributes for the current value and available
> values, like cpufreq does. I know there are other examples of the above
> format, but not everything already in sysfs is a *good* example!
Well I liked the io scheduler format the most as you may have guessed:
noop deadline [cfq]
so I used exactly that format... I didn't invent it. I found that the
most intuitive and simpler so you deal with a single file, it's faster
and more intuitive to use when you're on the shell and you twiddle
with the values. You simply cannot get it wrong.
> This, on the other hand, is totally ridiculous:
>
> if (test_bit(flag, &transparent_hugepage_flags))
> return sprintf(buf, "[yes] no\n");
> else
> return sprintf(buf, "yes [no]\n");
>
> Why show the possible values of a boolean? I can't even find any
> examples of 'yes' and 'no' rather than '1' and '0'.
As said I like that format and I've been consistent in using it. If
you write a parser for that format in userland it's probably easier to
be consistent. Anyway this got into 2.6.38 only. For other kernels
that shipped THP before 2.6.38 there is no
/sys/kernel/mm/transparent_hugepage directory at all (it's renamed
exactly to avoid any risk of sysfs ABI clashes). I doubt anybody wrote
any parser for /sys/kernel/mm/transparent_hugepage so if this is a big
deal I suggest you send patches to whatever you prefer. Or if you tell
me exactly how you want it, I can try to implement it and if others
agree I don't see a problem in altering it. But others may
disagree. Clearly best would have been if you requested a change
during 2.6.38-rc, everyone was aware of the format as everyone has
been twiddling with these sysfs controls. Comments welcome.
> And really, why add boolean flags for a tristate at all?
I don't get the question sorry.
--
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>