Re: [PATCH 2/3] xfs: convert inode cache lookups to use RCU locking

[Nick Piggin <npiggin@kernel.dk>]

On Wed, Dec 15, 2010 at 05:35:34PM +1100, Dave Chinner wrote:
> On Wed, Dec 15, 2010 at 03:30:43AM +0000, Nick Piggin wrote:
> > In this case, if you can observe something that has happened after the
> > inode is removed from the tree (ie. i_ino has changed), then you should
> > not find it in the tree after a subsequent lookup (no synchronize_rcu
> > required, just appropriate locking or barriers).
> 
> Ok, that's what I thought was supposed to be the case. Thanks
> for confirming that, Nick.
> 
> > BTW. I wondered if you can also do the radix_tree tag lookup for reclaim
> > under RCU?
> 
> It's currently under the ->pag_ici_lock using a
> radix_tree_gang_lookup_tag, though I think this was a mismerge bug
> from an earlier version.
> 
> I intended it to be under RCU as the "inode OK for reclaim"
> validation checks won't touch an inode that already has XFS_IRECLAIM
> already set (i.e. already under reclaim or freed), so the
> reliability of tag lookups is not a big deal.

That would be nice, a while back I was seeing lock contention in
reclaim there, and tried some hacks to improve it, but this should
solve it properly.

The tag lookups should be reliable too -- we use them in pagecache
dirty writeout. Basically same causality rules: if we observe (given
the required barriers or locks) an event we know to have happened after
the tag was set and before it is cleared, then the tag lookup should
find the item.


> The lookup probably needs to check if XFS_IRECLAIMABLE is set
> (rather than asserting it is set) to avoid so as to only reclaim
> inodes that are really in the reclaimable state. Note that
> ->i_flags_lock controls all the state changes, so it should provide
> the necessary item memory barriers to ensure that only reclaimable
> inodes are found for reclaim.

If you are checking and clearing those flags under lock, then I
think everything will be fine.

lock()
set deleted flag
unlock()
delete from radix tree

versus
look up from radix tree
lock()
check deleted flag...

Then at this point we would know that if the deleted flag is not set,
then the item can not have been deleted from the radix tree by the
1st sequence.

The stores that delete the item from the radix tree may be visible
before the deleted flag is visible (due to release-barrier ordering),
but that's not a problem here.

Thanks,
Nick

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