On Thu, Jan 07, 2010 at 10:05:24AM +1100, Dave Chinner wrote:
> Make the inode tree reclaim walk exclusive to avoid races with
> concurrent sync walkers and lookups. This is a version of
> a patch posted by Christoph Hellwig that avoids all the code
> duplication.
I don't like the write_lock flag very much, but given that the other
option is duplication we might have to live it.
> - /*
> - * If we can't get a reference on the inode, it must be in reclaim.
> - * Leave it for the reclaim code to flush. Also avoid inodes that
> - * haven't been fully initialised.
> - */
> + /* avoid new or reclaimable inodes. Leave for reclaim code to flush */
> + if (xfs_iflags_test(ip, XFS_INEW | XFS_IRECLAIMABLE | XFS_IRECLAIM)) {
> + read_unlock(&pag->pag_ici_lock);
> + return ENOENT;
> + }
> +
> + /* If we can't get a reference on the inode, it must be in reclaim. */
> if (!igrab(inode)) {
> read_unlock(&pag->pag_ici_lock);
> return ENOENT;
> }
> read_unlock(&pag->pag_ici_lock);
>
> - if (is_bad_inode(inode) || xfs_iflags_test(ip, XFS_INEW)) {
> + if (is_bad_inode(inode)) {
> IRELE(ip);
> return ENOENT;
That's an unrelated change and should be a separate patch.
> @@ -791,12 +779,22 @@ xfs_reclaim_inode_now(
> struct xfs_perag *pag,
> int flags)
> {
> + /*
> + * The radix tree lock here protects a thread in xfs_iget from racing
> + * with us starting reclaim on the inode. Once we have the
> + * XFS_IRECLAIM flag set it will not touch us.
> + */
> + spin_lock(&ip->i_flags_lock);
> + ASSERT_ALWAYS(__xfs_iflags_test(ip, XFS_IRECLAIMABLE));
> + if (__xfs_iflags_test(ip, XFS_IRECLAIM)) {
> + /* ignore as it is already under reclaim */
> + spin_unlock(&ip->i_flags_lock);
> + write_unlock(&pag->pag_ici_lock);
> return 0;
> }
> + __xfs_iflags_set(ip, XFS_IRECLAIM);
> + spin_unlock(&ip->i_flags_lock);
> + write_unlock(&pag->pag_ici_lock);
>
> return xfs_reclaim_inode(ip, flags);
Once you move things around please merge xfs_reclaim_inode_now and
xfs_reclaim_inode into a single function.
And yes, all this currently doesn't apply against the XFS tree or
mainline, but you know that already.
|