Linus Torvalds <torvalds@transmeta.com> writes:
> I found the cause for this, and it's apparently been there forever (well,
> since the new NFS client went in some time fairly early in 2.1.x).
>
> The reason some people see it a lot more especially after 2.1.130 is that
> newer kernels can have a lot more pending writes, and as such the old bug
> can show up a lot more easily.
>
> I'll make a 2.1.132 soonish, I'm trying to survive my email backlog.
>
There are still a few issues remaining in pre-patch-2.1.132-1 though.
- There remains an ambiguity concerning the status of cancelled writes:
if they were already in progress, they may still be committed to the
server's storage, but this will not be registered by the inode
attribute cache.
- Shouldn't a cancellation also invalidate the page if it was marked
PG_uptodate?
- Another (minor) point is whether nfs_cancel_dirty should really be
cancelling unfreed write requests that are marked as
NFS_WRITE_COMPLETE?
- If you run 'setattr' on a file in order to truncate it, there is no
synchronization to ensure that async writes which would be cancelled
are committed before the call to setattr.
- If you read a page and the call to nfs_wb_page fails, the PG_locked bit
is never cleared, and many processes end up hanging on wait_on_page.
(Please note my first patch to cure this contained a bug in that it didn't
free the page in the case of a successful synchronous read).
Would you therefore please consider the merits of the following patch:
Cheers,
Trond
--Multipart_Thu_Dec_17_15:11:46_1998-1
Content-Type: application/octet-stream
Content-Disposition: attachment; filename="nfs-2.1.132-0.1.dif"
Content-Transfer-Encoding: 7bit
--- linux/fs/nfs/inode.c-2.1.132-0.1 Thu Dec 17 14:33:23 1998
+++ linux/fs/nfs/inode.c Thu Dec 17 14:36:11 1998
@@ -597,8 +597,11 @@
sattr.gid = attr->ia_gid;
sattr.size = (u32) -1;
- if ((attr->ia_valid & ATTR_SIZE) && S_ISREG(inode->i_mode))
+ if ((attr->ia_valid & ATTR_SIZE) && S_ISREG(inode->i_mode)) {
+ nfs_truncate_dirty_pages(inode, attr->ia_size);
+ nfs_wb_all(inode);
sattr.size = attr->ia_size;
+ }
sattr.mtime.seconds = sattr.mtime.useconds = (u32) -1;
if (attr->ia_valid & ATTR_MTIME) {
--- linux/fs/nfs/read.c-2.1.132-0.1 Mon Nov 16 04:56:33 1998
+++ linux/fs/nfs/read.c Thu Dec 17 15:06:16 1998
@@ -229,6 +229,7 @@
dprintk("NFS: nfs_readpage (%p %ld@%ld)\n",
page, PAGE_SIZE, page->offset);
+ atomic_inc(&page->count);
set_bit(PG_locked, &page->flags);
/*
@@ -240,18 +241,24 @@
*/
error = nfs_wb_page(inode, page);
if (error)
- return error;
+ goto out_error;
error = -1;
- atomic_inc(&page->count);
if (!IS_SWAPFILE(inode) && !PageError(page) &&
NFS_SERVER(inode)->rsize >= PAGE_SIZE)
error = nfs_readpage_async(dentry, inode, page);
- if (error < 0) { /* couldn't enqueue */
+ if (error >= 0)
+ goto out;
+
error = nfs_readpage_sync(dentry, inode, page);
- if (error < 0 && IS_SWAPFILE(inode))
- printk("Aiee.. nfs swap-in of page failed!\n");
- free_page(page_address(page));
- }
+ if (error >= 0)
+ goto out_free;
+out_error:
+ if (IS_SWAPFILE(inode))
+ printk("Aiee.. nfs swap-in of page failed!\n");
+ clear_bit(PG_locked, &page->flags);
+out_free:
+ free_page(page_address(page));
+out:
return error;
}
--- linux/fs/nfs/write.c-2.1.132-0.1 Wed Dec 2 18:45:25 1998
+++ linux/fs/nfs/write.c Thu Dec 17 14:50:51 1998
@@ -497,7 +497,8 @@
static void
nfs_cancel_request(struct nfs_wreq *req)
{
- req->wb_flags |= NFS_WRITE_CANCELLED;
+ req->wb_flags |= NFS_WRITE_CANCELLED | NFS_WRITE_INVALIDATE;
+ clear_bit(PG_uptodate, &req->wb_page->flags);
if (!WB_INPROGRESS(req)) {
rpc_exit(&req->wb_task, 0);
rpc_wake_up_task(&req->wb_task);
@@ -514,8 +515,9 @@
req = head = NFS_WRITEBACK(inode);
while (req != NULL) {
- if (pid == 0 || req->wb_pid == pid)
- nfs_cancel_request(req);
+ if (!WB_COMPLETE(req))
+ if (pid == 0 || req->wb_pid == pid)
+ nfs_cancel_request(req);
if ((req = WB_NEXT(req)) == head)
break;
}
@@ -680,7 +682,7 @@
if (status < 0) {
req->wb_flags |= NFS_WRITE_INVALIDATE;
- } else if (!WB_CANCELLED(req)) {
+ } else if (WB_INPROGRESS(req)) {
struct nfs_fattr *fattr = &req->wb_fattr;
/* Update attributes as result of writeback.
* Beware: when UDP replies arrive out of order, we
--Multipart_Thu_Dec_17_15:11:46_1998-1--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/