NFSD: Rervert "knfsd: locks: flag NFSv4-owned locks"

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock for nfsd4_lock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt always be NULL too.

So that, using fl_lmops cannot checking nfsd4 lockowner for it always be NULL.

This patch save the strhashval in nfs4_lockowner for searching lockownerlike before.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4state.c | 35 +++++++++++++++++++++++------------
 fs/nfsd/state.h     |  1 +
 2 files changed, 24 insertions(+), 12 deletions(-)

fs/nfsd/nfs4state.c +0 -3

index 9b9474c..1b5afc1 100644
@@ -4282,18 +4282,30 @@ nfs4_transform_lock_offset(struct file_lock *lock)
 		lock->fl_end = OFFSET_MAX;
 }
 
-/* Hack!: For now, we're defining this just so we can use a pointer to it
- * as a unique cookie to identify our (NFSv4's) posix locks. */
-static const struct lock_manager_operations nfsd_posix_mng_ops  = {

fs/nfsd/nfs4state.c +14 -8

+	list_for_each_entry(local, &nn->ownerstr_hashtbl[hashval], so_strhash) {
+		if (local == &lo->lo_owner)
+			return true;
+	}
+
+	return false;
+}
 
 static inline void
-nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
+nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny,
+			struct nfsd_net *nn)
 {
-	struct nfs4_lockowner *lo;
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *) fl->fl_owner;
 
