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(-)
--
2.4.0
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 <[email protected]>
---
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);
}
/**
--
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.
Signed-off-by: Viresh Kumar <[email protected]>
---
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;
--
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.
Signed-off-by: Viresh Kumar <[email protected]>
---
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);
}
--
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.
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(-)
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
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(-)
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
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/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));
--
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.
Signed-off-by: Viresh Kumar <[email protected]>
---
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;
}
--
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.
Signed-off-by: Viresh Kumar <[email protected]>
---
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;
--
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.
Signed-off-by: Viresh Kumar <[email protected]>
---
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
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 <[email protected]>
---
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))
--
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.
Signed-off-by: Viresh Kumar <[email protected]>
---
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;
--
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.
Signed-off-by: Viresh Kumar <[email protected]>
---
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",
--
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.
Signed-off-by: Viresh Kumar <[email protected]>
---
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);
--
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.
Signed-off-by: Viresh Kumar <[email protected]>
---
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;
--
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.
Signed-off-by: Viresh Kumar <[email protected]>
---
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);
--
2.4.0
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 <[email protected]>
Acked-by: Kirill A. Shutemov <[email protected]>
--
Kirill A. Shutemov
Viresh Kumar <[email protected]> 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 <[email protected]>
Reviewed-by: David Howells <[email protected]>
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(-)
--- a/drivers/rtc/rtc-gemini.c
+++ b/drivers/rtc/rtc-gemini.c
@@ -148,10 +148,7 @@ static int gemini_rtc_probe(struct platf
rtc->rtc_dev = rtc_device_register(pdev->name, dev,
&gemini_rtc_ops, THIS_MODULE);
- if (IS_ERR(rtc->rtc_dev))
- return PTR_ERR(rtc->rtc_dev);
-
- return 0;
+ return PTR_ERR_OR_ZERO(rtc->rtc_dev);
}
static int gemini_rtc_remove(struct platform_device *pdev)
> On Jul 31, 2015, at 16:56, Kirill A. Shutemov <[email protected]> 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 <[email protected]>
>
> Acked-by: Kirill A. Shutemov <[email protected]>
>
> --
> 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/
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 <[email protected]> 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 <[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. :)
> # 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 :)
--
viresh
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.
--
viresh
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.
--
Kirill A. Shutemov
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 <[email protected]> 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 <[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. :)
>
> > # 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 <[email protected]>
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.
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(-)
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
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" <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
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);
--
2.4.0
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).
--
viresh
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.
--
viresh
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 <[email protected]>
Acked-by: Tejun Heo <[email protected]>
Thanks.
--
tejun
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 <[email protected]>
> ---
> 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;
>
--
Murali Karicheri
Linux Kernel, Keystone
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
This patch: Reviewed-by: Sinclair Yeh <[email protected]>
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 <[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(-)
>
> 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 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
--
viresh
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 <[email protected]>
> ---
> 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.
--
Dmitry
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 :)
--
viresh
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.
--
viresh
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?
--
Pali Rohár
[email protected]
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
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
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 ;-)
--
Pali Rohár
[email protected]
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 <[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(-)
>
> 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 <[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]>
@ from include/linux/err.h
#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
...
static inline bool __must_check IS_ERR_OR_NULL(__force const void *ptr)
{
return !ptr || IS_ERR_VALUE((unsigned long)ptr);
}
"!ptr" appears not covered with IS_ERR_OR_NULL.
(only the IS_ERR part seems covered)
Cheers,
MyungJoo
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
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 <[email protected]>
> ---
> 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!
On 03-08-15, 05:10, MyungJoo Ham 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 <[email protected]>
>
> @ from include/linux/err.h
> #define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
> ...
> static inline bool __must_check IS_ERR_OR_NULL(__force const void *ptr)
> {
> return !ptr || IS_ERR_VALUE((unsigned long)ptr);
> }
>
> "!ptr" appears not covered with IS_ERR_OR_NULL.
> (only the IS_ERR part seems covered)
Right, the first patch of the series has fixed that.
http://permalink.gmane.org/gmane.linux.kernel/2009151
--
viresh
> On 03-08-15, 05:10, MyungJoo Ham 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 <[email protected]>
> >
> > @ from include/linux/err.h
> > #define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
> > ...
> > static inline bool __must_check IS_ERR_OR_NULL(__force const void *ptr)
> > {
> > return !ptr || IS_ERR_VALUE((unsigned long)ptr);
> > }
> >
> > "!ptr" appears not covered with IS_ERR_OR_NULL.
> > (only the IS_ERR part seems covered)
>
> Right, the first patch of the series has fixed that.
>
> http://permalink.gmane.org/gmane.linux.kernel/2009151
Ah.. ok, then,
Acked-by: MyungJoo Ham <[email protected]>
please let me know when the series is ready to go.
Cheers,
MyungJoo
>
> --
> viresh
>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On 03-08-15, 05:50, MyungJoo Ham wrote:
> Acked-by: MyungJoo Ham <[email protected]>
>
>
> please let me know when the series is ready to go.
It wouldn't harm even if you apply it without 1/15. So, just go on and
apply :)
--
viresh
On Fri, Jul 31, 2015 at 08:53:04AM -0700, Sinclair Yeh wrote:
> This patch: Reviewed-by: Sinclair Yeh <[email protected]>
>
> 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 <[email protected]>
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
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
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:
>
> http://permalink.gmane.org/gmane.linux.kernel/2009151
>
Assuming the above change is merged, this patch looks good.
Acked-by: Murali Karicheri <[email protected]>
--
Murali Karicheri
Linux Kernel, Keystone
On Fri, 31 Jul 2015 13:23:10 +0300
"Kirill A. Shutemov" <[email protected]> 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" <[email protected]> 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 :)
--
viresh
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 <[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(-)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On 31/07/2015 at 15:01:04 +0530, Viresh Kumar wrote :
> 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(-)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com