* [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