[PATCH 1/3] NFS: Don't do a full flush to disk on close() if we hold a delegation

[PATCH 1/3] NFS: Don't do a full flush to disk on close() if we hold a delegation

[Trond Myklebust]

If we hold a delegation then we know that it should be safe to continue
to cache the data beyond the close(). However since the process that wrote
the data may die after close(), we may still want to send the data to
server before those RPCSEC_GSS credentials expire. We therefore compromise
by starting writeback to the server, but don't wait for completion.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/file.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index aa9b709..8eda8a6 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -174,6 +174,13 @@ nfs_file_flush(struct file *file, fl_owner_t id)
 	if ((file->f_mode & FMODE_WRITE) == 0)
 		return 0;
 
+	/*
+	 * If we're holding a write delegation, then just start the i/o
+	 * but don't wait for completion (or send a commit).
+	 */
+	if (nfs_have_delegation(inode, FMODE_WRITE))
+		return filemap_fdatawrite(file->f_mapping);
+
 	/* Flush writes to the server and return any errors */
 	return vfs_fsync(file, 0);
 }
-- 
1.7.7.6

[PATCH 2/3] NFS: nfs_inode_return_delegation() should always flush dirty data

[Trond Myklebust]

The assumption is that if you are in a situation where you need to
return the delegation, then you should probably stop caching the
data anyway.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/delegation.c |    6 +++++-
 fs/nfs/delegation.h |    1 +
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 89af1d2..a19cb5a 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -380,6 +380,10 @@ void nfs_inode_return_delegation_noreclaim(struct inode *inode)
  * nfs_inode_return_delegation - synchronously return a delegation
  * @inode: inode to process
  *
+ * This routine will always flush any dirty data to disk on the
+ * assumption that if we need to return the delegation, then
+ * we should stop caching.
+ *
  * Returns zero on success, or a negative errno value.
  */
 int nfs_inode_return_delegation(struct inode *inode)
@@ -389,10 +393,10 @@ int nfs_inode_return_delegation(struct inode *inode)
 	struct nfs_delegation *delegation;
 	int err = 0;
 
+	nfs_wb_all(inode);
 	if (rcu_access_pointer(nfsi->delegation) != NULL) {
 		delegation = nfs_detach_delegation(nfsi, server);
 		if (delegation != NULL) {
-			nfs_wb_all(inode);
 			err = __nfs_inode_return_delegation(inode, delegation, 1);
 		}
 	}
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index cd6a7a8..72709c4 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -66,6 +66,7 @@ static inline int nfs_have_delegation(struct inode *inode, fmode_t flags)
 
 static inline int nfs_inode_return_delegation(struct inode *inode)
 {
+	nfs_wb_all(inode);
 	return 0;
 }
 #endif
-- 
1.7.7.6

[PATCH 3/3] NFSv4: nfs_client_return_marked_delegations can't flush data

[Trond Myklebust]

Since even filemap_flush() needs to lock pages that are dirty, we
cannot risk calling it from the state manager context. Therefore,
we need to move the call to filemap_flush() to
nfs_async_inode_return_delegation().

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/delegation.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index a19cb5a..bd3a960 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -316,6 +316,10 @@ out:
  * nfs_client_return_marked_delegations - return previously marked delegations
  * @clp: nfs_client to process
  *
+ * Note that this function is designed to be called by the state
+ * manager thread. For this reason, it cannot flush the dirty data,
+ * since that could deadlock in case of a state recovery error.
+ *
  * Returns zero on success, or a negative errno value.
  */
 int nfs_client_return_marked_delegations(struct nfs_client *clp)
@@ -340,11 +344,9 @@ restart:
 								server);
 			rcu_read_unlock();
 
-			if (delegation != NULL) {
-				filemap_flush(inode->i_mapping);
+			if (delegation != NULL)
 				err = __nfs_inode_return_delegation(inode,
 								delegation, 0);
-			}
 			iput(inode);
 			if (!err)
 				goto restart;
@@ -542,6 +544,8 @@ int nfs_async_inode_return_delegation(struct inode *inode,
 	struct nfs_client *clp = server->nfs_client;
 	struct nfs_delegation *delegation;
 
+	filemap_flush(inode->i_mapping);
+
 	rcu_read_lock();
 	delegation = rcu_dereference(NFS_I(inode)->delegation);
 
-- 
1.7.7.6

Re: [PATCH 1/3] NFS: Don't do a full flush to disk on close() if we hold a delegation

[Chuck Lever]

I thought the reason we wait for flush completion on close() was to report errors.

Sent from my iPad

On May 7, 2012, at 10:07 AM, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:

> If we hold a delegation then we know that it should be safe to continue
> to cache the data beyond the close(). However since the process that wrote
> the data may die after close(), we may still want to send the data to
> server before those RPCSEC_GSS credentials expire. We therefore compromise
> by starting writeback to the server, but don't wait for completion.
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> fs/nfs/file.c |    7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index aa9b709..8eda8a6 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -174,6 +174,13 @@ nfs_file_flush(struct file *file, fl_owner_t id)
>    if ((file->f_mode & FMODE_WRITE) == 0)
>        return 0;
> 
> +    /*
> +     * If we're holding a write delegation, then just start the i/o
> +     * but don't wait for completion (or send a commit).
> +     */
> +    if (nfs_have_delegation(inode, FMODE_WRITE))
> +        return filemap_fdatawrite(file->f_mapping);
> +
>    /* Flush writes to the server and return any errors */
>    return vfs_fsync(file, 0);
> }
> -- 
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] NFS: Don't do a full flush to disk on close() if we hold a delegation

