Commit b67b3c16 authored by Trond Myklebust's avatar Trond Myklebust

[PATCH] Fix spurious ETXTBSY errors due to late release of struct file

  The following patch should fix a problem of ETXTBSY sometimes
occurring if one tries to run a file straight after compilation.

The problem is that both NFS read and write requests can currently
hold a count on the struct file. This is done partly so as to be able
to pass along the RPC credential (which is cached in the struct file),
and partly so that asynchronous writes can report any errors via the
file->f_error mechanism.

The problem is that both the read and write requests may persist even
after file close() occurs. For O_RDONLY files, this is not a problem,
but for O_WRONLY, and O_RDWR files, the fact that the struct file is
not released until the last call to nfs_release_request() means that
inode->i_writecount does not necessarily get cleared upon file
close().

The following patch fixes both these issues.

  - NFS read requests no longer hold the struct file. They take a
    count on the the RPC credential itself.

  - NFS write requests still hold the struct file, since they want to
    report errors to sys_close() using the file->f_error mechanism.
    However they are made to release the page, credential, and file
    structures as soon as the write is completed instead of following
    the current practice of waiting for the last nfs_page request
    release.
parent 0d86f2ec
...@@ -53,7 +53,7 @@ static int nfs_try_to_free_pages(struct nfs_server *); ...@@ -53,7 +53,7 @@ static int nfs_try_to_free_pages(struct nfs_server *);
/** /**
* nfs_create_request - Create an NFS read/write request. * nfs_create_request - Create an NFS read/write request.
* @file: file that owns this request * @cred: RPC credential to use
* @inode: inode to which the request is attached * @inode: inode to which the request is attached
* @page: page to write * @page: page to write
* @offset: starting offset within the page for the write * @offset: starting offset within the page for the write
...@@ -66,7 +66,7 @@ static int nfs_try_to_free_pages(struct nfs_server *); ...@@ -66,7 +66,7 @@ static int nfs_try_to_free_pages(struct nfs_server *);
* User should ensure it is safe to sleep in this function. * User should ensure it is safe to sleep in this function.
*/ */
struct nfs_page * struct nfs_page *
nfs_create_request(struct file *file, struct inode *inode, nfs_create_request(struct rpc_cred *cred, struct inode *inode,
struct page *page, struct page *page,
unsigned int offset, unsigned int count) unsigned int offset, unsigned int count)
{ {
...@@ -107,34 +107,49 @@ nfs_create_request(struct file *file, struct inode *inode, ...@@ -107,34 +107,49 @@ nfs_create_request(struct file *file, struct inode *inode,
req->wb_offset = offset; req->wb_offset = offset;
req->wb_bytes = count; req->wb_bytes = count;
/* If we have a struct file, use its cached credentials */ if (cred)
if (file) { req->wb_cred = get_rpccred(cred);
req->wb_file = file;
get_file(file);
req->wb_cred = nfs_file_cred(file);
}
req->wb_inode = inode; req->wb_inode = inode;
req->wb_count = 1; req->wb_count = 1;
return req; return req;
} }
/**
* nfs_clear_request - Free up all resources allocated to the request
* @req:
*
* Release all resources associated with a write request after it
* has completed.
*/
void nfs_clear_request(struct nfs_page *req)
{
/* Release struct file or cached credential */
if (req->wb_file) {
fput(req->wb_file);
req->wb_file = NULL;
}
if (req->wb_cred) {
put_rpccred(req->wb_cred);
req->wb_cred = NULL;
}
if (req->wb_page) {
page_cache_release(req->wb_page);
req->wb_page = NULL;
atomic_dec(&NFS_REQUESTLIST(req->wb_inode)->nr_requests);
}
}
/** /**
* nfs_release_request - Release the count on an NFS read/write request * nfs_release_request - Release the count on an NFS read/write request
* @req: request to release * @req: request to release
* *
* Release all resources associated with a write request after it
* has been committed to stable storage
*
* Note: Should never be called with the spinlock held! * Note: Should never be called with the spinlock held!
*/ */
void void
nfs_release_request(struct nfs_page *req) nfs_release_request(struct nfs_page *req)
{ {
struct inode *inode = req->wb_inode;
struct nfs_reqlist *cache = NFS_REQUESTLIST(inode);
spin_lock(&nfs_wreq_lock); spin_lock(&nfs_wreq_lock);
if (--req->wb_count) { if (--req->wb_count) {
spin_unlock(&nfs_wreq_lock); spin_unlock(&nfs_wreq_lock);
...@@ -142,7 +157,6 @@ nfs_release_request(struct nfs_page *req) ...@@ -142,7 +157,6 @@ nfs_release_request(struct nfs_page *req)
} }
__nfs_del_lru(req); __nfs_del_lru(req);
spin_unlock(&nfs_wreq_lock); spin_unlock(&nfs_wreq_lock);
atomic_dec(&cache->nr_requests);
#ifdef NFS_PARANOIA #ifdef NFS_PARANOIA
if (!list_empty(&req->wb_list)) if (!list_empty(&req->wb_list))
...@@ -151,16 +165,12 @@ nfs_release_request(struct nfs_page *req) ...@@ -151,16 +165,12 @@ nfs_release_request(struct nfs_page *req)
BUG(); BUG();
if (NFS_WBACK_BUSY(req)) if (NFS_WBACK_BUSY(req))
BUG(); BUG();
if (atomic_read(&cache->nr_requests) < 0) if (atomic_read(&NFS_REQUESTLIST(req->wb_inode)->nr_requests) < 0)
BUG(); BUG();
#endif #endif
/* Release struct file or cached credential */ /* Release struct file or cached credential */
if (req->wb_file) nfs_clear_request(req);
fput(req->wb_file);
else if (req->wb_cred)
put_rpccred(req->wb_cred);
page_cache_release(req->wb_page);
nfs_page_free(req); nfs_page_free(req);
} }
......
...@@ -171,7 +171,7 @@ nfs_readpage_async(struct file *file, struct inode *inode, struct page *page) ...@@ -171,7 +171,7 @@ nfs_readpage_async(struct file *file, struct inode *inode, struct page *page)
struct nfs_inode *nfsi = NFS_I(inode); struct nfs_inode *nfsi = NFS_I(inode);
struct nfs_page *new; struct nfs_page *new;
new = nfs_create_request(file, inode, page, 0, PAGE_CACHE_SIZE); new = nfs_create_request(nfs_file_cred(file), inode, page, 0, PAGE_CACHE_SIZE);
if (IS_ERR(new)) if (IS_ERR(new))
return PTR_ERR(new); return PTR_ERR(new);
nfs_mark_request_read(new); nfs_mark_request_read(new);
...@@ -227,8 +227,9 @@ nfs_async_read_error(struct list_head *head) ...@@ -227,8 +227,9 @@ nfs_async_read_error(struct list_head *head)
nfs_list_remove_request(req); nfs_list_remove_request(req);
SetPageError(page); SetPageError(page);
UnlockPage(page); UnlockPage(page);
nfs_unlock_request(req); nfs_clear_request(req);
nfs_release_request(req); nfs_release_request(req);
nfs_unlock_request(req);
} }
} }
...@@ -433,6 +434,7 @@ nfs_readpage_result(struct rpc_task *task) ...@@ -433,6 +434,7 @@ nfs_readpage_result(struct rpc_task *task)
(long long)NFS_FILEID(req->wb_inode), (long long)NFS_FILEID(req->wb_inode),
req->wb_bytes, req->wb_bytes,
(long long)(page_offset(page) + req->wb_offset)); (long long)(page_offset(page) + req->wb_offset));
nfs_clear_request(req);
nfs_release_request(req); nfs_release_request(req);
nfs_unlock_request(req); nfs_unlock_request(req);
} }
......
...@@ -354,6 +354,7 @@ nfs_inode_remove_request(struct nfs_page *req) ...@@ -354,6 +354,7 @@ nfs_inode_remove_request(struct nfs_page *req)
iput(inode); iput(inode);
} else } else
spin_unlock(&nfs_wreq_lock); spin_unlock(&nfs_wreq_lock);
nfs_clear_request(req);
nfs_release_request(req); nfs_release_request(req);
} }
...@@ -684,9 +685,13 @@ nfs_update_request(struct file* file, struct inode *inode, struct page *page, ...@@ -684,9 +685,13 @@ nfs_update_request(struct file* file, struct inode *inode, struct page *page,
} }
spin_unlock(&nfs_wreq_lock); spin_unlock(&nfs_wreq_lock);
new = nfs_create_request(file, inode, page, offset, bytes); new = nfs_create_request(nfs_file_cred(file), inode, page, offset, bytes);
if (IS_ERR(new)) if (IS_ERR(new))
return new; return new;
if (file) {
new->wb_file = file;
get_file(file);
}
/* If the region is locked, adjust the timeout */ /* If the region is locked, adjust the timeout */
if (region_locked(inode, new)) if (region_locked(inode, new))
new->wb_timeout = jiffies + NFS_WRITEBACK_LOCKDELAY; new->wb_timeout = jiffies + NFS_WRITEBACK_LOCKDELAY;
......
...@@ -250,7 +250,9 @@ extern struct address_space_operations nfs_file_aops; ...@@ -250,7 +250,9 @@ extern struct address_space_operations nfs_file_aops;
static __inline__ struct rpc_cred * static __inline__ struct rpc_cred *
nfs_file_cred(struct file *file) nfs_file_cred(struct file *file)
{ {
struct rpc_cred *cred = (struct rpc_cred *)(file->private_data); struct rpc_cred *cred = NULL;
if (file)
cred = (struct rpc_cred *)file->private_data;
#ifdef RPC_DEBUG #ifdef RPC_DEBUG
if (cred && cred->cr_magic != RPCAUTH_CRED_MAGIC) if (cred && cred->cr_magic != RPCAUTH_CRED_MAGIC)
BUG(); BUG();
......
...@@ -41,9 +41,10 @@ struct nfs_page { ...@@ -41,9 +41,10 @@ struct nfs_page {
#define NFS_WBACK_BUSY(req) (test_bit(PG_BUSY,&(req)->wb_flags)) #define NFS_WBACK_BUSY(req) (test_bit(PG_BUSY,&(req)->wb_flags))
extern struct nfs_page *nfs_create_request(struct file *, struct inode *, extern struct nfs_page *nfs_create_request(struct rpc_cred *, struct inode *,
struct page *, struct page *,
unsigned int, unsigned int); unsigned int, unsigned int);
extern void nfs_clear_request(struct nfs_page *req);
extern void nfs_release_request(struct nfs_page *req); extern void nfs_release_request(struct nfs_page *req);
......
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