From: ebiederm@xmission.com (Eric W. Biederman)
To: Chen Gang <gang.chen@asianux.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>, xi.wang@gmail.com,
        nicolas.dichtel@6wind.com, Andrew Morton <akpm@linux-foundation.org>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <5200A5E6.9020803@asianux.com>
Date: Tue, 06 Aug 2013 14:46:03 -0700
In-Reply-To: <5200A5E6.9020803@asianux.com> (Chen Gang's message of "Tue, 06
	Aug 2013 15:29:42 +0800")
Message-ID: <8761vibihw.fsf@xmission.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH] kernel/sysctl_binary.c: improve the usage of return value 'result'
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1850
Lines: 47

Chen Gang <gang.chen@asianux.com> writes:

Have you tested this code?  Do you have anything that actually the
uses sysctl binary interface?

If you do have code that actually uses this interface please fix it not
to use it.  This code is fundamentally a stop gap measure and will
bit-rot in time and then we will remove it.

> Improve the usage of return value 'result', so not only can make code
> clearer to readers, but also can improve the performance.
>
> Assign the return error code only when error occurs, so can save some
> instructions (some of them are inside looping).

That actually is a code pessimization, not an optimization.  As are most
of your proposed changes.  Only assigning the error code simply can not
generate better assembly code.

> Rewrite the sysctl_getname() to make code clearer to readers, and also
> can save some instructions (it is mainly related with the usage of the
> variable 'result').

Again you are introducing branches and pessimizing the code in the name
of optimization.

> Remove useless variable 'result' for sysctl() and compat sysctl(), when
> do_sysctl() fails, it is meaningless to assign 'oldlenp ' with original
> same value.

> Also modify the related code to pass "./scripts/checkpatch.pl" checking.

I really don't care about checpatch.pl at this point.

The right answer to the code is to config it out and then you don't have
to worry about it one way or another.

The sysctl binary path has never been properly maintained and I don't
intend to start.   But I will spend 5 minutes to say this patch seems to
make the code worse not better.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/