aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSteven Whitehouse <swhiteho@redhat.com>2008-06-02 09:14:54 +0100
committerSteven Whitehouse <swhiteho@redhat.com>2008-06-27 09:39:37 +0100
commit01b7c7ae88a6376c508b35a22bb61e04cb1b37f0 (patch)
tree01276b4382270285375540682d1eddf2a167103d
parent80274737220f8c5ea75696dde4c5c7feba39456f (diff)
[GFS2] Revise readpage locking
The previous attempt to fix the locking in readpage failed due to the use of a "try lock" which resulted in occasional high cpu usage during testing (due to repeated tries) and also it did not resolve all the ordering problems wrt the transaction lock (although it did solve all the inode lock ordering problems). This patch avoids the problem by unlocking the page and getting the locks in the correct order. This means that we have to retest the page to ensure that it hasn't changed when we relock the page. This now passes the tests which were previously failing. Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
-rw-r--r--fs/gfs2/ops_address.c29
1 files changed, 16 insertions, 13 deletions
diff --git a/fs/gfs2/ops_address.c b/fs/gfs2/ops_address.c
index 2b556dd034b..e64a1b04117 100644
--- a/fs/gfs2/ops_address.c
+++ b/fs/gfs2/ops_address.c
@@ -499,31 +499,34 @@ static int __gfs2_readpage(void *file, struct page *page)
* @file: The file to read
* @page: The page of the file
*
- * This deals with the locking required. We use a trylock in order to
- * avoid the page lock / glock ordering problems returning AOP_TRUNCATED_PAGE
- * in the event that we are unable to get the lock.
+ * This deals with the locking required. We have to unlock and
+ * relock the page in order to get the locking in the right
+ * order.
*/
static int gfs2_readpage(struct file *file, struct page *page)
{
- struct gfs2_inode *ip = GFS2_I(page->mapping->host);
+ struct address_space *mapping = page->mapping;
+ struct gfs2_inode *ip = GFS2_I(mapping->host);
struct gfs2_holder gh;
int error;
- gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME|LM_FLAG_TRY_1CB, &gh);
+ unlock_page(page);
+ gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME, &gh);
error = gfs2_glock_nq_atime(&gh);
- if (unlikely(error)) {
- unlock_page(page);
+ if (unlikely(error))
goto out;
- }
- error = __gfs2_readpage(file, page);
+ error = AOP_TRUNCATED_PAGE;
+ lock_page(page);
+ if (page->mapping == mapping && !PageUptodate(page))
+ error = __gfs2_readpage(file, page);
+ else
+ unlock_page(page);
gfs2_glock_dq(&gh);
out:
gfs2_holder_uninit(&gh);
- if (error == GLR_TRYFAILED) {
- yield();
- return AOP_TRUNCATED_PAGE;
- }
+ if (error && error != AOP_TRUNCATED_PAGE)
+ lock_page(page);
return error;
}