diff mbox

Grace period NEVER ends

Message ID 4E456BBB.1030605@panasas.com (mailing list archive)
State New, archived
Headers show

Commit Message

Boaz Harrosh Aug. 12, 2011, 6:06 p.m. UTC 
On 08/12/2011 07:32 AM, J. Bruce Fields wrote:
> On Fri, Aug 12, 2011 at 10:08:03AM -0400, Casey Bodley wrote:
>> On Thu, Aug 11, 2011 at 10:15 PM, J. Bruce Fields <bfields@redhat.com> wrote:
>>> On Thu, Aug 11, 2011 at 06:29:20PM -0700, Boaz Harrosh wrote:
>>>> With this patch I'm back to the previous behavior. That is
>>>> wait your grace period then continue.
>>>
>>> Is it true for some reason that the client never sends RECLAIM_COMPLETE?
>>
>> I tested this yesterday with the windows client and saw the same
>> never-ending grace period on OPEN.  We do send RECLAIM_COMPLETE, and
>> it completes successfully.  Other operations like CREATE and REMOVE
>> succeed as well.
> 
> Argh.  Does this help?
> 
> Unfortunately, this doesn't explain Malcolm Locke's problem, as it's 4.1
> specific.
> 
> --b.
> 
> commit d43b4d070a24edcbe5f5e9ffcf7a33bbeccdd47d
> Author: J. Bruce Fields <bfields@redhat.com>
> Date:   Fri Aug 12 10:27:18 2011 -0400
> 
>     nfsd4: fix failure to end nfsd4 grace period
>     
>     Even if we fail to write a recovery record to stable storage, we should
>     still mark the client as having acquired its first state.  Otherwise we
>     leave 4.1 clients with indefinite ERR_GRACE returns.
>     
>     Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> 
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index 29d77f6..4c7537d 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -156,10 +156,9 @@ out_put:
>  	dput(dentry);
>  out_unlock:
>  	mutex_unlock(&dir->d_inode->i_mutex);
> -	if (status == 0) {
> -		clp->cl_firststate = 1;
> +	if (status == 0)
>  		vfs_fsync(rec_file, 0);
> -	}
> +	clp->cl_firststate = 1;
>  	nfs4_reset_creds(original_cred);
>  	dprintk("NFSD: nfsd4_create_clid_dir returns %d\n", status);
>  	return status;

Bruce hi

I'm confused is this suppose to fix my problem? Because I do not believe
it will. There should not be any error writing a recovery record.

Please note that the case I have is that the client is a new client. That
loaded after the server loaded and started it's grace. Does a client suppose
to send RECLAIM_COMPLETE in that case too. .i.e send RECLAIM_COMPLETE as first
message after mount?

Also there is something I don't understand. I thought RECLAIM_COMPLETE is 
so the server can see all clients have completed the reclaim and end the grace
period earlier then usual, that client's RECLAIM_COMPLETE pertains to other
clients. But in anyway when the grace period ends then things have
opened up for everybody. no? how can a client suicide by failing to send
RECLAIM_COMPLETE?

what about:



Thanks
Boaz

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

Comments

Bruce Fields Aug. 12, 2011, 6:39 p.m. UTC | #1 
On Fri, Aug 12, 2011 at 11:06:51AM -0700, Boaz Harrosh wrote:
> I'm confused is this suppose to fix my problem? Because I do not believe
> it will. There should not be any error writing a recovery record.
> 
> Please note that the case I have is that the client is a new client. That
> loaded after the server loaded and started it's grace. Does a client suppose
> to send RECLAIM_COMPLETE in that case too. .i.e send RECLAIM_COMPLETE as first
> message after mount?

It must send one before it sends any non-reclaim open, yes.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Boaz Harrosh Aug. 12, 2011, 7 p.m. UTC | #2 
On 08/12/2011 11:39 AM, J. Bruce Fields wrote:
> On Fri, Aug 12, 2011 at 11:06:51AM -0700, Boaz Harrosh wrote:
>> I'm confused is this suppose to fix my problem? Because I do not believe
>> it will. There should not be any error writing a recovery record.
>>
>> Please note that the case I have is that the client is a new client. That
>> loaded after the server loaded and started it's grace. Does a client suppose
>> to send RECLAIM_COMPLETE in that case too. .i.e send RECLAIM_COMPLETE as first
>> message after mount?
> 
> It must send one before it sends any non-reclaim open, yes.
> 

