2008-03-19 19:35:30

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 2/2] nfs: don't ignore return value from nfs_pageio_add_request


On Wed, 2008-03-19 at 10:58 -0400, Trond Myklebust wrote:
> On Wed, 2008-03-19 at 10:13 -0400, Fred Isaman wrote:
> > Ignoring the return value from nfs_pageio_add_request can cause deadlocks.
> >
> > In read path:
> > call nfs_pageio_add_request from readpage_async_filler
> > assume at this point that there are requests already in desc, that
> > can't be merged with the current request.
> > so nfs_pageio_doio is fired up to clear out desc.
> > assume something goes wrong in setting up the io, so desc->pg_error is set.
> > This causes nfs_pageio_add_request to return 0, *WITHOUT* adding the original
> > request.
> > BUT, since return code is ignored, readpage_async_filler assumes it has
> > been added, and does nothing further, leaving page locked.
> > do_generic_mapping_read will eventually call lock_page, resulting in deadlock
>
> BTW: the same bug may occur in nfs_readpage_async(). I'll add a fixup
> for that to your patch.

Oops... My bad. nfs_readpage_async() appears to be fine. I'll go have
another coffee...

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com