trivial: Drop unlikely before IS_ERR(_OR_NULL)

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/gpu/drm/ttm/ttm_tt.c            | 4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_context.c | 2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c     | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

drivers/gpu/drm/ttm/ttm_tt.c +2 -2

index bf080abc86d1..4e19d0f9cc30 100644
@@ -340,7 +340,7 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage)
 		swap_storage = shmem_file_setup("ttm swap",
 						ttm->num_pages << PAGE_SHIFT,
 						0);
-		if (unlikely(IS_ERR(swap_storage))) {
+		if (IS_ERR(swap_storage)) {
 			pr_err("Failed allocating swap storage\n");
 			return PTR_ERR(swap_storage);
 		}

@@ -354,7 +354,7 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage)
 		if (unlikely(from_page == NULL))
 			continue;
 		to_page = shmem_read_mapping_page(swap_space, i);
-		if (unlikely(IS_ERR(to_page))) {
+		if (IS_ERR(to_page)) {
 			ret = PTR_ERR(to_page);
 			goto out_err;
 		}

drivers/gpu/drm/vmwgfx/vmwgfx_context.c +1 -1

index 5ac92874404d..44e6ecba3de7 100644
@@ -159,7 +159,7 @@ static int vmw_gb_context_init(struct vmw_private *dev_priv,
 
 	if (dev_priv->has_mob) {
 		uctx->man = vmw_cmdbuf_res_man_create(dev_priv);
-		if (unlikely(IS_ERR(uctx->man))) {
+		if (IS_ERR(uctx->man)) {
 			ret = PTR_ERR(uctx->man);
 			uctx->man = NULL;
 			goto out_err;

drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +1 -1

index 620bb5cf617c..6218a36cf01a 100644
@@ -1054,7 +1054,7 @@ static long vmw_generic_ioctl(struct file *filp, unsigned int cmd,
 		return -EINVAL;
 
 	vmaster = vmw_master_check(dev, file_priv, flags);
-	if (unlikely(IS_ERR(vmaster))) {
+	if (IS_ERR(vmaster)) {
 		ret = PTR_ERR(vmaster);
 
 		if (ret != -ERESTARTSYS)

Signature · 1 line

2.4.0

This patch: Reviewed-by: Sinclair Yeh <[email protected]>

Quoted 4 lines · Viresh Kumar

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

Signed-off-by Viresh Kumar <[email protected]>

Quoted 57 lines

---
 drivers/gpu/drm/ttm/ttm_tt.c            | 4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_context.c | 2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c     | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

drivers/gpu/drm/ttm/ttm_tt.c +2 -2

index bf080abc86d1..4e19d0f9cc30 100644
@@ -340,7 +340,7 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage)
 		swap_storage = shmem_file_setup("ttm swap",
 						ttm->num_pages << PAGE_SHIFT,
 						0);
-		if (unlikely(IS_ERR(swap_storage))) {
+		if (IS_ERR(swap_storage)) {
 			pr_err("Failed allocating swap storage\n");
 			return PTR_ERR(swap_storage);
 		}

@@ -354,7 +354,7 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage)
 		if (unlikely(from_page == NULL))
 			continue;
 		to_page = shmem_read_mapping_page(swap_space, i);
-		if (unlikely(IS_ERR(to_page))) {
+		if (IS_ERR(to_page)) {
 			ret = PTR_ERR(to_page);
 			goto out_err;
 		}

drivers/gpu/drm/vmwgfx/vmwgfx_context.c +1 -1

index 5ac92874404d..44e6ecba3de7 100644
@@ -159,7 +159,7 @@ static int vmw_gb_context_init(struct vmw_private *dev_priv,
 
 	if (dev_priv->has_mob) {
 		uctx->man = vmw_cmdbuf_res_man_create(dev_priv);
-		if (unlikely(IS_ERR(uctx->man))) {
+		if (IS_ERR(uctx->man)) {
 			ret = PTR_ERR(uctx->man);
 			uctx->man = NULL;
 			goto out_err;

drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +1 -1

index 620bb5cf617c..6218a36cf01a 100644
@@ -1054,7 +1054,7 @@ static long vmw_generic_ioctl(struct file *filp, unsigned int cmd,
 		return -EINVAL;
 
 	vmaster = vmw_master_check(dev, file_priv, flags);
-	if (unlikely(IS_ERR(vmaster))) {
+	if (IS_ERR(vmaster)) {
 		ret = PTR_ERR(vmaster);
 
 		if (ret != -ERESTARTSYS)

Signature · 1 line

2.4.0

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/input/mouse/alps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/input/mouse/alps.c +1 -1

index 113d6f1516a5..cef3611a4ccd 100644
@@ -1365,7 +1365,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,

Signature · 1 line

2.4.0

Quoted 2 lines · Viresh Kumar

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

I'd rather keep it as it documents the expected behavior and double unlikely should work just fine.

Quoted 6 lines · Dmitry Torokhov

Quoted 2 lines · Viresh Kumar

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

I'd rather keep it as it documents the expected behavior and double unlikely should work just fine.

TBH, I don't really agree that it is there for documentation. The only purpose of such compiler flags is to try make code more efficient.

Anyway, I got to this series as someone asked me to fix this for one of my patches which used unlikely(IS_ERR_OR_NULL()). And so I thought about fixing all sites that are doing double unlikely (that shouldn't hurt for sure).

I will leave it to you.

Quoted 17 lines · On Saturday 01 August 2015 13:22:51 Viresh Kumar wrote:

Quoted 6 lines · Dmitry Torokhov

Quoted 2 lines · Viresh Kumar

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

I'd rather keep it as it documents the expected behavior and double unlikely should work just fine.

TBH, I don't really agree that it is there for documentation. The only purpose of such compiler flags is to try make code more efficient.

Anyway, I got to this series as someone asked me to fix this for one of my patches which used unlikely(IS_ERR_OR_NULL()). And so I thought about fixing all sites that are doing double unlikely (that shouldn't hurt for sure).

Quoted 1 line

I will leave it to you.

I think that unlikely() macro here make code more readable. Yes, it is also for compiler optimization, but also for me it looks like Clean Code pattern <https://de.wikipedia.org/wiki/Clean_Code> -- is not it?

Quoted 21 lines · Pali Rohár

Quoted 19 lines · On Saturday 01 August 2015 13:22:51 Viresh Kumar wrote:

Quoted 7 lines · Dmitry Torokhov

Quoted 3 lines · Viresh Kumar

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

I'd rather keep it as it documents the expected behavior and double unlikely should work just fine.

TBH, I don't really agree that it is there for documentation. The only purpose of such compiler flags is to try make code more efficient.

Anyway, I got to this series as someone asked me to fix this for one of my patches which used unlikely(IS_ERR_OR_NULL()). And so I thought about fixing all sites that are doing double unlikely (that shouldn't hurt for sure).

I will leave it to you.

Quoted 4 lines

I think that unlikely() macro here make code more readable. Yes, it is also for compiler optimization, but also for me it looks like Clean Code pattern <https://de.wikipedia.org/wiki/Clean_Code> -- is not it?

People know that errors are unlikely, no need to tell them. I'd remove it.
Pavel

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

Signed-off-by Viresh Kumar <[email protected]>

---
  drivers/net/ethernet/ti/netcp_core.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

drivers/net/ethernet/ti/netcp_core.c +2 -2

index ec8ed30196f3..f685a19a3703 100644
@@ -1016,7 +1016,7 @@ netcp_tx_map_skb(struct sk_buff *skb, struct netcp_intf *netcp)
  	}
 
  	desc = knav_pool_desc_get(netcp->tx_pool);
-	if (unlikely(IS_ERR_OR_NULL(desc))) {
+	if (IS_ERR_OR_NULL(desc)) {
  		dev_err(netcp->ndev_dev, "out of TX desc\n");
  		dma_unmap_single(dev, dma_addr, pkt_len, DMA_TO_DEVICE);
  		return NULL;

@@ -1049,7 +1049,7 @@ netcp_tx_map_skb(struct sk_buff *skb, struct netcp_intf *netcp)
  		}
 
  		ndesc = knav_pool_desc_get(netcp->tx_pool);
-		if (unlikely(IS_ERR_OR_NULL(ndesc))) {
+		if (IS_ERR_OR_NULL(ndesc)) {
  			dev_err(netcp->ndev_dev, "out of TX desc for frags\n");
  			dma_unmap_page(dev, dma_addr, buf_len, DMA_TO_DEVICE);
  			goto free_descs;

Sent: 31 July 2015 16:04

Quoted 3 lines · Viresh Kumar

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

IS_ERR_OR_NULL() is defined as

static inline bool __must_check IS_ERR_OR_NULL(__force const void *ptr)

{
         return !ptr || IS_ERR_VALUE((unsigned long)ptr);
}

So the unlikely() applies only to second part. Wouldn't that be a problem for optimization?

Quoted 3 lines · Viresh Kumar

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

IS_ERR_OR_NULL() is defined as

static inline bool __must_check IS_ERR_OR_NULL(__force const void *ptr)

{
        return !ptr || IS_ERR_VALUE((unsigned long)ptr);
}

So the unlikely() applies only to second part. Wouldn't that be a problem for optimization?

Quoted 12 lines · Murali Karicheri

Quoted 3 lines · Viresh Kumar

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

IS_ERR_OR_NULL() is defined as

static inline bool __must_check IS_ERR_OR_NULL(__force const void *ptr)

{
         return !ptr || IS_ERR_VALUE((unsigned long)ptr);
}

Quoted 2 lines

So the unlikely() applies only to second part. Wouldn't that be a problem for optimization?

This is what the first patch of the series does:

http://permalink.gmane.org/gmane.linux.kernel/2009151

From: kbuild test robot <[email protected]>

drivers/rtc/rtc-gemini.c:151:1-3: WARNING: PTR_ERR_OR_ZERO can be used

Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR

Generated by: scripts/coccinelle/api/ptr_ret.cocci

Signed-off-by Fengguang Wu <[email protected]>

Signed-off-by Viresh Kumar <[email protected]>

---
Got a cleanup patch from Fengguang, maybe we can apply this too.

 rtc-gemini.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

gemini driver was using likely() for a failure case while the rtc driver is getting registered. That looks wrong and it should really be unlikely. But because we are killing all the unlikely() flags, lets kill that too.

Signed-off-by Viresh Kumar <[email protected]>

---
V1->V2:
- Removed the likely() part from gemini driver and the changelog wasn't updated to match that. Fixed the changelog now.

 drivers/rtc/interface.c  | 2 +-
 drivers/rtc/rtc-bfin.c   | 2 +-
 drivers/rtc/rtc-gemini.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

drivers/rtc/interface.c +1 -1

index 11b639067312..5836751b8203 100644
@@ -564,7 +564,7 @@ enum hrtimer_restart rtc_pie_update_irq(struct hrtimer *timer)
 void rtc_update_irq(struct rtc_device *rtc,
 		unsigned long num, unsigned long events)
 {
-	if (unlikely(IS_ERR_OR_NULL(rtc)))
+	if (IS_ERR_OR_NULL(rtc))
 		return;
 
 	pm_stay_awake(rtc->dev.parent);

drivers/rtc/rtc-bfin.c +1 -1

index 3d44b11721ea..535a5f9338d0 100644
@@ -361,7 +361,7 @@ static int bfin_rtc_probe(struct platform_device *pdev)
 	/* Register our RTC with the RTC framework */
 	rtc->rtc_dev = devm_rtc_device_register(dev, pdev->name, &bfin_rtc_ops,
 						THIS_MODULE);
-	if (unlikely(IS_ERR(rtc->rtc_dev)))
+	if (IS_ERR(rtc->rtc_dev))
 		return PTR_ERR(rtc->rtc_dev);
 
 	/* Grab the IRQ and init the hardware */

drivers/rtc/rtc-gemini.c +1 -1

index 35f4486738fc..2fed93e1114a 100644
@@ -148,7 +148,7 @@ static int gemini_rtc_probe(struct platform_device *pdev)
 
 	rtc->rtc_dev = rtc_device_register(pdev->name, dev,
 					   &gemini_rtc_ops, THIS_MODULE);
-	if (likely(IS_ERR(rtc->rtc_dev)))
+	if (IS_ERR(rtc->rtc_dev))
 		return PTR_ERR(rtc->rtc_dev);
 
 	return 0;

Signature · 1 line

2.4.0

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

gemini driver was using likely() for a failure case while the rtc driver is getting registered. That looks wrong and it should really be unlikely. But because we are killing all the unlikely() flags, lets kill that too.

Signed-off-by Viresh Kumar <[email protected]>

---
V1->V2:
- Removed the likely() part from gemini driver and the changelog wasn't updated to match that. Fixed the changelog now.

 drivers/rtc/interface.c  | 2 +-
 drivers/rtc/rtc-bfin.c   | 2 +-
 drivers/rtc/rtc-gemini.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/target/tcm_fc/tfc_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/target/tcm_fc/tfc_cmd.c +0 -0

index 68031723e5be..aa3caca8bace 100644

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

Signed-off-by Viresh Kumar <[email protected]>

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

Signed-off-by Viresh Kumar <[email protected]>

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

Signed-off-by Viresh Kumar <[email protected]>

Quoted 4 lines · Viresh Kumar

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

Signed-off-by Viresh Kumar <[email protected]>

Acked-by Kirill A. Shutemov <[email protected]>

Signature · 6 lines

Kirill A. Shutemov
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Quoted 4 lines · Kirill A. Shutemov

On Fri, Jul 31, 2015 at 02:08:34PM +0530, Viresh Kumar wrote:

Quoted 6 lines

Quoted 4 lines

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

Signed-off-by Viresh Kumar <[email protected]>

Acked-by Kirill A. Shutemov <[email protected]>

search in code, there are lots of using like this , does need add this check into checkpatch ?

# grep -r 'likely.*IS_ERR'  .
./include/linux/blk-cgroup.h:	if (unlikely(IS_ERR(blkg)))
./fs/nfs/objlayout/objio_osd.c:	if (unlikely(IS_ERR(od))) {
./fs/cifs/readdir.c:	if (unlikely(IS_ERR(dentry)))
./fs/ext4/extents.c:		if (unlikely(IS_ERR(bh))) {
./fs/ext4/extents.c:		if (unlikely(IS_ERR(path1))) {
./fs/ext4/extents.c:		if (unlikely(IS_ERR(path2))) {

Quoted 4 lines · yalin wang

Quoted 4 lines · Kirill A. Shutemov

On Fri, Jul 31, 2015 at 02:08:34PM +0530, Viresh Kumar wrote:

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.

Signed-off-by Viresh Kumar <[email protected]>

Acked-by Kirill A. Shutemov <[email protected]>

search in code, there are lots of using like this , does need add this check into checkpatch ?

cc'd Joe for that. :)

Quoted 7 lines

# grep -r 'likely.*IS_ERR'  .
./include/linux/blk-cgroup.h:	if (unlikely(IS_ERR(blkg)))
./fs/nfs/objlayout/objio_osd.c:	if (unlikely(IS_ERR(od))) {
./fs/cifs/readdir.c:	if (unlikely(IS_ERR(dentry)))
./fs/ext4/extents.c:		if (unlikely(IS_ERR(bh))) {
./fs/ext4/extents.c:		if (unlikely(IS_ERR(path1))) {
./fs/ext4/extents.c:		if (unlikely(IS_ERR(path2))) {

Btw, my series has fixed all of them :)

If it's all fixed, then it's unlikely to be needed in checkpatch.

Quoted 1 line · Joe Perches

If it's all fixed, then it's unlikely to be needed in checkpatch.

I thought checkpatch is more about not committing new mistakes, rather than finding them in old code.

Quoted 5 lines · Viresh Kumar

Quoted 1 line · Joe Perches

If it's all fixed, then it's unlikely to be needed in checkpatch.

I thought checkpatch is more about not committing new mistakes, rather than finding them in old code.

True, but checkpatch is more about style than substance.

There are a lot of things that _could_ be added to the script but don't have to be because of relative rarity.

The unanswered fundamental though is whether the unlikely use in #define IS_ERR_VALUE is useful.

include/linux/err.h:21:#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)

How often does using unlikely here make the code smaller/faster with more recent compilers than gcc 3.4? Or even using gcc 3.4.

Hi,

This cleans up the usage of IS_ERR(_OR_NULL)(), where the callers have added additional unlikely compiler flag to them. It also fixes the definition of IS_ERR_OR_NULL(), to use unlikely for all checks it does.

Viresh Kumar (15):
err.h: add (missing) unlikely() to IS_ERR_OR_NULL() PM / OPP: Drop unlikely before IS_ERR(_OR_NULL)
drivers: devfreq: Drop unlikely before IS_ERR(_OR_NULL)
drivers: gpu: Drop unlikely before IS_ERR(_OR_NULL)
drivers: input: Drop unlikely before IS_ERR(_OR_NULL)
drivers: md: Drop unlikely before IS_ERR(_OR_NULL)
drivers: misc: Drop unlikely before IS_ERR(_OR_NULL)
drivers: net: Drop unlikely before IS_ERR(_OR_NULL)
drivers: rtc: Drop unlikely before IS_ERR(_OR_NULL)
drivers: staging: Drop unlikely before IS_ERR(_OR_NULL)
drivers: target: Drop unlikely before IS_ERR(_OR_NULL)
fs: Drop unlikely before IS_ERR(_OR_NULL)
blk-cgroup: Drop unlikely before IS_ERR(_OR_NULL)
mm: Drop unlikely before IS_ERR(_OR_NULL)
net: Drop unlikely before IS_ERR(_OR_NULL)

  drivers/base/power/opp.c                             | 6 +++---
  drivers/devfreq/devfreq.c                            | 4 ++--
  drivers/gpu/drm/ttm/ttm_tt.c                         | 4 ++--
  drivers/gpu/drm/vmwgfx/vmwgfx_context.c              | 2 +-
  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c                  | 2 +-
  drivers/input/mouse/alps.c                           | 2 +-
  drivers/md/dm-snap-persistent.c                      | 2 +-
  drivers/md/dm-verity.c                               | 2 +-
  drivers/md/persistent-data/dm-block-manager.c        | 8 ++++----
  drivers/misc/c2port/core.c                           | 2 +-
  drivers/net/ethernet/ti/netcp_core.c                 | 4 ++--
  drivers/rtc/interface.c                              | 2 +-
  drivers/rtc/rtc-bfin.c                               | 2 +-
  drivers/rtc/rtc-gemini.c                             | 2 +-
  drivers/staging/android/ashmem.c                     | 2 +-
  drivers/staging/lustre/include/linux/libcfs/libcfs.h | 2 +-
  drivers/staging/lustre/lustre/obdclass/lu_object.c   | 6 +++---
  drivers/target/tcm_fc/tfc_cmd.c                      | 2 +-
  fs/cifs/readdir.c                                    | 2 +-
  fs/ecryptfs/inode.c                                  | 2 +-
  fs/ext4/extents.c                                    | 6 +++---
  fs/ext4/namei.c                                      | 2 +-
  fs/namei.c                                           | 4 ++--
  fs/ncpfs/dir.c                                       | 2 +-
  fs/nfs/objlayout/objio_osd.c                         | 2 +-
  fs/proc/proc_sysctl.c                                | 2 +-
  include/linux/blk-cgroup.h                           | 2 +-
  include/linux/err.h                                  | 2 +-
  mm/huge_memory.c                                     | 2 +-
  net/openvswitch/datapath.c                           | 2 +-
  net/sctp/socket.c                                    | 2 +-
  net/socket.c                                         | 6 +++---
  32 files changed, 47 insertions(+), 47 deletions(-)

[+CC Steven Rostedt]

Any idea what the compiler does in the case of
"if (likely(IS_ERR(...)))"? There are apparently such cases in the source.

does the "likely" somehow override the "unlikely" of IS_ERR, or is the resulting code a mess?

Quoted 5 lines · Viresh Kumar

Hi,

This cleans up the usage of IS_ERR(_OR_NULL)(), where the callers have added additional unlikely compiler flag to them. It also fixes the definition of IS_ERR_OR_NULL(), to use unlikely for all checks it does.

[+CC Steven Rostedt]

Any idea what the compiler does in the case of
"if (likely(IS_ERR(...)))"? There are apparently such cases in the source.

Quoted 7 lines · Vlastimil Babka

Quoted 5 lines · Viresh Kumar

Hi,

This cleans up the usage of IS_ERR(_OR_NULL)(), where the callers have added additional unlikely compiler flag to them. It also fixes the definition of IS_ERR_OR_NULL(), to use unlikely for all checks it does.

Quoted 4 lines

[+CC Steven Rostedt]

Any idea what the compiler does in the case of
"if (likely(IS_ERR(...)))"? There are apparently such cases in the source.

We have two cases in code:

drivers/rtc/rtc-gemini.c: if (likely(IS_ERR(rtc->rtc_dev)))

drivers/staging/lustre/lustre/obdclass/lu_object.c: if (likely(IS_ERR(shadow) && PTR_ERR(shadow) == -ENOENT)) {

Quoted 11 lines · Vlastimil Babka

Quoted 5 lines · Viresh Kumar

Hi,

This cleans up the usage of IS_ERR(_OR_NULL)(), where the callers have added additional unlikely compiler flag to them. It also fixes the definition of IS_ERR_OR_NULL(), to use unlikely for all checks it does.

[+CC Steven Rostedt]

Any idea what the compiler does in the case of
"if (likely(IS_ERR(...)))"? There are apparently such cases in the source.

We have two cases in code:

drivers/rtc/rtc-gemini.c: if (likely(IS_ERR(rtc->rtc_dev)))
drivers/staging/lustre/lustre/obdclass/lu_object.c: if (likely(IS_ERR(shadow) && PTR_ERR(shadow) == -ENOENT)) {

The first one is mistake, I think. Or do we expect rtc_device_register() to fail?

The second is redundant. "if (PTR_ERR(shadow) == -ENOENT)" should do the job.

Quoted 5 lines · On Fri, 31 Jul 2015 13:23:10 +0300 "Kirill A. Shutemov" <[email protected]>…

We have two cases in code:

drivers/rtc/rtc-gemini.c: if (likely(IS_ERR(rtc->rtc_dev)))
drivers/staging/lustre/lustre/obdclass/lu_object.c: if (likely(IS_ERR(shadow) && PTR_ERR(shadow) == -ENOENT)) {

Quoted 6 lines

The first one is mistake, I think. Or do we expect rtc_device_register() to fail?

The second is redundant. "if (PTR_ERR(shadow) == -ENOENT)" should do the job.

Yep, those look like bugs to me.