diff options
author | Fred Isaman <iisaman@citi.umich.edu> | 2008-03-19 11:24:39 -0400 |
---|---|---|
committer | Trond Myklebust <Trond.Myklebust@netapp.com> | 2008-03-19 17:59:02 -0400 |
commit | f8512ad0da16cbe156f3a7627971cdf0b39c4138 (patch) | |
tree | 2658c63faeda07505793ccc747ee4efbffdaa69c /fs | |
parent | 264e3e889d86e552b4191d69bb60f4f3b383135a (diff) |
nfs: don't ignore return value from nfs_pageio_add_request
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
In write path:
page is marked dirty by generic_perform_write
nfs_writepages is called
call nfs_pageio_add_request from nfs_page_async_flush
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_page_async_flush to return 0, *WITHOUT* adding the original
request, yet marking the request as locked (PG_BUSY) and in writeback,
clearing dirty marks.
The next time a write is done to the page, deadlock will result as
nfs_write_end calls nfs_update_request
Signed-off-by: Fred Isaman <iisaman@citi.umich.edu>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Diffstat (limited to 'fs')
-rw-r--r-- | fs/nfs/read.c | 5 | ||||
-rw-r--r-- | fs/nfs/write.c | 8 |
2 files changed, 11 insertions, 2 deletions
diff --git a/fs/nfs/read.c b/fs/nfs/read.c index 3d7d9631e125..5a70be589bbe 100644 --- a/fs/nfs/read.c +++ b/fs/nfs/read.c | |||
@@ -533,7 +533,10 @@ readpage_async_filler(void *data, struct page *page) | |||
533 | 533 | ||
534 | if (len < PAGE_CACHE_SIZE) | 534 | if (len < PAGE_CACHE_SIZE) |
535 | zero_user_segment(page, len, PAGE_CACHE_SIZE); | 535 | zero_user_segment(page, len, PAGE_CACHE_SIZE); |
536 | nfs_pageio_add_request(desc->pgio, new); | 536 | if (!nfs_pageio_add_request(desc->pgio, new)) { |
537 | error = desc->pgio->pg_error; | ||
538 | goto out_unlock; | ||
539 | } | ||
537 | return 0; | 540 | return 0; |
538 | out_error: | 541 | out_error: |
539 | error = PTR_ERR(new); | 542 | error = PTR_ERR(new); |
diff --git a/fs/nfs/write.c b/fs/nfs/write.c index 80c61fdb2720..bed63416a55b 100644 --- a/fs/nfs/write.c +++ b/fs/nfs/write.c | |||
@@ -39,6 +39,7 @@ static struct nfs_page * nfs_update_request(struct nfs_open_context*, | |||
39 | unsigned int, unsigned int); | 39 | unsigned int, unsigned int); |
40 | static void nfs_pageio_init_write(struct nfs_pageio_descriptor *desc, | 40 | static void nfs_pageio_init_write(struct nfs_pageio_descriptor *desc, |
41 | struct inode *inode, int ioflags); | 41 | struct inode *inode, int ioflags); |
42 | static void nfs_redirty_request(struct nfs_page *req); | ||
42 | static const struct rpc_call_ops nfs_write_partial_ops; | 43 | static const struct rpc_call_ops nfs_write_partial_ops; |
43 | static const struct rpc_call_ops nfs_write_full_ops; | 44 | static const struct rpc_call_ops nfs_write_full_ops; |
44 | static const struct rpc_call_ops nfs_commit_ops; | 45 | static const struct rpc_call_ops nfs_commit_ops; |
@@ -288,7 +289,12 @@ static int nfs_page_async_flush(struct nfs_pageio_descriptor *pgio, | |||
288 | BUG(); | 289 | BUG(); |
289 | } | 290 | } |
290 | spin_unlock(&inode->i_lock); | 291 | spin_unlock(&inode->i_lock); |
291 | nfs_pageio_add_request(pgio, req); | 292 | if (!nfs_pageio_add_request(pgio, req)) { |
293 | nfs_redirty_request(req); | ||
294 | nfs_end_page_writeback(page); | ||
295 | nfs_clear_page_tag_locked(req); | ||
296 | return pgio->pg_error; | ||
297 | } | ||
292 | return 0; | 298 | return 0; |
293 | } | 299 | } |
294 | 300 | ||