On Wed, 2007-11-14 at 22:31 +0100, Peter Zijlstra wrote: > On Wed, 2007-11-14 at 22:22 +0100, Nick Piggin wrote: > > On Wed, Nov 14, 2007 at 09:01:39PM +0100, Peter Zijlstra wrote: > > > Normal locking order is: > > > > > > i_mutex > > > mmap_sem > > > > > > However NFS's ->mmap hook, which is called under mmap_sem, can take > > > i_mutex. > > > Avoid this potential deadlock by doing the work that requires i_mutex from > > > the new ->mmap_prepare(). > > > > > > [ Is this sufficient, or does it introduce a race? ] > > > > Seems like an OK patchset in my opinion. I don't know much about NFS > > unfortunately, but I wonder what prevents the condition fixed by > > nfs_revalidate_mapping from happening again while the mmap is active...? > > As the changelog might have suggested, I'm not overly sure of the nfs > requirements myself. I think it just does a best effort at getting the > pages coherent with other clients, and then hopes for the best. > > I'll let Trond enlighten us further before I make an utter fool of > myself :-)
The NFS client needs to check the validity of already cached data before it allows those pages to be mmapped. If it finds out that the cache is stale, then we need to call invalidate_inode_pages2() to clear out the cache and refresh it from the server. The inode->i_mutex is necessary in order to prevent races between the new writes and the cache invalidation code. Cheers Trond - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html