nfsd: print status when nfsd4_open fails to open file it just created

PATCH Superseded Jeff Layton 3 participants 5 messages Jul 21, 2014 – Jul 30, 2014

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Jeff Layton

author

2014-07-21 13:37 UTC #0 lore raw

PATCH 1/1 nfsd: print status when nfsd4_open fails to open file it just created

It's possible for nfsd to fail opening a file that it has just created. When that happens, we throw a WARN but it doesn't include any info about the error code. Print the status code to give us a bit more info.

Our QA group hit some of these warnings under some very heavy stress testing. My suspicion is that they hit the file-max limit, but it's hard to know for sure. Go ahead and add a -ENFILE mapping to nfserr_serverfault to make the error more distinct (and correct).

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

---
 fs/nfsd/nfs4proc.c | 4 +++-
 fs/nfsd/nfsproc.c  | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

fs/nfsd/nfs4proc.c +3 -1

index 29a617ebe38c..8611585f739d 100644
@@ -460,7 +460,9 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	 * set, (2) sets open->op_stateid, (3) sets open->op_delegation.
 	 */
 	status = nfsd4_process_open2(rqstp, resfh, open);
-	WARN_ON(status && open->op_created);
+	WARN(status && open->op_created,
+	     "nfsd4_process_open2 failed to open newly-created file! status=%u\n",
+	     be32_to_cpu(status));
 out:
 	if (resfh && resfh != &cstate->current_fh) {
 		fh_dup2(&cstate->current_fh, resfh);

fs/nfsd/nfsproc.c +1 -0

index b19c7e8bf64c..b8680738f588 100644
@@ -745,6 +745,7 @@ nfserrno (int errno)
 		{ nfserr_notsupp, -EOPNOTSUPP },
 		{ nfserr_toosmall, -ETOOSMALL },
 		{ nfserr_serverfault, -ESERVERFAULT },
+		{ nfserr_serverfault, -ENFILE },
 	};
 	int	i;

Signature · 1 line

1.9.3

J. Bruce Fields

2014-07-21 20:13 UTC #1 lore raw

Quoted 8 lines · Jeff Layton

It's possible for nfsd to fail opening a file that it has just created. When that happens, we throw a WARN but it doesn't include any info about the error code. Print the status code to give us a bit more info.
Our QA group hit some of these warnings under some very heavy stress testing. My suspicion is that they hit the file-max limit, but it's hard to know for sure. Go ahead and add a -ENFILE mapping to nfserr_serverfault to make the error more distinct (and correct).

Thanks, applying.

We do need to fix this. The open code is complicated, but I think there probably aren't too many failure cases after a successful create left.

--b.

Quoted 37 lines

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

---
 fs/nfsd/nfs4proc.c | 4 +++-
 fs/nfsd/nfsproc.c  | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

fs/nfsd/nfs4proc.c +3 -1

index 29a617ebe38c..8611585f739d 100644
@@ -460,7 +460,9 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	 * set, (2) sets open->op_stateid, (3) sets open->op_delegation.
 	 */
 	status = nfsd4_process_open2(rqstp, resfh, open);
-	WARN_ON(status && open->op_created);
+	WARN(status && open->op_created,
+	     "nfsd4_process_open2 failed to open newly-created file! status=%u\n",
+	     be32_to_cpu(status));
 out:
 	if (resfh && resfh != &cstate->current_fh) {
 		fh_dup2(&cstate->current_fh, resfh);

fs/nfsd/nfsproc.c +1 -0

index b19c7e8bf64c..b8680738f588 100644
@@ -745,6 +745,7 @@ nfserrno (int errno)
 		{ nfserr_notsupp, -EOPNOTSUPP },
 		{ nfserr_toosmall, -ETOOSMALL },
 		{ nfserr_serverfault, -ESERVERFAULT },
+		{ nfserr_serverfault, -ENFILE },
 	};
 	int	i;

Signature · 1 line

1.9.3

Jeff Layton

2014-07-21 20:17 UTC #2 lore raw

Quoted 10 lines · On Mon, 21 Jul 2014 16:13:20 -0400 "J. Bruce Fields" <[email protected]>…

Quoted 8 lines · Jeff Layton
It's possible for nfsd to fail opening a file that it has just created. When that happens, we throw a WARN but it doesn't include any info about the error code. Print the status code to give us a bit more info.
Our QA group hit some of these warnings under some very heavy stress testing. My suspicion is that they hit the file-max limit, but it's hard to know for sure. Go ahead and add a -ENFILE mapping to nfserr_serverfault to make the error more distinct (and correct).

Quoted 7 lines

Thanks, applying.
We do need to fix this. The open code is complicated, but I think there probably aren't too many failure cases after a successful create left.
--b.

Yeah, this is just an interim step to help us nail down some of the failure scenarios. It doesn't really fix anything...

