Artem Bityutskiy had written, on 07/09/2010 08:12 AM, the following:
On Fri, 2010-06-25 at 16:46 -0500, Nishanth Menon wrote:
Add unlikely checks to better optimize the rare occurrance of
erroneous conditions.
Cc: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
Cc: Thara Gopinath <thara@xxxxxx>
Signed-off-by: Nishanth Menon <nm@xxxxxx>
unlikely and friends make sens only in realy hot path places. In other
places like you touch, they are pointless - better let gcc make a choice
@@ -43,8 +43,9 @@ static void __init sr_read_efuse(struct omap_sr_dev_data *dev_data,
{
int i;
- if (!dev_data || !dev_data->volts_supported || !dev_data->volt_data ||
- !dev_data->efuse_nvalues_offs) {
+ if (unlikely(!dev_data || !dev_data->volts_supported ||
+ !dev_data->volt_data ||
+ !dev_data->efuse_nvalues_offs)) {
@@ -87,8 +88,8 @@ static void __init sr_set_testing_nvalues(struct omap_sr_dev_data *dev_data,
{
int i;
- if (!dev_data || !dev_data->volts_supported ||
- !dev_data->volt_data || !dev_data->test_nvalues) {
+ if (unlikely(!dev_data || !dev_data->volts_supported ||
+ !dev_data->volt_data || !dev_data->test_nvalues)) {
and other places, why do you think that these are checks that should be
expected? - would be great if you can explain inline to the patch which
unlikely checks dont make sense.
static functions such as these are helpers for the maincode, unless
something horribly wrong occurs within the codepath calling these
helpers, they are not expected to be invalid parameters. hence the
rationale for adding unlikely.
of how to arrange code. And they only make the code less readable by
adding extra braces and making ifs longer.
Thus, NACK for this from my side.
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
