Commit 3aff4ebb authored by Trond Myklebust's avatar Trond Myklebust

NFS: Prevent a deadlock in the new writeback code

We have to unlock the nfs_page before we call nfs_end_page_writeback
to avoid races with functions that expect the page to be unlocked
when PG_locked and PG_writeback are not set.
The problem is that nfs_unlock_request also releases the nfs_page,
causing a deadlock if the release of the nfs_open_context
triggers an iput() while the PG_writeback flag is still set...

The solution is to separate the unlocking and release of the nfs_page,
so that we can do the former before nfs_end_page_writeback and the
latter after.
Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
Cc: Fred Isaman <iisaman@netapp.com>
parent dc327ed4
...@@ -128,10 +128,10 @@ nfs_create_request(struct nfs_open_context *ctx, struct inode *inode, ...@@ -128,10 +128,10 @@ nfs_create_request(struct nfs_open_context *ctx, struct inode *inode,
} }
/** /**
* nfs_unlock_request - Unlock request and wake up sleepers. * nfs_unlock_request_dont_release - Unlock request and wake up sleepers.
* @req: * @req:
*/ */
void nfs_unlock_request(struct nfs_page *req) void nfs_unlock_request_dont_release(struct nfs_page *req)
{ {
if (!NFS_WBACK_BUSY(req)) { if (!NFS_WBACK_BUSY(req)) {
printk(KERN_ERR "NFS: Invalid unlock attempted\n"); printk(KERN_ERR "NFS: Invalid unlock attempted\n");
...@@ -141,6 +141,14 @@ void nfs_unlock_request(struct nfs_page *req) ...@@ -141,6 +141,14 @@ void nfs_unlock_request(struct nfs_page *req)
clear_bit(PG_BUSY, &req->wb_flags); clear_bit(PG_BUSY, &req->wb_flags);
smp_mb__after_clear_bit(); smp_mb__after_clear_bit();
wake_up_bit(&req->wb_flags, PG_BUSY); wake_up_bit(&req->wb_flags, PG_BUSY);
}
/**
* nfs_unlock_request - Unlock request and release the nfs_page
*/
void nfs_unlock_request(struct nfs_page *req)
{
nfs_unlock_request_dont_release(req);
nfs_release_request(req); nfs_release_request(req);
} }
......
...@@ -628,8 +628,9 @@ static void nfs_write_completion(struct nfs_pgio_header *hdr) ...@@ -628,8 +628,9 @@ static void nfs_write_completion(struct nfs_pgio_header *hdr)
remove_req: remove_req:
nfs_inode_remove_request(req); nfs_inode_remove_request(req);
next: next:
nfs_unlock_request(req); nfs_unlock_request_dont_release(req);
nfs_end_page_writeback(page); nfs_end_page_writeback(page);
nfs_release_request(req);
} }
out: out:
hdr->release(hdr); hdr->release(hdr);
...@@ -1042,8 +1043,9 @@ static void nfs_redirty_request(struct nfs_page *req) ...@@ -1042,8 +1043,9 @@ static void nfs_redirty_request(struct nfs_page *req)
struct page *page = req->wb_page; struct page *page = req->wb_page;
nfs_mark_request_dirty(req); nfs_mark_request_dirty(req);
nfs_unlock_request(req); nfs_unlock_request_dont_release(req);
nfs_end_page_writeback(page); nfs_end_page_writeback(page);
nfs_release_request(req);
} }
static void nfs_async_write_error(struct list_head *head) static void nfs_async_write_error(struct list_head *head)
......
...@@ -96,6 +96,7 @@ extern bool nfs_generic_pg_test(struct nfs_pageio_descriptor *desc, ...@@ -96,6 +96,7 @@ extern bool nfs_generic_pg_test(struct nfs_pageio_descriptor *desc,
struct nfs_page *req); struct nfs_page *req);
extern int nfs_wait_on_request(struct nfs_page *); extern int nfs_wait_on_request(struct nfs_page *);
extern void nfs_unlock_request(struct nfs_page *req); extern void nfs_unlock_request(struct nfs_page *req);
extern void nfs_unlock_request_dont_release(struct nfs_page *req);
/* /*
* Lock the page of an asynchronous request without getting a new reference * Lock the page of an asynchronous request without getting a new reference
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment