Re: [PATCH 2/4] XFS: Use the inode tree for finding dirty inodes

[Christoph Hellwig <hch@infradead.org>]

On Sun, Jul 20, 2008 at 10:19:52PM +1000, Dave Chinner wrote:
> Update xfs_sync_inodes to walk the inode radix tree cache to find
> dirty inodes. This removes a huge bunch of nasty, messy code for
> traversing the mount inode list safely and removes another user of
> the mount inode list.

Looks good, some minor nits below:

>  	xfs_inode_t	*ip = NULL;
>  	bhv_vnode_t	*vp = NULL;

As you're touching most uses of vp what about just turning it
into a plain Linux inode?

> +	if (!pag->pag_ici_init)
> +		return 0;

I think it would be cleaner to move this into the caller and not even
call into this function for uninitialized AGs.

> +		read_lock(&pag->pag_ici_lock);
> +		nr_found = radix_tree_gang_lookup(&pag->pag_ici_root,
> +				(void**)&ip, first_index, 1);

This needs a big comment on why you're using the gang lookup for
a single lookup.  I guess that's because the gang lookup skips to
the next actually existing entry instead of returning NULL, but that's
not really obvious to the reader.

> +			VN_RELE(vp);

This should either be an iput or IRELE, VN_RELE is on it's way out.

Btw, all these also apply to the next patch, I won't comment on them
there again.