What we'll eventually need to do is reorganize the code so that the creates and opens are atomic. That'll take some work to untangle though since there are several steps between creating a file and opening it now.

Quoted 37 lines

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

---
 fs/nfsd/nfs4proc.c | 4 +++-
 fs/nfsd/nfsproc.c  | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

fs/nfsd/nfs4proc.c +3 -1

index 29a617ebe38c..8611585f739d 100644
@@ -460,7 +460,9 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	 * set, (2) sets open->op_stateid, (3) sets open->op_delegation.
 	 */
 	status = nfsd4_process_open2(rqstp, resfh, open);
-	WARN_ON(status && open->op_created);
+	WARN(status && open->op_created,
+	     "nfsd4_process_open2 failed to open newly-created file! status=%u\n",
+	     be32_to_cpu(status));
 out:
 	if (resfh && resfh != &cstate->current_fh) {
 		fh_dup2(&cstate->current_fh, resfh);

fs/nfsd/nfsproc.c +1 -0

index b19c7e8bf64c..b8680738f588 100644
@@ -745,6 +745,7 @@ nfserrno (int errno)
 		{ nfserr_notsupp, -EOPNOTSUPP },
 		{ nfserr_toosmall, -ETOOSMALL },
 		{ nfserr_serverfault, -ESERVERFAULT },
+		{ nfserr_serverfault, -ENFILE },
 	};
 	int	i;

Signature · 1 line

1.9.3

Signature · 1 line

Jeff Layton <[email protected]>

Jeff Layton

author

2014-07-30 01:37 UTC #3 lore raw

PATCH RESEND 1/1 nfsd: print status when nfsd4_open fails to open file it just created

In reply to Jeff Layton

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

---
 fs/nfsd/nfs4proc.c | 4 +++-
 fs/nfsd/nfsproc.c  | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

fs/nfsd/nfs4proc.c +3 -1

index 29a617ebe38c..8611585f739d 100644
@@ -460,7 +460,9 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	 * set, (2) sets open->op_stateid, (3) sets open->op_delegation.
 	 */
 	status = nfsd4_process_open2(rqstp, resfh, open);
-	WARN_ON(status && open->op_created);
+	WARN(status && open->op_created,
+	     "nfsd4_process_open2 failed to open newly-created file! status=%u\n",
+	     be32_to_cpu(status));
 out:
 	if (resfh && resfh != &cstate->current_fh) {
 		fh_dup2(&cstate->current_fh, resfh);

fs/nfsd/nfsproc.c +1 -0

index b19c7e8bf64c..b8680738f588 100644
@@ -745,6 +745,7 @@ nfserrno (int errno)
 		{ nfserr_notsupp, -EOPNOTSUPP },
 		{ nfserr_toosmall, -ETOOSMALL },
 		{ nfserr_serverfault, -ESERVERFAULT },
+		{ nfserr_serverfault, -ENFILE },
 	};
 	int	i;

Signature · 1 line

1.9.3

J. Bruce Fields

2014-07-30 13:28 UTC #4 lore raw

Thanks for the resend, applied.--b.

Quoted 45 lines · Jeff Layton

It's possible for nfsd to fail opening a file that it has just created. When that happens, we throw a WARN but it doesn't include any info about the error code. Print the status code to give us a bit more info.
Our QA group hit some of these warnings under some very heavy stress testing. My suspicion is that they hit the file-max limit, but it's hard to know for sure. Go ahead and add a -ENFILE mapping to nfserr_serverfault to make the error more distinct (and correct).
Signed-off-by Jeff Layton <[email protected]>
---
 fs/nfsd/nfs4proc.c | 4 +++-
 fs/nfsd/nfsproc.c  | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)
fs/nfsd/nfs4proc.c +3 -1
index 29a617ebe38c..8611585f739d 100644
@@ -460,7 +460,9 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	 * set, (2) sets open->op_stateid, (3) sets open->op_delegation.
 	 */
 	status = nfsd4_process_open2(rqstp, resfh, open);
-	WARN_ON(status && open->op_created);
+	WARN(status && open->op_created,
+	     "nfsd4_process_open2 failed to open newly-created file! status=%u\n",
+	     be32_to_cpu(status));
 out:
 	if (resfh && resfh != &cstate->current_fh) {
 		fh_dup2(&cstate->current_fh, resfh);
fs/nfsd/nfsproc.c +1 -0
index b19c7e8bf64c..b8680738f588 100644
@@ -745,6 +745,7 @@ nfserrno (int errno)
 		{ nfserr_notsupp, -EOPNOTSUPP },
 		{ nfserr_toosmall, -ETOOSMALL },
 		{ nfserr_serverfault, -ESERVERFAULT },
+		{ nfserr_serverfault, -ENFILE },
 	};
 	int	i;
 
Signature · 1 line
1.9.3