2011-08-12 00:51:31

by Boaz Harrosh

[permalink] [raw]
Subject: Grace period NEVER ends

Hi

I have this weird problem with latest code. It is Benny's latest based
on 3.1-rc1 + linus/master of today + trond/linux-next.

For my testing I use a pNFS client mounted on localhost over a pNFS-nfsd on
the same machine. If I wait the 90 seconds or so and then mount all is well
but if I mount right away, or stop the server, then start and mount.
The Grace period NEVER ends. On prints I see the server returning 100013
continuously forever.

This is my usual test setup. In the passed it would wait the annoying grace
and continue. Now it never returns.

Before I start to bisect back to a good point I though I might ask for pointers
on what might have changed in this respect.

Thanks for any input
Boaz



2011-08-12 18:39:53

by J. Bruce Fields

[permalink] [raw]
Subject: Re: Grace period NEVER ends

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.

2011-08-12 20:36:24

by Boaz Harrosh

[permalink] [raw]
Subject: Re: Grace period NEVER ends

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 <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
>
> 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;

I don't think this fix is enough what about the failure of nfs4_save_creds
It can only fail with -ENOMEM do you hang the client in this case?
What about:

diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 29d77f6..3becde7 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -129,9 +129,12 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
if (!rec_file || clp->cl_firststate)
return 0;

+ clp->cl_firststate = 1;
status = nfs4_save_creds(&original_cred);
- if (status < 0)
+ if (unlikely(status < 0)) {
+ printk(KERN_ERR "!!!nfs4_save_creds Returned => %d\n", status);
return status;
+ }

dir = rec_file->f_path.dentry;
/* lock the parent */
@@ -140,6 +143,7 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
dentry = lookup_one_len(dname, dir, HEXDIR_LEN-1);
if (IS_ERR(dentry)) {
status = PTR_ERR(dentry);
+ printk(KERN_ERR "NFSD: lookup_one_len => %d\n", status);
goto out_unlock;
}
status = -EEXIST;
@@ -148,18 +152,21 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
goto out_put;
}
status = mnt_want_write(rec_file->f_path.mnt);
- if (status)
+ if (unlikely(status)) {
+ printk(KERN_ERR "!!!mnt_want_write Returned => %d\n", status);
goto out_put;
+ }
status = vfs_mkdir(dir->d_inode, dentry, S_IRWXU);
+ if (unlikely(status))
+ printk(KERN_ERR "!!!vfs_mkdir Returned => %d\n", status);
+
mnt_drop_write(rec_file->f_path.mnt);
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);
- }
nfs4_reset_creds(original_cred);
dprintk("NFSD: nfsd4_create_clid_dir returns %d\n", status);
return status;

I think Some of these prints should be delegated to a KERN_ERR since
it is a possible setup problem.

I get these a lot:
NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS

what is suppose to delete this directory? I did a clean
umount and reboot. Next time up it is there.

I guess that explains my problem. So yes this fixes it for
me too. I'm able to run as usual.

Thanks
Boaz

2011-08-12 01:29:28

by Boaz Harrosh

[permalink] [raw]
Subject: Re: Grace period NEVER ends

With this patch I'm back to the previous behavior. That is
wait your grace period then continue.

---
NFSD: Remove a wrong check in nfs4_open

We are already doing the proper grace period checking
farther down in nfs4_open. This check was just checking
nothing and was totally unrelated to the comment about
"RECLAIM_COMPLETE". It was a bug because if an open was
coming before the grace period end, it would then never
pass the condition of not being cl_firststate.

Boaz

---
@@ -295,15 +295,6 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (open->op_create && open->op_claim_type != NFS4_OPEN_CLAIM_NULL)
return nfserr_inval;

