Andrew Morton <a...@linux-foundation.org> writes:
> On Tue, 06 Aug 2013 15:29:42 +0800 Chen Gang <gang.c...@asianux.com> wrote:
>
>> Improve the usage of return value 'result', so not only can make code
>> clearer to readers, but also can improve the performance.
>
> It used to be pervasive kernel style do to
>
> ret = -ENOMEM;
> foo = alloc(...);
> if (!foo)
> goto out;
>
> whereas nowadays people usually do the more straightforward
>
> foo = alloc(...);
> if (!foo) {
> ret = -ENOMEM;
> goto out;
> }
>
> The thinking was that the old style generated better code, but for the
> life of me I can't remember why :(
Because doing the assignment outside of the if() goto . Allows the
compiler to emit the if() goto as a single branch.
While a smart compiler may perform the code motion across the branch,
it is much easier for the compiler to branch to somewhere else perform
the assignment and then branch out.
Eric
> Your patch switches from old-style to new-style. And it appears to
> have increased the text size. I did this, to switch three sites back
> to old-style:
>
> ---
> a/kernel/sysctl_binary.c~kernel-sysctl_binaryc-improve-the-usage-of-return-value-result-fix
> +++ a/kernel/sysctl_binary.c
> @@ -941,17 +941,15 @@ static ssize_t bin_string(struct file *f
> copied = result;
> lastp = oldval + copied - 1;
>
> - if (get_user(ch, lastp)) {
> - result = -EFAULT;
> + result = -EFAULT;
> + if (get_user(ch, lastp))
> goto out;
> - }
>
> /* Trim off the trailing newline */
> if (ch == '\n') {
> - if (put_user('\0', lastp)) {
> - result = -EFAULT;
> + result = -EFAULT;
> + if (put_user('\0', lastp))
> goto out;
> - }
> copied -= 1;
> }
> }
> @@ -976,11 +974,10 @@ static ssize_t bin_intvec(struct file *f
> char *buffer;
> ssize_t result;
>
> + result = -ENOMEM;
> buffer = kmalloc(BUFSZ, GFP_KERNEL);
> - if (!buffer) {
> - result = -ENOMEM;
> + if (!buffer)
> goto out;
> - }
>
> if (oldval && oldlen) {
> unsigned __user *vec = oldval;
> _
>
> and kernel/sysctl_binary.o's .text got six bytes smaller.
>
> Now, smaller text doesn't mean faster code. But it probably means
> larger cache footprint, which can mean slower code.
>
> IOW, it isn't obvious that this was an improvement.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
