xfs
[Top] [All Lists]

Re: [PATCH 1/5] xfs: avoid taking the ilock unnessecarily in xfs_qm_dqat

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/5] xfs: avoid taking the ilock unnessecarily in xfs_qm_dqattach
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 27 Mar 2012 09:13:30 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20120326211603.074328642@xxxxxxxxxxxxxxxxxxxxxx>
References: <20120326211421.518374058@xxxxxxxxxxxxxxxxxxxxxx> <20120326211603.074328642@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Mar 26, 2012 at 05:14:22PM -0400, Christoph Hellwig wrote:
> Check if we actually need to attach a dquot before taking the ilock in
> xfs_qm_dqattach.  This avoid superflous lock roundtrips for the common cases
> of quota support compiled in but not activated on a filesystem and an
> inode that already has the dquots attached.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Looks good. One minor thing:

> +static inline bool
> +xfs_qm_need_dqattach(
> +     struct xfs_inode        *ip)
> +{
> +     struct xfs_mount        *mp = ip->i_mount;
> +
> +     if (!XFS_IS_QUOTA_RUNNING(mp))
> +             return false;
> +     if (!XFS_IS_QUOTA_ON(mp))
> +             return false;
> +     if (!XFS_NOT_DQATTACHED(mp, ip))
> +             return false;
> +     if (ip->i_ino == mp->m_sb.sb_uquotino ||
> +         ip->i_ino == mp->m_sb.sb_gquotino)
> +             return false;
> +     return true;
> +}

That's probably a little large for an inline function. Let the
compiler decide whether to inline it or not. In most cases, it will
inline it because it is a static function....

Otherwise,

Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>