Re: [Y2038] [PATCH 2/5] fs: nfsd: Replace time_t with u32 in nfsd4_lease

[Arnd Bergmann <arnd@arndb.de>]

On Tuesday 27 October 2015 09:09:35 Ksenija Stanojevic wrote:
> Replace time_t type and get_seconds function which are not y2038 safe on
> 32-bit systems. Function ktime_get_seconds use monotonic instead of real
> time and therefore will not cause overflow.
> 
> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>

You are missing an explanation about why the u32 type is used. The
change to that type looks absolutely appropriate here, but it seems
unrelated to what you write above.

> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index d5cce15..85b114f 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -94,7 +94,7 @@ struct nfsd_net {
>  	bool in_grace;
>  	struct nfsd4_client_tracking_ops *client_tracking_ops;
>  
> -	time_t nfsd4_lease;
> +	u32 nfsd4_lease;
>  	time_t nfsd4_grace;
>  
>  	bool nfsd_net_up;
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index e7f50c4..8a177c6 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -681,7 +681,7 @@ static const struct rpc_program cb_program = {
>  static int max_cb_time(struct net *net)
>  {
>  	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> -	return max(nn->nfsd4_lease/10, (time_t)1) * HZ;
> +	return max(nn->nfsd4_lease/10, (u32)1) * HZ;
>  }

coding style: it would be helpful to convert this to use max_t()
instead of max() to avoid the cast.

>  static struct rpc_cred *callback_cred;
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 8245236..4bceb878 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4332,8 +4332,8 @@ nfs4_laundromat(struct nfsd_net *nn)
>  	struct nfs4_delegation *dp;
>  	struct nfs4_ol_stateid *stp;
>  	struct list_head *pos, *next, reaplist;
> -	time_t cutoff = get_seconds() - nn->nfsd4_lease;
> -	time_t t, new_timeo = nn->nfsd4_lease;
> +	u32 cutoff = ktime_get_seconds() - nn->nfsd4_lease;
> +	u32 t, new_timeo = nn->nfsd4_lease;
>  
>  	dprintk("NFSD: laundromat service - starting\n");
>  	nfsd4_end_grace(nn);

The change to "cutoff" needs to be done together with the 'cl_time'
change from patch 1, and all the users of that variable.

> @@ -4399,7 +4399,7 @@ nfs4_laundromat(struct nfsd_net *nn)
>  	}
>  	spin_unlock(&nn->client_lock);
>  
> -	new_timeo = max_t(time_t, new_timeo, NFSD_LAUNDROMAT_MINTIMEOUT);
> +	new_timeo = max_t(u32, new_timeo, NFSD_LAUNDROMAT_MINTIMEOUT);
>  	return new_timeo;
>  }
>  
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 9690cb4..6e0cfd6 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -932,7 +932,7 @@ static ssize_t write_maxconn(struct file *file, char *buf, size_t size)
>  
>  #ifdef CONFIG_NFSD_V4
>  static ssize_t __nfsd4_write_time(struct file *file, char *buf, size_t size,
> -				  time_t *time, struct nfsd_net *nn)
> +				  u32 *time, struct nfsd_net *nn)
>  {
>  	char *mesg = buf;
>  	int rv, i;
> @@ -964,7 +964,7 @@ static ssize_t __nfsd4_write_time(struct file *file, char *buf, size_t size,
>  }
>  
>  static ssize_t nfsd4_write_time(struct file *file, char *buf, size_t size,
> -				time_t *time, struct nfsd_net *nn)
> +				u32 *time, struct nfsd_net *nn)
>  {
>  	ssize_t rv;

This function is called for both nfsd4_lease and nfsd4_grace, so by changing
only one of the two types, you generate a compiler warning.

	Arnd