Re: [PATCH] NFS: Always use the same SETCLIENTID boot verifier

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2012-03-09 at 17:35 -0500, Chuck Lever wrote:
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 11b3864..e236d0b 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -180,7 +180,6 @@ static struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_
>  	spin_lock_init(&clp->cl_lock);
>  	INIT_DELAYED_WORK(&clp->cl_renewd, nfs4_renew_state);
>  	rpc_init_wait_queue(&clp->cl_rpcwaitq, "NFS client");
> -	clp->cl_boot_time = CURRENT_TIME;
>  	clp->cl_state = 1 << NFS4CLNT_LEASE_EXPIRED;
>  	clp->cl_minorversion = cl_init->minorversion;
>  	clp->cl_mvops = nfs_v4_minor_ops[cl_init->minorversion];
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 6c619de..13c27ff 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -64,6 +64,8 @@ static int nfs_update_inode(struct inode *, struct nfs_fattr *);
>  
>  static struct kmem_cache * nfs_inode_cachep;
>  
> +struct timespec nfs_boot_time __read_mostly;

Can we make this a per-net-namespace global variable? IOW: add it to
struct nfs_net

That will allow individual containers on the client to crash or reboot,
and be able to tell the server to clear out their old lease and state
when they come back up again.

> +
>  static inline unsigned long
>  nfs_fattr_to_ino_t(struct nfs_fattr *fattr)
>  {
> @@ -1649,6 +1651,8 @@ static int __init init_nfs_fs(void)
>  #endif
>  	if ((err = register_nfs_fs()) != 0)
>  		goto out;
> +
> +	nfs_boot_time = CURRENT_TIME;
>  	return 0;
>  out:
>  #ifdef CONFIG_PROC_FS
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index 62ffefa..47b3384 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -248,6 +248,7 @@ extern int nfs_access_cache_shrinker(struct shrinker *shrink,
>  					struct shrink_control *sc);
>  
>  /* inode.c */
> +extern struct timespec nfs_boot_time;
>  extern struct workqueue_struct *nfsiod_workqueue;
>  extern struct inode *nfs_alloc_inode(struct super_block *sb);
>  extern void nfs_destroy_inode(struct inode *);
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index c0d7220..747bf7a 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3773,13 +3773,12 @@ wait_on_recovery:
>  	return -EAGAIN;
>  }
>  
> -static void nfs4_construct_boot_verifier(struct nfs_client *clp,
> -					 nfs4_verifier *bootverf)
> +static void nfs4_gen_boot_verifier(nfs4_verifier *bootverf)

How about nfs4_init_boot_verifier? 'gen' sounds so dynamic.

>  {
>  	__be32 verf[2];
>  
> -	verf[0] = htonl((u32)clp->cl_boot_time.tv_sec);
> -	verf[1] = htonl((u32)clp->cl_boot_time.tv_nsec);
> +	verf[0] = cpu_to_be32((u32)nfs_boot_time.tv_sec);
> +	verf[1] = cpu_to_be32((u32)nfs_boot_time.tv_nsec);
>  	memcpy(bootverf->data, verf, sizeof(bootverf->data));
>  }
>  
> @@ -3802,7 +3801,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
>  	int loop = 0;
>  	int status;
>  
> -	nfs4_construct_boot_verifier(clp, &sc_verifier);
> +	nfs4_gen_boot_verifier(&sc_verifier);
>  
>  	for(;;) {
>  		rcu_read_lock();
> @@ -4888,7 +4887,7 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
>  	dprintk("--> %s\n", __func__);
>  	BUG_ON(clp == NULL);
>  
> -	nfs4_construct_boot_verifier(clp, &verifier);
> +	nfs4_gen_boot_verifier(&verifier);
>  
>  	args.id_len = scnprintf(args.id, sizeof(args.id),
>  				"%s/%s.%s/%u",
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index 4db330d..a2b7c47 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -1570,7 +1570,6 @@ void nfs41_handle_recall_slot(struct nfs_client *clp)
>  static void nfs4_reset_all_state(struct nfs_client *clp)
>  {
>  	if (test_and_set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) == 0) {
> -		clp->cl_boot_time = CURRENT_TIME;
>  		nfs4_state_start_reclaim_nograce(clp);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This line is now problematic. If we don't declare a reboot, then we may
end up with all sorts of BAD_SEQID errors above.

Hmm... Thinking about this, the best solution might be simply to declare
2 reboots:
     1. Reboot using a CURRENT_TIME based verifier
     2. Reboot again using the nfs_boot_time based verifier.

That guarantees that the server clears out all the state that we weren't
interested in keeping, while ensuring that in the end, we continue to
use the nfs_boot_time verifier for all future state generation attempts.


-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com

��.n��������+%������w��{.n�����{��w���jg��������ݢj����G�������j:+v���w�m������w�������h�����٥



[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux