The nfs client code has a function "nfs_flush_incompatible" which is called when a process writes to a page to see if the page is already dirty in a way that means it should be flushed before the write is permitted. As I understand it, this is needed so that if two different credentials are used to write to a file, it is important that the server see the different writes each with their own credentials. Similarly if two different processes have different locks, then the writes they send should be kept separate. However if two processes both have the same credentials and neither are holding any locks on the file, then there can be no need to flush writes from one before writes from the other are allowed. I have a customer who has hit exactly this problem. It particularly involves mmapped files. Two different processes both mmap a file which happens to be stored on NFS. They both update the same pages (with some independent synchronisation) and when this happens each update requires that the change made by the other process be flushed first. This was did not happen before 2.6.24, but since the addition of nfs_vm_page_mkwrite, it has a serious impact on performance. So my question is: is it safe to make nfs_flush_incompatible" a bit less heavy handed, and in particular allow different processes with the same credentials to update the same page without flushing. The patch I have been experimenting with is below. It probably don't help on NFSv4 because there probably needs to be some care taken with different 'state' values. But my customer uses NFSv3 so that is what I focussed on. Any ideas? Thanks, NeilBrown diff --git a/fs/nfs/write.c b/fs/nfs/write.c index c483cc5..df40c57 100644 --- a/fs/nfs/write.c +++ b/fs/nfs/write.c @@ -860,11 +860,16 @@ int nfs_flush_incompatible(struct file *file, struct page *page) if (req == NULL) return 0; l_ctx = req->wb_lock_context; - do_flush = req->wb_page != page || req->wb_context != ctx; - if (l_ctx) { + + do_flush = req->wb_page != page; + if (req->wb_context != ctx) + do_flush |= + (req->wb_context->dentry != ctx->dentry || + req->wb_context->cred != ctx->cred ); + if (l_ctx && (req->wb_context->state || ctx->state)) do_flush |= l_ctx->lockowner.l_owner != current->files || l_ctx->lockowner.l_pid != current->tgid; - } + nfs_release_request(req); if (!do_flush) return 0;
Attachment:
signature.asc
Description: PGP signature