On Mon, Jul 22, 2013 at 12:00:07PM -0400, Dwight Engen wrote: > On Fri, 19 Jul 2013 16:02:21 +1000 > Dave Chinner <david@xxxxxxxxxxxxx> wrote: > > [...] > > > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c > > > index d873ab9e..728283a 100644 > > > --- a/fs/xfs/xfs_icache.c > > > +++ b/fs/xfs/xfs_icache.c > > > @@ -1247,6 +1247,10 @@ xfs_inode_free_eofblocks( > > > if (!xfs_inode_match_id(ip, eofb)) > > > return 0; > > > > > > + if (eofb->eof_flags & XFS_EOF_FLAGS_PERM_CHECK && > > > + inode_permission(VFS_I(ip), MAY_WRITE)) > > > + return 0; > > > > This assumes we are walking fully instantiated VFS inodes. That's > > not necessarily true - we may be walking inodes that have already > > been dropped from the VFS and are waiting for background reclaim to > > Hi Dave, in looking at this a bit I don't see how they can be dropped > from the VFS since they are igrab()ed in the flow: > > xfs_icache_free_eofblocks > xfs_inode_ag_iterator_tag > xfs_inode_ag_walk > xfs_inode_ag_walk_grab > igrab Ah, right, I forgot that we have two different methods of walking that tree, and the one that xfs_icache_free_eofblocks() avoids inodes in reclaim. > and I don't see a way for xfs_inode_free_eofblocks() to be called other > than the ag_walk flow. > > If there is a way to get into xfs_inode_free_eofblocks where we can't > use VFS_I(ip) then it will be a problem for the new code in > xfs_inode_match_id() as well. We can always use VFS_I(ip) because the struct inode is embedded in the struct xfs_inode, so that's never an issue. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
