Use IS_ERR_OR_NULL where appropriate.
Cc: Bob Peterson <[email protected]>
Cc: Andreas Gruenbacher <[email protected]>
Cc: [email protected]
Signed-off-by: Kefeng Wang <[email protected]>
---
fs/gfs2/dir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index db9a05244a35..3925efd3fd83 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -857,7 +857,7 @@ static struct gfs2_dirent *gfs2_dirent_search(struct inode *inode,
return ERR_PTR(error);
dent = gfs2_dirent_scan(inode, bh->b_data, bh->b_size, scan, name, NULL);
got_dent:
- if (unlikely(dent == NULL || IS_ERR(dent))) {
+ if (IS_ERR_OR_NULL(dent)) {
brelse(bh);
bh = NULL;
}
--
2.20.1
IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag,
so no need to do that again from its callers. Drop it.
Cc: Jens Axboe <[email protected]>
Cc: [email protected]
Signed-off-by: Kefeng Wang <[email protected]>
---
block/blk-cgroup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index b97b479e4f64..1f7127b03490 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -881,7 +881,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
blkg_free(new_blkg);
} else {
blkg = blkg_create(pos, q, new_blkg);
- if (unlikely(IS_ERR(blkg))) {
+ if (IS_ERR(blkg)) {
ret = PTR_ERR(blkg);
goto fail_unlock;
}
--
2.20.1
IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag,
so no need to do that again from its callers. Drop it.
Cc: "Pali Rohár" <[email protected]>
Cc: Dmitry Torokhov <[email protected]>
Cc: [email protected]
Signed-off-by: Kefeng Wang <[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 0a6f7ca883e7..791ef0f826c5 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1478,7 +1478,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
/* On V2 devices the DualPoint Stick reports bare packets */
dev = priv->dev2;
dev2 = psmouse->dev;
- } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) {
+ } else if (IS_ERR_OR_NULL(priv->dev3)) {
/* Register dev3 mouse if we received PS/2 packet first time */
if (!IS_ERR(priv->dev3))
psmouse_queue_work(psmouse, &priv->dev3_register_work,
--
2.20.1
On 6/5/19 8:24 AM, Kefeng Wang wrote:
> IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag,
> so no need to do that again from its callers. Drop it.
Applied, thanks.
--
Jens Axboe
On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote:
> IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag,
> so no need to do that again from its callers. Drop it.
Hi! I already reviewed this patch and rejected it, see:
https://patchwork.kernel.org/patch/10817475/
> Cc: "Pali Rohár" <[email protected]>
> Cc: Dmitry Torokhov <[email protected]>
> Cc: [email protected]
> Signed-off-by: Kefeng Wang <[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 0a6f7ca883e7..791ef0f826c5 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -1478,7 +1478,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
> /* On V2 devices the DualPoint Stick reports bare packets */
> dev = priv->dev2;
> dev2 = psmouse->dev;
> - } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) {
> + } else if (IS_ERR_OR_NULL(priv->dev3)) {
> /* Register dev3 mouse if we received PS/2 packet first time */
> if (!IS_ERR(priv->dev3))
> psmouse_queue_work(psmouse, &priv->dev3_register_work,
--
Pali Rohár
[email protected]
On 05.06.19 14:24, Kefeng Wang wrote:
> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> index b97b479e4f64..1f7127b03490 100644
> --- a/block/blk-cgroup.c
> +++ b/block/blk-cgroup.c
> @@ -881,7 +881,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
> blkg_free(new_blkg);
> } else {
> blkg = blkg_create(pos, q, new_blkg);
> - if (unlikely(IS_ERR(blkg))) {
> + if (IS_ERR(blkg)) {
> ret = PTR_ERR(blkg);
> goto fail_unlock;
> }
>
I think this cocci script should do such things automatically:
virtual patch
virtual context
virtual org
virtual report
@@
expression E;
@@
- unlikely(IS_ERR(E))
+ IS_ERR(E)
Do you already do it this way and have a cocci scrip on the way
to mainline ?
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On Wed, 2019-06-05 at 18:24 +0000, Enrico Weigelt, metux IT consult
wrote:
> On 05.06.19 14:24, Kefeng Wang wrote:
>
> > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> > index b97b479e4f64..1f7127b03490 100644
> > --- a/block/blk-cgroup.c
> > +++ b/block/blk-cgroup.c
> > @@ -881,7 +881,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
> > blkg_free(new_blkg);
> > } else {
> > blkg = blkg_create(pos, q, new_blkg);
> > - if (unlikely(IS_ERR(blkg))) {
> > + if (IS_ERR(blkg)) {
> > ret = PTR_ERR(blkg);
> > goto fail_unlock;
> > }
> >
>
> I think this cocci script should do such things automatically:
>
> virtual patch
> virtual context
> virtual org
> virtual report
>
> @@
> expression E;
> @@
> - unlikely(IS_ERR(E))
> + IS_ERR(E)
Likely change all of these too:
$ git grep -P '\blikely.*IS_ERR'
drivers/net/vxlan.c: if (likely(!IS_ERR(rt))) {
fs/ntfs/lcnalloc.c: if (likely(page && !IS_ERR(page))) {
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 (likely(!IS_ERR(rl2)))
fs/ntfs/namei.c: if (likely(!IS_ERR(dent_inode))) {
fs/ntfs/runlist.c: if (likely(!IS_ERR(old_rl)))
net/openvswitch/datapath.c: if (likely(!IS_ERR(reply))) {
net/socket.c: if (likely(!IS_ERR(newfile))) {
On 05.06.19 18:32, Joe Perches wrote:
> Likely change all of these too:
>
> $ git grep -P '\blikely.*IS_ERR'
> drivers/net/vxlan.c: if (likely(!IS_ERR(rt))) {
> fs/ntfs/lcnalloc.c: if (likely(page && !IS_ERR(page))) {
> 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 (likely(!IS_ERR(rl2)))
> fs/ntfs/namei.c: if (likely(!IS_ERR(dent_inode))) {
> fs/ntfs/runlist.c: if (likely(!IS_ERR(old_rl)))
> net/openvswitch/datapath.c: if (likely(!IS_ERR(reply))) {
> net/socket.c: if (likely(!IS_ERR(newfile))) {
Good point, thx.
Added that to my cocci script, which I've just sent to lkml.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On 2019/6/5 22:42, Pali Rohár wrote:
> On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote:
>> IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag,
>> so no need to do that again from its callers. Drop it.
> Hi! I already reviewed this patch and rejected it, see:
> https://patchwork.kernel.org/patch/10817475/
OK, please ignore it.
>> Cc: "Pali Rohár" <[email protected]>
>> Cc: Dmitry Torokhov <[email protected]>
>> Cc: [email protected]
>> Signed-off-by: Kefeng Wang <[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 0a6f7ca883e7..791ef0f826c5 100644
>> --- a/drivers/input/mouse/alps.c
>> +++ b/drivers/input/mouse/alps.c
>> @@ -1478,7 +1478,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
>> /* On V2 devices the DualPoint Stick reports bare packets */
>> dev = priv->dev2;
>> dev2 = psmouse->dev;
>> - } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) {
>> + } else if (IS_ERR_OR_NULL(priv->dev3)) {
>> /* Register dev3 mouse if we received PS/2 packet first time */
>> if (!IS_ERR(priv->dev3))
>> psmouse_queue_work(psmouse, &priv->dev3_register_work,
On Thu, 2019-06-06 at 09:08 +0800, Kefeng Wang wrote:
> On 2019/6/5 22:42, Pali Roh?r wrote:
> > On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote:
> > > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag,
> > > so no need to do that again from its callers. Drop it.
> > Hi! I already reviewed this patch and rejected it, see:
> > https://patchwork.kernel.org/patch/10817475/
> OK, please ignore it.
I think the stated reason of better readability isn't
particularly sensible as the object code produced is
actually slightly larger.
x86-64 defconfig (gcc 8.3.0)
$ size drivers/input/mouse/alps.o*
text data bss dec hex filename
29416 56 0 29472 7320 drivers/input/mouse/alps.o.new
29432 56 0 29488 7330 drivers/input/mouse/alps.o.old
Also if this unlikely is _really_ useful, perhaps the
!IS_ERR immediately after could also use likely as the
test seems only done for an OOM condition.
> > > Cc: "Pali Roh?r" <[email protected]>
> > > Cc: Dmitry Torokhov <[email protected]>
> > > Cc: [email protected]
> > > Signed-off-by: Kefeng Wang <[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 0a6f7ca883e7..791ef0f826c5 100644
> > > --- a/drivers/input/mouse/alps.c
> > > +++ b/drivers/input/mouse/alps.c
> > > @@ -1478,7 +1478,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
> > > /* On V2 devices the DualPoint Stick reports bare packets */
> > > dev = priv->dev2;
> > > dev2 = psmouse->dev;
> > > - } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) {
> > > + } else if (IS_ERR_OR_NULL(priv->dev3)) {
> > > /* Register dev3 mouse if we received PS/2 packet first time */
> > > if (!IS_ERR(priv->dev3))
> > > psmouse_queue_work(psmouse, &priv->dev3_register_work,
Kefeng,
On Wed, 5 Jun 2019 at 16:17, Kefeng Wang <[email protected]> wrote:
> Use IS_ERR_OR_NULL where appropriate.
It seems there are several more instances in which IS_ERR_OR_NULL should
be used (see below).
Thanks,
Andreas
---
fs/gfs2/dir.c | 2 +-
fs/gfs2/glock.c | 2 +-
fs/gfs2/inode.c | 2 +-
fs/gfs2/ops_fstype.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index 3925efd3fd83..761a37a3c656 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -753,7 +753,7 @@ static struct gfs2_dirent *gfs2_dirent_split_alloc(struct inode *inode,
struct gfs2_dirent *dent;
dent = gfs2_dirent_scan(inode, bh->b_data, bh->b_size,
gfs2_dirent_find_offset, name, ptr);
- if (!dent || IS_ERR(dent))
+ if (IS_ERR_OR_NULL(dent))
return dent;
return do_init_dirent(inode, dent, name, bh,
(unsigned)(ptr - (void *)dent));
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 15c605cfcfc8..f6281470a182 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -684,7 +684,7 @@ static void delete_work_func(struct work_struct *work)
goto out;
inode = gfs2_lookup_by_inum(sdp, no_addr, NULL, GFS2_BLKST_UNLINKED);
- if (inode && !IS_ERR(inode)) {
+ if (!IS_ERR_OR_NULL(inode)) {
d_prune_aliases(inode);
iput(inode);
}
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 998051c4aea7..1cc99da705fc 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -796,7 +796,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
fail_gunlock:
gfs2_dir_no_add(&da);
gfs2_glock_dq_uninit(ghs);
- if (inode && !IS_ERR(inode)) {
+ if (!IS_ERR_OR_NULL(inode)) {
clear_nlink(inode);
if (!free_vfs_inode)
mark_inode_dirty(inode);
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index f5312f3b58ee..d35772d90b0a 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -579,7 +579,7 @@ static int gfs2_jindex_hold(struct gfs2_sbd *sdp, struct gfs2_holder *ji_gh)
INIT_WORK(&jd->jd_work, gfs2_recover_func);
jd->jd_inode = gfs2_lookupi(sdp->sd_jindex, &name, 1);
- if (!jd->jd_inode || IS_ERR(jd->jd_inode)) {
+ if (IS_ERR_OR_NULL(jd->jd_inode)) {
if (!jd->jd_inode)
error = -ENOENT;
else
--
2.20.1
Hi Joe,
On Wed, Jun 05, 2019 at 07:28:53PM -0700, Joe Perches wrote:
> On Thu, 2019-06-06 at 09:08 +0800, Kefeng Wang wrote:
> > On 2019/6/5 22:42, Pali Roh?r wrote:
> > > On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote:
> > > > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag,
> > > > so no need to do that again from its callers. Drop it.
> > > Hi! I already reviewed this patch and rejected it, see:
> > > https://patchwork.kernel.org/patch/10817475/
> > OK, please ignore it.
>
> I think the stated reason of better readability isn't
> particularly sensible as the object code produced is
> actually slightly larger.
>
> x86-64 defconfig (gcc 8.3.0)
>
> $ size drivers/input/mouse/alps.o*
> text data bss dec hex filename
> 29416 56 0 29472 7320 drivers/input/mouse/alps.o.new
> 29432 56 0 29488 7330 drivers/input/mouse/alps.o.old
If gcc produces worse code for double unlikely, you should probably
report it to gcc folks, no? Or double unlikely turns into likely?
>
> Also if this unlikely is _really_ useful, perhaps the
> !IS_ERR immediately after could also use likely as the
> test seems only done for an OOM condition.
No, once you take the IS_ERR_OR_NULL(priv->dev3) == true branch it stops
being hot path and additional annotations are completely unneeded.
And if we failed to create and register priv->dev3 device - that's an
error and system is degraded. Can't do much here.
Thanks.
--
Dmitry
On 2019/6/12 0:23, Andreas Gruenbacher wrote:
> Kefeng,
>
> On Wed, 5 Jun 2019 at 16:17, Kefeng Wang <[email protected]> wrote:
>> Use IS_ERR_OR_NULL where appropriate.
>
> It seems there are several more instances in which IS_ERR_OR_NULL should
> be used (see below).
>
Right, will collect the following changes and send a new one, thanks.
> Thanks,
> Andreas
>
> ---
> fs/gfs2/dir.c | 2 +-
> fs/gfs2/glock.c | 2 +-
> fs/gfs2/inode.c | 2 +-
> fs/gfs2/ops_fstype.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
> index 3925efd3fd83..761a37a3c656 100644
> --- a/fs/gfs2/dir.c
> +++ b/fs/gfs2/dir.c
> @@ -753,7 +753,7 @@ static struct gfs2_dirent *gfs2_dirent_split_alloc(struct inode *inode,
> struct gfs2_dirent *dent;
> dent = gfs2_dirent_scan(inode, bh->b_data, bh->b_size,
> gfs2_dirent_find_offset, name, ptr);
> - if (!dent || IS_ERR(dent))
> + if (IS_ERR_OR_NULL(dent))
> return dent;
> return do_init_dirent(inode, dent, name, bh,
> (unsigned)(ptr - (void *)dent));
> diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> index 15c605cfcfc8..f6281470a182 100644
> --- a/fs/gfs2/glock.c
> +++ b/fs/gfs2/glock.c
> @@ -684,7 +684,7 @@ static void delete_work_func(struct work_struct *work)
> goto out;
>
> inode = gfs2_lookup_by_inum(sdp, no_addr, NULL, GFS2_BLKST_UNLINKED);
> - if (inode && !IS_ERR(inode)) {
> + if (!IS_ERR_OR_NULL(inode)) {
> d_prune_aliases(inode);
> iput(inode);
> }
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index 998051c4aea7..1cc99da705fc 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -796,7 +796,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
> fail_gunlock:
> gfs2_dir_no_add(&da);
> gfs2_glock_dq_uninit(ghs);
> - if (inode && !IS_ERR(inode)) {
> + if (!IS_ERR_OR_NULL(inode)) {
> clear_nlink(inode);
> if (!free_vfs_inode)
> mark_inode_dirty(inode);
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index f5312f3b58ee..d35772d90b0a 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -579,7 +579,7 @@ static int gfs2_jindex_hold(struct gfs2_sbd *sdp, struct gfs2_holder *ji_gh)
>
> INIT_WORK(&jd->jd_work, gfs2_recover_func);
> jd->jd_inode = gfs2_lookupi(sdp->sd_jindex, &name, 1);
> - if (!jd->jd_inode || IS_ERR(jd->jd_inode)) {
> + if (IS_ERR_OR_NULL(jd->jd_inode)) {
> if (!jd->jd_inode)
> error = -ENOENT;
> else
>
Use IS_ERR_OR_NULL where appropriate.
Signed-off-by: Kefeng Wang <[email protected]>
Signed-off-by: Andreas Gruenbacher <[email protected]>
---
fs/gfs2/dir.c | 4 ++--
fs/gfs2/glock.c | 2 +-
fs/gfs2/inode.c | 2 +-
fs/gfs2/ops_fstype.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index 88e4f955c518..6f35d19eec25 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -750,7 +750,7 @@ static struct gfs2_dirent *gfs2_dirent_split_alloc(struct inode *inode,
struct gfs2_dirent *dent;
dent = gfs2_dirent_scan(inode, bh->b_data, bh->b_size,
gfs2_dirent_find_offset, name, ptr);
- if (!dent || IS_ERR(dent))
+ if (IS_ERR_OR_NULL(dent))
return dent;
return do_init_dirent(inode, dent, name, bh,
(unsigned)(ptr - (void *)dent));
@@ -854,7 +854,7 @@ static struct gfs2_dirent *gfs2_dirent_search(struct inode *inode,
return ERR_PTR(error);
dent = gfs2_dirent_scan(inode, bh->b_data, bh->b_size, scan, name, NULL);
got_dent:
- if (unlikely(dent == NULL || IS_ERR(dent))) {
+ if (IS_ERR_OR_NULL(dent)) {
brelse(bh);
bh = NULL;
}
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index f1ebcb42cbf5..44718098cc60 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -681,7 +681,7 @@ static void delete_work_func(struct work_struct *work)
goto out;
inode = gfs2_lookup_by_inum(sdp, no_addr, NULL, GFS2_BLKST_UNLINKED);
- if (inode && !IS_ERR(inode)) {
+ if (!IS_ERR_OR_NULL(inode)) {
d_prune_aliases(inode);
iput(inode);
}
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index b296c59832a7..2e2a8a2fb51d 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -793,7 +793,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
fail_gunlock:
gfs2_dir_no_add(&da);
gfs2_glock_dq_uninit(ghs);
- if (inode && !IS_ERR(inode)) {
+ if (!IS_ERR_OR_NULL(inode)) {
clear_nlink(inode);
if (!free_vfs_inode)
mark_inode_dirty(inode);
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 08823bb3b2d0..9683d534e1a1 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -568,7 +568,7 @@ static int gfs2_jindex_hold(struct gfs2_sbd *sdp, struct gfs2_holder *ji_gh)
INIT_WORK(&jd->jd_work, gfs2_recover_func);
jd->jd_inode = gfs2_lookupi(sdp->sd_jindex, &name, 1);
- if (!jd->jd_inode || IS_ERR(jd->jd_inode)) {
+ if (IS_ERR_OR_NULL(jd->jd_inode)) {
if (!jd->jd_inode)
error = -ENOENT;
else
--
2.20.1
On Tuesday 11 June 2019 17:59:13 Dmitry Torokhov wrote:
> Hi Joe,
>
> On Wed, Jun 05, 2019 at 07:28:53PM -0700, Joe Perches wrote:
> > On Thu, 2019-06-06 at 09:08 +0800, Kefeng Wang wrote:
> > > On 2019/6/5 22:42, Pali Rohár wrote:
> > > > On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote:
> > > > > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag,
> > > > > so no need to do that again from its callers. Drop it.
> > > > Hi! I already reviewed this patch and rejected it, see:
> > > > https://patchwork.kernel.org/patch/10817475/
> > > OK, please ignore it.
> >
> > I think the stated reason of better readability isn't
> > particularly sensible as the object code produced is
> > actually slightly larger.
> >
> > x86-64 defconfig (gcc 8.3.0)
> >
> > $ size drivers/input/mouse/alps.o*
> > text data bss dec hex filename
> > 29416 56 0 29472 7320 drivers/input/mouse/alps.o.new
> > 29432 56 0 29488 7330 drivers/input/mouse/alps.o.old
>
> If gcc produces worse code for double unlikely, you should probably
> report it to gcc folks, no? Or double unlikely turns into likely?
Is measured size of stripped or unstripped binary? Plus with or without
debug symbols? Double unlikely version should have more debug symbols
and therefore also larger size.
But if unstripped version with double unlikely is larger then it is for
sure compiler bug.
--
Pali Rohár
[email protected]
On Wed, 2019-06-12 at 09:14 +0200, Pali Roh?r wrote:
> On Tuesday 11 June 2019 17:59:13 Dmitry Torokhov wrote:
> > Hi Joe,
> >
> > On Wed, Jun 05, 2019 at 07:28:53PM -0700, Joe Perches wrote:
> > > On Thu, 2019-06-06 at 09:08 +0800, Kefeng Wang wrote:
> > > > On 2019/6/5 22:42, Pali Roh?r wrote:
> > > > > On Wednesday 05 June 2019 22:24:28 Kefeng Wang wrote:
> > > > > > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag,
> > > > > > so no need to do that again from its callers. Drop it.
> > > > > Hi! I already reviewed this patch and rejected it, see:
> > > > > https://patchwork.kernel.org/patch/10817475/
> > > > OK, please ignore it.
> > >
> > > I think the stated reason of better readability isn't
> > > particularly sensible as the object code produced is
> > > actually slightly larger.
> > >
> > > x86-64 defconfig (gcc 8.3.0)
> > >
> > > $ size drivers/input/mouse/alps.o*
> > > text data bss dec hex filename
> > > 29416 56 0 29472 7320 drivers/input/mouse/alps.o.new
> > > 29432 56 0 29488 7330 drivers/input/mouse/alps.o.old
> >
> > If gcc produces worse code for double unlikely, you should probably
> > report it to gcc folks, no? Or double unlikely turns into likely?
>
> Is measured size of stripped or unstripped binary? Plus with or without
> debug symbols? Double unlikely version should have more debug symbols
> and therefore also larger size.
>
> But if unstripped version with double unlikely is larger then it is for
> sure compiler bug.
defconfig so no debug symbols.
It's not necessarily a gcc bug as gcc doesn't
guarantee compiler repeatability.