-	if (fl->fl_lmops == &nfsd_posix_mng_ops) {
-		lo = (struct nfs4_lockowner *) fl->fl_owner;
+	if (nfs4_verify_lock_stateowner(lo, nn)) {
 		deny->ld_owner.data = kmemdup(lo->lo_owner.so_owner.data,
 					lo->lo_owner.so_owner.len, GFP_KERNEL);
 		if (!deny->ld_owner.data)

@@ -4350,6 +4362,7 @@ alloc_init_lock_stateowner(unsigned int strhashval, struct nfs4_client *clp, str
 	if (!lo)
 		return NULL;
 	INIT_LIST_HEAD(&lo->lo_owner.so_stateids);
+	lo->lo_hashval = strhashval;
 	lo->lo_owner.so_is_open_owner = 0;
 	/* It is the openowner seqid that will be incremented in encode in the
 	 * case of new lockowners; so increment the lock seqid manually: */

@@ -4564,7 +4577,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
 	file_lock->fl_start = lock->lk_offset;
 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
 	nfs4_transform_lock_offset(file_lock);

@@ -4587,7 +4599,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	case (EAGAIN):		/* conflock holds conflicting lock */
 		status = nfserr_denied;
 		dprintk("NFSD: nfsd4_lock: conflicting lock found!\n");
-		nfs4_set_lock_denied(conflock, &lock->lk_denied);
+		nfs4_set_lock_denied(conflock, &lock->lk_denied, nn);
 		break;
 	case (EDEADLK):
 		status = nfserr_deadlock;

@@ -4695,7 +4707,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	if (file_lock->fl_type != F_UNLCK) {
 		status = nfserr_denied;
-		nfs4_set_lock_denied(file_lock, &lockt->lt_denied);
+		nfs4_set_lock_denied(file_lock, &lockt->lt_denied, nn);
 	}
 out:
 	nfs4_unlock_state();

@@ -4746,7 +4758,6 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
 	file_lock->fl_start = locku->lu_offset;
 
 	file_lock->fl_end = last_byte_offset(locku->lu_offset,

fs/nfsd/state.h +0 -0

index 1848fed..9ecae58 100644

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock for nfsd4_lock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt always be NULL too.

So that, using fl_lmops cannot checking nfsd4 lockowner for it always be NULL.

This patch save the strhashval in nfs4_lockowner for searching lockownerlike before.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4state.c | 35 +++++++++++++++++++++++------------
 fs/nfsd/state.h     |  1 +
 2 files changed, 24 insertions(+), 12 deletions(-)

fs/nfsd/nfs4state.c +23 -12

index 9b9474c..1b5afc1 100644
@@ -4282,18 +4282,30 @@ nfs4_transform_lock_offset(struct file_lock *lock)
 		lock->fl_end = OFFSET_MAX;
 }
 
-/* Hack!: For now, we're defining this just so we can use a pointer to it
- * as a unique cookie to identify our (NFSv4's) posix locks. */
-static const struct lock_manager_operations nfsd_posix_mng_ops  = {
-};
+static bool
+nfs4_verify_lock_stateowner(struct nfs4_lockowner *lo, struct nfsd_net *nn)
+{
+	struct nfs4_stateowner *local = NULL;
+	unsigned hashval = lo->lo_hashval;
+
+	if (hashval >= OWNER_HASH_SIZE)
+		return false;
+
+	list_for_each_entry(local, &nn->ownerstr_hashtbl[hashval], so_strhash) {
+		if (local == &lo->lo_owner)
+			return true;
+	}
+
+	return false;
+}
 
 static inline void
-nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
+nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny,
+			struct nfsd_net *nn)
 {
-	struct nfs4_lockowner *lo;
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *) fl->fl_owner;
 
-	if (fl->fl_lmops == &nfsd_posix_mng_ops) {
-		lo = (struct nfs4_lockowner *) fl->fl_owner;
+	if (nfs4_verify_lock_stateowner(lo, nn)) {
 		deny->ld_owner.data = kmemdup(lo->lo_owner.so_owner.data,
 					lo->lo_owner.so_owner.len, GFP_KERNEL);
 		if (!deny->ld_owner.data)

@@ -4350,6 +4362,7 @@ alloc_init_lock_stateowner(unsigned int strhashval, struct nfs4_client *clp, str
 	if (!lo)
 		return NULL;
 	INIT_LIST_HEAD(&lo->lo_owner.so_stateids);
+	lo->lo_hashval = strhashval;
 	lo->lo_owner.so_is_open_owner = 0;
 	/* It is the openowner seqid that will be incremented in encode in the
 	 * case of new lockowners; so increment the lock seqid manually: */

@@ -4564,7 +4577,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
 	file_lock->fl_start = lock->lk_offset;
 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
 	nfs4_transform_lock_offset(file_lock);

@@ -4587,7 +4599,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	case (EAGAIN):		/* conflock holds conflicting lock */
 		status = nfserr_denied;
 		dprintk("NFSD: nfsd4_lock: conflicting lock found!\n");
-		nfs4_set_lock_denied(conflock, &lock->lk_denied);
+		nfs4_set_lock_denied(conflock, &lock->lk_denied, nn);
 		break;
 	case (EDEADLK):
 		status = nfserr_deadlock;

@@ -4695,7 +4707,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	if (file_lock->fl_type != F_UNLCK) {
 		status = nfserr_denied;
-		nfs4_set_lock_denied(file_lock, &lockt->lt_denied);
+		nfs4_set_lock_denied(file_lock, &lockt->lt_denied, nn);
 	}
 out:
 	nfs4_unlock_state();

@@ -4746,7 +4758,6 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
 	file_lock->fl_start = locku->lu_offset;
 
 	file_lock->fl_end = last_byte_offset(locku->lu_offset,

fs/nfsd/state.h +0 -0

index 1848fed..9ecae58 100644

fs/nfsd/state.h +0 -0

 };
 
 static inline struct nfs4_openowner * openowner(struct

nfs4_stateowner *so)

Quoted 25 lines · On Mon, 07 Jul 2014 22:12:21 +0800 Kinglong Mee <[email protected]> wrote:

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock for nfsd4_lock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt always be NULL too.

So that, using fl_lmops cannot checking nfsd4 lockowner for it always be NULL.

This patch save the strhashval in nfs4_lockowner for searching lockownerlike before.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4state.c | 35 +++++++++++++++++++++++------------
 fs/nfsd/state.h     |  1 +
 2 files changed, 24 insertions(+), 12 deletions(-)

fs/nfsd/nfs4state.c +0 -0

index 9b9474c..1b5afc1 100644
@@ -4282,18 +4282,30 @@ nfs4_transform_lock_offset(struct file_lock *lock)
 		lock->fl_end = OFFSET_MAX;
 }
 

fs/nfsd/nfs4state.c nfs4_transform_lock_offset(struct file_lock *lock) · line 4293

Quoted 13 lines

fs/nfsd/nfs4state.c +9 -4

-/* Hack!: For now, we're defining this just so we can use a pointer to it
- * as a unique cookie to identify our (NFSv4's) posix locks. */
-static const struct lock_manager_operations nfsd_posix_mng_ops  = {
-};
+static bool
+nfs4_verify_lock_stateowner(struct nfs4_lockowner *lo, struct nfsd_net *nn)
+{
+	struct nfs4_stateowner *local = NULL;
+	unsigned hashval = lo->lo_hashval;
+
+	if (hashval >= OWNER_HASH_SIZE)
+		return false;
+

^^^^
Maybe a WARN_ON_ONCE or something here would be appropriate?

Quoted 76 lines

fs/nfsd/nfs4state.c +14 -8

+	list_for_each_entry(local, &nn->ownerstr_hashtbl[hashval], so_strhash) {
+		if (local == &lo->lo_owner)
+			return true;
+	}
+
+	return false;
+}
 
 static inline void
-nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
+nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny,
+			struct nfsd_net *nn)
 {
-	struct nfs4_lockowner *lo;
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *) fl->fl_owner;
 
-	if (fl->fl_lmops == &nfsd_posix_mng_ops) {
-		lo = (struct nfs4_lockowner *) fl->fl_owner;
+	if (nfs4_verify_lock_stateowner(lo, nn)) {
 		deny->ld_owner.data = kmemdup(lo->lo_owner.so_owner.data,
 					lo->lo_owner.so_owner.len, GFP_KERNEL);
 		if (!deny->ld_owner.data)

@@ -4350,6 +4362,7 @@ alloc_init_lock_stateowner(unsigned int strhashval, struct nfs4_client *clp, str
 	if (!lo)
 		return NULL;
 	INIT_LIST_HEAD(&lo->lo_owner.so_stateids);
+	lo->lo_hashval = strhashval;
 	lo->lo_owner.so_is_open_owner = 0;
 	/* It is the openowner seqid that will be incremented in encode in the
 	 * case of new lockowners; so increment the lock seqid manually: */

@@ -4564,7 +4577,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
 	file_lock->fl_start = lock->lk_offset;
 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
 	nfs4_transform_lock_offset(file_lock);

@@ -4587,7 +4599,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	case (EAGAIN):		/* conflock holds conflicting lock */
 		status = nfserr_denied;
 		dprintk("NFSD: nfsd4_lock: conflicting lock found!\n");
-		nfs4_set_lock_denied(conflock, &lock->lk_denied);
+		nfs4_set_lock_denied(conflock, &lock->lk_denied, nn);
 		break;
 	case (EDEADLK):
 		status = nfserr_deadlock;

@@ -4695,7 +4707,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	if (file_lock->fl_type != F_UNLCK) {
 		status = nfserr_denied;
-		nfs4_set_lock_denied(file_lock, &lockt->lt_denied);
+		nfs4_set_lock_denied(file_lock, &lockt->lt_denied, nn);
 	}
 out:
 	nfs4_unlock_state();

@@ -4746,7 +4758,6 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
 	file_lock->fl_start = locku->lu_offset;
 
 	file_lock->fl_end = last_byte_offset(locku->lu_offset,

fs/nfsd/state.h +1 -0

index 1848fed..9ecae58 100644
@@ -362,6 +362,7 @@ struct nfs4_openowner {
 
 struct nfs4_lockowner {
 	struct nfs4_stateowner	lo_owner; /* must be first element */
+	unsigned int		lo_hashval;
 };
 

fs/nfsd/state.h struct nfs4_openowner { · line 368

Quoted 1 line

fs/nfsd/state.h +0 -0

 static inline struct nfs4_openowner * openowner(struct nfs4_stateowner *so)

Looks fine other than the comment about the warning. I do have some patches that change the ownerstr_hashtbl to be a per-client entity. I'll need to respin them to account for this change, but that shouldn't be too onerous.

Quoted 108 lines · On Mon, 07 Jul 2014 22:12:21 +0800 Kinglong Mee <[email protected]> wrote:

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock for nfsd4_lock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt always be NULL too.

So that, using fl_lmops cannot checking nfsd4 lockowner for it always be NULL.

This patch save the strhashval in nfs4_lockowner for searching lockownerlike before.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4state.c | 35 +++++++++++++++++++++++------------
 fs/nfsd/state.h     |  1 +
 2 files changed, 24 insertions(+), 12 deletions(-)

fs/nfsd/nfs4state.c +23 -12

index 9b9474c..1b5afc1 100644
@@ -4282,18 +4282,30 @@ nfs4_transform_lock_offset(struct file_lock *lock)
 		lock->fl_end = OFFSET_MAX;
 }
 
-/* Hack!: For now, we're defining this just so we can use a pointer to it
- * as a unique cookie to identify our (NFSv4's) posix locks. */
-static const struct lock_manager_operations nfsd_posix_mng_ops  = {
-};
+static bool
+nfs4_verify_lock_stateowner(struct nfs4_lockowner *lo, struct nfsd_net *nn)
+{
+	struct nfs4_stateowner *local = NULL;
+	unsigned hashval = lo->lo_hashval;
+
+	if (hashval >= OWNER_HASH_SIZE)
+		return false;
+
+	list_for_each_entry(local, &nn->ownerstr_hashtbl[hashval], so_strhash) {
+		if (local == &lo->lo_owner)
+			return true;
+	}
+
+	return false;
+}
 
 static inline void
-nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
+nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny,
+			struct nfsd_net *nn)
 {
-	struct nfs4_lockowner *lo;
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *) fl->fl_owner;
 
-	if (fl->fl_lmops == &nfsd_posix_mng_ops) {
-		lo = (struct nfs4_lockowner *) fl->fl_owner;
+	if (nfs4_verify_lock_stateowner(lo, nn)) {
 		deny->ld_owner.data = kmemdup(lo->lo_owner.so_owner.data,
 					lo->lo_owner.so_owner.len, GFP_KERNEL);
 		if (!deny->ld_owner.data)

@@ -4350,6 +4362,7 @@ alloc_init_lock_stateowner(unsigned int strhashval, struct nfs4_client *clp, str
 	if (!lo)
 		return NULL;
 	INIT_LIST_HEAD(&lo->lo_owner.so_stateids);
+	lo->lo_hashval = strhashval;
 	lo->lo_owner.so_is_open_owner = 0;
 	/* It is the openowner seqid that will be incremented in encode in the
 	 * case of new lockowners; so increment the lock seqid manually: */

@@ -4564,7 +4577,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
 	file_lock->fl_start = lock->lk_offset;
 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
 	nfs4_transform_lock_offset(file_lock);

@@ -4587,7 +4599,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	case (EAGAIN):		/* conflock holds conflicting lock */
 		status = nfserr_denied;
 		dprintk("NFSD: nfsd4_lock: conflicting lock found!\n");
-		nfs4_set_lock_denied(conflock, &lock->lk_denied);
+		nfs4_set_lock_denied(conflock, &lock->lk_denied, nn);
 		break;
 	case (EDEADLK):
 		status = nfserr_deadlock;

@@ -4695,7 +4707,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	if (file_lock->fl_type != F_UNLCK) {
 		status = nfserr_denied;
-		nfs4_set_lock_denied(file_lock, &lockt->lt_denied);
+		nfs4_set_lock_denied(file_lock, &lockt->lt_denied, nn);
 	}
 out:
 	nfs4_unlock_state();

@@ -4746,7 +4758,6 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
 	file_lock->fl_start = locku->lu_offset;
 
 	file_lock->fl_end = last_byte_offset(locku->lu_offset,

fs/nfsd/state.h +0 -0

index 1848fed..9ecae58 100644
@@ -362,6 +362,7 @@ struct nfs4_openowner {
 

fs/nfsd/state.h struct nfs4_openowner { · line 365

Quoted 3 lines

fs/nfsd/state.h +1 -0

 struct nfs4_lockowner {
 	struct nfs4_stateowner	lo_owner; /* must be first element */
+	unsigned int		lo_hashval;

Actually, I have another comment here too. I'm not sure there's a lot of value in tracking the lo_hashval here. Why not just recompute it in nfs4_verify_lock_stateowner?

You'd need to pass down a pointer to the nfs4_client in order to do so, but you could do that instead of passing down 'nn' and then just call net_generic on clp->net where you needed it.

Quoted 111 lines · Jeff Layton

Quoted 108 lines · On Mon, 07 Jul 2014 22:12:21 +0800 Kinglong Mee <[email protected]> wrote:

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock for nfsd4_lock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt always be NULL too.

So that, using fl_lmops cannot checking nfsd4 lockowner for it always be NULL.

This patch save the strhashval in nfs4_lockowner for searching lockownerlike before.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4state.c | 35 +++++++++++++++++++++++------------
 fs/nfsd/state.h     |  1 +
 2 files changed, 24 insertions(+), 12 deletions(-)

fs/nfsd/nfs4state.c +23 -12

index 9b9474c..1b5afc1 100644
@@ -4282,18 +4282,30 @@ nfs4_transform_lock_offset(struct file_lock *lock)
 		lock->fl_end = OFFSET_MAX;
 }
 
-/* Hack!: For now, we're defining this just so we can use a pointer to it
- * as a unique cookie to identify our (NFSv4's) posix locks. */
-static const struct lock_manager_operations nfsd_posix_mng_ops  = {
-};
+static bool
+nfs4_verify_lock_stateowner(struct nfs4_lockowner *lo, struct nfsd_net *nn)
+{
+	struct nfs4_stateowner *local = NULL;
+	unsigned hashval = lo->lo_hashval;
+
+	if (hashval >= OWNER_HASH_SIZE)
+		return false;
+
+	list_for_each_entry(local, &nn->ownerstr_hashtbl[hashval], so_strhash) {
+		if (local == &lo->lo_owner)
+			return true;
+	}
+
+	return false;
+}
 
 static inline void
-nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
+nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny,
+			struct nfsd_net *nn)
 {
-	struct nfs4_lockowner *lo;
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *) fl->fl_owner;
 
-	if (fl->fl_lmops == &nfsd_posix_mng_ops) {
-		lo = (struct nfs4_lockowner *) fl->fl_owner;
+	if (nfs4_verify_lock_stateowner(lo, nn)) {
 		deny->ld_owner.data = kmemdup(lo->lo_owner.so_owner.data,
 					lo->lo_owner.so_owner.len, GFP_KERNEL);
 		if (!deny->ld_owner.data)

@@ -4350,6 +4362,7 @@ alloc_init_lock_stateowner(unsigned int strhashval, struct nfs4_client *clp, str
 	if (!lo)
 		return NULL;
 	INIT_LIST_HEAD(&lo->lo_owner.so_stateids);
+	lo->lo_hashval = strhashval;
 	lo->lo_owner.so_is_open_owner = 0;
 	/* It is the openowner seqid that will be incremented in encode in the
 	 * case of new lockowners; so increment the lock seqid manually: */

@@ -4564,7 +4577,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
 	file_lock->fl_start = lock->lk_offset;
 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
 	nfs4_transform_lock_offset(file_lock);

@@ -4587,7 +4599,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	case (EAGAIN):		/* conflock holds conflicting lock */
 		status = nfserr_denied;
 		dprintk("NFSD: nfsd4_lock: conflicting lock found!\n");
-		nfs4_set_lock_denied(conflock, &lock->lk_denied);
+		nfs4_set_lock_denied(conflock, &lock->lk_denied, nn);
 		break;
 	case (EDEADLK):
 		status = nfserr_deadlock;

@@ -4695,7 +4707,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	if (file_lock->fl_type != F_UNLCK) {
 		status = nfserr_denied;
-		nfs4_set_lock_denied(file_lock, &lockt->lt_denied);
+		nfs4_set_lock_denied(file_lock, &lockt->lt_denied, nn);
 	}
 out:
 	nfs4_unlock_state();

@@ -4746,7 +4758,6 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
 	file_lock->fl_start = locku->lu_offset;
 
 	file_lock->fl_end = last_byte_offset(locku->lu_offset,

fs/nfsd/state.h +0 -0

index 1848fed..9ecae58 100644
@@ -362,6 +362,7 @@ struct nfs4_openowner {
 

Quoted 7 lines

fs/nfsd/state.h struct nfs4_openowner { · line 365

Quoted 3 lines

fs/nfsd/state.h +1 -0

 struct nfs4_lockowner {
 	struct nfs4_stateowner	lo_owner; /* must be first element */
+	unsigned int		lo_hashval;

Actually, I have another comment here too. I'm not sure there's a lot of value in tracking the lo_hashval here. Why not just recompute it in nfs4_verify_lock_stateowner?

No, ownerstr_hashval() needs a client which holds the conflicting lock, in nfsd4_lock/nfsd4_lockt cannot know the client from vfs_lock_file()
or vfs_test_lock().

Quoted 4 lines

You'd need to pass down a pointer to the nfs4_client in order to do so, but you could do that instead of passing down 'nn' and then just call net_generic on clp->net where you needed it.

I find patch "[PATCH v3 069/114] nfsd: Move the open owner hash table into struct nfs4_client" have remove the client argument from ownerstr_hashval().

So, the lo_hashval isn't needed after that patch.
After Bruce merging that patch, I will resend a new patch recomputing the hash value instead recording in lo_hashval.

Quoted 4 lines · On Tue, 08 Jul 2014 11:23:49 +0800 Kinglong Mee <[email protected]> wrote:

Quoted 3 lines · Jeff Layton

On Mon, 07 Jul 2014 22:12:21 +0800
Kinglong Mee <[email protected]> wrote:

-------------------------[snip]------------------------------

[PATCH] NFSD: Rervert "knfsd: locks: flag NFSv4-owned locks"

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock for nfsd4_lock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt always be NULL too.

So that, using fl_lmops cannot checking nfsd4 lockowner for it always be NULL.

Signed-off-by Kinglong Mee <[email protected]>

Signed-off-by Jeff Layton <[email protected]>

---
 fs/nfsd/nfs4state.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

fs/nfsd/nfs4state.c +0 -1

index c400ec17915e..00cd7fd17b0f 100644
@@ -4312,18 +4312,29 @@ nfs4_transform_lock_offset(struct file_lock *lock)
 		lock->fl_end = OFFSET_MAX;
 }
 
-/* Hack!: For now, we're defining this just so we can use a pointer to it

fs/nfsd/nfs4state.c +14 -6

+
+	list_for_each_entry(local, &nn->ownerstr_hashtbl[hashval], so_strhash) {
+		if (local == &lo->lo_owner)
+			return true;
+	}
+
+	return false;
+}
 
 static inline void
-nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
+nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny,
+			struct nfs4_client *clp)
 {
-	struct nfs4_lockowner *lo;
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *)fl->fl_owner;
 
-	if (fl->fl_lmops == &nfsd_posix_mng_ops) {
-		lo = (struct nfs4_lockowner *) fl->fl_owner;
+	if (nfs4_verify_lock_stateowner(lo, clp)) {
 		deny->ld_owner.data = kmemdup(lo->lo_owner.so_owner.data,
 					lo->lo_owner.so_owner.len, GFP_KERNEL);
 		if (!deny->ld_owner.data)

@@ -4594,7 +4605,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
 	file_lock->fl_start = lock->lk_offset;
 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
 	nfs4_transform_lock_offset(file_lock);

@@ -4617,7 +4627,8 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	case (EAGAIN):		/* conflock holds conflicting lock */
 		status = nfserr_denied;
 		dprintk("NFSD: nfsd4_lock: conflicting lock found!\n");
-		nfs4_set_lock_denied(conflock, &lock->lk_denied);
+		nfs4_set_lock_denied(conflock, &lock->lk_denied,
+					cstate->clp);
 		break;
 	case (EDEADLK):
 		status = nfserr_deadlock;

Quoted 35 lines · Jeff Layton

Quoted 4 lines · On Tue, 08 Jul 2014 11:23:49 +0800 Kinglong Mee <[email protected]> wrote:

Quoted 3 lines · Jeff Layton

On Mon, 07 Jul 2014 22:12:21 +0800
Kinglong Mee <[email protected]> wrote:

-------------------------[snip]------------------------------

[PATCH] NFSD: Rervert "knfsd: locks: flag NFSv4-owned locks"

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock for nfsd4_lock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt always be NULL too.

So that, using fl_lmops cannot checking nfsd4 lockowner for it always be NULL.

Signed-off-by Kinglong Mee <[email protected]>

Signed-off-by Jeff Layton <[email protected]>

---
 fs/nfsd/nfs4state.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

fs/nfsd/nfs4state.c +0 -1

index c400ec17915e..00cd7fd17b0f 100644
@@ -4312,18 +4312,29 @@ nfs4_transform_lock_offset(struct file_lock *lock)
 		lock->fl_end = OFFSET_MAX;
 }
 
-/* Hack!: For now, we're defining this just so we can use a pointer to it

fs/nfsd/nfs4state.c nfs4_transform_lock_offset(struct file_lock *lock) · line 4321

Quoted 10 lines

fs/nfsd/nfs4state.c +7 -3

- * as a unique cookie to identify our (NFSv4's) posix locks. */
-static const struct lock_manager_operations nfsd_posix_mng_ops  = {
-};
+static bool
+nfs4_verify_lock_stateowner(struct nfs4_lockowner *lo, struct nfs4_client *clp)
+{
+	struct nfs4_stateowner *local = NULL;
+	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+	unsigned int hashval = ownerstr_hashval(clp->cl_clientid.cl_id,
+						&lo->lo_owner.so_owner);

Needs the conflicting client which holds the lock, Not the new client which try to locking the file has be locked.

Quoted 42 lines

fs/nfsd/nfs4state.c +14 -6

+
+	list_for_each_entry(local, &nn->ownerstr_hashtbl[hashval], so_strhash) {
+		if (local == &lo->lo_owner)
+			return true;
+	}
+
+	return false;
+}
 
 static inline void
-nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
+nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny,
+			struct nfs4_client *clp)
 {
-	struct nfs4_lockowner *lo;
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *)fl->fl_owner;
 
-	if (fl->fl_lmops == &nfsd_posix_mng_ops) {
-		lo = (struct nfs4_lockowner *) fl->fl_owner;
+	if (nfs4_verify_lock_stateowner(lo, clp)) {
 		deny->ld_owner.data = kmemdup(lo->lo_owner.so_owner.data,
 					lo->lo_owner.so_owner.len, GFP_KERNEL);
 		if (!deny->ld_owner.data)

@@ -4594,7 +4605,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
 	file_lock->fl_start = lock->lk_offset;
 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
 	nfs4_transform_lock_offset(file_lock);

@@ -4617,7 +4627,8 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	case (EAGAIN):		/* conflock holds conflicting lock */
 		status = nfserr_denied;
 		dprintk("NFSD: nfsd4_lock: conflicting lock found!\n");
-		nfs4_set_lock_denied(conflock, &lock->lk_denied);
+		nfs4_set_lock_denied(conflock, &lock->lk_denied,
+					cstate->clp);
 		break;
 	case (EDEADLK):
 		status = nfserr_deadlock;

fs/nfsd/nfs4state.c nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, · line 4739

Quoted 5 lines

fs/nfsd/nfs4state.c +1 -1

@@ -4725,7 +4736,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	if (file_lock->fl_type != F_UNLCK) {
 		status = nfserr_denied;
-		nfs4_set_lock_denied(file_lock, &lockt->lt_denied);
+		nfs4_set_lock_denied(file_lock, &lockt->lt_denied, cstate->clp);

The clp is the client which try to lock the file here, but the file have be locked by other client or server.

With this patch, the hashval is wrong.
*correct* *this patch*
clid conflicting client new request lock client
owner from vfs_lock_file() from vfs_lock_file()
or vfs_test_lock() or vfs_test_lock()
hashval right hashval bad hashval (never exist)

With this patch, LOCK/LOCKT compound gets client with all zero, and owner with all zero.

thanks,
Kinglong Mee
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

<facepalm>
Ahh ok, I see what you're saying now. I'll leave it to you to respin once the rest of the client_mutex removal series is merged. </facepalm>

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock for nfsd4_lock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt always be NULL too.

So that, nfsd4 lockowner for it always be NULL too.

This patch re-coping the fl_lmops to conflock.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/locks.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

fs/locks.c +1 -1

index 717fbc4..cc1219a 100644
@@ -279,7 +279,7 @@ void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
        new->fl_start = fl->fl_start;
        new->fl_end = fl->fl_end;
        new->fl_ops = NULL;
-       new->fl_lmops = NULL;
+       new->fl_lmops = fl->fl_lmops;
 }
 EXPORT_SYMBOL(__locks_copy_lock);
 

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock for nfsd4_lock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt always be NULL too.

So that, nfsd4 lockowner for it always be NULL too.

This patch re-coping the fl_lmops to conflock.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/locks.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

fs/locks.c +1 -1

index 717fbc4..cc1219a 100644
@@ -279,7 +279,7 @@ void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
 	new->fl_start = fl->fl_start;
 	new->fl_end = fl->fl_end;
 	new->fl_ops = NULL;
-	new->fl_lmops = NULL;
+	new->fl_lmops = fl->fl_lmops;
 }
 EXPORT_SYMBOL(__locks_copy_lock);
 

Quoted 39 lines · On Sat, 02 Aug 2014 22:45:17 +0800 Kinglong Mee…

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock for nfsd4_lock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt always be NULL too.

So that, nfsd4 lockowner for it always be NULL too.

This patch re-coping the fl_lmops to conflock.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/locks.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

fs/locks.c +1 -2

index 717fbc4..cc1219a 100644
@@ -279,7 +279,7 @@ void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
 	new->fl_start = fl->fl_start;
 	new->fl_end = fl->fl_end;
 	new->fl_ops = NULL;
-	new->fl_lmops = NULL;
+	new->fl_lmops = fl->fl_lmops;
 }
 EXPORT_SYMBOL(__locks_copy_lock);
 

@@ -290,7 +290,6 @@ void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 	__locks_copy_lock(new, fl);
 	new->fl_file = fl->fl_file;
 	new->fl_ops = fl->fl_ops;
-	new->fl_lmops = fl->fl_lmops;
 

fs/locks.c void locks_copy_lock(struct file_lock *new, struct file_lock *fl) · line 295

Quoted 2 lines

fs/locks.c +0 -0

 	locks_copy_private(new, fl);
 }

This looks sane to me AFAICT.

I'll run a few tests with it, and put it into I'll plan to pick this up since I have some other fs/locks.c related patches slated for 3.17.

Quoted 47 lines · Jeff Layton

Quoted 34 lines · On Sat, 02 Aug 2014 22:45:17 +0800 Kinglong Mee…

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock for nfsd4_lock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt always be NULL too.

So that, nfsd4 lockowner for it always be NULL too.

This patch re-coping the fl_lmops to conflock.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/locks.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

fs/locks.c +1 -1

index 717fbc4..cc1219a 100644
@@ -279,7 +279,7 @@ void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
 	new->fl_start = fl->fl_start;
 	new->fl_end = fl->fl_end;
 	new->fl_ops = NULL;
-	new->fl_lmops = NULL;
+	new->fl_lmops = fl->fl_lmops;
 }
 EXPORT_SYMBOL(__locks_copy_lock);
 

fs/locks.c void locks_copy_lock(struct file_lock *new, struct file_lock *fl) · line 295

Quoted 7 lines

fs/locks.c +0 -1

@@ -290,7 +290,6 @@ void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 	__locks_copy_lock(new, fl);
 	new->fl_file = fl->fl_file;
 	new->fl_ops = fl->fl_ops;
-	new->fl_lmops = fl->fl_lmops;
 
 	locks_copy_private(new, fl);
 }

This looks sane to me AFAICT.

Quoted 2 lines

I'll run a few tests with it, and put it into I'll plan to pick this up since I have some other fs/locks.c related patches slated for 3.17.

Looks like your mail and Trond's crossed? I'd need to go remind myself of the __locks_copy_lock callers, but I'm pretty sure Trond's right....

--b.

Using fl_flags instead of fl_lmops for marking file_lock belongs to NFSD.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4state.c | 15 ++++-----------
 include/linux/fs.h  |  1 +
 2 files changed, 5 insertions(+), 11 deletions(-)

fs/nfsd/nfs4state.c +4 -11

index 2e80a59..24168ae 100644
@@ -4867,17 +4867,12 @@ nfs4_transform_lock_offset(struct file_lock *lock)
 		lock->fl_end = OFFSET_MAX;
 }
 
-/* Hack!: For now, we're defining this just so we can use a pointer to it
- * as a unique cookie to identify our (NFSv4's) posix locks. */
-static const struct lock_manager_operations nfsd_posix_mng_ops  = {
-};
-
 static inline void
 nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
 {
 	struct nfs4_lockowner *lo;
 
-	if (fl->fl_lmops == &nfsd_posix_mng_ops) {
+	if (fl->fl_flags & FL_NFSD) {
 		lo = (struct nfs4_lockowner *) fl->fl_owner;
 		deny->ld_owner.data = kmemdup(lo->lo_owner.so_owner.data,
 					lo->lo_owner.so_owner.len, GFP_KERNEL);

@@ -5241,8 +5236,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_owner = (fl_owner_t)lock_sop;
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
-	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
+	file_lock->fl_flags = FL_POSIX | FL_NFSD;
 	file_lock->fl_start = lock->lk_offset;
 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
 	nfs4_transform_lock_offset(file_lock);

@@ -5375,7 +5369,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	if (lo)
 		file_lock->fl_owner = (fl_owner_t)lo;
 	file_lock->fl_pid = current->tgid;
-	file_lock->fl_flags = FL_POSIX;
+	file_lock->fl_flags = FL_POSIX | FL_NFSD;
 
 	file_lock->fl_start = lockt->lt_offset;
 	file_lock->fl_end = last_byte_offset(lockt->lt_offset, lockt->lt_length);

@@ -5437,8 +5431,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_owner = (fl_owner_t)lockowner(stp->st_stateowner);
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
-	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
+	file_lock->fl_flags = FL_POSIX | FL_NFSD;
 	file_lock->fl_start = locku->lu_offset;
 
 	file_lock->fl_end = last_byte_offset(locku->lu_offset,

include/linux/fs.h +0 -0

index e11d60c..4d40097 100644

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4state.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

fs/nfsd/nfs4state.c +29 -1

index 24168ae..07e4b5c 100644
@@ -4867,6 +4867,29 @@ nfs4_transform_lock_offset(struct file_lock *lock)
 		lock->fl_end = OFFSET_MAX;
 }
 
+static inline struct nfs4_lockowner *get_lockowner(struct nfs4_lockowner *lo)
+{
+	atomic_inc(&lo->lo_owner.so_count);
+	return lo;
+}
+
+static void nfsd4_fl_copy_lock(struct file_lock *dst, struct file_lock *src)
+{
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *) src->fl_owner;
+	get_lockowner(lo);
+}
+
+static void nfsd4_fl_release_lock(struct file_lock *fl)
+{
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *) fl->fl_owner;
+	nfs4_put_stateowner(&lo->lo_owner);
+}
+
+static const struct file_lock_operations nfsd4_fl_lock_ops = {
+	.fl_copy_lock = nfsd4_fl_copy_lock,
+	.fl_release_private = nfsd4_fl_release_lock,
+};
+
 static inline void
 nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
 {

@@ -5233,10 +5256,12 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		status = nfserr_openmode;
 		goto out;
 	}
-	file_lock->fl_owner = (fl_owner_t)lock_sop;
+
+	file_lock->fl_owner = (fl_owner_t)get_lockowner(lock_sop);
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX | FL_NFSD;
+	file_lock->fl_ops = &nfsd4_fl_lock_ops;
 	file_lock->fl_start = lock->lk_offset;
 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
 	nfs4_transform_lock_offset(file_lock);

@@ -5399,6 +5424,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfs4_ol_stateid *stp;
 	struct file *filp = NULL;
 	struct file_lock *file_lock = NULL;
+	struct nfs4_lockowner *lock_sop = NULL;
 	__be32 status;
 	int err;
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);

@@ -5420,6 +5446,8 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		status = nfserr_lock_range;
 		goto put_stateid;
 	}
+
+	lock_sop = lockowner(stp->st_stateowner);
 	file_lock = locks_alloc_lock();
 	if (!file_lock) {
 		dprintk("NFSD: %s: unable to allocate lock!\n", __func__);

@@ -5428,10 +5456,11 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	}
 	locks_init_lock(file_lock);

Quoted 65 lines · On Wed, 06 Aug 2014 21:33:25 +0800 Kinglong Mee…

Using fl_flags instead of fl_lmops for marking file_lock belongs to NFSD.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4state.c | 15 ++++-----------
 include/linux/fs.h  |  1 +
 2 files changed, 5 insertions(+), 11 deletions(-)

fs/nfsd/nfs4state.c +4 -11

index 2e80a59..24168ae 100644
@@ -4867,17 +4867,12 @@ nfs4_transform_lock_offset(struct file_lock *lock)
 		lock->fl_end = OFFSET_MAX;
 }
 
-/* Hack!: For now, we're defining this just so we can use a pointer to it
- * as a unique cookie to identify our (NFSv4's) posix locks. */
-static const struct lock_manager_operations nfsd_posix_mng_ops  = {
-};
-
 static inline void
 nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
 {
 	struct nfs4_lockowner *lo;
 
-	if (fl->fl_lmops == &nfsd_posix_mng_ops) {
+	if (fl->fl_flags & FL_NFSD) {
 		lo = (struct nfs4_lockowner *) fl->fl_owner;
 		deny->ld_owner.data = kmemdup(lo->lo_owner.so_owner.data,
 					lo->lo_owner.so_owner.len, GFP_KERNEL);

@@ -5241,8 +5236,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_owner = (fl_owner_t)lock_sop;
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
-	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
+	file_lock->fl_flags = FL_POSIX | FL_NFSD;
 	file_lock->fl_start = lock->lk_offset;
 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
 	nfs4_transform_lock_offset(file_lock);

@@ -5375,7 +5369,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	if (lo)
 		file_lock->fl_owner = (fl_owner_t)lo;
 	file_lock->fl_pid = current->tgid;
-	file_lock->fl_flags = FL_POSIX;
+	file_lock->fl_flags = FL_POSIX | FL_NFSD;
 
 	file_lock->fl_start = lockt->lt_offset;
 	file_lock->fl_end = last_byte_offset(lockt->lt_offset, lockt->lt_length);

@@ -5437,8 +5431,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_owner = (fl_owner_t)lockowner(stp->st_stateowner);
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
-	file_lock->fl_flags = FL_POSIX;
-	file_lock->fl_lmops = &nfsd_posix_mng_ops;
+	file_lock->fl_flags = FL_POSIX | FL_NFSD;
 	file_lock->fl_start = locku->lu_offset;
 
 	file_lock->fl_end = last_byte_offset(locku->lu_offset,

include/linux/fs.h +0 -0

index e11d60c..4d40097 100644

include/linux/fs.h static inline struct file *get_file(struct file *f) · line 825

Quoted 7 lines

include/linux/fs.h +1 -0

@@ -819,6 +819,7 @@ static inline struct file *get_file(struct file *f)
 #define FL_DOWNGRADE_PENDING	256 /* Lease is being downgraded */
 #define FL_UNLOCK_PENDING	512 /* Lease is being broken */
 #define FL_OFDLCK	1024	/* lock is "owned" by struct file */
+#define FL_NFSD		2048	/* NFSD holds this lock */
 
 /*
  * Special return value from posix_lock_file() and vfs_lock_file() for

Honestly, I'd prefer not using a fl_flag for this. Why, might you ask?

Currently, there are two alternate lock managers in the kernel (nfsd and lockd) but there could (in principle) be an arbitrary number of them later. What's so special about knfsd that it warrants its own flag?

I think existing use of an empty lock_manager_operations is a cleaner solution than this for identifying locks owned by knfsd.

Quoted 73 lines · On Wed, 06 Aug 2014 21:35:40 +0800 Kinglong Mee…

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4state.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

fs/nfsd/nfs4state.c +29 -1

index 24168ae..07e4b5c 100644
@@ -4867,6 +4867,29 @@ nfs4_transform_lock_offset(struct file_lock *lock)
 		lock->fl_end = OFFSET_MAX;
 }
 
+static inline struct nfs4_lockowner *get_lockowner(struct nfs4_lockowner *lo)
+{
+	atomic_inc(&lo->lo_owner.so_count);
+	return lo;
+}
+
+static void nfsd4_fl_copy_lock(struct file_lock *dst, struct file_lock *src)
+{
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *) src->fl_owner;
+	get_lockowner(lo);
+}
+
+static void nfsd4_fl_release_lock(struct file_lock *fl)
+{
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *) fl->fl_owner;
+	nfs4_put_stateowner(&lo->lo_owner);
+}
+
+static const struct file_lock_operations nfsd4_fl_lock_ops = {
+	.fl_copy_lock = nfsd4_fl_copy_lock,
+	.fl_release_private = nfsd4_fl_release_lock,
+};
+
 static inline void
 nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
 {

@@ -5233,10 +5256,12 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		status = nfserr_openmode;
 		goto out;
 	}
-	file_lock->fl_owner = (fl_owner_t)lock_sop;
+
+	file_lock->fl_owner = (fl_owner_t)get_lockowner(lock_sop);
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX | FL_NFSD;
+	file_lock->fl_ops = &nfsd4_fl_lock_ops;
 	file_lock->fl_start = lock->lk_offset;
 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
 	nfs4_transform_lock_offset(file_lock);

@@ -5399,6 +5424,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfs4_ol_stateid *stp;
 	struct file *filp = NULL;
 	struct file_lock *file_lock = NULL;
+	struct nfs4_lockowner *lock_sop = NULL;
 	__be32 status;
 	int err;
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);

@@ -5420,6 +5446,8 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		status = nfserr_lock_range;
 		goto put_stateid;
 	}
+
+	lock_sop = lockowner(stp->st_stateowner);
 	file_lock = locks_alloc_lock();
 	if (!file_lock) {
 		dprintk("NFSD: %s: unable to allocate lock!\n", __func__);

@@ -5428,10 +5456,11 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	}
 	locks_init_lock(file_lock);

fs/nfsd/nfs4state.c nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, · line 5466

Quoted 10 lines

fs/nfsd/nfs4state.c +2 -1

 	file_lock->fl_type = F_UNLCK;
-	file_lock->fl_owner = (fl_owner_t)lockowner(stp->st_stateowner);
+	file_lock->fl_owner = (fl_owner_t)get_lockowner(lock_sop);
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX | FL_NFSD;
+	file_lock->fl_ops = &nfsd4_fl_lock_ops;
 	file_lock->fl_start = locku->lu_offset;
 
 	file_lock->fl_end = last_byte_offset(locku->lu_offset,

(Sorry I didn't respond to these before -- I no longer work for Red
Hat, so emails to [email protected] don't get to me).

This looks really wrong. In this case, knfsd is acting as a (server-side) lock manager, not a filesystem:

const struct file_lock_operations *fl_ops; /* Callbacks for filesystems */
const struct lock_manager_operations *fl_lmops; /* Callbacks for lockmanagers */

So really, taking and putting of lockowner references is really the purview of the lockmanager. If you had a filesystem that set fl_ops and was exportable, then this will likely break it.

The fact that the file locking code uses struct file_lock for lock requests and for tracking the locks themselves makes for a horribly confusing interface. If I were designing this from scratch I would have made those two distinct structures. Maybe we should do that anyway sometime.

What you probably ought want to do is to declare some new fl_lmops for taking and putting lockowner references and add hooks to call them at the appropriate places.

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock always be NULL too.

v2: Only change the order from 3/3 to 1/3 now.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/lockd/svclock.c |  2 +-
 fs/locks.c         | 25 ++++++-------------------
 include/linux/fs.h |  6 ------
 3 files changed, 7 insertions(+), 26 deletions(-)

fs/lockd/svclock.c +1 -1

index ab798a8..e1f209c 100644
@@ -677,7 +677,7 @@ nlmsvc_update_deferred_block(struct nlm_block *block, struct file_lock *conf,
 		block->b_flags |= B_TIMED_OUT;
 	if (conf) {
 		if (block->b_fl)
-			__locks_copy_lock(block->b_fl, conf);
+			locks_copy_lock(block->b_fl, conf);
 	}
 }
 

fs/locks.c +4 -9

index 717fbc4..91b0f03 100644
@@ -266,35 +266,22 @@ static void locks_copy_private(struct file_lock *new, struct file_lock *fl)
 		new->fl_lmops = fl->fl_lmops;
 }
 
-/*
- * Initialize a new lock from an existing file_lock structure.
- */
-void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
+void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 {
+	locks_release_private(new);
+
 	new->fl_owner = fl->fl_owner;
 	new->fl_pid = fl->fl_pid;
-	new->fl_file = NULL;
+	new->fl_file = fl->fl_file;
 	new->fl_flags = fl->fl_flags;
 	new->fl_type = fl->fl_type;
 	new->fl_start = fl->fl_start;
 	new->fl_end = fl->fl_end;
 	new->fl_ops = NULL;
 	new->fl_lmops = NULL;
-}
-EXPORT_SYMBOL(__locks_copy_lock);
-
-void locks_copy_lock(struct file_lock *new, struct file_lock *fl)

fs/locks.c +2 -3

-
 EXPORT_SYMBOL(locks_copy_lock);
 
 static inline int flock_translate_cmd(int cmd) {

@@ -718,7 +705,7 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 			break;
 	}
 	if (cfl) {
-		__locks_copy_lock(fl, cfl);
+		locks_copy_lock(fl, cfl);
 		if (cfl->fl_nspid)
 			fl->fl_pid = pid_vnr(cfl->fl_nspid);
 	} else

@@ -921,7 +908,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 			if (!posix_locks_conflict(request, fl))
 				continue;
 			if (conflock)
-				__locks_copy_lock(conflock, fl);
+				locks_copy_lock(conflock, fl);
 			error = -EAGAIN;
 			if (!(request->fl_flags & FL_SLEEP))
 				goto out;

include/linux/fs.h +0 -6

index e11d60c..ced023d 100644
@@ -941,7 +941,6 @@ void locks_free_lock(struct file_lock *fl);
 extern void locks_init_lock(struct file_lock *);
 extern struct file_lock * locks_alloc_lock(void);
 extern void locks_copy_lock(struct file_lock *, struct file_lock *);
-extern void __locks_copy_lock(struct file_lock *, const struct file_lock *);
 extern void locks_remove_posix(struct file *, fl_owner_t);
 extern void locks_remove_file(struct file *);
 extern void locks_release_private(struct file_lock *);

@@ -1001,11 +1000,6 @@ static inline void locks_init_lock(struct file_lock *fl)
 	return;
 }
 
-static inline void __locks_copy_lock(struct file_lock *new, struct file_lock *fl)
-{
-	return;
-}
-
 static inline void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 {
 	return;

Quoted 69 lines · On Sun, 10 Aug 2014 23:38:25 +0800 Kinglong Mee…

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock always be NULL too.

v2: Only change the order from 3/3 to 1/3 now.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/lockd/svclock.c |  2 +-
 fs/locks.c         | 25 ++++++-------------------
 include/linux/fs.h |  6 ------
 3 files changed, 7 insertions(+), 26 deletions(-)

fs/lockd/svclock.c +1 -1

index ab798a8..e1f209c 100644
@@ -677,7 +677,7 @@ nlmsvc_update_deferred_block(struct nlm_block *block, struct file_lock *conf,
 		block->b_flags |= B_TIMED_OUT;
 	if (conf) {
 		if (block->b_fl)
-			__locks_copy_lock(block->b_fl, conf);
+			locks_copy_lock(block->b_fl, conf);
 	}
 }
 

fs/locks.c +4 -16

index 717fbc4..91b0f03 100644
@@ -266,35 +266,22 @@ static void locks_copy_private(struct file_lock *new, struct file_lock *fl)
 		new->fl_lmops = fl->fl_lmops;
 }
 
-/*
- * Initialize a new lock from an existing file_lock structure.
- */
-void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
+void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 {
+	locks_release_private(new);
+
 	new->fl_owner = fl->fl_owner;
 	new->fl_pid = fl->fl_pid;
-	new->fl_file = NULL;
+	new->fl_file = fl->fl_file;
 	new->fl_flags = fl->fl_flags;
 	new->fl_type = fl->fl_type;
 	new->fl_start = fl->fl_start;
 	new->fl_end = fl->fl_end;
 	new->fl_ops = NULL;
 	new->fl_lmops = NULL;
-}
-EXPORT_SYMBOL(__locks_copy_lock);
-
-void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
-{
-	locks_release_private(new);
-
-	__locks_copy_lock(new, fl);
-	new->fl_file = fl->fl_file;
-	new->fl_ops = fl->fl_ops;
-	new->fl_lmops = fl->fl_lmops;
 

fs/locks.c static void locks_copy_private(struct file_lock *new, struct file_lock *fl) · line 284

Quoted 2 lines

fs/locks.c +0 -0

 	locks_copy_private(new, fl);
 }

(cc'ing Joe Perches)

Ok, so you're basically just reverting 1a747ee0cc11a19. The catch there is that you now need to ensure that any conflock structures are properly initialized before passing them to locks_copy_lock.

The nfsv4 server code currently doesn't do that and it will need to be fixed to do so or that will be a regression.

For the NLM code, Joe Perches has proposed a patch to remove the conflock parameter from lm_grant since the callers always pass in NULL anyway. You may want to pull in his patch and rebase yours on top of it since it'll remove that __locks_copy_lock call altogether.

Joe, is Andrew merging that patch or do I need to pull it into the locks tree?

v2: Fix bad using of struct file_lock_operations for handle the owner.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4state.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

fs/nfsd/nfs4state.c +0 -0

index 2e80a59..24a8d91 100644

fs/nfsd/nfs4state.c +8 -0

+static void nfsd4_fl_copy_owner(struct file_lock *dst, struct file_lock *src)
+{
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *) src->fl_owner;
+	dst->fl_owner = (fl_owner_t) get_lockowner(lo);
+}
+
+static void nfsd4_fl_release_owner(struct file_lock *fl)
+{

static const struct lock_manager_operations nfsd_posix_mng_ops = {

fs/nfsd/nfs4state.c +8 -2

+	.lm_copy_owner = nfsd4_fl_copy_owner,
+	.lm_release_owner = nfsd4_fl_release_owner,
 };
 
 static inline void

@@ -5238,7 +5262,8 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		status = nfserr_openmode;
 		goto out;
 	}
-	file_lock->fl_owner = (fl_owner_t)lock_sop;
+
+	file_lock->fl_owner = (fl_owner_t)get_lockowner(lock_sop);
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;

@@ -5405,6 +5430,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfs4_ol_stateid *stp;
 	struct file *filp = NULL;
 	struct file_lock *file_lock = NULL;
+	struct nfs4_lockowner *lock_sop = NULL;
 	__be32 status;
 	int err;
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);

@@ -5426,6 +5452,8 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		status = nfserr_lock_range;
 		goto put_stateid;
 	}
+
+	lock_sop = lockowner(stp->st_stateowner);
 	file_lock = locks_alloc_lock();
 	if (!file_lock) {
 		dprintk("NFSD: %s: unable to allocate lock!\n", __func__);

@@ -5434,7 +5462,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	}
 	locks_init_lock(file_lock);
 	file_lock->fl_type = F_UNLCK;
-	file_lock->fl_owner = (fl_owner_t)lockowner(stp->st_stateowner);
+	file_lock->fl_owner = (fl_owner_t)get_lockowner(lock_sop);
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;

Quoted 61 lines · On Sun, 10 Aug 2014 23:38:25 +0800 Kinglong Mee…

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock always be NULL too.

v2: Only change the order from 3/3 to 1/3 now.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/lockd/svclock.c |  2 +-
 fs/locks.c         | 25 ++++++-------------------
 include/linux/fs.h |  6 ------
 3 files changed, 7 insertions(+), 26 deletions(-)

fs/lockd/svclock.c +1 -1

index ab798a8..e1f209c 100644
@@ -677,7 +677,7 @@ nlmsvc_update_deferred_block(struct nlm_block *block, struct file_lock *conf,
 		block->b_flags |= B_TIMED_OUT;
 	if (conf) {
 		if (block->b_fl)
-			__locks_copy_lock(block->b_fl, conf);
+			locks_copy_lock(block->b_fl, conf);
 	}
 }
 

fs/locks.c +4 -9

index 717fbc4..91b0f03 100644
@@ -266,35 +266,22 @@ static void locks_copy_private(struct file_lock *new, struct file_lock *fl)
 		new->fl_lmops = fl->fl_lmops;
 }
 
-/*
- * Initialize a new lock from an existing file_lock structure.
- */
-void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
+void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 {
+	locks_release_private(new);
+
 	new->fl_owner = fl->fl_owner;
 	new->fl_pid = fl->fl_pid;
-	new->fl_file = NULL;
+	new->fl_file = fl->fl_file;
 	new->fl_flags = fl->fl_flags;
 	new->fl_type = fl->fl_type;
 	new->fl_start = fl->fl_start;
 	new->fl_end = fl->fl_end;
 	new->fl_ops = NULL;
 	new->fl_lmops = NULL;
-}
-EXPORT_SYMBOL(__locks_copy_lock);
-
-void locks_copy_lock(struct file_lock *new, struct file_lock *fl)

fs/locks.c static void locks_copy_private(struct file_lock *new, struct file_lock *fl) · line 284

Quoted 10 lines

fs/locks.c +0 -7

-{
-	locks_release_private(new);
-
-	__locks_copy_lock(new, fl);
-	new->fl_file = fl->fl_file;
-	new->fl_ops = fl->fl_ops;
-	new->fl_lmops = fl->fl_lmops;
 
 	locks_copy_private(new, fl);
 }

(cc'ing Joe Perches)

Ok, so you're basically just reverting 1a747ee0cc11a19. The catch there is that you now need to ensure that any conflock structures are properly initialized before passing them to locks_copy_lock.

The nfsv4 server code currently doesn't do that and it will need to be fixed to do so or that will be a regression.

For the NLM code, Joe Perches has proposed a patch to remove the conflock parameter from lm_grant since the callers always pass in NULL anyway. You may want to pull in his patch and rebase yours on top of it since it'll remove that __locks_copy_lock call altogether.

Joe, is Andrew merging that patch or do I need to pull it into the locks tree?

Quoted 39 lines

fs/locks.c +2 -3

-
 EXPORT_SYMBOL(locks_copy_lock);
 
 static inline int flock_translate_cmd(int cmd) {

@@ -718,7 +705,7 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 			break;
 	}
 	if (cfl) {
-		__locks_copy_lock(fl, cfl);
+		locks_copy_lock(fl, cfl);
 		if (cfl->fl_nspid)
 			fl->fl_pid = pid_vnr(cfl->fl_nspid);
 	} else

@@ -921,7 +908,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 			if (!posix_locks_conflict(request, fl))
 				continue;
 			if (conflock)
-				__locks_copy_lock(conflock, fl);
+				locks_copy_lock(conflock, fl);
 			error = -EAGAIN;
 			if (!(request->fl_flags & FL_SLEEP))
 				goto out;

include/linux/fs.h +0 -1

index e11d60c..ced023d 100644
@@ -941,7 +941,6 @@ void locks_free_lock(struct file_lock *fl);
 extern void locks_init_lock(struct file_lock *);
 extern struct file_lock * locks_alloc_lock(void);
 extern void locks_copy_lock(struct file_lock *, struct file_lock *);
-extern void __locks_copy_lock(struct file_lock *, const struct file_lock *);
 extern void locks_remove_posix(struct file *, fl_owner_t);
 extern void locks_remove_file(struct file *);
 extern void locks_release_private(struct file_lock *);

@@ -1001,11 +1000,6 @@ static inline void locks_init_lock(struct file_lock *fl)
 	return;
 }
 

include/linux/fs.h static inline void locks_init_lock(struct file_lock *fl) · line 1005

Quoted 8 lines

include/linux/fs.h +0 -5

-static inline void __locks_copy_lock(struct file_lock *new, struct file_lock *fl)
-{
-	return;
-}
-
 static inline void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 {
 	return;

Quoted 15 lines · On Sun, 10 Aug 2014 23:43:57 +0800 Kinglong Mee…

v2: Fix bad using of struct file_lock_operations for handle the owner.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4state.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

fs/nfsd/nfs4state.c +0 -0

index 2e80a59..24a8d91 100644
@@ -4867,9 +4867,33 @@ nfs4_transform_lock_offset(struct file_lock *lock)
 		lock->fl_end = OFFSET_MAX;
 }
 

fs/nfsd/nfs4state.c nfs4_transform_lock_offset(struct file_lock *lock) · line 4875

Quoted 6 lines

fs/nfsd/nfs4state.c +6 -0

+static inline struct nfs4_lockowner *get_lockowner(struct nfs4_lockowner *lo)
+{
+	atomic_inc(&lo->lo_owner.so_count);
+	return lo;
+}
+

The other places that increment the so_count on a lockowner should probably be changed to use get_lockowner.

Or what may be better is to make a nfs4_get_stateowner call and change all of the places that bump the so_count to use it. Then you could just do something like this in the right spot below:

    file_lock->fl_owner = (fl_owner_t)lockowner(nfs4_get_stateowner(lock_sop));

Quoted 12 lines

fs/nfsd/nfs4state.c +12 -0

+static void nfsd4_fl_copy_owner(struct file_lock *dst, struct file_lock *src)
+{
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *) src->fl_owner;
+	dst->fl_owner = (fl_owner_t) get_lockowner(lo);
+}
+
+static void nfsd4_fl_release_owner(struct file_lock *fl)
+{
+	struct nfs4_lockowner *lo = (struct nfs4_lockowner *) fl->fl_owner;
+
+	if (lo) {
+		nfs4_put_stateowner(&lo->lo_owner);

fs/nfsd/nfs4state.c nfs4_transform_lock_offset(struct file_lock *lock) · line 4893

Quoted 6 lines

fs/nfsd/nfs4state.c +4 -0

+		fl->fl_owner = NULL;
+	}
+}
+
 /* Hack!: For now, we're defining this just so we can use a pointer to it
  * as a unique cookie to identify our (NFSv4's) posix locks. */

The above comment should be removed now.

Quoted 35 lines

fs/nfsd/nfs4state.c +7 -1

 static const struct lock_manager_operations nfsd_posix_mng_ops  = {
+	.lm_copy_owner = nfsd4_fl_copy_owner,
+	.lm_release_owner = nfsd4_fl_release_owner,
 };
 
 static inline void

@@ -5238,7 +5262,8 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		status = nfserr_openmode;
 		goto out;
 	}
-	file_lock->fl_owner = (fl_owner_t)lock_sop;
+
+	file_lock->fl_owner = (fl_owner_t)get_lockowner(lock_sop);
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;

@@ -5405,6 +5430,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfs4_ol_stateid *stp;
 	struct file *filp = NULL;
 	struct file_lock *file_lock = NULL;
+	struct nfs4_lockowner *lock_sop = NULL;
 	__be32 status;
 	int err;
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);

@@ -5426,6 +5452,8 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		status = nfserr_lock_range;
 		goto put_stateid;
 	}
+
+	lock_sop = lockowner(stp->st_stateowner);
 	file_lock = locks_alloc_lock();
 	if (!file_lock) {
 		dprintk("NFSD: %s: unable to allocate lock!\n", __func__);

@@ -5434,7 +5462,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,

fs/nfsd/nfs4state.c nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, · line 5468

Quoted 8 lines

fs/nfsd/nfs4state.c +1 -1

 	}
 	locks_init_lock(file_lock);
 	file_lock->fl_type = F_UNLCK;
-	file_lock->fl_owner = (fl_owner_t)lockowner(stp->st_stateowner);
+	file_lock->fl_owner = (fl_owner_t)get_lockowner(lock_sop);
 	file_lock->fl_pid = current->tgid;
 	file_lock->fl_file = filp;
 	file_lock->fl_flags = FL_POSIX;

Quoted 72 lines · Jeff Layton

Quoted 69 lines · On Sun, 10 Aug 2014 23:38:25 +0800 Kinglong Mee…

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock always be NULL too.

v2: Only change the order from 3/3 to 1/3 now.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/lockd/svclock.c |  2 +-
 fs/locks.c         | 25 ++++++-------------------
 include/linux/fs.h |  6 ------
 3 files changed, 7 insertions(+), 26 deletions(-)

fs/lockd/svclock.c +1 -1

index ab798a8..e1f209c 100644
@@ -677,7 +677,7 @@ nlmsvc_update_deferred_block(struct nlm_block *block, struct file_lock *conf,
 		block->b_flags |= B_TIMED_OUT;
 	if (conf) {
 		if (block->b_fl)
-			__locks_copy_lock(block->b_fl, conf);
+			locks_copy_lock(block->b_fl, conf);
 	}
 }
 

fs/locks.c +4 -16

index 717fbc4..91b0f03 100644
@@ -266,35 +266,22 @@ static void locks_copy_private(struct file_lock *new, struct file_lock *fl)
 		new->fl_lmops = fl->fl_lmops;
 }
 
-/*
- * Initialize a new lock from an existing file_lock structure.
- */
-void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
+void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 {
+	locks_release_private(new);
+
 	new->fl_owner = fl->fl_owner;
 	new->fl_pid = fl->fl_pid;
-	new->fl_file = NULL;
+	new->fl_file = fl->fl_file;
 	new->fl_flags = fl->fl_flags;
 	new->fl_type = fl->fl_type;
 	new->fl_start = fl->fl_start;
 	new->fl_end = fl->fl_end;
 	new->fl_ops = NULL;
 	new->fl_lmops = NULL;
-}
-EXPORT_SYMBOL(__locks_copy_lock);
-
-void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
-{
-	locks_release_private(new);
-
-	__locks_copy_lock(new, fl);
-	new->fl_file = fl->fl_file;
-	new->fl_ops = fl->fl_ops;
-	new->fl_lmops = fl->fl_lmops;
 

Quoted 4 lines

fs/locks.c static void locks_copy_private(struct file_lock *new, struct file_lock *fl) · line 284

Quoted 2 lines

fs/locks.c +0 -0

 	locks_copy_private(new, fl);
 }

(cc'ing Joe Perches)

(cc'ing Andrew Morton too)

Quoted 12 lines

Ok, so you're basically just reverting 1a747ee0cc11a19. The catch there is that you now need to ensure that any conflock structures are properly initialized before passing them to locks_copy_lock.

The nfsv4 server code currently doesn't do that and it will need to be fixed to do so or that will be a regression.

For the NLM code, Joe Perches has proposed a patch to remove the conflock parameter from lm_grant since the callers always pass in NULL anyway. You may want to pull in his patch and rebase yours on top of it since it'll remove that __locks_copy_lock call altogether.

Quoted 2 lines

Joe, is Andrew merging that patch or do I need to pull it into the locks tree?

I believe Andrew is merging it.

Quoted 81 lines · Jeff Layton

Quoted 61 lines · On Sun, 10 Aug 2014 23:38:25 +0800 Kinglong Mee <[email protected]> wrote:

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock always be NULL too.

v2: Only change the order from 3/3 to 1/3 now.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/lockd/svclock.c |  2 +-
 fs/locks.c         | 25 ++++++-------------------
 include/linux/fs.h |  6 ------
 3 files changed, 7 insertions(+), 26 deletions(-)

fs/lockd/svclock.c +1 -1

index ab798a8..e1f209c 100644
@@ -677,7 +677,7 @@ nlmsvc_update_deferred_block(struct nlm_block *block, struct file_lock *conf,
 		block->b_flags |= B_TIMED_OUT;
 	if (conf) {
 		if (block->b_fl)
-			__locks_copy_lock(block->b_fl, conf);
+			locks_copy_lock(block->b_fl, conf);
 	}
 }
 

fs/locks.c +4 -9

index 717fbc4..91b0f03 100644
@@ -266,35 +266,22 @@ static void locks_copy_private(struct file_lock *new, struct file_lock *fl)
 		new->fl_lmops = fl->fl_lmops;
 }
 
-/*
- * Initialize a new lock from an existing file_lock structure.
- */
-void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
+void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 {
+	locks_release_private(new);
+
 	new->fl_owner = fl->fl_owner;
 	new->fl_pid = fl->fl_pid;
-	new->fl_file = NULL;
+	new->fl_file = fl->fl_file;
 	new->fl_flags = fl->fl_flags;
 	new->fl_type = fl->fl_type;
 	new->fl_start = fl->fl_start;
 	new->fl_end = fl->fl_end;
 	new->fl_ops = NULL;
 	new->fl_lmops = NULL;
-}
-EXPORT_SYMBOL(__locks_copy_lock);
-
-void locks_copy_lock(struct file_lock *new, struct file_lock *fl)

fs/locks.c static void locks_copy_private(struct file_lock *new, struct file_lock *fl) · line 284

Quoted 10 lines

fs/locks.c +0 -7

-{
-	locks_release_private(new);
-
-	__locks_copy_lock(new, fl);
-	new->fl_file = fl->fl_file;
-	new->fl_ops = fl->fl_ops;
-	new->fl_lmops = fl->fl_lmops;
 
 	locks_copy_private(new, fl);
 }

(cc'ing Joe Perches)

Ok, so you're basically just reverting 1a747ee0cc11a19. The catch there is that you now need to ensure that any conflock structures are properly initialized before passing them to locks_copy_lock.

Quoted 2 lines

The nfsv4 server code currently doesn't do that and it will need to be fixed to do so or that will be a regression.

I don't think so.
locks_alloc_lock() has initialize the file_lock struct, the same as locks_init_lock().

I will clean the duplicate initialize for file_lock in nfs4state.c in v3.

Quoted 5 lines

For the NLM code, Joe Perches has proposed a patch to remove the conflock parameter from lm_grant since the callers always pass in NULL anyway. You may want to pull in his patch and rebase yours on top of it since it'll remove that __locks_copy_lock call altogether.

Quoted 2 lines

Joe, is Andrew merging that patch or do I need to pull it into the locks tree?

I will update this patch based on that patch and your new patch for locks.c.

thanks,
Kinglong Mee

Quoted 39 lines

Quoted 39 lines

fs/locks.c +2 -3

-
 EXPORT_SYMBOL(locks_copy_lock);
 
 static inline int flock_translate_cmd(int cmd) {

@@ -718,7 +705,7 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 			break;
 	}
 	if (cfl) {
-		__locks_copy_lock(fl, cfl);
+		locks_copy_lock(fl, cfl);
 		if (cfl->fl_nspid)
 			fl->fl_pid = pid_vnr(cfl->fl_nspid);
 	} else

@@ -921,7 +908,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 			if (!posix_locks_conflict(request, fl))
 				continue;
 			if (conflock)
-				__locks_copy_lock(conflock, fl);
+				locks_copy_lock(conflock, fl);
 			error = -EAGAIN;
 			if (!(request->fl_flags & FL_SLEEP))
 				goto out;

include/linux/fs.h +0 -1

index e11d60c..ced023d 100644
@@ -941,7 +941,6 @@ void locks_free_lock(struct file_lock *fl);
 extern void locks_init_lock(struct file_lock *);
 extern struct file_lock * locks_alloc_lock(void);
 extern void locks_copy_lock(struct file_lock *, struct file_lock *);
-extern void __locks_copy_lock(struct file_lock *, const struct file_lock *);
 extern void locks_remove_posix(struct file *, fl_owner_t);
 extern void locks_remove_file(struct file *);
 extern void locks_release_private(struct file_lock *);

@@ -1001,11 +1000,6 @@ static inline void locks_init_lock(struct file_lock *fl)
 	return;
 }
 

Quoted 10 lines

include/linux/fs.h static inline void locks_init_lock(struct file_lock *fl) · line 1005

Quoted 8 lines

include/linux/fs.h +0 -5

-static inline void __locks_copy_lock(struct file_lock *new, struct file_lock *fl)
-{
-	return;
-}
-
 static inline void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 {
 	return;

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 89 lines · On Thu, 14 Aug 2014 20:26:03 +0800 Kinglong Mee…

Quoted 77 lines · Jeff Layton

Quoted 61 lines · On Sun, 10 Aug 2014 23:38:25 +0800 Kinglong Mee…

Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) using fl_lmops field in file_lock for checking nfsd4 lockowner.

But, commit 1a747ee0cc (locks: don't call ->copy_lock methods on return of conflicting locks) causes the fl_lmops of conflock always be NULL.

Also, commit 0996905f93 (lockd: posix_test_lock() should not call locks_copy_lock()) caused the fl_lmops of conflock always be NULL too.

v2: Only change the order from 3/3 to 1/3 now.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/lockd/svclock.c |  2 +-
 fs/locks.c         | 25 ++++++-------------------
 include/linux/fs.h |  6 ------
 3 files changed, 7 insertions(+), 26 deletions(-)

fs/lockd/svclock.c +1 -1

index ab798a8..e1f209c 100644
@@ -677,7 +677,7 @@ nlmsvc_update_deferred_block(struct nlm_block *block, struct file_lock *conf,
 		block->b_flags |= B_TIMED_OUT;
 	if (conf) {
 		if (block->b_fl)
-			__locks_copy_lock(block->b_fl, conf);
+			locks_copy_lock(block->b_fl, conf);
 	}
 }
 

fs/locks.c +4 -9

index 717fbc4..91b0f03 100644
@@ -266,35 +266,22 @@ static void locks_copy_private(struct file_lock *new, struct file_lock *fl)
 		new->fl_lmops = fl->fl_lmops;
 }
 
-/*
- * Initialize a new lock from an existing file_lock structure.
- */
-void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
+void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 {
+	locks_release_private(new);
+
 	new->fl_owner = fl->fl_owner;
 	new->fl_pid = fl->fl_pid;
-	new->fl_file = NULL;
+	new->fl_file = fl->fl_file;
 	new->fl_flags = fl->fl_flags;
 	new->fl_type = fl->fl_type;
 	new->fl_start = fl->fl_start;
 	new->fl_end = fl->fl_end;
 	new->fl_ops = NULL;
 	new->fl_lmops = NULL;
-}
-EXPORT_SYMBOL(__locks_copy_lock);
-
-void locks_copy_lock(struct file_lock *new, struct file_lock *fl)

fs/locks.c static void locks_copy_private(struct file_lock *new, struct file_lock *fl) · line 284

Quoted 10 lines

fs/locks.c +0 -7

-{
-	locks_release_private(new);
-
-	__locks_copy_lock(new, fl);
-	new->fl_file = fl->fl_file;
-	new->fl_ops = fl->fl_ops;
-	new->fl_lmops = fl->fl_lmops;
 
 	locks_copy_private(new, fl);
 }

(cc'ing Joe Perches)

Quoted 6 lines

Ok, so you're basically just reverting 1a747ee0cc11a19. The catch there is that you now need to ensure that any conflock structures are properly initialized before passing them to locks_copy_lock.

The nfsv4 server code currently doesn't do that and it will need to be fixed to do so or that will be a regression.

I don't think so.
locks_alloc_lock() has initialize the file_lock struct, the same as locks_init_lock().

Quoted 2 lines

I will clean the duplicate initialize for file_lock in nfs4state.c in v3.

Ahh, you're correct. Yes, please just remove that instead. You might also want to look for other places in the kernel that call locks_init_lock unnecessarily. We might as well get rid of all of them while we're looking.

Quoted 11 lines

Quoted 8 lines

For the NLM code, Joe Perches has proposed a patch to remove the conflock parameter from lm_grant since the callers always pass in NULL anyway. You may want to pull in his patch and rebase yours on top of it since it'll remove that __locks_copy_lock call altogether.

Joe, is Andrew merging that patch or do I need to pull it into the locks tree?

I will update this patch based on that patch and your new patch for locks.c.

Quoted 3 lines

thanks,
Kinglong Mee

Thanks. I wiggled Joe's patch on top of my current set of locking patches and will plan to merge it for v3.18 unless there are any objections.

Quoted 50 lines

Quoted 50 lines

Quoted 48 lines

fs/locks.c +2 -3

-
 EXPORT_SYMBOL(locks_copy_lock);
 
 static inline int flock_translate_cmd(int cmd) {

@@ -718,7 +705,7 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 			break;
 	}
 	if (cfl) {
-		__locks_copy_lock(fl, cfl);
+		locks_copy_lock(fl, cfl);
 		if (cfl->fl_nspid)
 			fl->fl_pid = pid_vnr(cfl->fl_nspid);
 	} else

@@ -921,7 +908,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 			if (!posix_locks_conflict(request, fl))
 				continue;
 			if (conflock)
-				__locks_copy_lock(conflock, fl);
+				locks_copy_lock(conflock, fl);
 			error = -EAGAIN;
 			if (!(request->fl_flags & FL_SLEEP))
 				goto out;

include/linux/fs.h +0 -6

index e11d60c..ced023d 100644
@@ -941,7 +941,6 @@ void locks_free_lock(struct file_lock *fl);
 extern void locks_init_lock(struct file_lock *);
 extern struct file_lock * locks_alloc_lock(void);
 extern void locks_copy_lock(struct file_lock *, struct file_lock *);
-extern void __locks_copy_lock(struct file_lock *, const struct file_lock *);
 extern void locks_remove_posix(struct file *, fl_owner_t);
 extern void locks_remove_file(struct file *);
 extern void locks_release_private(struct file_lock *);

@@ -1001,11 +1000,6 @@ static inline void locks_init_lock(struct file_lock *fl)
 	return;
 }
 
-static inline void __locks_copy_lock(struct file_lock *new, struct file_lock *fl)
-{
-	return;
-}
-
 static inline void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 {
 	return;

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html