All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH - alt-2] NFSv4:  Don't try to recover NFSv4 locks when they are lost.
@ 2013-09-04  7:04 NeilBrown
  2013-09-04 14:31 ` Myklebust, Trond
  0 siblings, 1 reply; 3+ messages in thread
From: NeilBrown @ 2013-09-04  7:04 UTC (permalink / raw)
  To: Myklebust, Trond; +Cc: NFS

[-- Attachment #1: Type: text/plain, Size: 11127 bytes --]



When an NFSv4 client loses contact with the server it can lose any
locks that it holds.

Currently when it reconnects to the server it simply tries to reclaim
those locks.  This might succeed even though some other client has
held and released a lock in the mean time.  So the first client might
think the file is unchanged, but it isn't.  This isn't good.

If, when recovery happens, the locks cannot be claimed because some
other client still holds the lock, then we get a message in the kernel
logs, but the client can still write.  So two clients can both think
they have a lock and can both write at the same time.  This is equally
not good.

There was a patch a while ago
  http://comments.gmane.org/gmane.linux.nfs/41917

which tried to address some of this, but it didn't seem to go
anywhere.  That patch would also send a signal to the process.  That
might be useful but for now this patch just causes writes to fail.

For NFSv4 (unlike v2/v3) there is a strong link between the lock and
the write request so we can fairly easily fail any IO of the lock is
gone.  While some applications might not expect this, it is still
safer than allowing the write to succeed.

Because this is a fairly big change in behaviour a module parameter,
"recover_locks", is introduced which defaults to true (the current
behaviour) but can be set to "false" to tell the client not to try to
recover things that were lost.

Signed-off-by: NeilBrown <neilb@suse.de>

--
This alternative uses a module parameter which defaults to the current,
incorrect, behaviour.
I suspect we don't want that one..

NeilBrown


diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index f5c84c3..de0229b 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -826,9 +826,10 @@ static void nfs3_proc_read_setup(struct nfs_read_data *data, struct rpc_message
 	msg->rpc_proc = &nfs3_procedures[NFS3PROC_READ];
 }
 
-static void nfs3_proc_read_rpc_prepare(struct rpc_task *task, struct nfs_read_data *data)
+static int nfs3_proc_read_rpc_prepare(struct rpc_task *task, struct nfs_read_data *data)
 {
 	rpc_call_start(task);
+	return 0;
 }
 
 static int nfs3_write_done(struct rpc_task *task, struct nfs_write_data *data)
@@ -847,9 +848,10 @@ static void nfs3_proc_write_setup(struct nfs_write_data *data, struct rpc_messag
 	msg->rpc_proc = &nfs3_procedures[NFS3PROC_WRITE];
 }
 
-static void nfs3_proc_write_rpc_prepare(struct rpc_task *task, struct nfs_write_data *data)
+static int nfs3_proc_write_rpc_prepare(struct rpc_task *task, struct nfs_write_data *data)
 {
 	rpc_call_start(task);
+	return 0;
 }
 
 static void nfs3_proc_commit_rpc_prepare(struct rpc_task *task, struct nfs_commit_data *data)
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index ee81e35..a468b345 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -135,6 +135,7 @@ struct nfs4_lock_state {
 	struct list_head	ls_locks;	/* Other lock stateids */
 	struct nfs4_state *	ls_state;	/* Pointer to open state */
 #define NFS_LOCK_INITIALIZED 0
+#define NFS_LOCK_LOST        1
 	unsigned long		ls_flags;
 	struct nfs_seqid_counter	ls_seqid;
 	nfs4_stateid		ls_stateid;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 108a774..6ef4016 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3905,15 +3905,19 @@ static void nfs4_proc_read_setup(struct nfs_read_data *data, struct rpc_message
 	nfs41_init_sequence(&data->args.seq_args, &data->res.seq_res, 0);
 }
 
-static void nfs4_proc_read_rpc_prepare(struct rpc_task *task, struct nfs_read_data *data)
+static int nfs4_proc_read_rpc_prepare(struct rpc_task *task, struct nfs_read_data *data)
 {
 	if (nfs4_setup_sequence(NFS_SERVER(data->header->inode),
 			&data->args.seq_args,
 			&data->res.seq_res,
 			task))
-		return;
-	nfs4_set_rw_stateid(&data->args.stateid, data->args.context,
-			data->args.lock_context, FMODE_READ);
+		return 0;
+	if (nfs4_set_rw_stateid(&data->args.stateid, data->args.context,
+				data->args.lock_context, FMODE_READ) == -EIO)
+		return -EIO;
+	if (unlikely(test_bit(NFS_CONTEXT_BAD, &data->args.context->flags)))
+		return -EIO;
+	return 0;
 }
 
 static int nfs4_write_done_cb(struct rpc_task *task, struct nfs_write_data *data)
@@ -3988,15 +3992,19 @@ static void nfs4_proc_write_setup(struct nfs_write_data *data, struct rpc_messag
 	nfs41_init_sequence(&data->args.seq_args, &data->res.seq_res, 1);
 }
 
-static void nfs4_proc_write_rpc_prepare(struct rpc_task *task, struct nfs_write_data *data)
+static int nfs4_proc_write_rpc_prepare(struct rpc_task *task, struct nfs_write_data *data)
 {
 	if (nfs4_setup_sequence(NFS_SERVER(data->header->inode),
 			&data->args.seq_args,
 			&data->res.seq_res,
 			task))
-		return;
-	nfs4_set_rw_stateid(&data->args.stateid, data->args.context,
-			data->args.lock_context, FMODE_WRITE);
+		return 0;
+	if (nfs4_set_rw_stateid(&data->args.stateid, data->args.context,
+				data->args.lock_context, FMODE_WRITE) == -EIO)
+		return -EIO;
+	if (unlikely(test_bit(NFS_CONTEXT_BAD, &data->args.context->flags)))
+		return -EIO;
+	return 0;
 }
 
 static void nfs4_proc_commit_rpc_prepare(struct rpc_task *task, struct nfs_commit_data *data)
@@ -5378,6 +5386,11 @@ static int nfs4_lock_reclaim(struct nfs4_state *state, struct file_lock *request
 	return err;
 }
 
+bool recover_locks = true;
+module_param(recover_locks, bool, 0644);
+MODULE_PARM_DESC(recovery_locks,
+		 "If the server reports that a lock might be lost, "
+		 "try to recovery it risking corruption.");
 static int nfs4_lock_expired(struct nfs4_state *state, struct file_lock *request)
 {
 	struct nfs_server *server = NFS_SERVER(state->inode);
@@ -5389,6 +5402,10 @@ static int nfs4_lock_expired(struct nfs4_state *state, struct file_lock *request
 	err = nfs4_set_lock_state(state, request);
 	if (err != 0)
 		return err;
+	if (!recover_locks) {
+		set_bit(NFS_LOCK_LOST, &request->fl_u.nfs4_fl.owner->ls_flags);
+		return 0;
+	}
 	do {
 		if (test_bit(NFS_DELEGATED_STATE, &state->flags) != 0)
 			return 0;
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index e22862f..1e26e8c 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -998,7 +998,9 @@ static int nfs4_copy_lock_stateid(nfs4_stateid *dst,
 	fl_pid = lockowner->l_pid;
 	spin_lock(&state->state_lock);
 	lsp = __nfs4_find_lock_state(state, fl_owner, fl_pid, NFS4_ANY_LOCK_TYPE);
-	if (lsp != NULL && test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags) != 0) {
+	if (lsp && test_bit(NFS_LOCK_LOST, &lsp->ls_flags))
+		ret = -EIO;
+	else if (lsp != NULL && test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags) != 0) {
 		nfs4_stateid_copy(dst, &lsp->ls_stateid);
 		ret = 0;
 		smp_rmb();
@@ -1038,11 +1040,17 @@ static int nfs4_copy_open_stateid(nfs4_stateid *dst, struct nfs4_state *state)
 int nfs4_select_rw_stateid(nfs4_stateid *dst, struct nfs4_state *state,
 		fmode_t fmode, const struct nfs_lockowner *lockowner)
 {
-	int ret = 0;
+	int ret = nfs4_copy_lock_stateid(dst, state, lockowner);
+	if (ret == -EIO)
+		/* A lost lock - don't even consider delegations */
+		goto out;
 	if (nfs4_copy_delegation_stateid(dst, state->inode, fmode))
 		goto out;
-	ret = nfs4_copy_lock_stateid(dst, state, lockowner);
 	if (ret != -ENOENT)
+		/* nfs4_copy_delegation_stateid() didn't over-write
+		 * dst, so it still has the lock stateid which we now
+		 * choose to use.
+		 */
 		goto out;
 	ret = nfs4_copy_open_stateid(dst, state);
 out:
diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
index c041c41..a8f57c7 100644
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -623,9 +623,10 @@ static void nfs_proc_read_setup(struct nfs_read_data *data, struct rpc_message *
 	msg->rpc_proc = &nfs_procedures[NFSPROC_READ];
 }
 
-static void nfs_proc_read_rpc_prepare(struct rpc_task *task, struct nfs_read_data *data)
+static int nfs_proc_read_rpc_prepare(struct rpc_task *task, struct nfs_read_data *data)
 {
 	rpc_call_start(task);
+	return 0;
 }
 
 static int nfs_write_done(struct rpc_task *task, struct nfs_write_data *data)
@@ -644,9 +645,10 @@ static void nfs_proc_write_setup(struct nfs_write_data *data, struct rpc_message
 	msg->rpc_proc = &nfs_procedures[NFSPROC_WRITE];
 }
 
-static void nfs_proc_write_rpc_prepare(struct rpc_task *task, struct nfs_write_data *data)
+static int nfs_proc_write_rpc_prepare(struct rpc_task *task, struct nfs_write_data *data)
 {
 	rpc_call_start(task);
+	return 0;
 }
 
 static void nfs_proc_commit_rpc_prepare(struct rpc_task *task, struct nfs_commit_data *data)
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 70a26c6..31db5c3 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -513,9 +513,10 @@ static void nfs_readpage_release_common(void *calldata)
 void nfs_read_prepare(struct rpc_task *task, void *calldata)
 {
 	struct nfs_read_data *data = calldata;
-	NFS_PROTO(data->header->inode)->read_rpc_prepare(task, data);
-	if (unlikely(test_bit(NFS_CONTEXT_BAD, &data->args.context->flags)))
-		rpc_exit(task, -EIO);
+	int err;
+	err = NFS_PROTO(data->header->inode)->read_rpc_prepare(task, data);
+	if (err)
+		rpc_exit(task, err);
 }
 
 static const struct rpc_call_ops nfs_read_common_ops = {
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index f1bdb72..7816801 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1265,9 +1265,10 @@ EXPORT_SYMBOL_GPL(nfs_pageio_reset_write_mds);
 void nfs_write_prepare(struct rpc_task *task, void *calldata)
 {
 	struct nfs_write_data *data = calldata;
-	NFS_PROTO(data->header->inode)->write_rpc_prepare(task, data);
-	if (unlikely(test_bit(NFS_CONTEXT_BAD, &data->args.context->flags)))
-		rpc_exit(task, -EIO);
+	int err;
+	err = NFS_PROTO(data->header->inode)->write_rpc_prepare(task, data);
+	if (err)
+		rpc_exit(task, err);
 }
 
 void nfs_commit_prepare(struct rpc_task *task, void *calldata)
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 8651574..c71e12b 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1419,12 +1419,12 @@ struct nfs_rpc_ops {
 	void	(*read_setup)   (struct nfs_read_data *, struct rpc_message *);
 	void	(*read_pageio_init)(struct nfs_pageio_descriptor *, struct inode *,
 				    const struct nfs_pgio_completion_ops *);
-	void	(*read_rpc_prepare)(struct rpc_task *, struct nfs_read_data *);
+	int	(*read_rpc_prepare)(struct rpc_task *, struct nfs_read_data *);
 	int	(*read_done)  (struct rpc_task *, struct nfs_read_data *);
 	void	(*write_setup)  (struct nfs_write_data *, struct rpc_message *);
 	void	(*write_pageio_init)(struct nfs_pageio_descriptor *, struct inode *, int,
 				     const struct nfs_pgio_completion_ops *);
-	void	(*write_rpc_prepare)(struct rpc_task *, struct nfs_write_data *);
+	int	(*write_rpc_prepare)(struct rpc_task *, struct nfs_write_data *);
 	int	(*write_done)  (struct rpc_task *, struct nfs_write_data *);
 	void	(*commit_setup) (struct nfs_commit_data *, struct rpc_message *);
 	void	(*commit_rpc_prepare)(struct rpc_task *, struct nfs_commit_data *);

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH - alt-2] NFSv4:  Don't try to recover NFSv4 locks when they are lost.
  2013-09-04  7:04 [PATCH - alt-2] NFSv4: Don't try to recover NFSv4 locks when they are lost NeilBrown
@ 2013-09-04 14:31 ` Myklebust, Trond
  2013-09-04 23:48   ` NeilBrown
  0 siblings, 1 reply; 3+ messages in thread
From: Myklebust, Trond @ 2013-09-04 14:31 UTC (permalink / raw)
  To: NeilBrown; +Cc: NFS

[-- Attachment #1: Type: text/plain, Size: 2391 bytes --]

On Wed, 2013-09-04 at 17:04 +1000, NeilBrown wrote:
> 
> When an NFSv4 client loses contact with the server it can lose any
> locks that it holds.
> 
> Currently when it reconnects to the server it simply tries to reclaim
> those locks.  This might succeed even though some other client has
> held and released a lock in the mean time.  So the first client might
> think the file is unchanged, but it isn't.  This isn't good.
> 
> If, when recovery happens, the locks cannot be claimed because some
> other client still holds the lock, then we get a message in the kernel
> logs, but the client can still write.  So two clients can both think
> they have a lock and can both write at the same time.  This is equally
> not good.
> 
> There was a patch a while ago
>   http://comments.gmane.org/gmane.linux.nfs/41917
> 
> which tried to address some of this, but it didn't seem to go
> anywhere.  That patch would also send a signal to the process.  That
> might be useful but for now this patch just causes writes to fail.
> 
> For NFSv4 (unlike v2/v3) there is a strong link between the lock and
> the write request so we can fairly easily fail any IO of the lock is
> gone.  While some applications might not expect this, it is still
> safer than allowing the write to succeed.
> 
> Because this is a fairly big change in behaviour a module parameter,
> "recover_locks", is introduced which defaults to true (the current
> behaviour) but can be set to "false" to tell the client not to try to
> recover things that were lost.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>

Thanks!

> --
> This alternative uses a module parameter which defaults to the current,
> incorrect, behaviour.
> I suspect we don't want that one..

Agreed. We also need to document the module parameter, so I'm adding a
little blurb in Documentation/kernel-parameters.txt (see attachment).

I'd also like to change the parameter name to "recover_lost_locks" to
make it a little more obvious.

Finally, I'd like to move the parameter to fs/nfs/super.c so that we can
use the same modprobe.conf 'options' lines for back ports of this patch
(yes I strongly suspect we will want to back port this patch to distro
kernels).

See attachment.

Cheers
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-NFSv4-Document-the-recover_lost_locks-kernel-paramet.patch --]
[-- Type: text/x-patch; name="0001-NFSv4-Document-the-recover_lost_locks-kernel-paramet.patch", Size: 4564 bytes --]

From 7722b2fa150b9b7b21218923ad01b757a6653b10 Mon Sep 17 00:00:00 2001
From: Trond Myklebust <Trond.Myklebust@netapp.com>
Date: Wed, 4 Sep 2013 10:08:54 -0400
Subject: [PATCH] NFSv4: Document the recover_lost_locks kernel parameter

Rename the new 'recover_locks' kernel parameter to 'recover_lost_locks'
and change the default to 'false'. Document why in
Documentation/kernel-parameters.txt

Move the 'recover_lost_locks' kernel parameter to fs/nfs/super.c to
make it easy to backport to kernels prior to 3.6.x, which don't have
a separate NFSv4 module.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 Documentation/kernel-parameters.txt | 12 ++++++++++++
 fs/nfs/nfs4_fs.h                    |  1 +
 fs/nfs/nfs4proc.c                   |  8 +-------
 fs/nfs/super.c                      |  8 ++++++++
 4 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 15356ac..30584b1 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1847,6 +1847,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			will be sent.
 			The default is to send the implementation identification
 			information.
+	
+	nfs.recover_lost_locks =
+			[NFSv4] Attempt to recover locks that were lost due
+			to a lease timeout on the server. Please note that
+			doing this risks data corruption, since there are
+			no guarantees that the file will remain unchanged
+			after the locks are lost.
+			If you want to enable the kernel legacy behaviour of
+			attempting to recover these locks, then set this
+			parameter to '1'.
+			The default parameter value of '0' causes the kernel
+			not to attempt recovery of lost locks.
 
 	nfsd.nfs4_disable_idmapping=
 			[NFSv4] When set to the default of '1', the NFSv4
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 6411831..277407d 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -372,6 +372,7 @@ struct dentry *nfs4_try_mount(int, const char *, struct nfs_mount_info *, struct
 extern bool nfs4_disable_idmapping;
 extern unsigned short max_session_slots;
 extern unsigned short send_implementation_id;
+extern bool recover_lost_locks;
 
 #define NFS4_CLIENT_ID_UNIQ_LEN		(64)
 extern char nfs4_client_id_uniquifier[NFS4_CLIENT_ID_UNIQ_LEN];
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 1eb694e..535011a 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5523,12 +5523,6 @@ static int nfs4_lock_reclaim(struct nfs4_state *state, struct file_lock *request
 	return err;
 }
 
-bool recover_locks = true;
-module_param(recover_locks, bool, 0644);
-MODULE_PARM_DESC(recover_locks,
-		 "If the server reports that a lock might be lost, "
-		 "try to recovery it risking corruption.");
-
 static int nfs4_lock_expired(struct nfs4_state *state, struct file_lock *request)
 {
 	struct nfs_server *server = NFS_SERVER(state->inode);
@@ -5540,7 +5534,7 @@ static int nfs4_lock_expired(struct nfs4_state *state, struct file_lock *request
 	err = nfs4_set_lock_state(state, request);
 	if (err != 0)
 		return err;
-	if (!recover_locks) {
+	if (!recover_lost_locks) {
 		set_bit(NFS_LOCK_LOST, &request->fl_u.nfs4_fl.owner->ls_flags);
 		return 0;
 	}
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index f2071d2..6ad9053 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2758,6 +2758,7 @@ bool nfs4_disable_idmapping = true;
 unsigned short max_session_slots = NFS4_DEF_SLOT_TABLE_SIZE;
 unsigned short send_implementation_id = 1;
 char nfs4_client_id_uniquifier[NFS4_CLIENT_ID_UNIQ_LEN] = "";
+bool recover_lost_locks = false;
 
 EXPORT_SYMBOL_GPL(nfs_callback_set_tcpport);
 EXPORT_SYMBOL_GPL(nfs_callback_tcpport);
@@ -2766,6 +2767,7 @@ EXPORT_SYMBOL_GPL(nfs4_disable_idmapping);
 EXPORT_SYMBOL_GPL(max_session_slots);
 EXPORT_SYMBOL_GPL(send_implementation_id);
 EXPORT_SYMBOL_GPL(nfs4_client_id_uniquifier);
+EXPORT_SYMBOL_GPL(recover_lost_locks);
 
 #define NFS_CALLBACK_MAXPORTNR (65535U)
 
@@ -2803,4 +2805,10 @@ MODULE_PARM_DESC(send_implementation_id,
 		"Send implementation ID with NFSv4.1 exchange_id");
 MODULE_PARM_DESC(nfs4_unique_id, "nfs_client_id4 uniquifier string");
 
+module_param(recover_lost_locks, bool, 0644);
+MODULE_PARM_DESC(recover_lost_locks,
+		 "If the server reports that a lock might be lost, "
+		 "try to recover it risking data corruption.");
+
+
 #endif /* CONFIG_NFS_V4 */
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH - alt-2] NFSv4:  Don't try to recover NFSv4 locks when they are lost.
  2013-09-04 14:31 ` Myklebust, Trond
