| Message ID | 081982d1188978b6020952afb4c2dcf3bc42fa1f.1443667610.git.geliangtang@163.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thursday 01 October 2015 10:55:30 Geliang Tang wrote: > IS_ERR_OR_NULL already contain an unlikely compiler flag. Drop it. > > Signed-off-by: Geliang Tang <geliangtang@163.com> > --- > drivers/input/mouse/alps.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c > index 4d24686..b4f146a 100644 > --- a/drivers/input/mouse/alps.c > +++ b/drivers/input/mouse/alps.c > @@ -1367,7 +1367,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse, > /* On V2 devices the DualPoint Stick reports bare packets */ > dev = priv->dev2; > dev2 = psmouse->dev; > - } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) { > + } else if (IS_ERR_OR_NULL(priv->dev3)) { > /* Register dev3 mouse if we received PS/2 packet first time */ > if (!IS_ERR(priv->dev3)) > psmouse_queue_work(psmouse, &priv->dev3_register_work, Hm... I do not like this change. If I read code if (unlikely(IS_ERR_OR_NULL(priv->dev3))) then I know that it is really unlikely that condition will be truth and so this is some case of error/exception or something that normally does not happen too much. But if I read code if (IS_ERR_OR_NULL(priv->dev3)) I know nothing about chance that this condition will be truth. Explicit unlikely in previous example give me more information.
On Thu, Oct 01, 2015 at 09:30:19AM +0200, Pali Rohár wrote: > On Thursday 01 October 2015 10:55:30 Geliang Tang wrote: > > IS_ERR_OR_NULL already contain an unlikely compiler flag. Drop it. > > > > Signed-off-by: Geliang Tang <geliangtang@163.com> > > --- > > drivers/input/mouse/alps.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c > > index 4d24686..b4f146a 100644 > > --- a/drivers/input/mouse/alps.c > > +++ b/drivers/input/mouse/alps.c > > @@ -1367,7 +1367,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse, > > /* On V2 devices the DualPoint Stick reports bare packets */ > > dev = priv->dev2; > > dev2 = psmouse->dev; > > - } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) { > > + } else if (IS_ERR_OR_NULL(priv->dev3)) { > > /* Register dev3 mouse if we received PS/2 packet first time */ > > if (!IS_ERR(priv->dev3)) > > psmouse_queue_work(psmouse, &priv->dev3_register_work, > > Hm... I do not like this change. If I read code > > if (unlikely(IS_ERR_OR_NULL(priv->dev3))) > > then I know that it is really unlikely that condition will be truth and > so this is some case of error/exception or something that normally does > not happen too much. > > But if I read code > > if (IS_ERR_OR_NULL(priv->dev3)) > > I know nothing about chance that this condition will be truth. Explicit > unlikely in previous example give me more information. Yes, given that this is in packet processing path I prefer having explicit unlikely there. Thanks.
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c index 4d24686..b4f146a 100644 --- a/drivers/input/mouse/alps.c +++ b/drivers/input/mouse/alps.c @@ -1367,7 +1367,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse, /* On V2 devices the DualPoint Stick reports bare packets */ dev = priv->dev2; dev2 = psmouse->dev; - } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) { + } else if (IS_ERR_OR_NULL(priv->dev3)) { /* Register dev3 mouse if we received PS/2 packet first time */ if (!IS_ERR(priv->dev3)) psmouse_queue_work(psmouse, &priv->dev3_register_work,
IS_ERR_OR_NULL already contain an unlikely compiler flag. Drop it. Signed-off-by: Geliang Tang <geliangtang@163.com> --- drivers/input/mouse/alps.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)