1. So you are saying the Linux client is broken? How do you test?
2. If a client is broken and never sends it. Do you claim that it should
   stay in denial forever. Or we can let him off the hook once the
   grace ends?

I can't see the logic in 2. It's that lawyer logic again. I don't mind
and it will harm no one if I do it, but I don't because I'm punishing you
for being a bad boy.

Sigh, what do you suggest we do? a client fix will only work for 3.1 Kernel
say we even send it to stable. Are you willing to sacrifice all the old clients
that do not update?

When was this breakage introduced I did not have it before yesterday?

Please decide?
Boaz

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bruce Fields Aug. 12, 2011, 8:36 p.m. UTC | #3 
On Fri, Aug 12, 2011 at 12:00:45PM -0700, Boaz Harrosh wrote:
> On 08/12/2011 11:39 AM, J. Bruce Fields wrote:
> > On Fri, Aug 12, 2011 at 11:06:51AM -0700, Boaz Harrosh wrote:
> >> I'm confused is this suppose to fix my problem? Because I do not believe
> >> it will. There should not be any error writing a recovery record.
> >>
> >> Please note that the case I have is that the client is a new client. That
> >> loaded after the server loaded and started it's grace. Does a client suppose
> >> to send RECLAIM_COMPLETE in that case too. .i.e send RECLAIM_COMPLETE as first
> >> message after mount?
> > 
> > It must send one before it sends any non-reclaim open, yes.
> > 
> 
> 1. So you are saying the Linux client is broken? How do you test?

Which linux client version, again?  See:

	"Whenever a client establishes a new client ID and before it does
	the first non-reclaim operation that obtains a lock, it MUST
	send a RECLAIM_COMPLETE with rca_one_fs set to FALSE, even if
	there are no locks to reclaim."

I thought it was only very early ("experimental") 4.1 clients that
omitted RECLAIM_COMPLETE?

Is this actually the deleg reclaim case that Trond mentioned?  Could I
see a trace?

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Boaz Harrosh Aug. 12, 2011, 8:44 p.m. UTC | #4 
On 08/12/2011 01:36 PM, J. Bruce Fields wrote:
> On Fri, Aug 12, 2011 at 12:00:45PM -0700, Boaz Harrosh wrote:
> 
> Is this actually the deleg reclaim case that Trond mentioned?  Could I
> see a trace?
> 

No it's the:
	NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS

problem that you sent a fix for. See my other mail

> --b.

Thanks
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields Aug. 12, 2011, 8:48 p.m. UTC | #5 
On Fri, Aug 12, 2011 at 01:44:20PM -0700, Boaz Harrosh wrote:
> On 08/12/2011 01:36 PM, J. Bruce Fields wrote:
> > On Fri, Aug 12, 2011 at 12:00:45PM -0700, Boaz Harrosh wrote:
> > 
> > Is this actually the deleg reclaim case that Trond mentioned?  Could I
> > see a trace?
> > 
> 
> No it's the:
> 	NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS
> 
> problem that you sent a fix for. See my other mail

Got it, thanks for your persistence!

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index a68384f..bd7fbae 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -286,6 +286,7 @@  nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 {
 	__be32 status;
 	struct nfsd4_compoundres *resp;
+	bool in_grace = locks_in_grace();
 
 	dprintk("NFSD: nfsd4_open filename %.*s op_stateowner %p\n",
 		(int)open->op_fname.len, open->op_fname.data,
@@ -299,10 +300,12 @@  nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	 * RFC5661 18.51.3
 	 * Before RECLAIM_COMPLETE done, server should deny new lock
 	 */
-	if (nfsd4_has_session(cstate) &&
+	if (in_grace && nfsd4_has_session(cstate) &&
 	    !cstate->session->se_client->cl_firststate &&
-	    open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
+	    open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS) {
+		printk(KERN_ERR "!!! Before RECLAIM_COMPLETE done\n");
 		return nfserr_grace;
+	}
 
 	if (nfsd4_has_session(cstate))
 		copy_clientid(&open->op_clientid, cstate->session);
@@ -334,10 +337,10 @@  nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	/* Openowner is now set, so sequence id will get bumped.  Now we need
 	 * these checks before we do any creates: */
 	status = nfserr_grace;
-	if (locks_in_grace() && open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
+	if (in_grace && open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
 		goto out;
 	status = nfserr_no_grace;
-	if (!locks_in_grace() && open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS)
+	if (!in_grace && open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS)
 		goto out;
 
 	switch (open->op_claim_type) {