@ 2013-09-04 23:48   ` NeilBrown
  0 siblings, 0 replies; 3+ messages in thread
From: NeilBrown @ 2013-09-04 23:48 UTC (permalink / raw)
  To: Myklebust, Trond; +Cc: NFS

[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]

On Wed, 4 Sep 2013 14:31:07 +0000 "Myklebust, Trond"
<Trond.Myklebust@netapp.com> wrote:

> On Wed, 2013-09-04 at 17:04 +1000, NeilBrown wrote:
> > 
> > When an NFSv4 client loses contact with the server it can lose any
> > locks that it holds.
> > 
> > Currently when it reconnects to the server it simply tries to reclaim
> > those locks.  This might succeed even though some other client has
> > held and released a lock in the mean time.  So the first client might
> > think the file is unchanged, but it isn't.  This isn't good.
> > 
> > If, when recovery happens, the locks cannot be claimed because some
> > other client still holds the lock, then we get a message in the kernel
> > logs, but the client can still write.  So two clients can both think
> > they have a lock and can both write at the same time.  This is equally
> > not good.
> > 
> > There was a patch a while ago
> >   http://comments.gmane.org/gmane.linux.nfs/41917
> > 
> > which tried to address some of this, but it didn't seem to go
> > anywhere.  That patch would also send a signal to the process.  That
> > might be useful but for now this patch just causes writes to fail.
> > 
> > For NFSv4 (unlike v2/v3) there is a strong link between the lock and
> > the write request so we can fairly easily fail any IO of the lock is
> > gone.  While some applications might not expect this, it is still
> > safer than allowing the write to succeed.
> > 
> > Because this is a fairly big change in behaviour a module parameter,
> > "recover_locks", is introduced which defaults to true (the current
> > behaviour) but can be set to "false" to tell the client not to try to
> > recover things that were lost.
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> 
> Thanks!
> 
> > --
> > This alternative uses a module parameter which defaults to the current,
> > incorrect, behaviour.
> > I suspect we don't want that one..
> 
> Agreed. We also need to document the module parameter, so I'm adding a
> little blurb in Documentation/kernel-parameters.txt (see attachment).
> 
> I'd also like to change the parameter name to "recover_lost_locks" to
> make it a little more obvious.
> 
> Finally, I'd like to move the parameter to fs/nfs/super.c so that we can
> use the same modprobe.conf 'options' lines for back ports of this patch
> (yes I strongly suspect we will want to back port this patch to distro
> kernels).
> 
> See attachment.

Looks good - thanks.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-09-04 23:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-04  7:04 [PATCH - alt-2] NFSv4: Don't try to recover NFSv4 locks when they are lost NeilBrown
2013-09-04 14:31 ` Myklebust, Trond
2013-09-04 23:48   ` NeilBrown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.