Re: [PATCH 6/7] xfs: use generic inode iterator in xfs_qm_dqrele_all_inodes

[Eric Sandeen <sandeen@sandeen.net>]

Christoph Hellwig wrote:
> On Fri, Jun 05, 2009 at 02:15:38PM -0500, Eric Sandeen wrote:
>> Does this need the same error propagation treatment as 5/7 ?
> 
> Yes, I've already fixed this up in my local copy:
> 
> Subject: xfs: use generic inode iterator in xfs_qm_dqrele_all_inodes
> From: Christoph Hellwig <hch@lst.de>
> 
> Use xfs_inode_ag_iterator instead of opencoding the inode walk in the
> quota code.  Mark xfs_inode_ag_iterator and xfs_sync_inode_valid non-static
> to allow using them from the quota code.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>

Ok, this looks fine to me.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> 
> Index: xfs/fs/xfs/quota/xfs_qm_syscalls.c
> ===================================================================
> --- xfs.orig/fs/xfs/quota/xfs_qm_syscalls.c	2009-06-04 12:47:30.742821242 +0200
> +++ xfs/fs/xfs/quota/xfs_qm_syscalls.c	2009-06-04 13:03:51.039832673 +0200
> @@ -847,105 +847,55 @@ xfs_qm_export_flags(
>  }
>  
>  
> -/*
> - * Release all the dquots on the inodes in an AG.
> - */
> -STATIC void
> -xfs_qm_dqrele_inodes_ag(
> -	xfs_mount_t	*mp,
> -	int		ag,
> -	uint		flags)
> +STATIC int
> +xfs_dqrele_inode(
> +	struct xfs_inode	*ip,
> +	struct xfs_perag	*pag,
> +	int			flags)
>  {
> -	xfs_inode_t	*ip = NULL;
> -	xfs_perag_t	*pag = &mp->m_perag[ag];
> -	int		first_index = 0;
> -	int		nr_found;
> -
> -	do {
> -		/*
> -		 * use a gang lookup to find the next inode in the tree
> -		 * as the tree is sparse and a gang lookup walks to find
> -		 * the number of objects requested.
> -		 */
> -		read_lock(&pag->pag_ici_lock);
> -		nr_found = radix_tree_gang_lookup(&pag->pag_ici_root,
> -				(void**)&ip, first_index, 1);
> -
> -		if (!nr_found) {
> -			read_unlock(&pag->pag_ici_lock);
> -			break;
> -		}
> -
> -		/*
> -		 * Update the index for the next lookup. Catch overflows
> -		 * into the next AG range which can occur if we have inodes
> -		 * in the last block of the AG and we are currently
> -		 * pointing to the last inode.
> -		 */
> -		first_index = XFS_INO_TO_AGINO(mp, ip->i_ino + 1);
> -		if (first_index < XFS_INO_TO_AGINO(mp, ip->i_ino)) {
> -			read_unlock(&pag->pag_ici_lock);
> -			break;
> -		}
> -
> -		/* skip quota inodes */
> -		if (ip == XFS_QI_UQIP(mp) || ip == XFS_QI_GQIP(mp)) {
> -			ASSERT(ip->i_udquot == NULL);
> -			ASSERT(ip->i_gdquot == NULL);
> -			read_unlock(&pag->pag_ici_lock);
> -			continue;
> -		}
> +	int			error;
>  
> -		/*
> -		 * If we can't get a reference on the inode, it must be
> -		 * in reclaim. Leave it for the reclaim code to flush.
> -		 */
> -		if (!igrab(VFS_I(ip))) {
> -			read_unlock(&pag->pag_ici_lock);
> -			continue;
> -		}
> +	/* skip quota inodes */
> +	if (ip == XFS_QI_UQIP(ip->i_mount) || ip == XFS_QI_GQIP(ip->i_mount)) {
> +		ASSERT(ip->i_udquot == NULL);
> +		ASSERT(ip->i_gdquot == NULL);
>  		read_unlock(&pag->pag_ici_lock);
> +		return 0;
> +	}
>  
> -		/* avoid new inodes though we shouldn't find any here */
> -		if (xfs_iflags_test(ip, XFS_INEW)) {
> -			IRELE(ip);
> -			continue;
> -		}
> +	error = xfs_sync_inode_valid(ip, pag);
> +	if (error)
> +		return error;
>  
> -		xfs_ilock(ip, XFS_ILOCK_EXCL);
> -		if ((flags & XFS_UQUOTA_ACCT) && ip->i_udquot) {
> -			xfs_qm_dqrele(ip->i_udquot);
> -			ip->i_udquot = NULL;
> -		}
> -		if (flags & (XFS_PQUOTA_ACCT|XFS_GQUOTA_ACCT) &&
> -		    ip->i_gdquot) {
> -			xfs_qm_dqrele(ip->i_gdquot);
> -			ip->i_gdquot = NULL;
> -		}
> -		xfs_iput(ip, XFS_ILOCK_EXCL);
> +	xfs_ilock(ip, XFS_ILOCK_EXCL);
> +	if ((flags & XFS_UQUOTA_ACCT) && ip->i_udquot) {
> +		xfs_qm_dqrele(ip->i_udquot);
> +		ip->i_udquot = NULL;
> +	}
> +	if (flags & (XFS_PQUOTA_ACCT|XFS_GQUOTA_ACCT) && ip->i_gdquot) {
> +		xfs_qm_dqrele(ip->i_gdquot);
> +		ip->i_gdquot = NULL;
> +	}
> +	xfs_iput(ip, XFS_ILOCK_EXCL);
> +	IRELE(ip);
>  
> -	} while (nr_found);
> +	return 0;
>  }
>  
> +
>  /*
>   * Go thru all the inodes in the file system, releasing their dquots.
> + *
>   * Note that the mount structure gets modified to indicate that quotas are off
> - * AFTER this, in the case of quotaoff. This also gets called from
> - * xfs_rootumount.
> + * AFTER this, in the case of quotaoff.
>   */
>  void
>  xfs_qm_dqrele_all_inodes(
>  	struct xfs_mount *mp,
>  	uint		 flags)
>  {
> -	int		i;
> -
>  	ASSERT(mp->m_quotainfo);
> -	for (i = 0; i < mp->m_sb.sb_agcount; i++) {
> -		if (!mp->m_perag[i].pag_ici_init)
> -			continue;
> -		xfs_qm_dqrele_inodes_ag(mp, i, flags);
> -	}
> +	xfs_inode_ag_iterator(mp, xfs_dqrele_inode, flags, -1);
>  }
>  
>  /*------------------------------------------------------------------------*/
> Index: xfs/fs/xfs/linux-2.6/xfs_sync.c
> ===================================================================
> --- xfs.orig/fs/xfs/linux-2.6/xfs_sync.c	2009-06-04 13:01:29.388941822 +0200
> +++ xfs/fs/xfs/linux-2.6/xfs_sync.c	2009-06-04 13:02:26.249965001 +0200
> @@ -141,7 +141,7 @@ restart:
>  	return last_error;
>  }
>  
> -STATIC int
> +int
>  xfs_inode_ag_iterator(
>  	struct xfs_mount	*mp,
>  	int			(*execute)(struct xfs_inode *ip,
> @@ -167,7 +167,7 @@ xfs_inode_ag_iterator(
>  }
>  
>  /* must be called with pag_ici_lock held and releases it */
> -STATIC int
> +int
>  xfs_sync_inode_valid(
>  	struct xfs_inode	*ip,
>  	struct xfs_perag	*pag)
> Index: xfs/fs/xfs/linux-2.6/xfs_sync.h
> ===================================================================
> --- xfs.orig/fs/xfs/linux-2.6/xfs_sync.h	2009-06-04 12:47:30.804939977 +0200
> +++ xfs/fs/xfs/linux-2.6/xfs_sync.h	2009-06-04 13:02:26.249965001 +0200
> @@ -54,4 +54,10 @@ void xfs_inode_set_reclaim_tag(struct xf
>  void xfs_inode_clear_reclaim_tag(struct xfs_inode *ip);
>  void __xfs_inode_clear_reclaim_tag(struct xfs_mount *mp, struct xfs_perag *pag,
>  				struct xfs_inode *ip);
> +
> +int xfs_sync_inode_valid(struct xfs_inode *ip, struct xfs_perag *pag);
> +int xfs_inode_ag_iterator(struct xfs_mount *mp,
> +	int (*execute)(struct xfs_inode *ip, struct xfs_perag *pag, int flags),
> +	int flags, int tag);
> +
>  #endif
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs