[PATCH 0/7] 3.6 nfsd patches

[PATCH 0/7] 3.6 nfsd patches

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

These are some miscellaneous patches for review: minor cleanup and fixes
for a couple problems found at the last bakeathon.

Absent objections they'll be queued up for 3.6.

--b.

J. Bruce Fields (7):
  nfsd4: remove unnecessary comment
  nfsd4: nfsd4_lock() cleanup
  nfsd4: process_open2 cleanup
  nfsd4: our filesystems are normally case sensitive
  nfsd4: release openowners on free in >=4.1 case
  nfsd: allow owner_override only for regular files
  nfsd: share some function prototypes

 fs/nfsd/nfs4callback.c |    1 -
 fs/nfsd/nfs4state.c    |   42 +++++++++++++++++++-----------------------
 fs/nfsd/nfs4xdr.c      |    2 +-
 fs/nfsd/nfsd.h         |    2 ++
 fs/nfsd/vfs.c          |   10 +++++++++-
 5 files changed, 31 insertions(+), 26 deletions(-)

-- 
1.7.9.5

[PATCH 1/7] nfsd4: remove unnecessary comment

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

For the most part readers of cl_cb_state only need a value that is
"eventually" right.  And the value is set only either 1) in response to
some change of state, in which case it's set to UNKNOWN and then a
callback rpc is sent to probe the real state, or b) in the handling of a
response to such a callback.  UNKNOWN is therefore always a "temporary"
state, and for the other states we're happy to accept last writer wins.

So I think we're OK here.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4callback.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index a5fd6b9..cbaf4f8 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -756,7 +756,6 @@ static void do_probe_callback(struct nfs4_client *clp)
  */
 void nfsd4_probe_callback(struct nfs4_client *clp)
 {
-	/* XXX: atomicity?  Also, should we be using cl_flags? */
 	clp->cl_cb_state = NFSD4_CB_UNKNOWN;
 	set_bit(NFSD4_CLIENT_CB_UPDATE, &clp->cl_flags);
 	do_probe_callback(clp);
-- 
1.7.9.5

[PATCH 2/7] nfsd4: nfsd4_lock() cleanup

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

Share a little common logic.  And note the comments here are a little
out of date (e.g. we don't always create new state in the "new" case any
more.)

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |   15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 59b9efc..de8f7e4 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4059,11 +4059,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	nfs4_lock_state();
 
 	if (lock->lk_is_new) {
-		/*
-		 * Client indicates that this is a new lockowner.
-		 * Use open owner and open stateid to create lock owner and
-		 * lock stateid.
-		 */
 		struct nfs4_ol_stateid *open_stp = NULL;
 
 		if (nfsd4_has_session(cstate))
@@ -4090,17 +4085,13 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 			goto out;
 		status = lookup_or_create_lock_state(cstate, open_stp, lock,
 							&lock_stp, &new_state);
-		if (status)
-			goto out;
-	} else {
-		/* lock (lock owner + lock stateid) already exists */
+	} else
 		status = nfs4_preprocess_seqid_op(cstate,
 				       lock->lk_old_lock_seqid,
 				       &lock->lk_old_lock_stateid,
 				       NFS4_LOCK_STID, &lock_stp);
-		if (status)
-			goto out;
-	}
+	if (status)
+		goto out;
 	lock_sop = lockowner(lock_stp->st_stateowner);
 
 	lkflg = setlkflg(lock->lk_type);
-- 
1.7.9.5

[PATCH 3/7] nfsd4: process_open2 cleanup

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

Note we can simplify the error handling a little by doing the truncate
earlier.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index de8f7e4..9efa405 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3013,14 +3013,12 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 		status = nfs4_get_vfs_file(rqstp, fp, current_fh, open);
 		if (status)
 			goto out;
+		status = nfsd4_truncate(rqstp, current_fh, open);
+		if (status)
+			goto out;
 		stp = open->op_stp;
 		open->op_stp = NULL;
 		init_open_stateid(stp, fp, open);
-		status = nfsd4_truncate(rqstp, current_fh, open);
-		if (status) {
-			release_open_stateid(stp);
-			goto out;
-		}
 	}
 	update_stateid(&stp->st_stid.sc_stateid);
 	memcpy(&open->op_stateid, &stp->st_stid.sc_stateid, sizeof(stateid_t));
-- 
1.7.9.5

[PATCH 4/7] nfsd4: our filesystems are normally case sensitive

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

Actually, xfs can optionally be case insensitive; we'll handle that case
in later patches.

Cc: stable@kernel.org
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4xdr.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 4949667..6322df3 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2259,7 +2259,7 @@ out_acl:
 	if (bmval0 & FATTR4_WORD0_CASE_INSENSITIVE) {
 		if ((buflen -= 4) < 0)
 			goto out_resource;
-		WRITE32(1);
+		WRITE32(0);
 	}
 	if (bmval0 & FATTR4_WORD0_CASE_PRESERVING) {
 		if ((buflen -= 4) < 0)
-- 
1.7.9.5

[PATCH 5/7] nfsd4: release openowners on free in >=4.1 case

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

We don't need to keep openowners around in the >=4.1 case, because they
aren't needed to handle CLOSE replays any more (that's a problem for
sessions).  And doing so causes unexpected failures on a subsequent
destroy_clientid to fail.

We probably also need something comparable for lock owners on last
unlock.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |   19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9efa405..e404fca 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3763,12 +3763,19 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	nfsd4_close_open_stateid(stp);
 	oo->oo_last_closed_stid = stp;
 
-	/* place unused nfs4_stateowners on so_close_lru list to be
-	 * released by the laundromat service after the lease period
-	 * to enable us to handle CLOSE replay
-	 */
-	if (list_empty(&oo->oo_owner.so_stateids))
-		move_to_close_lru(oo);
+	if (list_empty(&oo->oo_owner.so_stateids)) {
+		if (cstate->minorversion) {
+			release_openowner(oo);
+			cstate->replay_owner = NULL;
+		} else {
+			/*
+			 * In the 4.0 case we need to keep the owners around a
+			 * little while to handle CLOSE replay.
+			 */
+			if (list_empty(&oo->oo_owner.so_stateids))
+				move_to_close_lru(oo);
+		}
+	}
 out:
 	if (!cstate->replay_owner)
 		nfs4_unlock_state();
-- 
1.7.9.5

[PATCH 6/7] nfsd: allow owner_override only for regular files

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

We normally allow the owner of a file to override permissions checks on
IO operations, since:
	- the client will take responsibility for doing an access check
	  on open;
	- the permission checks offer no protection against malicious
	  clients--if they can authenticate as the file's owner then
	  they can always just change its permissions;
	- checking permission on each IO operation breaks the usual
	  posix rule that permission is checked only on open.

However, we've never allowed the owner to override permissions on
readdir operations, even though the above logic would also apply to
directories.  I've never heard of this causing a problem, probably
because a) simultaneously opening and creating a directory (with
restricted mode) isn't possible, and b) opening a directory, then
chmod'ing it, is rare.

Our disallowal of owner-override on directories appears to be an
accident, though--the readdir itself succeeds, and then we fail just
because lookup_one_len() calls in our filldir methods fail.

I'm not sure what the easiest fix for that would be.  For now, just make
this behavior obvious by denying the override right at the start.

This also fixes some odd v4 behavior: with the rdattr_error attribute
requested, it would perform the readdir but return an ACCES error with
each entry.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/vfs.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index c8bd9c3..3256b5c 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -757,8 +757,16 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type,
 	 * If we get here, then the client has already done an "open",
 	 * and (hopefully) checked permission - so allow OWNER_OVERRIDE
 	 * in case a chmod has now revoked permission.
+	 *
+	 * Arguably we should also allow the owner override for
+	 * directories, but we never have and it doesn't seem to have
+	 * caused anyone a problem.  If we were to change this, note
+	 * also that our filldir callbacks would need a variant of
+	 * lookup_one_len that doesn't check permissions.
 	 */
-	err = fh_verify(rqstp, fhp, type, may_flags | NFSD_MAY_OWNER_OVERRIDE);
+	if (type == S_IFREG)
+		may_flags |= NFSD_MAY_OWNER_OVERRIDE;
+	err = fh_verify(rqstp, fhp, type, may_flags);
 	if (err)
 		goto out;
 
-- 
1.7.9.5

[PATCH 7/7] nfsd: share some function prototypes

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfsd.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 1671429..6d425c2 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -72,6 +72,8 @@ int		nfsd_nrthreads(void);
 int		nfsd_nrpools(void);
 int		nfsd_get_nrthreads(int n, int *);
 int		nfsd_set_nrthreads(int n, int *);
+int		nfsd_pool_stats_open(struct inode *, struct file *);
+int		nfsd_pool_stats_release(struct inode *, struct file *);
 
 #if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
 #ifdef CONFIG_NFSD_V2_ACL
-- 
1.7.9.5

Re: [PATCH 4/7] nfsd4: our filesystems are normally case sensitive

[Christoph Hellwig]

On Tue, Jul 10, 2012 at 12:07:43PM -0400, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> Actually, xfs can optionally be case insensitive; we'll handle that case
> in later patches.

The same is true for jfs, btw.

Re: [PATCH 4/7] nfsd4: our filesystems are normally case sensitive

[J. Bruce Fields]

On Tue, Jul 10, 2012 at 01:34:32PM -0400, Christoph Hellwig wrote:
> On Tue, Jul 10, 2012 at 12:07:43PM -0400, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > Actually, xfs can optionally be case insensitive; we'll handle that case
> > in later patches.
> 
> The same is true for jfs, btw.

Oops, I missed that.

I need to resend the patches to test for that soon.

--b.