* [PATCH] Input: alps: Drop unlikely before IS_ERR(_OR_NULL) [not found] <20190605142428.84784-1-wangkefeng.wang@huawei.com> @ 2019-06-05 14:24 ` Kefeng Wang 2019-06-05 14:42 ` Pali Rohár 0 siblings, 1 reply; 7+ messages in thread From: Kefeng Wang @ 2019-06-05 14:24 UTC (permalink / raw) To: linux-kernel; +Cc: Kefeng Wang, Pali Rohár, Dmitry Torokhov, linux-input IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag, so no need to do that again from its callers. Drop it. Cc: "Pali Rohár" <pali.rohar@gmail.com> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: linux-input@vger.kernel.org Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.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 0a6f7ca883e7..791ef0f826c5 100644 --- a/drivers/input/mouse/alps.c +++ b/drivers/input/mouse/alps.c @@ -1478,7 +1478,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, -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Input: alps: Drop unlikely before IS_ERR(_OR_NULL) 2019-06-05 14:24 ` [PATCH] Input: alps: Drop unlikely before IS_ERR(_OR_NULL) Kefeng Wang @ 2019-06-05 14:42 ` Pali Rohár 2019-06-06 1:08 ` Kefeng Wang 0 siblings, 1 reply; 7+ messages in thread From: Pali Rohár @ 2019-06-05 14:42 UTC (permalink / raw) To: Kefeng Wang; +Cc: linux-kernel, Dmitry Torokhov, linux-input On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote: > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag, > so no need to do that again from its callers. Drop it. Hi! I already reviewed this patch and rejected it, see: https://patchwork.kernel.org/patch/10817475/ > Cc: "Pali Rohár" <pali.rohar@gmail.com> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Cc: linux-input@vger.kernel.org > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.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 0a6f7ca883e7..791ef0f826c5 100644 > --- a/drivers/input/mouse/alps.c > +++ b/drivers/input/mouse/alps.c > @@ -1478,7 +1478,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, -- Pali Rohár pali.rohar@gmail.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Input: alps: Drop unlikely before IS_ERR(_OR_NULL) 2019-06-05 14:42 ` Pali Rohár @ 2019-06-06 1:08 ` Kefeng Wang 2019-06-06 2:28 ` Joe Perches 0 siblings, 1 reply; 7+ messages in thread From: Kefeng Wang @ 2019-06-06 1:08 UTC (permalink / raw) To: Pali Rohár; +Cc: linux-kernel, Dmitry Torokhov, linux-input On 2019/6/5 22:42, Pali Rohár wrote: > On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote: >> IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag, >> so no need to do that again from its callers. Drop it. > Hi! I already reviewed this patch and rejected it, see: > https://patchwork.kernel.org/patch/10817475/ OK, please ignore it. >> Cc: "Pali Rohár" <pali.rohar@gmail.com> >> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> >> Cc: linux-input@vger.kernel.org >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.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 0a6f7ca883e7..791ef0f826c5 100644 >> --- a/drivers/input/mouse/alps.c >> +++ b/drivers/input/mouse/alps.c >> @@ -1478,7 +1478,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, ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Input: alps: Drop unlikely before IS_ERR(_OR_NULL) 2019-06-06 1:08 ` Kefeng Wang @ 2019-06-06 2:28 ` Joe Perches 2019-06-12 0:59 ` Dmitry Torokhov 0 siblings, 1 reply; 7+ messages in thread From: Joe Perches @ 2019-06-06 2:28 UTC (permalink / raw) To: Kefeng Wang, Pali Rohár; +Cc: linux-kernel, Dmitry Torokhov, linux-input On Thu, 2019-06-06 at 09:08 +0800, Kefeng Wang wrote: > On 2019/6/5 22:42, Pali Rohár wrote: > > On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote: > > > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag, > > > so no need to do that again from its callers. Drop it. > > Hi! I already reviewed this patch and rejected it, see: > > https://patchwork.kernel.org/patch/10817475/ > OK, please ignore it. I think the stated reason of better readability isn't particularly sensible as the object code produced is actually slightly larger. x86-64 defconfig (gcc 8.3.0) $ size drivers/input/mouse/alps.o* text data bss dec hex filename 29416 56 0 29472 7320 drivers/input/mouse/alps.o.new 29432 56 0 29488 7330 drivers/input/mouse/alps.o.old Also if this unlikely is _really_ useful, perhaps the !IS_ERR immediately after could also use likely as the test seems only done for an OOM condition. > > > Cc: "Pali Rohár" <pali.rohar@gmail.com> > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > > Cc: linux-input@vger.kernel.org > > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.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 0a6f7ca883e7..791ef0f826c5 100644 > > > --- a/drivers/input/mouse/alps.c > > > +++ b/drivers/input/mouse/alps.c > > > @@ -1478,7 +1478,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, ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Input: alps: Drop unlikely before IS_ERR(_OR_NULL) 2019-06-06 2:28 ` Joe Perches @ 2019-06-12 0:59 ` Dmitry Torokhov 2019-06-12 7:14 ` Pali Rohár 0 siblings, 1 reply; 7+ messages in thread From: Dmitry Torokhov @ 2019-06-12 0:59 UTC (permalink / raw) To: Joe Perches; +Cc: Kefeng Wang, Pali Rohár, linux-kernel, linux-input Hi Joe, On Wed, Jun 05, 2019 at 07:28:53PM -0700, Joe Perches wrote: > On Thu, 2019-06-06 at 09:08 +0800, Kefeng Wang wrote: > > On 2019/6/5 22:42, Pali Rohár wrote: > > > On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote: > > > > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag, > > > > so no need to do that again from its callers. Drop it. > > > Hi! I already reviewed this patch and rejected it, see: > > > https://patchwork.kernel.org/patch/10817475/ > > OK, please ignore it. > > I think the stated reason of better readability isn't > particularly sensible as the object code produced is > actually slightly larger. > > x86-64 defconfig (gcc 8.3.0) > > $ size drivers/input/mouse/alps.o* > text data bss dec hex filename > 29416 56 0 29472 7320 drivers/input/mouse/alps.o.new > 29432 56 0 29488 7330 drivers/input/mouse/alps.o.old If gcc produces worse code for double unlikely, you should probably report it to gcc folks, no? Or double unlikely turns into likely? > > Also if this unlikely is _really_ useful, perhaps the > !IS_ERR immediately after could also use likely as the > test seems only done for an OOM condition. No, once you take the IS_ERR_OR_NULL(priv->dev3) == true branch it stops being hot path and additional annotations are completely unneeded. And if we failed to create and register priv->dev3 device - that's an error and system is degraded. Can't do much here. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Input: alps: Drop unlikely before IS_ERR(_OR_NULL) 2019-06-12 0:59 ` Dmitry Torokhov @ 2019-06-12 7:14 ` Pali Rohár 2019-06-12 8:35 ` Joe Perches 0 siblings, 1 reply; 7+ messages in thread From: Pali Rohár @ 2019-06-12 7:14 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Joe Perches, Kefeng Wang, linux-kernel, linux-input On Tuesday 11 June 2019 17:59:13 Dmitry Torokhov wrote: > Hi Joe, > > On Wed, Jun 05, 2019 at 07:28:53PM -0700, Joe Perches wrote: > > On Thu, 2019-06-06 at 09:08 +0800, Kefeng Wang wrote: > > > On 2019/6/5 22:42, Pali Rohár wrote: > > > > On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote: > > > > > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag, > > > > > so no need to do that again from its callers. Drop it. > > > > Hi! I already reviewed this patch and rejected it, see: > > > > https://patchwork.kernel.org/patch/10817475/ > > > OK, please ignore it. > > > > I think the stated reason of better readability isn't > > particularly sensible as the object code produced is > > actually slightly larger. > > > > x86-64 defconfig (gcc 8.3.0) > > > > $ size drivers/input/mouse/alps.o* > > text data bss dec hex filename > > 29416 56 0 29472 7320 drivers/input/mouse/alps.o.new > > 29432 56 0 29488 7330 drivers/input/mouse/alps.o.old > > If gcc produces worse code for double unlikely, you should probably > report it to gcc folks, no? Or double unlikely turns into likely? Is measured size of stripped or unstripped binary? Plus with or without debug symbols? Double unlikely version should have more debug symbols and therefore also larger size. But if unstripped version with double unlikely is larger then it is for sure compiler bug. -- Pali Rohár pali.rohar@gmail.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Input: alps: Drop unlikely before IS_ERR(_OR_NULL) 2019-06-12 7:14 ` Pali Rohár @ 2019-06-12 8:35 ` Joe Perches 0 siblings, 0 replies; 7+ messages in thread From: Joe Perches @ 2019-06-12 8:35 UTC (permalink / raw) To: Pali Rohár, Dmitry Torokhov; +Cc: Kefeng Wang, linux-kernel, linux-input On Wed, 2019-06-12 at 09:14 +0200, Pali Rohár wrote: > On Tuesday 11 June 2019 17:59:13 Dmitry Torokhov wrote: > > Hi Joe, > > > > On Wed, Jun 05, 2019 at 07:28:53PM -0700, Joe Perches wrote: > > > On Thu, 2019-06-06 at 09:08 +0800, Kefeng Wang wrote: > > > > On 2019/6/5 22:42, Pali Rohár wrote: > > > > > On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote: > > > > > > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag, > > > > > > so no need to do that again from its callers. Drop it. > > > > > Hi! I already reviewed this patch and rejected it, see: > > > > > https://patchwork.kernel.org/patch/10817475/ > > > > OK, please ignore it. > > > > > > I think the stated reason of better readability isn't > > > particularly sensible as the object code produced is > > > actually slightly larger. > > > > > > x86-64 defconfig (gcc 8.3.0) > > > > > > $ size drivers/input/mouse/alps.o* > > > text data bss dec hex filename > > > 29416 56 0 29472 7320 drivers/input/mouse/alps.o.new > > > 29432 56 0 29488 7330 drivers/input/mouse/alps.o.old > > > > If gcc produces worse code for double unlikely, you should probably > > report it to gcc folks, no? Or double unlikely turns into likely? > > Is measured size of stripped or unstripped binary? Plus with or without > debug symbols? Double unlikely version should have more debug symbols > and therefore also larger size. > > But if unstripped version with double unlikely is larger then it is for > sure compiler bug. defconfig so no debug symbols. It's not necessarily a gcc bug as gcc doesn't guarantee compiler repeatability. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-06-12 8:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20190605142428.84784-1-wangkefeng.wang@huawei.com>
2019-06-05 14:24 ` [PATCH] Input: alps: Drop unlikely before IS_ERR(_OR_NULL) Kefeng Wang
2019-06-05 14:42 ` Pali Rohár
2019-06-06 1:08 ` Kefeng Wang
2019-06-06 2:28 ` Joe Perches
2019-06-12 0:59 ` Dmitry Torokhov
2019-06-12 7:14 ` Pali Rohár
2019-06-12 8:35 ` Joe Perches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).