- /*
- * RFC5661 18.51.3
- * Before RECLAIM_COMPLETE done, server should deny new lock
- */
- if (nfsd4_has_session(cstate) &&
- !cstate->session->se_client->cl_firststate &&
- open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
- return nfserr_grace;
-
if (nfsd4_has_session(cstate))
copy_clientid(&open->op_clientid, cstate->session);



2011-08-12 19:00:58

by Boaz Harrosh

[permalink] [raw]
Subject: Re: Grace period NEVER ends

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


2011-08-12 20:36:15

by J. Bruce Fields

[permalink] [raw]
Subject: Re: Grace period NEVER ends

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.

2011-08-12 19:30:18

by Casey Bodley

[permalink] [raw]
Subject: Re: Grace period NEVER ends

On Fri, Aug 12, 2011 at 10:32 AM, J. Bruce Fields <[email protected]> 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 <[email protected]> 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?

Bruce, I've not been able to reproduce since applying this patch.

>
> Unfortunately, this doesn't explain Malcolm Locke's problem, as it's 4.1
> specific.
>
> --b.
>
> commit d43b4d070a24edcbe5f5e9ffcf7a33bbeccdd47d
> Author: J. Bruce Fields <[email protected]>
> 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 <[email protected]>
>
> 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;
>

2011-08-12 01:14:14

by J. Bruce Fields

[permalink] [raw]
Subject: Re: Grace period NEVER ends

On Thu, Aug 11, 2011 at 05:51:22PM -0700, Boaz Harrosh wrote:
> I have this weird problem with latest code. It is Benny's latest based
> on 3.1-rc1 + linus/master of today + trond/linux-next.
>
> For my testing I use a pNFS client mounted on localhost over a pNFS-nfsd on
> the same machine. If I wait the 90 seconds or so and then mount all is well
> but if I mount right away, or stop the server, then start and mount.
> The Grace period NEVER ends. On prints I see the server returning 100013
> continuously forever.
>
> This is my usual test setup. In the passed it would wait the annoying grace
> and continue. Now it never returns.
>
> Before I start to bisect back to a good point I though I might ask for pointers
> on what might have changed in this respect.

Hm, I don't know. We turned on the reply cache for nfsv4, so it could
be mistakenly treating the calls as replays, but that seems unlikely.

There was one other report of such behavior here:

http://marc.info/?l=linux-nfs&m=131280896721834&w=2

--b.

2011-08-12 15:52:23

by Myklebust, Trond

[permalink] [raw]
Subject: Re: Grace period NEVER ends

On Thu, 2011-08-11 at 18:29 -0700, Boaz Harrosh wrote:
> With this patch I'm back to the previous behavior. That is
> wait your grace period then continue.
>
> ---
> NFSD: Remove a wrong check in nfs4_open
>
> We are already doing the proper grace period checking
> farther down in nfs4_open. This check was just checking
> nothing and was totally unrelated to the comment about
> "RECLAIM_COMPLETE". It was a bug because if an open was
> coming before the grace period end, it would then never
> pass the condition of not being cl_firststate.
>
> Boaz
>
> ---
> @@ -295,15 +295,6 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> if (open->op_create && open->op_claim_type != NFS4_OPEN_CLAIM_NULL)
> return nfserr_inval;
>
> - /*
> - * RFC5661 18.51.3
> - * Before RECLAIM_COMPLETE done, server should deny new lock
> - */
> - if (nfsd4_has_session(cstate) &&
> - !cstate->session->se_client->cl_firststate &&
> - open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
> - return nfserr_grace;
> -

BTW: restricting opens to CLAIM_PREVIOUS only during the grace period
seems wrong.

If I have already reclaimed my delegation, then why shouldn't I be able
to do a CLAIM_DELEGATE_CUR and/or CLAIM_DELEG_CUR_FH open? It is not as
if those can ever cause a lock that will conflict with some other
client's lock reclaims.

Trond
--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2011-08-12 16:26:06

by J. Bruce Fields

[permalink] [raw]
Subject: Re: Grace period NEVER ends

On Fri, Aug 12, 2011 at 11:52:05AM -0400, Trond Myklebust wrote:
> On Thu, 2011-08-11 at 18:29 -0700, Boaz Harrosh wrote:
> > With this patch I'm back to the previous behavior. That is
> > wait your grace period then continue.
> >
> > ---
> > NFSD: Remove a wrong check in nfs4_open
> >
> > We are already doing the proper grace period checking
> > farther down in nfs4_open. This check was just checking
> > nothing and was totally unrelated to the comment about
> > "RECLAIM_COMPLETE". It was a bug because if an open was
> > coming before the grace period end, it would then never
> > pass the condition of not being cl_firststate.
> >
> > Boaz
> >
> > ---
> > @@ -295,15 +295,6 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > if (open->op_create && open->op_claim_type != NFS4_OPEN_CLAIM_NULL)
> > return nfserr_inval;
> >
> > - /*
> > - * RFC5661 18.51.3
> > - * Before RECLAIM_COMPLETE done, server should deny new lock
> > - */
> > - if (nfsd4_has_session(cstate) &&
> > - !cstate->session->se_client->cl_firststate &&
> > - open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
> > - return nfserr_grace;
> > -
>
> BTW: restricting opens to CLAIM_PREVIOUS only during the grace period
> seems wrong.
>
> If I have already reclaimed my delegation, then why shouldn't I be able
> to do a CLAIM_DELEGATE_CUR and/or CLAIM_DELEG_CUR_FH open? It is not as
> if those can ever cause a lock that will conflict with some other
> client's lock reclaims.

I was assuming the client had to send a reclaim_complete before it could
do that, but... I think you're right, those must fall under the "reclaim
operations" that a client is allowed to do before the reclaim_complete.

I need to look at rfc 5661 10.2.1--I don't think we have that stuff
right at all. Argh.

--b.

2011-08-12 14:32:37

by J. Bruce Fields

[permalink] [raw]
Subject: Re: Grace period NEVER ends

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 <[email protected]> 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 <[email protected]>
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 <[email protected]>

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;

2011-08-12 01:16:36

by Boaz Harrosh

[permalink] [raw]
Subject: Re: Grace period NEVER ends

On 08/11/2011 05:51 PM, Boaz Harrosh wrote:
> Hi
>
> I have this weird problem with latest code. It is Benny's latest based
> on 3.1-rc1 + linus/master of today + trond/linux-next.
>
> For my testing I use a pNFS client mounted on localhost over a pNFS-nfsd on
> the same machine. If I wait the 90 seconds or so and then mount all is well
> but if I mount right away, or stop the server, then start and mount.
> The Grace period NEVER ends. On prints I see the server returning 100013
> continuously forever.
>
> This is my usual test setup. In the passed it would wait the annoying grace
> and continue. Now it never returns.
>
> Before I start to bisect back to a good point I though I might ask for pointers
> on what might have changed in this respect.
>
> Thanks for any input
> Boaz
>
> --
> 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

I put some prints and I see that the nfserr_grace is returned from here:
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index a68384f..8eae742 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -301,8 +301,10 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
*/
if (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);

Is that new code?
and this print below is never shown:
diff --git a/fs/lockd/grace.c b/fs/lockd/grace.c
index 183cc1f..202c3e7 100644
--- a/fs/lockd/grace.c
+++ b/fs/lockd/grace.c
@@ -54,6 +54,10 @@ EXPORT_SYMBOL_GPL(locks_end_grace);
*/
int locks_in_grace(void)
{
- return !list_empty(&grace_list);
+ int ret = !list_empty(&grace_list);
+
+ if (ret)
+ printk(KERN_ERR "locks_in_grace => true\n");
+ return ret;
}
EXPORT_SYMBOL_GPL(locks_in_grace);


If you ask me that if() in nfsd4_open above is totally bogus.
So what if this is not the cl_firststate? Don't we have to
actually ask is this a grace period at all? which we do below.
It looks to me like the complete if (and its comment) is not
needed. We already check for the proper things below.

Boaz





2011-08-12 02:16:00

by J. Bruce Fields

[permalink] [raw]
Subject: Re: Grace period NEVER ends

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?

--b.

>
> ---
> NFSD: Remove a wrong check in nfs4_open
>
> We are already doing the proper grace period checking
> farther down in nfs4_open. This check was just checking
> nothing and was totally unrelated to the comment about
> "RECLAIM_COMPLETE". It was a bug because if an open was
> coming before the grace period end, it would then never
> pass the condition of not being cl_firststate.
>
> Boaz
>
> ---
> @@ -295,15 +295,6 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> if (open->op_create && open->op_claim_type != NFS4_OPEN_CLAIM_NULL)
> return nfserr_inval;
>
> - /*
> - * RFC5661 18.51.3
> - * Before RECLAIM_COMPLETE done, server should deny new lock
> - */
> - if (nfsd4_has_session(cstate) &&
> - !cstate->session->se_client->cl_firststate &&
> - open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
> - return nfserr_grace;
> -
> if (nfsd4_has_session(cstate))
> copy_clientid(&open->op_clientid, cstate->session);
>
>

2011-08-12 20:49:05

by J. Bruce Fields

[permalink] [raw]
Subject: Re: Grace period NEVER ends

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.

2011-08-12 21:10:08

by Boaz Harrosh

[permalink] [raw]
Subject: Re: Grace period NEVER ends

On 08/12/2011 01:36 PM, Boaz Harrosh wrote:
>
> I get these a lot:
> NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS
>
> what is suppose to delete this directory? I did a clean
> umount and reboot. Next time up it is there.
>

Bruce please look into this.

I do an:
[1]
[]$ service nfs start
Starting NFS daemon: NFSD: Using /var/lib/nfs/v4recovery as the NFSv4 state recovery directory
NFSD: starting 90-second grace period
[]$ mount -t nfs4 -o minorversion=1 localhost:/ $MNT_PNFS
Here I get the: NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS
[]$ dd if=/dev/zero of=$MNT_PNFS/dd1_pnfs bs=4k count=2048
Here it waits, waits, waits, then boom done.

[2]
Then I do:
[]$ umount $MNT_PNFS
[]$ service nfs stop

And back to [1]

I constantly get the NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS.
Also if I do reboot after [2] then [1] after reboot I still get
the "DIRECTORY EXISTS"

Boaz

2011-08-12 18:07:11

by Boaz Harrosh

[permalink] [raw]
Subject: Re: Grace period NEVER ends

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 <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
>
> 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:

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) {


Thanks
Boaz


2011-08-12 14:08:04

by Casey Bodley

[permalink] [raw]
Subject: Re: Grace period NEVER ends

On Thu, Aug 11, 2011 at 10:15 PM, J. Bruce Fields <[email protected]> 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.

>
> --b.
>
>>
>> ---
>> NFSD: Remove a wrong check in nfs4_open
>>
>> We are already doing the proper grace period checking
>> farther down in nfs4_open. This check was just checking
>> nothing and was totally unrelated to the comment about
>> "RECLAIM_COMPLETE". It was a bug because if an open was
>> coming before the grace period end, it would then never
>> pass the condition of not being cl_firststate.
>>
>> Boaz
>>
>> ---
>> @@ -295,15 +295,6 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>> ? ? ? if (open->op_create && open->op_claim_type != NFS4_OPEN_CLAIM_NULL)
>> ? ? ? ? ? ? ? return nfserr_inval;
>>
>> - ? ? /*
>> - ? ? ?* RFC5661 18.51.3
>> - ? ? ?* Before RECLAIM_COMPLETE done, server should deny new lock
>> - ? ? ?*/
>> - ? ? if (nfsd4_has_session(cstate) &&
>> - ? ? ? ? !cstate->session->se_client->cl_firststate &&
>> - ? ? ? ? open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
>> - ? ? ? ? ? ? return nfserr_grace;
>> -
>> ? ? ? if (nfsd4_has_session(cstate))
>> ? ? ? ? ? ? ? copy_clientid(&open->op_clientid, cstate->session);
>>
>>
> --
> 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
>

2011-08-12 20:44:32

by Boaz Harrosh

[permalink] [raw]
Subject: Re: Grace period NEVER ends

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