Grace period NEVER ends

Commit Message

Boaz Harrosh Aug. 12, 2011, 8:36 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 <[e-mail]> 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 <[e-mail]>
> 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 <[e-mail]>
> 
> 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:


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
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Boaz Harrosh Aug. 12, 2011, 9:09 p.m. UTC | #1

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
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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;