* [PATCH 1/1] XFS: Deletion of unnecessary checks before two function calls
[not found] ` <5317A59D.4@users.sourceforge.net>
@ 2014-11-29 16:45 ` SF Markus Elfring
2014-11-30 23:09 ` Dave Chinner
0 siblings, 1 reply; 6+ messages in thread
From: SF Markus Elfring @ 2014-11-29 16:45 UTC (permalink / raw)
To: Dave Chinner, xfs; +Cc: Julia Lawall, kernel-janitors, LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 29 Nov 2014 17:40:22 +0100
The functions xfs_blkdev_put() and xfs_qm_dqrele() test whether their argument
is NULL and then return immediately.
Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
fs/xfs/xfs_qm.c | 12 +++++-------
fs/xfs/xfs_super.c | 3 +--
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index d68f230..9a4b50a 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1749,23 +1749,21 @@ xfs_qm_vop_dqalloc(
xfs_iunlock(ip, lockflags);
if (O_udqpp)
*O_udqpp = uq;
- else if (uq)
+ else
xfs_qm_dqrele(uq);
if (O_gdqpp)
*O_gdqpp = gq;
- else if (gq)
+ else
xfs_qm_dqrele(gq);
if (O_pdqpp)
*O_pdqpp = pq;
- else if (pq)
+ else
xfs_qm_dqrele(pq);
return 0;
error_rele:
- if (gq)
- xfs_qm_dqrele(gq);
- if (uq)
- xfs_qm_dqrele(uq);
+ xfs_qm_dqrele(gq);
+ xfs_qm_dqrele(uq);
return error;
}
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 206b97f..97eaa25 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -796,8 +796,7 @@ xfs_open_devices(
out_free_ddev_targ:
xfs_free_buftarg(mp, mp->m_ddev_targp);
out_close_rtdev:
- if (rtdev)
- xfs_blkdev_put(rtdev);
+ xfs_blkdev_put(rtdev);
out_close_logdev:
if (logdev && logdev != ddev)
xfs_blkdev_put(logdev);
--
2.1.3
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] XFS: Deletion of unnecessary checks before two function calls
2014-11-29 16:45 ` [PATCH 1/1] XFS: Deletion of unnecessary checks before two function calls SF Markus Elfring
@ 2014-11-30 23:09 ` Dave Chinner
2015-06-26 9:15 ` [PATCH] XFS: Delete unnecessary checks before the function call "xfs_qm_dqrele" SF Markus Elfring
0 siblings, 1 reply; 6+ messages in thread
From: Dave Chinner @ 2014-11-30 23:09 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: Julia Lawall, kernel-janitors, LKML, xfs
On Sat, Nov 29, 2014 at 05:45:23PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 29 Nov 2014 17:40:22 +0100
>
> The functions xfs_blkdev_put() and xfs_qm_dqrele() test whether their argument
> is NULL and then return immediately.
> Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Looks fine.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] XFS: Delete unnecessary checks before the function call "xfs_qm_dqrele"
2014-11-30 23:09 ` Dave Chinner
@ 2015-06-26 9:15 ` SF Markus Elfring
2015-06-29 21:43 ` Dave Chinner
0 siblings, 1 reply; 6+ messages in thread
From: SF Markus Elfring @ 2015-06-26 9:15 UTC (permalink / raw)
To: Dave Chinner, xfs; +Cc: Julia Lawall, kernel-janitors, LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 26 Jun 2015 11:05:41 +0200
The xfs_qm_dqrele() function tests whether its argument is NULL and
then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
fs/xfs/xfs_qm_syscalls.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 3640c6e..6deee1e 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -735,15 +735,15 @@ xfs_dqrele_inode(
}
xfs_ilock(ip, XFS_ILOCK_EXCL);
- if ((flags & XFS_UQUOTA_ACCT) && ip->i_udquot) {
+ if (flags & XFS_UQUOTA_ACCT) {
xfs_qm_dqrele(ip->i_udquot);
ip->i_udquot = NULL;
}
- if ((flags & XFS_GQUOTA_ACCT) && ip->i_gdquot) {
+ if (flags & XFS_GQUOTA_ACCT) {
xfs_qm_dqrele(ip->i_gdquot);
ip->i_gdquot = NULL;
}
- if ((flags & XFS_PQUOTA_ACCT) && ip->i_pdquot) {
+ if (flags & XFS_PQUOTA_ACCT) {
xfs_qm_dqrele(ip->i_pdquot);
ip->i_pdquot = NULL;
}
--
2.4.4
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] XFS: Delete unnecessary checks before the function call "xfs_qm_dqrele"
2015-06-26 9:15 ` [PATCH] XFS: Delete unnecessary checks before the function call "xfs_qm_dqrele" SF Markus Elfring
@ 2015-06-29 21:43 ` Dave Chinner
2015-07-01 7:50 ` XFS: Fine-tuning for checks before the function call "xfs_qm_dqrele"? SF Markus Elfring
0 siblings, 1 reply; 6+ messages in thread
From: Dave Chinner @ 2015-06-29 21:43 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: Julia Lawall, kernel-janitors, LKML, xfs
On Fri, Jun 26, 2015 at 11:15:31AM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 26 Jun 2015 11:05:41 +0200
>
> The xfs_qm_dqrele() function tests whether its argument is NULL and
> then returns immediately.
True.
> Thus the test around the call is not needed.
But wrong.
xfs_dqrele_inode() gets called on every inode in the inode cache,
and this change results in a cacheline in every inode being dirtied
even if they don't have dquots attached. Given the inode cache can
hold tens to hundreds of millions of inodes on large machines, we
don't want to dirty any cachelines we don't need to while walking
the inode cache and releasing dquots...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: XFS: Fine-tuning for checks before the function call "xfs_qm_dqrele"?
2015-06-29 21:43 ` Dave Chinner
@ 2015-07-01 7:50 ` SF Markus Elfring
2015-07-02 0:19 ` Dave Chinner
0 siblings, 1 reply; 6+ messages in thread
From: SF Markus Elfring @ 2015-07-01 7:50 UTC (permalink / raw)
To: Dave Chinner; +Cc: Julia Lawall, kernel-janitors, LKML, xfs
> xfs_dqrele_inode() gets called on every inode in the inode cache,
> and this change results in a cacheline in every inode being dirtied
> even if they don't have dquots attached. Given the inode cache can
> hold tens to hundreds of millions of inodes on large machines, we
> don't want to dirty any cachelines we don't need to while walking
> the inode cache and releasing dquots...
Would it make sense to annotate checks before such function calls
as "LIKELY"?
Regards,
Markus
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: XFS: Fine-tuning for checks before the function call "xfs_qm_dqrele"?
2015-07-01 7:50 ` XFS: Fine-tuning for checks before the function call "xfs_qm_dqrele"? SF Markus Elfring
@ 2015-07-02 0:19 ` Dave Chinner
0 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2015-07-02 0:19 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: Julia Lawall, kernel-janitors, LKML, xfs
On Wed, Jul 01, 2015 at 09:50:00AM +0200, SF Markus Elfring wrote:
> > xfs_dqrele_inode() gets called on every inode in the inode cache,
> > and this change results in a cacheline in every inode being dirtied
> > even if they don't have dquots attached. Given the inode cache can
> > hold tens to hundreds of millions of inodes on large machines, we
> > don't want to dirty any cachelines we don't need to while walking
> > the inode cache and releasing dquots...
>
> Would it make sense to annotate checks before such function calls
> as "LIKELY"?
No - it will be random as to whether the inodes have dquots attached
or not and so a static hint is always going to be wrong for
someone....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-07-02 0:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5307CAA2.8060406@users.sourceforge.net>
[not found] ` <alpine.DEB.2.02.1402212321410.2043@localhost6.localdomain6>
[not found] ` <530A086E.8010901@users.sourceforge.net>
[not found] ` <alpine.DEB.2.02.1402231635510.1985@localhost6.localdomain6>
[not found] ` <530A72AA.3000601@users.sourceforge.net>
[not found] ` <alpine.DEB.2.02.1402240658210.2090@localhost6.localdomain6>
[not found] ` <530B5FB6.6010207@users.sourceforge.net>
[not found] ` <alpine.DEB.2.10.1402241710370.2074@hadrien>
[not found] ` <530C5E18.1020800@users.sourceforge.net>
[not found] ` <alpine.DEB.2.10.1402251014170.2080@hadrien>
[not found] ` <530CD2C4.4050903@users.sourceforge.net>
[not found] ` <alpine.DEB.2.10.1402251840450.7035@hadrien>
[not found] ` <530CF8FF.8080600@users.sourceforge.net>
[not found] ` <alpine.DEB.2.02.1402252117150.2047@localhost6.localdomain6>
[not found] ` <530DD06F.4090703@users.sourceforge.net>
[not found] ` <alpine.DEB.2.02.1402262129250.2221@localhost6.localdomain6>
[not found] ` <5317A59D.4@users.sourceforge.net>
2014-11-29 16:45 ` [PATCH 1/1] XFS: Deletion of unnecessary checks before two function calls SF Markus Elfring
2014-11-30 23:09 ` Dave Chinner
2015-06-26 9:15 ` [PATCH] XFS: Delete unnecessary checks before the function call "xfs_qm_dqrele" SF Markus Elfring
2015-06-29 21:43 ` Dave Chinner
2015-07-01 7:50 ` XFS: Fine-tuning for checks before the function call "xfs_qm_dqrele"? SF Markus Elfring
2015-07-02 0:19 ` Dave Chinner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox