Artem Bityutskiy had written, on 07/09/2010 09:15 AM, the following:
On Fri, 2010-07-09 at 08:40 -0500, Nishanth Menon wrote:
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.
Sorry, I do not really understand you. All I said is that
unlikely()/likely() are usually used in hot-paths / tight loops.
unlikely()/likely() are micro optimization. They make no difference when
you use them in initialization paths.
So what I said, that in your patch they will make no difference
performance wise. So no benefits.
But they make if () statements a bit more difficult to read, this is a
drawback.
So your patch introduces no benefits, just a drawback. Thus, it is not
good.
There were many flamewars about unlikely and likely in lkml in the past.
And the outcome was always - do not use them anywhere except of
performance-critical tight loops / hot paths.
alright.. thanks for the review. will drop this patch.
--
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
