From 6f2ce4cdc216ad5a3e8ed09a011a5f0681f098b4 Mon Sep 17 00:00:00 2001 From: Simon Wilkinson Date: Fri, 16 Oct 2009 16:39:24 +0100 Subject: [PATCH] Always unlock pages when returning from writepage Writepage has a return path which returns an error with a locked page. However, all returns that are not AOP_WRITEPAGE_ACTIVATE must unlock their pages - using this codepath would leave a stray page lock, which would eventually hang the machine. The logic behind the -EIO return was also incorrect. In the Linux page cache model, truncates simply reduce the size recorded in the inode. If there are pages pending writeback then they may still have writepage() called upon them - it's up to the writepage routine to discard the write request (rather than returning an error) Reviewed-on: http://gerrit.openafs.org/685 Tested-by: Derrick Brashear Reviewed-by: Derrick Brashear --- src/afs/LINUX/osi_vnodeops.c | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/src/afs/LINUX/osi_vnodeops.c b/src/afs/LINUX/osi_vnodeops.c index 75a5103..4be7eea 100644 --- a/src/afs/LINUX/osi_vnodeops.c +++ b/src/afs/LINUX/osi_vnodeops.c @@ -2067,31 +2067,41 @@ afs_linux_writepage(struct page *pp) { struct address_space *mapping = pp->mapping; struct inode *inode; - unsigned long end_index; - unsigned offset = PAGE_CACHE_SIZE; - long status; + unsigned int to = PAGE_CACHE_SIZE; + loff_t isize; + int status = 0; if (PageReclaim(pp)) { return AOP_WRITEPAGE_ACTIVATE; + /* XXX - Do we need to redirty the page here? */ } + page_cache_get(pp); + inode = (struct inode *)mapping->host; - end_index = i_size_read(inode) >> PAGE_CACHE_SHIFT; - - /* easy case */ - if (pp->index < end_index) - goto do_it; - /* things got complicated... */ - offset = i_size_read(inode) & (PAGE_CACHE_SIZE - 1); - /* OK, are we completely out? */ - if (pp->index >= end_index + 1 || !offset) - return -EIO; - do_it: - status = afs_linux_writepage_sync(inode, pp, 0, offset); + isize = i_size_read(inode); + + /* Don't defeat an earlier truncate */ + if (page_offset(pp) > isize) + goto done; + + /* If this is the final page, then just write the number of bytes that + * are actually in it */ + if ((isize - page_offset(pp)) < to ) + to = isize - page_offset(pp); + + status = afs_linux_writepage_sync(inode, pp, 0, to); + +done: SetPageUptodate(pp); - if ( status != AOP_WRITEPAGE_ACTIVATE ) + if ( status != AOP_WRITEPAGE_ACTIVATE ) { + /* XXX - do we need to redirty the page here? */ UnlockPage(pp); - if (status == offset) + } + + page_cache_release(pp); + + if (status == to) return 0; else return status; -- 1.9.4