Re: [PATCH 3/3] nfs: use ->mmap_prepare() to avoid an AB-BA deadlock

[Trond Myklebust <trond.myklebust@fys.uio.no>]

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-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/