[PATCH 1/8] lockd: nfsd4: use same grace period for lockd and nfsd4

[Trond Myklebust]

On Tue, 2007-06-19 at 15:28 -0400, J. Bruce Fields wrote:
> From: J. Bruce Fields <bfields at fieldses.org>
> 
> Both lockd and (in the nfsv4 case) nfsd enforce a "grace period" after
> reboot, during which clients may reclaim locks from the previous server
> instance, but may not acquire new locks.
> 
> Currently the lockd and nfsd enforce grace periods of different lengths.
> This may cause problems when we reboot a server with both v2/v3 and v4
> clients.  For example, if the lockd grace period is shorter (as is likely
> the case), then a v3 client might acquire a new lock that conflicts with a
> lock already held (but not yet reclaimed) by a v4 client.
> 
> This patch calculates a lease time that lockd and nfsd can both use.
> 
> Signed-off-by: Marc Eshel <eshel at almaden.ibm.com>
> Signed-off-by: J. Bruce Fields <bfields at citi.umich.edu>
> ---
>  fs/lockd/svc.c             |   31 ++++++++++++++++++++++++-------
>  fs/nfsd/lockd.c            |    1 +
>  fs/nfsd/nfs4state.c        |    9 ++++++++-
>  include/linux/lockd/bind.h |    9 +++++++++
>  4 files changed, 42 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> index 126b1bf..6378572 100644
> --- a/fs/lockd/svc.c
> +++ b/fs/lockd/svc.c
> @@ -75,18 +75,35 @@ static const int		nlm_port_min = 0, nlm_port_max = 65535;
>  
>  static struct ctl_table_header * nlm_sysctl_table;
>  
> -static unsigned long set_grace_period(void)
> +static time_t get_lockd_grace_period(void)
>  {
> -	unsigned long grace_period;
> -
>  	/* Note: nlm_timeout should always be nonzero */
>  	if (nlm_grace_period)
> -		grace_period = ((nlm_grace_period + nlm_timeout - 1)
> -				/ nlm_timeout) * nlm_timeout * HZ;
> +		return ((nlm_grace_period + nlm_timeout - 1)
> +				/ nlm_timeout) * nlm_timeout;
>  	else
> -		grace_period = nlm_timeout * 5 * HZ;
> +		return nlm_timeout * 5;
> +}

Why on earth are you changing the units here? The natural unit inside
the kernel is the jiffy, not the second. You're just forcing all the
callers to convert...

> +
> +time_t get_nfs_grace_period(void)
> +{
> +	time_t lockdgrace = get_lockd_grace_period();
> +	time_t nfsdgrace = 0;
> +
> +	if (nlmsvc_ops)
> +		nfsdgrace = nlmsvc_ops->get_grace_period();
> +
> +	return max(lockdgrace, nfsdgrace);
> +}
> +EXPORT_SYMBOL(get_nfs_grace_period);
> +
> +static unsigned long set_grace_period(void)
> +{
> +	time_t grace_period;
> +
> +	grace_period = get_nfs_grace_period();
>  	nlmsvc_grace_period = 1;
> -	return grace_period + jiffies;
> +	return grace_period * HZ + jiffies;
>  }
>  
>  static inline void clear_grace_period(void)
> diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> index 221acd1..9e4a568 100644
> --- a/fs/nfsd/lockd.c
> +++ b/fs/nfsd/lockd.c
> @@ -65,6 +65,7 @@ nlm_fclose(struct file *filp)
>  static struct nlmsvc_binding	nfsd_nlm_ops = {
>  	.fopen		= nlm_fopen,		/* open file for locking */
>  	.fclose		= nlm_fclose,		/* close file */
> +	.get_grace_period = get_nfs4_grace_period,
>  };
>  
>  void
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 3cc8ce4..50e0d44 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -51,6 +51,7 @@
>  #include <linux/namei.h>
>  #include <linux/mutex.h>
>  #include <linux/lockd/bind.h>
> +#include <linux/module.h>
>  
>  #define NFSDDBG_FACILITY                NFSDDBG_PROC
>  
> @@ -3191,6 +3192,12 @@ nfsd4_load_reboot_recovery_data(void)
>  		printk("NFSD: Failure reading reboot recovery data\n");
>  }
>  
> +time_t
> +get_nfs4_grace_period(void)
> +{
> +	return max(user_lease_time, lease_time);
> +}
> +
>  /* initialization to perform when the nfsd service is started: */
>  
>  static void
> @@ -3199,7 +3206,7 @@ __nfs4_state_start(void)
>  	time_t grace_time;
>  
>  	boot_time = get_seconds();
> -	grace_time = max(user_lease_time, lease_time);
> +	grace_time = get_nfs_grace_period();
>  	lease_time = user_lease_time;
>  	in_grace = 1;
>  	printk("NFSD: starting %ld-second grace period\n", grace_time);
> diff --git a/include/linux/lockd/bind.h b/include/linux/lockd/bind.h
> index 246de1d..df008eb 100644
> --- a/include/linux/lockd/bind.h
> +++ b/include/linux/lockd/bind.h
> @@ -27,6 +27,7 @@ struct nlmsvc_binding {
>  						struct nfs_fh *,
>  						struct file **);
>  	void			(*fclose)(struct file *);
> +	time_t			(*get_grace_period)(void);
>  };
>  
>  extern struct nlmsvc_binding *	nlmsvc_ops;
> @@ -38,4 +39,12 @@ extern int	nlmclnt_proc(struct inode *, int, struct file_lock *);
>  extern int	lockd_up(int proto);
>  extern void	lockd_down(void);
>  
> +time_t get_nfs_grace_period(void);
> +
> +#ifdef CONFIG_NFSD_V4
> +time_t get_nfs4_grace_period(void);
> +#else
> +static inline void get_nfs4_grace_period(void) {return 0;}
> +#endif
> +
>  #endif /* LINUX_LOCKD_BIND_H */