From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752846AbdARRoa (ORCPT ); Wed, 18 Jan 2017 12:44:30 -0500 Received: from zeniv.linux.org.uk ([195.92.253.2]:46922 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752105AbdARRoB (ORCPT ); Wed, 18 Jan 2017 12:44:01 -0500 Date: Wed, 18 Jan 2017 17:43:20 +0000 From: Al Viro To: SF Markus Elfring Cc: linux-alpha@vger.kernel.org, Ivan Kokshaysky , Jan-Benedict Glaw , Matt Turner , Nicolas Pitre , Richard Cochran , Richard Henderson , Thomas Gleixner , LKML , kernel-janitors@vger.kernel.org Subject: Re: alpha: Checking source code positions for the setting of error codes Message-ID: <20170118174320.GE1555@ZenIV.linux.org.uk> References: <986573fa-e636-f864-14dc-5e65a31b454e@users.sourceforge.net> <20170118141103.GD1555@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jan 18, 2017 at 04:41:10PM +0100, SF Markus Elfring wrote: > >> A local variable was set to an error code in two cases before a concrete > >> error situation was detected. Thus move the corresponding assignment into > >> an if branch to indicate a software failure there. > >> > >> This issue was detected by using the Coccinelle software. > > > > Why the hell is that an issue? > > * Can misplaced variable assignments result in unwanted run time consequences > because of the previous approach for a control flow specification? More like the opposite. load constant to register test branch usually not taken is considerably cheaper than test branch usually taken Something like if (unlikely(foo)) { err = -ESOMETHING; goto sod_off; } would be more or less on par (and quite possibly would be compiled into the same code - depends upon the scheduling details for processor, but speculative load of constant can be an optimization). However, that has an effect of splattering the source with tons of those unlikely() *and* visually cluttering the common path. > * How do you think about to achieve that error codes will only be set > after a specific software failure was detected? Sounds like an arbitrary requirement, TBH... Again, loading a constant into register tends to be cheap and easy to combine with other instructions at CPU pipeline level. If anything, this pattern is a microoptimization, often in spots that are not on hotpaths by any stretch of imagination. But estimating whether a given place is on a hot path takes a lot more delicate analysis than feasible for cocci scripts. And visual cluttering of the common execution path remains - it doesn't matter for compiler, but it can matter a lot for human readers.