[Myklebust, Trond]

T24gTW9uLCAyMDEyLTA1LTA3IGF0IDEwOjQ4IC0wNDAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
SSB0aG91Z2h0IHRoZSByZWFzb24gd2Ugd2FpdCBmb3IgZmx1c2ggY29tcGxldGlvbiBvbiBjbG9z
ZSgpIHdhcyB0byByZXBvcnQgZXJyb3JzLg0KDQpOby4gVGhlIHJlYXNvbiB3aHkgd2Ugd2FpdCBp
cyB0byBlbnN1cmUgY2xvc2UtdG8tb3BlbiBjYWNoZSBjb25zaXN0ZW5jeS4NCg0KQ2hlZXJzDQog
IFRyb25kDQoNCj4gU2VudCBmcm9tIG15IGlQYWQNCj4gDQo+IE9uIE1heSA3LCAyMDEyLCBhdCAx
MDowNyBBTSwgVHJvbmQgTXlrbGVidXN0IDxUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbT4gd3Jv
dGU6DQo+IA0KPiA+IElmIHdlIGhvbGQgYSBkZWxlZ2F0aW9uIHRoZW4gd2Uga25vdyB0aGF0IGl0
IHNob3VsZCBiZSBzYWZlIHRvIGNvbnRpbnVlDQo+ID4gdG8gY2FjaGUgdGhlIGRhdGEgYmV5b25k
IHRoZSBjbG9zZSgpLiBIb3dldmVyIHNpbmNlIHRoZSBwcm9jZXNzIHRoYXQgd3JvdGUNCj4gPiB0
aGUgZGF0YSBtYXkgZGllIGFmdGVyIGNsb3NlKCksIHdlIG1heSBzdGlsbCB3YW50IHRvIHNlbmQg
dGhlIGRhdGEgdG8NCj4gPiBzZXJ2ZXIgYmVmb3JlIHRob3NlIFJQQ1NFQ19HU1MgY3JlZGVudGlh
bHMgZXhwaXJlLiBXZSB0aGVyZWZvcmUgY29tcHJvbWlzZQ0KPiA+IGJ5IHN0YXJ0aW5nIHdyaXRl
YmFjayB0byB0aGUgc2VydmVyLCBidXQgZG9uJ3Qgd2FpdCBmb3IgY29tcGxldGlvbi4NCj4gPiAN
Cj4gPiBTaWduZWQtb2ZmLWJ5OiBUcm9uZCBNeWtsZWJ1c3QgPFRyb25kLk15a2xlYnVzdEBuZXRh
cHAuY29tPg0KPiA+IC0tLQ0KPiA+IGZzL25mcy9maWxlLmMgfCAgICA3ICsrKysrKysNCj4gPiAx
IGZpbGVzIGNoYW5nZWQsIDcgaW5zZXJ0aW9ucygrKSwgMCBkZWxldGlvbnMoLSkNCj4gPiANCj4g
PiBkaWZmIC0tZ2l0IGEvZnMvbmZzL2ZpbGUuYyBiL2ZzL25mcy9maWxlLmMNCj4gPiBpbmRleCBh
YTliNzA5Li44ZWRhOGE2IDEwMDY0NA0KPiA+IC0tLSBhL2ZzL25mcy9maWxlLmMNCj4gPiArKysg
Yi9mcy9uZnMvZmlsZS5jDQo+ID4gQEAgLTE3NCw2ICsxNzQsMTMgQEAgbmZzX2ZpbGVfZmx1c2go
c3RydWN0IGZpbGUgKmZpbGUsIGZsX293bmVyX3QgaWQpDQo+ID4gICAgaWYgKChmaWxlLT5mX21v
ZGUgJiBGTU9ERV9XUklURSkgPT0gMCkNCj4gPiAgICAgICAgcmV0dXJuIDA7DQo+ID4gDQo+ID4g
KyAgICAvKg0KPiA+ICsgICAgICogSWYgd2UncmUgaG9sZGluZyBhIHdyaXRlIGRlbGVnYXRpb24s
IHRoZW4ganVzdCBzdGFydCB0aGUgaS9vDQo+ID4gKyAgICAgKiBidXQgZG9uJ3Qgd2FpdCBmb3Ig
Y29tcGxldGlvbiAob3Igc2VuZCBhIGNvbW1pdCkuDQo+ID4gKyAgICAgKi8NCj4gPiArICAgIGlm
IChuZnNfaGF2ZV9kZWxlZ2F0aW9uKGlub2RlLCBGTU9ERV9XUklURSkpDQo+ID4gKyAgICAgICAg
cmV0dXJuIGZpbGVtYXBfZmRhdGF3cml0ZShmaWxlLT5mX21hcHBpbmcpOw0KPiA+ICsNCj4gPiAg
ICAvKiBGbHVzaCB3cml0ZXMgdG8gdGhlIHNlcnZlciBhbmQgcmV0dXJuIGFueSBlcnJvcnMgKi8N
Cj4gPiAgICByZXR1cm4gdmZzX2ZzeW5jKGZpbGUsIDApOw0KPiA+IH0NCj4gPiAtLSANCj4gPiAx
LjcuNy42DQo+ID4gDQo+ID4gLS0NCj4gPiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDog
c2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtbmZzIiBpbg0KPiA+IHRoZSBib2R5IG9m
IGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwub3JnDQo+ID4gTW9yZSBtYWpvcmRv
bW8gaW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQoN
Ci0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0
QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K