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.
|