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(-)
IS_ERR_VALUE() already contains it and so we need to add this only to the !ptr check. That will allow users of IS_ERR_OR_NULL(), to not add this compiler flag.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- include/linux/err.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/err.h b/include/linux/err.h index a729120644d5..56762ab41713 100644 --- a/include/linux/err.h +++ b/include/linux/err.h @@ -37,7 +37,7 @@ static inline bool __must_check IS_ERR(__force const void *ptr)
static inline bool __must_check IS_ERR_OR_NULL(__force const void *ptr) { - return !ptr || IS_ERR_VALUE((unsigned long)ptr); + return unlikely(!ptr) || IS_ERR_VALUE((unsigned long)ptr); }
/**
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 viresh.kumar@linaro.org --- drivers/base/power/opp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 677fb2843553..f60bdd5cbb71 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -134,7 +134,7 @@ static struct device_opp *_find_device_opp(struct device *dev) { struct device_opp *tmp_dev_opp, *dev_opp = ERR_PTR(-ENODEV);
- if (unlikely(IS_ERR_OR_NULL(dev))) { + if (IS_ERR_OR_NULL(dev)) { pr_err("%s: Invalid parameters\n", __func__); return ERR_PTR(-EINVAL); } @@ -172,7 +172,7 @@ unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp) opp_rcu_lockdep_assert();
tmp_opp = rcu_dereference(opp); - if (unlikely(IS_ERR_OR_NULL(tmp_opp)) || !tmp_opp->available) + if (IS_ERR_OR_NULL(tmp_opp) || !tmp_opp->available) pr_err("%s: Invalid parameters\n", __func__); else v = tmp_opp->u_volt; @@ -204,7 +204,7 @@ unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp) opp_rcu_lockdep_assert();
tmp_opp = rcu_dereference(opp); - if (unlikely(IS_ERR_OR_NULL(tmp_opp)) || !tmp_opp->available) + if (IS_ERR_OR_NULL(tmp_opp) || !tmp_opp->available) pr_err("%s: Invalid parameters\n", __func__); else f = tmp_opp->rate;
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 viresh.kumar@linaro.org --- drivers/devfreq/devfreq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index ca1b362d77e2..f28a1fae5691 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -53,7 +53,7 @@ static struct devfreq *find_device_devfreq(struct device *dev) { struct devfreq *tmp_devfreq;
- if (unlikely(IS_ERR_OR_NULL(dev))) { + if (IS_ERR_OR_NULL(dev)) { pr_err("DEVFREQ: %s: Invalid parameters\n", __func__); return ERR_PTR(-EINVAL); } @@ -133,7 +133,7 @@ static struct devfreq_governor *find_devfreq_governor(const char *name) { struct devfreq_governor *tmp_governor;
- if (unlikely(IS_ERR_OR_NULL(name))) { + if (IS_ERR_OR_NULL(name)) { pr_err("DEVFREQ: %s: Invalid parameters\n", __func__); return ERR_PTR(-EINVAL); }
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 viresh.kumar@linaro.org --- 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(-)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index bf080abc86d1..4e19d0f9cc30 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -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; } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_context.c b/drivers/gpu/drm/vmwgfx/vmwgfx_context.c index 5ac92874404d..44e6ecba3de7 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_context.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_context.c @@ -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; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index 620bb5cf617c..6218a36cf01a 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -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)
This patch: Reviewed-by: Sinclair Yeh syeh@vmware.com
On Fri, Jul 31, 2015 at 02:08:24PM +0530, Viresh Kumar wrote:
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 viresh.kumar@linaro.org
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(-)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index bf080abc86d1..4e19d0f9cc30 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -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;diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_context.c b/drivers/gpu/drm/vmwgfx/vmwgfx_context.c index 5ac92874404d..44e6ecba3de7 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_context.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_context.c @@ -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;diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index 620bb5cf617c..6218a36cf01a 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -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) -- 2.4.0
On Fri, Jul 31, 2015 at 08:53:04AM -0700, Sinclair Yeh wrote:
This patch: Reviewed-by: Sinclair Yeh syeh@vmware.com
On Fri, Jul 31, 2015 at 02:08:24PM +0530, Viresh Kumar wrote:
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 viresh.kumar@linaro.org
Applied to drm-misc, thanks. -Daniel
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(-)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index bf080abc86d1..4e19d0f9cc30 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -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;diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_context.c b/drivers/gpu/drm/vmwgfx/vmwgfx_context.c index 5ac92874404d..44e6ecba3de7 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_context.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_context.c @@ -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;diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index 620bb5cf617c..6218a36cf01a 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -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) -- 2.4.0
dri-devel mailing list dri-devel@lists.freedesktop.org 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 viresh.kumar@linaro.org --- 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 113d6f1516a5..cef3611a4ccd 100644 --- a/drivers/input/mouse/alps.c +++ b/drivers/input/mouse/alps.c @@ -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,
On Fri, Jul 31, 2015 at 02:08:25PM +0530, Viresh Kumar wrote:
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.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
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 113d6f1516a5..cef3611a4ccd 100644 --- a/drivers/input/mouse/alps.c +++ b/drivers/input/mouse/alps.c @@ -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,
-- 2.4.0
Thanks.
On 31-07-15, 09:58, Dmitry Torokhov wrote:
On Fri, Jul 31, 2015 at 02:08:25PM +0530, Viresh Kumar wrote:
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.
On Saturday 01 August 2015 13:22:51 Viresh Kumar wrote:
On 31-07-15, 09:58, Dmitry Torokhov wrote:
On Fri, Jul 31, 2015 at 02:08:25PM +0530, Viresh Kumar wrote:
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.
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?
On Sat 2015-08-01 13:44:59, Pali Rohár wrote:
On Saturday 01 August 2015 13:22:51 Viresh Kumar wrote:
On 31-07-15, 09:58, Dmitry Torokhov wrote:
On Fri, Jul 31, 2015 at 02:08:25PM +0530, Viresh Kumar wrote:
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.
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
On Sunday 02 August 2015 17:43:52 Pavel Machek wrote:
On Sat 2015-08-01 13:44:59, Pali Rohár wrote:
On Saturday 01 August 2015 13:22:51 Viresh Kumar wrote:
On 31-07-15, 09:58, Dmitry Torokhov wrote:
On Fri, Jul 31, 2015 at 02:08:25PM +0530, Viresh Kumar wrote:
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.
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
Errors and bugs are always unlikely ;-)
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 viresh.kumar@linaro.org --- drivers/md/dm-snap-persistent.c | 2 +- drivers/md/dm-verity.c | 2 +- drivers/md/persistent-data/dm-block-manager.c | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/md/dm-snap-persistent.c b/drivers/md/dm-snap-persistent.c index 808b8419bc48..bf71583296f7 100644 --- a/drivers/md/dm-snap-persistent.c +++ b/drivers/md/dm-snap-persistent.c @@ -533,7 +533,7 @@ static int read_exceptions(struct pstore *ps, chunk = area_location(ps, ps->current_area);
area = dm_bufio_read(client, chunk, &bp); - if (unlikely(IS_ERR(area))) { + if (IS_ERR(area)) { r = PTR_ERR(area); goto ret_destroy_bufio; } diff --git a/drivers/md/dm-verity.c b/drivers/md/dm-verity.c index bb9c6a00e4b0..1d131519ed21 100644 --- a/drivers/md/dm-verity.c +++ b/drivers/md/dm-verity.c @@ -271,7 +271,7 @@ static int verity_verify_level(struct dm_verity_io *io, sector_t block, verity_hash_at_level(v, block, level, &hash_block, &offset);
data = dm_bufio_read(v->bufio, hash_block, &buf); - if (unlikely(IS_ERR(data))) + if (IS_ERR(data)) return PTR_ERR(data);
aux = dm_bufio_get_aux_data(buf); diff --git a/drivers/md/persistent-data/dm-block-manager.c b/drivers/md/persistent-data/dm-block-manager.c index 4d6c9b689eaa..88dbe7b97c2c 100644 --- a/drivers/md/persistent-data/dm-block-manager.c +++ b/drivers/md/persistent-data/dm-block-manager.c @@ -454,7 +454,7 @@ int dm_bm_read_lock(struct dm_block_manager *bm, dm_block_t b, int r;
p = dm_bufio_read(bm->bufio, b, (struct dm_buffer **) result); - if (unlikely(IS_ERR(p))) + if (IS_ERR(p)) return PTR_ERR(p);
aux = dm_bufio_get_aux_data(to_buffer(*result)); @@ -490,7 +490,7 @@ int dm_bm_write_lock(struct dm_block_manager *bm, return -EPERM;
p = dm_bufio_read(bm->bufio, b, (struct dm_buffer **) result); - if (unlikely(IS_ERR(p))) + if (IS_ERR(p)) return PTR_ERR(p);
aux = dm_bufio_get_aux_data(to_buffer(*result)); @@ -523,7 +523,7 @@ int dm_bm_read_try_lock(struct dm_block_manager *bm, int r;
p = dm_bufio_get(bm->bufio, b, (struct dm_buffer **) result); - if (unlikely(IS_ERR(p))) + if (IS_ERR(p)) return PTR_ERR(p); if (unlikely(!p)) return -EWOULDBLOCK; @@ -559,7 +559,7 @@ int dm_bm_write_lock_zero(struct dm_block_manager *bm, return -EPERM;
p = dm_bufio_new(bm->bufio, b, (struct dm_buffer **) result); - if (unlikely(IS_ERR(p))) + if (IS_ERR(p)) return PTR_ERR(p);
memset(p, 0, dm_bm_block_size(bm));
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 viresh.kumar@linaro.org --- drivers/misc/c2port/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/c2port/core.c b/drivers/misc/c2port/core.c index 464419b36440..cc8645b5369d 100644 --- a/drivers/misc/c2port/core.c +++ b/drivers/misc/c2port/core.c @@ -926,7 +926,7 @@ struct c2port_device *c2port_device_register(char *name,
c2dev->dev = device_create(c2port_class, NULL, 0, c2dev, "c2port%d", c2dev->id); - if (unlikely(IS_ERR(c2dev->dev))) { + if (IS_ERR(c2dev->dev)) { ret = PTR_ERR(c2dev->dev); goto error_device_create; }
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 viresh.kumar@linaro.org --- drivers/net/ethernet/ti/netcp_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c index ec8ed30196f3..f685a19a3703 100644 --- a/drivers/net/ethernet/ti/netcp_core.c +++ b/drivers/net/ethernet/ti/netcp_core.c @@ -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;
On 07/31/2015 04:38 AM, Viresh Kumar wrote:
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?
Murali
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
drivers/net/ethernet/ti/netcp_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c index ec8ed30196f3..f685a19a3703 100644 --- a/drivers/net/ethernet/ti/netcp_core.c +++ b/drivers/net/ethernet/ti/netcp_core.c @@ -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;
From: Murali Karicheri
Sent: 31 July 2015 16:04 On 07/31/2015 04:38 AM, Viresh Kumar wrote:
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?
Ugg...
The unlikely() in IS_ERR_VALUE() is likely to stop the compiler doing a single 'window' comparison for the range [-MAX_ERROR .. 0]. So you are likely to end up with 2 comparisons. I suspect that:
return IS_ERR_VALUE((unsigned long)ptr - 1);
would be a much better test. (Ignoring the off-by-one for the highest error.)
David
On 31-07-15, 11:04, Murali Karicheri wrote:
On 07/31/2015 04:38 AM, Viresh Kumar wrote:
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?
This is what the first patch of the series does:
http://permalink.gmane.org/gmane.linux.kernel/2009151
On 07/31/2015 12:20 PM, Viresh Kumar wrote:
On 31-07-15, 11:04, Murali Karicheri wrote:
On 07/31/2015 04:38 AM, Viresh Kumar wrote:
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?
This is what the first patch of the series does:
Assuming the above change is merged, this patch looks good.
Acked-by: Murali Karicheri m-karicheri2@ti.com
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 viresh.kumar@linaro.org --- drivers/rtc/interface.c | 2 +- drivers/rtc/rtc-bfin.c | 2 +- drivers/rtc/rtc-gemini.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c index 11b639067312..5836751b8203 100644 --- a/drivers/rtc/interface.c +++ b/drivers/rtc/interface.c @@ -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); diff --git a/drivers/rtc/rtc-bfin.c b/drivers/rtc/rtc-bfin.c index 3d44b11721ea..535a5f9338d0 100644 --- a/drivers/rtc/rtc-bfin.c +++ b/drivers/rtc/rtc-bfin.c @@ -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 */ diff --git a/drivers/rtc/rtc-gemini.c b/drivers/rtc/rtc-gemini.c index 35f4486738fc..2fed93e1114a 100644 --- a/drivers/rtc/rtc-gemini.c +++ b/drivers/rtc/rtc-gemini.c @@ -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;
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 viresh.kumar@linaro.org --- 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(-)
diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c index 11b639067312..5836751b8203 100644 --- a/drivers/rtc/interface.c +++ b/drivers/rtc/interface.c @@ -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); diff --git a/drivers/rtc/rtc-bfin.c b/drivers/rtc/rtc-bfin.c index 3d44b11721ea..535a5f9338d0 100644 --- a/drivers/rtc/rtc-bfin.c +++ b/drivers/rtc/rtc-bfin.c @@ -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 */ diff --git a/drivers/rtc/rtc-gemini.c b/drivers/rtc/rtc-gemini.c index 35f4486738fc..2fed93e1114a 100644 --- a/drivers/rtc/rtc-gemini.c +++ b/drivers/rtc/rtc-gemini.c @@ -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;
Hi
On Fri, 31 Jul 2015, Viresh Kumar wrote:
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 viresh.kumar@linaro.org
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(-)
diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c index 11b639067312..5836751b8203 100644 --- a/drivers/rtc/interface.c +++ b/drivers/rtc/interface.c @@ -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); diff --git a/drivers/rtc/rtc-bfin.c b/drivers/rtc/rtc-bfin.c index 3d44b11721ea..535a5f9338d0 100644 --- a/drivers/rtc/rtc-bfin.c +++ b/drivers/rtc/rtc-bfin.c @@ -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 */ diff --git a/drivers/rtc/rtc-gemini.c b/drivers/rtc/rtc-gemini.c index 35f4486738fc..2fed93e1114a 100644 --- a/drivers/rtc/rtc-gemini.c +++ b/drivers/rtc/rtc-gemini.c @@ -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; -- 2.4.0
For the mach-gemini part Acked-by: Hans Ulli Kroll ulli.kroll@googlemail.com
On 31/07/2015 at 16:23:43 +0530, Viresh Kumar wrote :
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 viresh.kumar@linaro.org
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(-)
Applied, thanks.
IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it.
This also replaces an IS_ERR(x) + (x == NULL) check to IS_ERR_OR_NULL check.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/staging/android/ashmem.c | 2 +- drivers/staging/lustre/include/linux/libcfs/libcfs.h | 2 +- drivers/staging/lustre/lustre/obdclass/lu_object.c | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c index c5c037ccf32c..9db321c63e79 100644 --- a/drivers/staging/android/ashmem.c +++ b/drivers/staging/android/ashmem.c @@ -388,7 +388,7 @@ static int ashmem_mmap(struct file *file, struct vm_area_struct *vma)
/* ... and allocate the backing shmem file */ vmfile = shmem_file_setup(name, asma->size, vma->vm_flags); - if (unlikely(IS_ERR(vmfile))) { + if (IS_ERR(vmfile)) { ret = PTR_ERR(vmfile); goto out; } diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs.h b/drivers/staging/lustre/include/linux/libcfs/libcfs.h index 5dd9cdfae30c..d585041041c7 100644 --- a/drivers/staging/lustre/include/linux/libcfs/libcfs.h +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs.h @@ -134,7 +134,7 @@ void cfs_get_random_bytes(void *buf, int size); /* container_of depends on "likely" which is defined in libcfs_private.h */ static inline void *__container_of(void *ptr, unsigned long shift) { - if (unlikely(IS_ERR(ptr) || ptr == NULL)) + if (IS_ERR_OR_NULL(ptr)) return ptr; return (char *)ptr - shift; } diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c index 4d9b6333eeae..6cd3af8c6237 100644 --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c @@ -602,7 +602,7 @@ static struct lu_object *lu_object_new(const struct lu_env *env, struct lu_site_bkt_data *bkt;
o = lu_object_alloc(env, dev, f, conf); - if (unlikely(IS_ERR(o))) + if (IS_ERR(o)) return o;
hs = dev->ld_site->ls_obj_hash; @@ -666,7 +666,7 @@ static struct lu_object *lu_object_find_try(const struct lu_env *env, * operations, including fld queries, inode loading, etc. */ o = lu_object_alloc(env, dev, f, conf); - if (unlikely(IS_ERR(o))) + if (IS_ERR(o)) return o;
LASSERT(lu_fid_eq(lu_object_fid(o), f)); @@ -1558,7 +1558,7 @@ static int keys_fill(struct lu_context *ctx) LINVRNT(key->lct_index == i);
value = key->lct_init(ctx, key); - if (unlikely(IS_ERR(value))) + if (IS_ERR(value)) return PTR_ERR(value);
if (!(ctx->lc_tags & LCT_NOREF))
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 viresh.kumar@linaro.org --- drivers/target/tcm_fc/tfc_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c index 68031723e5be..aa3caca8bace 100644 --- a/drivers/target/tcm_fc/tfc_cmd.c +++ b/drivers/target/tcm_fc/tfc_cmd.c @@ -255,7 +255,7 @@ static void ft_recv_seq(struct fc_seq *sp, struct fc_frame *fp, void *arg) struct ft_cmd *cmd = arg; struct fc_frame_header *fh;
- if (unlikely(IS_ERR(fp))) { + if (IS_ERR(fp)) { /* XXX need to find cmd if queued */ cmd->seq = NULL; cmd->aborted = true;
On Fri, 2015-07-31 at 14:08 +0530, Viresh Kumar wrote:
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 viresh.kumar@linaro.org
drivers/target/tcm_fc/tfc_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c index 68031723e5be..aa3caca8bace 100644 --- a/drivers/target/tcm_fc/tfc_cmd.c +++ b/drivers/target/tcm_fc/tfc_cmd.c @@ -255,7 +255,7 @@ static void ft_recv_seq(struct fc_seq *sp, struct fc_frame *fp, void *arg) struct ft_cmd *cmd = arg; struct fc_frame_header *fh;
- if (unlikely(IS_ERR(fp))) {
- if (IS_ERR(fp)) { /* XXX need to find cmd if queued */ cmd->seq = NULL; cmd->aborted = true;
Looks fine. Applied to target-pending/for-next.
Thanks Viresh!
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 viresh.kumar@linaro.org --- 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 +- 8 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index b1eede3678a9..0557c45e9c33 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c @@ -84,7 +84,7 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name, cifs_dbg(FYI, "%s: for %s\n", __func__, name->name);
dentry = d_hash_and_lookup(parent, name); - if (unlikely(IS_ERR(dentry))) + if (IS_ERR(dentry)) return;
if (dentry) { diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 3c4db1172d22..e2e47ba5d313 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -270,7 +270,7 @@ ecryptfs_create(struct inode *directory_inode, struct dentry *ecryptfs_dentry,
ecryptfs_inode = ecryptfs_do_create(directory_inode, ecryptfs_dentry, mode); - if (unlikely(IS_ERR(ecryptfs_inode))) { + if (IS_ERR(ecryptfs_inode)) { ecryptfs_printk(KERN_WARNING, "Failed to create file in" "lower filesystem\n"); rc = PTR_ERR(ecryptfs_inode); diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 2553aa8b608d..799f01714767 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -899,7 +899,7 @@ ext4_find_extent(struct inode *inode, ext4_lblk_t block,
bh = read_extent_tree_block(inode, path[ppos].p_block, --i, flags); - if (unlikely(IS_ERR(bh))) { + if (IS_ERR(bh)) { ret = PTR_ERR(bh); goto err; } @@ -5792,7 +5792,7 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1, int split = 0;
path1 = ext4_find_extent(inode1, lblk1, NULL, EXT4_EX_NOCACHE); - if (unlikely(IS_ERR(path1))) { + if (IS_ERR(path1)) { *erp = PTR_ERR(path1); path1 = NULL; finish: @@ -5800,7 +5800,7 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1, goto repeat; } path2 = ext4_find_extent(inode2, lblk2, NULL, EXT4_EX_NOCACHE); - if (unlikely(IS_ERR(path2))) { + if (IS_ERR(path2)) { *erp = PTR_ERR(path2); path2 = NULL; goto finish; diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 011dcfb5cce3..0554e4be1fb8 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -1429,7 +1429,7 @@ static struct buffer_head * ext4_find_entry (struct inode *dir, } num++; bh = ext4_getblk(NULL, dir, b++, 0); - if (unlikely(IS_ERR(bh))) { + if (IS_ERR(bh)) { if (ra_max == 0) { ret = bh; goto cleanup_and_exit; diff --git a/fs/namei.c b/fs/namei.c index ae4e4c18b2ac..04fdec230c45 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1942,7 +1942,7 @@ static int link_path_walk(const char *name, struct nameidata *nd) if (err) { const char *s = get_link(nd);
- if (unlikely(IS_ERR(s))) + if (IS_ERR(s)) return PTR_ERR(s); err = 0; if (unlikely(!s)) { @@ -3351,7 +3351,7 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt, return ERR_PTR(-ELOOP);
filename = getname_kernel(name); - if (unlikely(IS_ERR(filename))) + if (IS_ERR(filename)) return ERR_CAST(filename);
set_nameidata(&nd, -1, filename); diff --git a/fs/ncpfs/dir.c b/fs/ncpfs/dir.c index 93575e91a7aa..356816e7bc90 100644 --- a/fs/ncpfs/dir.c +++ b/fs/ncpfs/dir.c @@ -597,7 +597,7 @@ ncp_fill_cache(struct file *file, struct dir_context *ctx, qname.name = __name;
newdent = d_hash_and_lookup(dentry, &qname); - if (unlikely(IS_ERR(newdent))) + if (IS_ERR(newdent)) goto end_advance; if (!newdent) { newdent = d_alloc(dentry, &qname); diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c index 5aaed363556a..5c0c6b58157f 100644 --- a/fs/nfs/objlayout/objio_osd.c +++ b/fs/nfs/objlayout/objio_osd.c @@ -124,7 +124,7 @@ objio_alloc_deviceid_node(struct nfs_server *server, struct pnfs_device *pdev,
retry_lookup: od = osduld_info_lookup(&odi); - if (unlikely(IS_ERR(od))) { + if (IS_ERR(od)) { err = PTR_ERR(od); dprintk("%s: osduld_info_lookup => %d\n", __func__, err); if (err == -ENODEV && retry_flag) { diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index fdda62e6115e..fe5b6e6c4671 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -948,7 +948,7 @@ static struct ctl_dir *get_subdir(struct ctl_dir *dir, found: subdir->header.nreg++; failed: - if (unlikely(IS_ERR(subdir))) { + if (IS_ERR(subdir)) { pr_err("sysctl could not get directory: "); sysctl_print_dir(dir); pr_cont("/%*.*s %ld\n",
Viresh Kumar viresh.kumar@linaro.org wrote:
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 viresh.kumar@linaro.org
Reviewed-by: David Howells dhowells@redhat.com
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 viresh.kumar@linaro.org --- include/linux/blk-cgroup.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h index 1b62d768c7df..a4cd1641e9e2 100644 --- a/include/linux/blk-cgroup.h +++ b/include/linux/blk-cgroup.h @@ -374,7 +374,7 @@ static inline struct request_list *blk_get_rl(struct request_queue *q, * root_rl in such cases. */ blkg = blkg_lookup_create(blkcg, q); - if (unlikely(IS_ERR(blkg))) + if (IS_ERR(blkg)) goto root_rl;
blkg_get(blkg);
On Fri, Jul 31, 2015 at 02:08:33PM +0530, Viresh Kumar wrote:
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 viresh.kumar@linaro.org
Acked-by: Tejun Heo tj@kernel.org
Thanks.
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 viresh.kumar@linaro.org --- mm/huge_memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c index c107094f79ba..e14652480c59 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -149,7 +149,7 @@ static int start_stop_khugepaged(void) if (!khugepaged_thread) khugepaged_thread = kthread_run(khugepaged, NULL, "khugepaged"); - if (unlikely(IS_ERR(khugepaged_thread))) { + if (IS_ERR(khugepaged_thread)) { pr_err("khugepaged: kthread_run(khugepaged) failed\n"); err = PTR_ERR(khugepaged_thread); khugepaged_thread = NULL;
On Fri, Jul 31, 2015 at 02:08:34PM +0530, Viresh Kumar wrote:
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 viresh.kumar@linaro.org
Acked-by: Kirill A. Shutemov kirill.shutemov@linux.intel.com
On Jul 31, 2015, at 16:56, Kirill A. Shutemov kirill@shutemov.name wrote:
On Fri, Jul 31, 2015 at 02:08:34PM +0530, Viresh Kumar wrote:
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 viresh.kumar@linaro.org
Acked-by: Kirill A. Shutemov kirill.shutemov@linux.intel.com
-- Kirill A. Shutemov -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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))) { ./fs/ext4/namei.c: if (unlikely(IS_ERR(bh))) { ./fs/ntfs/lcnalloc.c: if (likely(page && !IS_ERR(page))) { ./fs/ntfs/runlist.c: if (likely(!IS_ERR(old_rl))) ./fs/ntfs/mft.c: if (likely(!IS_ERR(page))) { ./fs/ntfs/mft.c: if (likely(!IS_ERR(m))) ./fs/ntfs/mft.c: if (likely(!IS_ERR(m))) { ./fs/ntfs/mft.c: if (unlikely(IS_ERR(rl) || !rl->length || rl->lcn < 0)) { ./fs/ntfs/mft.c: if (unlikely(IS_ERR(rl) || !rl->length || rl->lcn < 0)) { ./fs/ntfs/mft.c: if (likely(!IS_ERR(rl2))) ./fs/ntfs/inode.c: if (unlikely(err || IS_ERR(m))) { ./fs/ntfs/namei.c: if (likely(!IS_ERR(dent_inode))) { ./fs/ntfs/super.c: if (unlikely(IS_ERR(tmp_ino) || is_bad_inode(tmp_ino))) { ./fs/namei.c: if (unlikely(IS_ERR(s))) ./fs/namei.c: if (unlikely(IS_ERR(filename))) ./fs/gfs2/dir.c: if (unlikely(dent == NULL || IS_ERR(dent))) { ./fs/ecryptfs/inode.c: if (unlikely(IS_ERR(ecryptfs_inode))) { ./fs/ncpfs/dir.c: if (unlikely(IS_ERR(newdent))) ./fs/proc/proc_sysctl.c: if (unlikely(IS_ERR(subdir))) { Binary file ./.git/objects/pack/pack-4a5df920db8b8d9df9a91893c9567b4b2f15b782.pack matches ./drivers/target/tcm_fc/tfc_cmd.c: if (unlikely(IS_ERR(fp))) { ./drivers/thermal/intel_powerclamp.c: if (likely(!IS_ERR(thread))) { ./drivers/thermal/intel_powerclamp.c: if (likely(!IS_ERR(thread))) { ./drivers/gpu/drm/vmwgfx/vmwgfx_drv.c: if (unlikely(IS_ERR(vmaster))) { ./drivers/gpu/drm/vmwgfx/vmwgfx_context.c: if (unlikely(IS_ERR(uctx->man))) { ./drivers/gpu/drm/ttm/ttm_tt.c: if (unlikely(IS_ERR(swap_storage))) { ./drivers/gpu/drm/ttm/ttm_tt.c: if (unlikely(IS_ERR(to_page))) { ./drivers/scsi/bnx2fc/bnx2fc_fcoe.c: if (likely(!IS_ERR(thread))) { ./drivers/scsi/bnx2i/bnx2i_init.c: if (likely(!IS_ERR(thread))) { ./drivers/scsi/fcoe/fcoe.c: if (likely(!IS_ERR(thread))) { ./drivers/base/power/opp.c: if (unlikely(IS_ERR_OR_NULL(dev))) { ./drivers/base/power/opp.c: if (unlikely(IS_ERR_OR_NULL(tmp_opp)) || !tmp_opp->available) ./drivers/base/power/opp.c: if (unlikely(IS_ERR_OR_NULL(tmp_opp)) || !tmp_opp->available) ./drivers/tty/serial/serial_core.c: if (likely(!IS_ERR(tty_dev))) { ./drivers/rtc/rtc-bfin.c: if (unlikely(IS_ERR(rtc->rtc_dev))) ./drivers/rtc/rtc-gemini.c: if (likely(IS_ERR(rtc->rtc_dev))) ./drivers/rtc/interface.c: if (unlikely(IS_ERR_OR_NULL(rtc))) ./drivers/md/dm-snap-persistent.c: if (unlikely(IS_ERR(area))) { ./drivers/md/dm-verity.c: if (unlikely(IS_ERR(data))) ./drivers/md/persistent-data/dm-block-manager.c: if (unlikely(IS_ERR(p))) ./drivers/md/persistent-data/dm-block-manager.c: if (unlikely(IS_ERR(p))) ./drivers/md/persistent-data/dm-block-manager.c: if (unlikely(IS_ERR(p))) ./drivers/md/persistent-data/dm-block-manager.c: if (unlikely(IS_ERR(p))) ./drivers/staging/lustre/include/linux/libcfs/libcfs.h: if (unlikely(IS_ERR(ptr) || ptr == NULL)) ./drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c: if (likely(!IS_ERR(pfmr))) { ./drivers/staging/lustre/lustre/obdclass/lu_object.c: if (unlikely(IS_ERR(o))) ./drivers/staging/lustre/lustre/obdclass/lu_object.c: if (unlikely(IS_ERR(o))) ./drivers/staging/lustre/lustre/obdclass/lu_object.c: if (likely(IS_ERR(shadow) && PTR_ERR(shadow) == -ENOENT)) { ./drivers/staging/lustre/lustre/obdclass/lu_object.c: if (unlikely(IS_ERR(value))) ./drivers/staging/android/ashmem.c: if (unlikely(IS_ERR(vmfile))) { ./drivers/input/mouse/alps.c: } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) { ./drivers/net/ethernet/marvell/sky2.c: if (unlikely(status & Y2_IS_ERROR)) ./drivers/net/ethernet/ti/netcp_core.c: if (unlikely(IS_ERR_OR_NULL(desc))) { ./drivers/net/ethernet/ti/netcp_core.c: if (unlikely(IS_ERR_OR_NULL(ndesc))) { ./drivers/devfreq/devfreq.c: if (unlikely(IS_ERR_OR_NULL(dev))) { ./drivers/devfreq/devfreq.c: if (unlikely(IS_ERR_OR_NULL(name))) { ./drivers/misc/c2port/core.c: if (unlikely(IS_ERR(c2dev->dev))) { ./net/socket.c: if (unlikely(IS_ERR(file))) { ./net/socket.c: if (likely(!IS_ERR(newfile))) { ./net/socket.c: if (unlikely(IS_ERR(newfile1))) { ./net/socket.c: if (unlikely(IS_ERR(newfile))) { ./net/openvswitch/datapath.c: if (unlikely(IS_ERR(reply))) { ./net/openvswitch/datapath.c: if (likely(!IS_ERR(reply))) { ./net/sctp/socket.c: if (unlikely(IS_ERR(newfile))) { ./mm/huge_memory.c: if (unlikely(IS_ERR(khugepaged_thread))) {
Thanks
On 31-07-15, 17:32, yalin wang wrote:
On Jul 31, 2015, at 16:56, Kirill A. Shutemov kirill@shutemov.name wrote:
On Fri, Jul 31, 2015 at 02:08:34PM +0530, Viresh Kumar wrote:
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 viresh.kumar@linaro.org
Acked-by: Kirill A. Shutemov kirill.shutemov@linux.intel.com
search in code, there are lots of using like this , does need add this check into checkpatch ?
cc'd Joe for that. :)
# 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 :)
On Fri, 2015-07-31 at 15:04 +0530, Viresh Kumar wrote:
On 31-07-15, 17:32, yalin wang wrote:
On Jul 31, 2015, at 16:56, Kirill A. Shutemov kirill@shutemov.name wrote:
On Fri, Jul 31, 2015 at 02:08:34PM +0530, Viresh Kumar wrote:
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 viresh.kumar@linaro.org
Acked-by: Kirill A. Shutemov kirill.shutemov@linux.intel.com
search in code, there are lots of using like this , does need add this check into checkpatch ?
cc'd Joe for that. :)
# 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.
But given the unlikely was added when using gcc3.4, I wonder if it's still appropriate to use unlikely in IS_ERR at all.
---
commit b5acea523151452c37cd428437e7576a291dd146 Author: Andrew Morton akpm@osdl.org Date: Sun Aug 22 23:04:49 2004 -0700
[PATCH] mark IS_ERR as unlikely()
It seems fair to assume that it is always unlikely that IS_ERR will return true.
This patch changes the gcc-3.4-generated kernel text by ~500 bytes (less) so it's fair to assume that the compiler is indeed propagating unlikeliness out of inline functions.
On 31-07-15, 03:28, Joe Perches wrote:
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.
On Fri, 2015-07-31 at 16:36 +0530, Viresh Kumar wrote:
On 31-07-15, 03:28, Joe Perches wrote:
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.
On 31-07-15, 11:00, Joe Perches wrote:
On Fri, 2015-07-31 at 16:36 +0530, Viresh Kumar wrote:
On 31-07-15, 03:28, Joe Perches wrote:
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.
No idea :)
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 viresh.kumar@linaro.org --- net/openvswitch/datapath.c | 2 +- net/sctp/socket.c | 2 +- net/socket.c | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index ff8c4a4c1609..01d69680ba5d 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -1143,7 +1143,7 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info) info, OVS_FLOW_CMD_NEW, false, ufid_flags);
- if (unlikely(IS_ERR(reply))) { + if (IS_ERR(reply)) { error = PTR_ERR(reply); goto err_unlock_ovs; } diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 1425ec2bbd5a..c1569432235e 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -4481,7 +4481,7 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int len, char __user *optval }
newfile = sock_alloc_file(newsock, 0, NULL); - if (unlikely(IS_ERR(newfile))) { + if (IS_ERR(newfile)) { put_unused_fd(retval); sock_release(newsock); return PTR_ERR(newfile); diff --git a/net/socket.c b/net/socket.c index 9963a0b53a64..dd2c247c99e3 100644 --- a/net/socket.c +++ b/net/socket.c @@ -373,7 +373,7 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
file = alloc_file(&path, FMODE_READ | FMODE_WRITE, &socket_file_ops); - if (unlikely(IS_ERR(file))) { + if (IS_ERR(file)) { /* drop dentry, keep inode */ ihold(d_inode(path.dentry)); path_put(&path); @@ -1303,7 +1303,7 @@ SYSCALL_DEFINE4(socketpair, int, family, int, type, int, protocol, }
newfile1 = sock_alloc_file(sock1, flags, NULL); - if (unlikely(IS_ERR(newfile1))) { + if (IS_ERR(newfile1)) { err = PTR_ERR(newfile1); goto out_put_unused_both; } @@ -1467,7 +1467,7 @@ SYSCALL_DEFINE4(accept4, int, fd, struct sockaddr __user *, upeer_sockaddr, goto out_put; } newfile = sock_alloc_file(newsock, flags, sock->sk->sk_prot_creator->name); - if (unlikely(IS_ERR(newfile))) { + if (IS_ERR(newfile)) { err = PTR_ERR(newfile); put_unused_fd(newfd); sock_release(newsock);
On 07/31/2015 10:38 AM, Viresh Kumar wrote:
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.
does the "likely" somehow override the "unlikely" of IS_ERR, or is the resulting code a mess?
Vlastimil
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(-)
On 31-07-15, 11:41, Vlastimil Babka wrote:
[+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?
Good point. While fixing all the sites, I saw some code like that. Then before posting the series, I tried to look at what compilers do to such codes and they generated exactly same code for:
likely(unlikely(x)) and unlikely(x).
So, either those call sites should drop the likely bits or we supply them with another raw version of the macro :)
Or if my tests were wrong, then please lemme know.
On Fri, Jul 31, 2015 at 11:41:09AM +0200, Vlastimil Babka wrote:
On 07/31/2015 10:38 AM, Viresh Kumar wrote:
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.
There is no need to verify that its an error, as we are anyway going to match the error value to -ENOENT. Drop the redundant check.
Reported-by: "Kirill A. Shutemov" kirill@shutemov.name Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/staging/lustre/lustre/obdclass/lu_object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c index 6cd3af8c6237..8e472327c880 100644 --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c @@ -674,7 +674,7 @@ static struct lu_object *lu_object_find_try(const struct lu_env *env, cfs_hash_bd_lock(hs, &bd, 1);
shadow = htable_lookup(s, &bd, f, waiter, &version); - if (likely(IS_ERR(shadow) && PTR_ERR(shadow) == -ENOENT)) { + if (likely(PTR_ERR(shadow) == -ENOENT)) { struct lu_site_bkt_data *bkt;
bkt = cfs_hash_bd_extra_get(hs, &bd);
On 31-07-15, 13:23, Kirill A. Shutemov wrote:
On Fri, Jul 31, 2015 at 11:41:09AM +0200, Vlastimil Babka wrote:
On 07/31/2015 10:38 AM, Viresh Kumar wrote:
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)))
This one is already fixed in my series (sent a v2 to fix changelog as well)
drivers/staging/lustre/lustre/obdclass/lu_object.c: if (likely(IS_ERR(shadow) && PTR_ERR(shadow) == -ENOENT)) {
And I have sent a patch for this (in reply the your mail).
On Fri, 31 Jul 2015 13:23:10 +0300 "Kirill A. Shutemov" kirill@shutemov.name wrote:
On Fri, Jul 31, 2015 at 11:41:09AM +0200, Vlastimil Babka wrote:
On 07/31/2015 10:38 AM, Viresh Kumar wrote:
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.
Yep, those look like bugs to me.
-- Steve
On 03-08-15, 17:38, Steven Rostedt wrote:
On Fri, 31 Jul 2015 13:23:10 +0300 "Kirill A. Shutemov" kirill@shutemov.name wrote:
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.
Yep, those look like bugs to me.
Yeah, I have fixed both of them :)
linaro-kernel@lists.linaro.org
-
Alexandre Belloni -
Daniel Vetter -
David Howells -
David Laight -
Dmitry Torokhov -
Hans Ulli Kroll -
Joe Perches -
Kirill A. Shutemov -
Murali Karicheri -
Nicholas A. Bellinger -
Pali Rohár -
Pavel Machek -
Sinclair Yeh -
Steven Rostedt -
Tejun Heo -
Viresh Kumar -
Vlastimil Babka -
yalin wang