[PATCH v2 00/12] nfsd: avoid 32-bit time_t

[PATCH v2 00/12] nfsd: avoid 32-bit time_t

[Arnd Bergmann]

Hi Bruce, Chuck,

NFSd is one of the last areas of the kernel that is not y2038 safe
yet, this series addresses the remaining issues here.

I did not get any comments for the first version I posted [1], and
I hope this just means that everything was fine and you plan to
merge this soon ;-)

I uploaded a git branch to [2] for testing.

Please review and merge for linux-5.6 so we can remove the 32-bit
time handling from that release.

      Arnd

Changes from v1:
- separate nfs and nfsd, as most of the nfs changes are merged now
- rebase to v5.5

[1] https://lore.kernel.org/lkml/20191111201639.2240623-1-arnd@arndb.de/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=y2038-nfsd

Arnd Bergmann (12):
  nfsd: use ktime_get_seconds() for timestamps
  nfsd: print 64-bit timestamps in client_info_show
  nfsd: handle nfs3 timestamps as unsigned
  nfsd: use timespec64 in encode_time_delta
  nfsd: make 'boot_time' 64-bit wide
  nfsd: pass a 64-bit guardtime to nfsd_setattr()
  nfsd: use time64_t in nfsd_proc_setattr() check
  nfsd: fix delay timer on 32-bit architectures
  nfsd: fix jiffies/time_t mixup in LRU list
  nfsd: use boottime for lease expiry alculation
  nfsd: use ktime_get_real_seconds() in nfs4_verifier
  nfsd: remove nfs4_reset_lease() declarations

 fs/nfsd/netns.h        |  6 ++--
 fs/nfsd/nfs3xdr.c      | 20 +++++--------
 fs/nfsd/nfs4callback.c |  7 ++++-
 fs/nfsd/nfs4layouts.c  |  2 +-
 fs/nfsd/nfs4proc.c     |  2 +-
 fs/nfsd/nfs4recover.c  |  8 ++---
 fs/nfsd/nfs4state.c    | 68 ++++++++++++++++++++----------------------
 fs/nfsd/nfs4xdr.c      |  4 +--
 fs/nfsd/nfsctl.c       |  6 ++--
 fs/nfsd/nfsd.h         |  2 --
 fs/nfsd/nfsfh.h        |  4 +--
 fs/nfsd/nfsproc.c      |  6 ++--
 fs/nfsd/state.h        | 10 +++----
 fs/nfsd/vfs.c          |  4 +--
 fs/nfsd/vfs.h          |  2 +-
 fs/nfsd/xdr3.h         |  2 +-
 16 files changed, 74 insertions(+), 79 deletions(-)

-- 
2.20.0

[PATCH v2 01/12] nfsd: use ktime_get_seconds() for timestamps

[Arnd Bergmann]

The delegation logic in nfsd uses the somewhat inefficient
seconds_since_boot() function to record time intervals.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nfsd/nfs4state.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 369e574c5092..bfdb3366239c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -806,7 +806,7 @@ static void nfs4_free_deleg(struct nfs4_stid *stid)
 static DEFINE_SPINLOCK(blocked_delegations_lock);
 static struct bloom_pair {
 	int	entries, old_entries;
-	time_t	swap_time;
+	time64_t swap_time;
 	int	new; /* index into 'set' */
 	DECLARE_BITMAP(set[2], 256);
 } blocked_delegations;
@@ -818,15 +818,15 @@ static int delegation_blocked(struct knfsd_fh *fh)
 
 	if (bd->entries == 0)
 		return 0;
-	if (seconds_since_boot() - bd->swap_time > 30) {
+	if (ktime_get_seconds() - bd->swap_time > 30) {
 		spin_lock(&blocked_delegations_lock);
-		if (seconds_since_boot() - bd->swap_time > 30) {
+		if (ktime_get_seconds() - bd->swap_time > 30) {
 			bd->entries -= bd->old_entries;
 			bd->old_entries = bd->entries;
 			memset(bd->set[bd->new], 0,
 			       sizeof(bd->set[0]));
 			bd->new = 1-bd->new;
-			bd->swap_time = seconds_since_boot();
+			bd->swap_time = ktime_get_seconds();
 		}
 		spin_unlock(&blocked_delegations_lock);
 	}
@@ -856,7 +856,7 @@ static void block_delegations(struct knfsd_fh *fh)
 	__set_bit((hash>>8)&255, bd->set[bd->new]);
 	__set_bit((hash>>16)&255, bd->set[bd->new]);
 	if (bd->entries == 0)
-		bd->swap_time = seconds_since_boot();
+		bd->swap_time = ktime_get_seconds();
 	bd->entries += 1;
 	spin_unlock(&blocked_delegations_lock);
 }
-- 
2.20.0

[PATCH v2 02/12] nfsd: print 64-bit timestamps in client_info_show

[Arnd Bergmann]

The nii_time field gets truncated to 'time_t' on 32-bit architectures
before printing.

Remove the use of 'struct timespec' to product the correct output
beyond 2038.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nfsd/nfs4state.c | 5 ++---
 fs/nfsd/state.h     | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index bfdb3366239c..27a629cc5a46 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2292,7 +2292,7 @@ static int client_info_show(struct seq_file *m, void *v)
 					clp->cl_nii_domain.len);
 		seq_printf(m, "\nImplementation name: ");
 		seq_quote_mem(m, clp->cl_nii_name.data, clp->cl_nii_name.len);
-		seq_printf(m, "\nImplementation time: [%ld, %ld]\n",
+		seq_printf(m, "\nImplementation time: [%lld, %ld]\n",
 			clp->cl_nii_time.tv_sec, clp->cl_nii_time.tv_nsec);
 	}
 	drop_client(clp);
@@ -2946,8 +2946,7 @@ static __be32 copy_impl_id(struct nfs4_client *clp,
 	xdr_netobj_dup(&clp->cl_nii_name, &exid->nii_name, GFP_KERNEL);
 	if (!clp->cl_nii_name.data)
 		return nfserr_jukebox;
-	clp->cl_nii_time.tv_sec = exid->nii_time.tv_sec;
-	clp->cl_nii_time.tv_nsec = exid->nii_time.tv_nsec;
+	clp->cl_nii_time = exid->nii_time;
 	return 0;
 }
 
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index d61b83b9654c..2b4165cd1d3e 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -320,7 +320,7 @@ struct nfs4_client {
 	/* NFSv4.1 client implementation id: */
 	struct xdr_netobj	cl_nii_domain;
 	struct xdr_netobj	cl_nii_name;
-	struct timespec		cl_nii_time;
+	struct timespec64	cl_nii_time;
 
 	/* for v4.0 and v4.1 callbacks: */
 	struct nfs4_cb_conn	cl_cb_conn;
-- 
2.20.0

[PATCH v2 03/12] nfsd: handle nfs3 timestamps as unsigned

[Arnd Bergmann]

The decode_time3 function behaves differently on 32-bit
and 64-bit architectures: on the former, a 32-bit timestamp
gets converted into an signed number and then into a timestamp
between 1902 and 2038, while on the latter it is interpreted
as unsigned in the range 1970-2106.

Change all the remaining 'timespec' in nfsd to 'timespec64'
to make the behavior the same, and use the current interpretation
of the dominant 64-bit architectures.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nfsd/nfs3xdr.c | 20 ++++++++------------
 fs/nfsd/nfsfh.h   |  4 ++--
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index 195ab7a0fc89..c997b710af27 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -32,14 +32,14 @@ static u32	nfs3_ftypes[] = {
  * XDR functions for basic NFS types
  */
 static __be32 *
-encode_time3(__be32 *p, struct timespec *time)
+encode_time3(__be32 *p, struct timespec64 *time)
 {
 	*p++ = htonl((u32) time->tv_sec); *p++ = htonl(time->tv_nsec);
 	return p;
 }
 
 static __be32 *
-decode_time3(__be32 *p, struct timespec *time)
+decode_time3(__be32 *p, struct timespec64 *time)
 {
 	time->tv_sec = ntohl(*p++);
 	time->tv_nsec = ntohl(*p++);
@@ -167,7 +167,6 @@ encode_fattr3(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp,
 	      struct kstat *stat)
 {
 	struct user_namespace *userns = nfsd_user_namespace(rqstp);
-	struct timespec ts;
 	*p++ = htonl(nfs3_ftypes[(stat->mode & S_IFMT) >> 12]);
 	*p++ = htonl((u32) (stat->mode & S_IALLUGO));
 	*p++ = htonl((u32) stat->nlink);
@@ -183,12 +182,9 @@ encode_fattr3(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp,
 	*p++ = htonl((u32) MINOR(stat->rdev));
 	p = encode_fsid(p, fhp);
 	p = xdr_encode_hyper(p, stat->ino);
-	ts = timespec64_to_timespec(stat->atime);
-	p = encode_time3(p, &ts);
-	ts = timespec64_to_timespec(stat->mtime);
-	p = encode_time3(p, &ts);
-	ts = timespec64_to_timespec(stat->ctime);
-	p = encode_time3(p, &ts);
+	p = encode_time3(p, &stat->atime);
+	p = encode_time3(p, &stat->mtime);
+	p = encode_time3(p, &stat->ctime);
 
 	return p;
 }
@@ -277,8 +273,8 @@ void fill_pre_wcc(struct svc_fh *fhp)
 		stat.size  = inode->i_size;
 	}
 
-	fhp->fh_pre_mtime = timespec64_to_timespec(stat.mtime);
-	fhp->fh_pre_ctime = timespec64_to_timespec(stat.ctime);
+	fhp->fh_pre_mtime = stat.mtime;
+	fhp->fh_pre_ctime = stat.ctime;
 	fhp->fh_pre_size  = stat.size;
 	fhp->fh_pre_change = nfsd4_change_attribute(&stat, inode);
 	fhp->fh_pre_saved = true;
@@ -330,7 +326,7 @@ nfs3svc_decode_sattrargs(struct svc_rqst *rqstp, __be32 *p)
 	p = decode_sattr3(p, &args->attrs, nfsd_user_namespace(rqstp));
 
 	if ((args->check_guard = ntohl(*p++)) != 0) { 
-		struct timespec time; 
+		struct timespec64 time;
 		p = decode_time3(p, &time);
 		args->guardtime = time.tv_sec;
 	}
diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index 755e256a9103..495540a248a1 100644
--- a/fs/nfsd/nfsfh.h
+++ b/fs/nfsd/nfsfh.h
@@ -42,8 +42,8 @@ typedef struct svc_fh {
 
 	/* Pre-op attributes saved during fh_lock */
 	__u64			fh_pre_size;	/* size before operation */
-	struct timespec		fh_pre_mtime;	/* mtime before oper */
-	struct timespec		fh_pre_ctime;	/* ctime before oper */
+	struct timespec64	fh_pre_mtime;	/* mtime before oper */
+	struct timespec64	fh_pre_ctime;	/* ctime before oper */
 	/*
 	 * pre-op nfsv4 change attr: note must check IS_I_VERSION(inode)
 	 *  to find out if it is valid.
-- 
2.20.0

[PATCH v2 04/12] nfsd: use timespec64 in encode_time_delta

[Arnd Bergmann]

The values in encode_time_delta are always small and don't
overflow the range of 'struct timespec', so changing it has
no effect.

Change it to timespec64 as a prerequisite for removing the
timespec definition later.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nfsd/nfs4xdr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index d2dc4c0e22e8..d95e9668eeff 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2024,11 +2024,11 @@ static __be32 *encode_change(__be32 *p, struct kstat *stat, struct inode *inode,
  */
 static __be32 *encode_time_delta(__be32 *p, struct inode *inode)
 {
-	struct timespec ts;
+	struct timespec64 ts;
 	u32 ns;
 
 	ns = max_t(u32, NSEC_PER_SEC/HZ, inode->i_sb->s_time_gran);
-	ts = ns_to_timespec(ns);
+	ts = ns_to_timespec64(ns);
 
 	p = xdr_encode_hyper(p, ts.tv_sec);
 	*p++ = cpu_to_be32(ts.tv_nsec);
-- 
2.20.0

[PATCH v2 05/12] nfsd: make 'boot_time' 64-bit wide

[Arnd Bergmann]

The local boot time variable gets truncated to time_t at the moment,
which can lead to slightly odd behavior on 32-bit architectures.

Use ktime_get_real_seconds() instead of get_seconds() to always
get a 64-bit result, and keep it that way wherever possible.

It still gets truncated in a few places:

- When assigning to cl_clientid.cl_boot, this is already documented
  and is only used as a unique identifier.

- In clients_still_reclaiming(), the truncation is to 'unsigned long'
  in order to use the 'time_before() helper.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nfsd/netns.h       |  2 +-
 fs/nfsd/nfs4recover.c |  8 ++++----
 fs/nfsd/nfs4state.c   | 14 +++++++-------
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 9a4ef815fb8c..29bbe28eda53 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -40,7 +40,7 @@ struct nfsd_net {
 
 	struct lock_manager nfsd4_manager;
 	bool grace_ended;
-	time_t boot_time;
+	time64_t boot_time;
 
 	/* internal mount of the "nfsd" pseudofilesystem: */
 	struct vfsmount *nfsd_mnt;
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 2481e7662128..a8fb18609146 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -1445,7 +1445,7 @@ nfsd4_cld_grace_done_v0(struct nfsd_net *nn)
 	}
 
 	cup->cu_u.cu_msg.cm_cmd = Cld_GraceDone;
-	cup->cu_u.cu_msg.cm_u.cm_gracetime = (int64_t)nn->boot_time;
+	cup->cu_u.cu_msg.cm_u.cm_gracetime = nn->boot_time;
 	ret = cld_pipe_upcall(cn->cn_pipe, &cup->cu_u.cu_msg);
 	if (!ret)
 		ret = cup->cu_u.cu_msg.cm_status;
@@ -1782,7 +1782,7 @@ nfsd4_cltrack_client_has_session(struct nfs4_client *clp)
 }
 
 static char *
-nfsd4_cltrack_grace_start(time_t grace_start)
+nfsd4_cltrack_grace_start(time64_t grace_start)
 {
 	int copied;
 	size_t len;
@@ -1795,7 +1795,7 @@ nfsd4_cltrack_grace_start(time_t grace_start)
 	if (!result)
 		return result;
 
-	copied = snprintf(result, len, GRACE_START_ENV_PREFIX "%ld",
+	copied = snprintf(result, len, GRACE_START_ENV_PREFIX "%lld",
 				grace_start);
 	if (copied >= len) {
 		/* just return nothing if output was truncated */
@@ -2004,7 +2004,7 @@ nfsd4_umh_cltrack_grace_done(struct nfsd_net *nn)
 	char *legacy;
 	char timestr[22]; /* FIXME: better way to determine max size? */
 
-	sprintf(timestr, "%ld", nn->boot_time);
+	sprintf(timestr, "%lld", nn->boot_time);
 	legacy = nfsd4_cltrack_legacy_topdir();
 	nfsd4_umh_cltrack_upcall("gracedone", timestr, legacy, NULL);
 	kfree(legacy);
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 27a629cc5a46..1ac431aa29c4 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -753,7 +753,7 @@ int nfs4_init_cp_state(struct nfsd_net *nn, struct nfsd4_copy *copy)
 	if (new_id < 0)
 		return 0;
 	copy->cp_stateid.si_opaque.so_id = new_id;
-	copy->cp_stateid.si_opaque.so_clid.cl_boot = nn->boot_time;
+	copy->cp_stateid.si_opaque.so_clid.cl_boot = (u32)nn->boot_time;
 	copy->cp_stateid.si_opaque.so_clid.cl_id = nn->s2s_cp_cl_id;
 	return 1;
 }
@@ -1862,7 +1862,7 @@ STALE_CLIENTID(clientid_t *clid, struct nfsd_net *nn)
 	 */
 	if (clid->cl_boot == (u32)nn->boot_time)
 		return 0;
-	dprintk("NFSD stale clientid (%08x/%08x) boot_time %08lx\n",
+	dprintk("NFSD stale clientid (%08x/%08x) boot_time %08llx\n",
 		clid->cl_boot, clid->cl_id, nn->boot_time);
 	return 1;
 }
@@ -2222,7 +2222,7 @@ static void gen_confirm(struct nfs4_client *clp, struct nfsd_net *nn)
 
 static void gen_clid(struct nfs4_client *clp, struct nfsd_net *nn)
 {
-	clp->cl_clientid.cl_boot = nn->boot_time;
+	clp->cl_clientid.cl_boot = (u32)nn->boot_time;
 	clp->cl_clientid.cl_id = nn->clientid_counter++;
 	gen_confirm(clp, nn);
 }
@@ -5183,9 +5183,9 @@ nfsd4_end_grace(struct nfsd_net *nn)
  */
 static bool clients_still_reclaiming(struct nfsd_net *nn)
 {
-	unsigned long now = get_seconds();
-	unsigned long double_grace_period_end = nn->boot_time +
-						2 * nn->nfsd4_lease;
+	unsigned long now = (unsigned long) ktime_get_real_seconds();
+	unsigned long double_grace_period_end = (unsigned long)nn->boot_time +
+					   2 * (unsigned long)nn->nfsd4_lease;
 
 	if (nn->track_reclaim_completes &&
 			atomic_read(&nn->nr_reclaim_complete) ==
@@ -7640,7 +7640,7 @@ static int nfs4_state_create_net(struct net *net)
 		INIT_LIST_HEAD(&nn->sessionid_hashtbl[i]);
 	nn->conf_name_tree = RB_ROOT;
 	nn->unconf_name_tree = RB_ROOT;
-	nn->boot_time = get_seconds();
+	nn->boot_time = ktime_get_real_seconds();
 	nn->grace_ended = false;
 	nn->nfsd4_manager.block_opens = true;
 	INIT_LIST_HEAD(&nn->nfsd4_manager.list);
-- 
2.20.0

[PATCH v2 06/12] nfsd: pass a 64-bit guardtime to nfsd_setattr()

[Arnd Bergmann]

Guardtime handling in nfs3 differs between 32-bit and 64-bit
architectures, and uses the deprecated time_t type.

Change it to using time64_t, which behaves the same way on
64-bit and 32-bit architectures, treating the number as an
unsigned 32-bit entity with a range of year 1970 to 2106
consistently, and avoiding the y2038 overflow.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nfsd/nfs4proc.c  | 2 +-
 fs/nfsd/nfs4state.c | 2 +-
 fs/nfsd/nfsproc.c   | 4 ++--
 fs/nfsd/vfs.c       | 4 ++--
 fs/nfsd/vfs.h       | 2 +-
 fs/nfsd/xdr3.h      | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4798667af647..bcf6803cf3a6 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -975,7 +975,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	if (status)
 		goto out;
 	status = nfsd_setattr(rqstp, &cstate->current_fh, &setattr->sa_iattr,
-				0, (time_t)0);
+				0, (time64_t)0);
 out:
 	fh_drop_write(&cstate->current_fh);
 	return status;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1ac431aa29c4..5cb0f774218a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4671,7 +4671,7 @@ nfsd4_truncate(struct svc_rqst *rqstp, struct svc_fh *fh,
 		return 0;
 	if (!(open->op_share_access & NFS4_SHARE_ACCESS_WRITE))
 		return nfserr_inval;
-	return nfsd_setattr(rqstp, fh, &iattr, 0, (time_t)0);
+	return nfsd_setattr(rqstp, fh, &iattr, 0, (time64_t)0);
 }
 
 static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index c83ddac22f38..aa013b736073 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -113,7 +113,7 @@ nfsd_proc_setattr(struct svc_rqst *rqstp)
 		}
 	}
 
-	nfserr = nfsd_setattr(rqstp, fhp, iap, 0, (time_t)0);
+	nfserr = nfsd_setattr(rqstp, fhp, iap, 0, (time64_t)0);
 done:
 	return nfsd_return_attrs(nfserr, resp);
 }
@@ -380,7 +380,7 @@ nfsd_proc_create(struct svc_rqst *rqstp)
 		 */
 		attr->ia_valid &= ATTR_SIZE;
 		if (attr->ia_valid)
-			nfserr = nfsd_setattr(rqstp, newfhp, attr, 0, (time_t)0);
+			nfserr = nfsd_setattr(rqstp, newfhp, attr, 0, (time64_t)0);
 	}
 
 out_unlock:
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index c0dc491537a6..fd5ba6997447 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -358,7 +358,7 @@ nfsd_get_write_access(struct svc_rqst *rqstp, struct svc_fh *fhp,
  */
 __be32
 nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
-	     int check_guard, time_t guardtime)
+	     int check_guard, time64_t guardtime)
 {
 	struct dentry	*dentry;
 	struct inode	*inode;
@@ -1123,7 +1123,7 @@ nfsd_create_setattr(struct svc_rqst *rqstp, struct svc_fh *resfhp,
 	if (!uid_eq(current_fsuid(), GLOBAL_ROOT_UID))
 		iap->ia_valid &= ~(ATTR_UID|ATTR_GID);
 	if (iap->ia_valid)
-		return nfsd_setattr(rqstp, resfhp, iap, 0, (time_t)0);
+		return nfsd_setattr(rqstp, resfhp, iap, 0, (time64_t)0);
 	/* Callers expect file metadata to be committed here */
 	return nfserrno(commit_metadata(resfhp));
 }
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index cc110a10bfe8..bbb485177b25 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -48,7 +48,7 @@ __be32		 nfsd_lookup_dentry(struct svc_rqst *, struct svc_fh *,
 				const char *, unsigned int,
 				struct svc_export **, struct dentry **);
 __be32		nfsd_setattr(struct svc_rqst *, struct svc_fh *,
-				struct iattr *, int, time_t);
+				struct iattr *, int, time64_t);
 int nfsd_mountpoint(struct dentry *, struct svc_export *);
 #ifdef CONFIG_NFSD_V4
 __be32          nfsd4_set_nfs4_label(struct svc_rqst *, struct svc_fh *,
diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
index 99ff9f403ff1..0fa12988fb6a 100644
--- a/fs/nfsd/xdr3.h
+++ b/fs/nfsd/xdr3.h
@@ -14,7 +14,7 @@ struct nfsd3_sattrargs {
 	struct svc_fh		fh;
 	struct iattr		attrs;
 	int			check_guard;
-	time_t			guardtime;
+	time64_t		guardtime;
 };
 
 struct nfsd3_diropargs {
-- 
2.20.0

[PATCH v2 07/12] nfsd: use time64_t in nfsd_proc_setattr() check

[Arnd Bergmann]

Change to time64_t and ktime_get_real_seconds() to make the
logic work correctly on 32-bit architectures beyond 2038.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nfsd/nfsproc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index aa013b736073..b25c90be29fb 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -94,7 +94,7 @@ nfsd_proc_setattr(struct svc_rqst *rqstp)
 		 * Solaris, at least, doesn't seem to care what the time
 		 * request is.  We require it be within 30 minutes of now.
 		 */
-		time_t delta = iap->ia_atime.tv_sec - get_seconds();
+		time64_t delta = iap->ia_atime.tv_sec - ktime_get_real_seconds();
 
 		nfserr = fh_verify(rqstp, fhp, 0, NFSD_MAY_NOP);
 		if (nfserr)
-- 
2.20.0

[PATCH v2 08/12] nfsd: fix delay timer on 32-bit architectures

[Arnd Bergmann]

The nfsd4_cb_layout_done() function takes a 'time_t' value,
multiplied by NSEC_PER_SEC*2 to get a nanosecond value.

This works fine on 64-bit architectures, but on 32-bit, any
value over 1 second results in a signed integer overflow
with unexpected results.

Cast one input to a 64-bit type in order to produce the
same result that we have on 64-bit architectures, regarless
of the type of nfsd4_lease.

Fixes: 6b9b21073d3b ("nfsd: give up on CB_LAYOUTRECALLs after two lease periods")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nfsd/nfs4layouts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
index 2681c70283ce..e12409eca7cc 100644
--- a/fs/nfsd/nfs4layouts.c
+++ b/fs/nfsd/nfs4layouts.c
@@ -675,7 +675,7 @@ nfsd4_cb_layout_done(struct nfsd4_callback *cb, struct rpc_task *task)
 
 		/* Client gets 2 lease periods to return it */
 		cutoff = ktime_add_ns(task->tk_start,
-					 nn->nfsd4_lease * NSEC_PER_SEC * 2);
+					 (u64)nn->nfsd4_lease * NSEC_PER_SEC * 2);
 
 		if (ktime_before(now, cutoff)) {
 			rpc_delay(task, HZ/100); /* 10 mili-seconds */
-- 
2.20.0

[PATCH v2 09/12] nfsd: fix jiffies/time_t mixup in LRU list

[Arnd Bergmann]

The nfsd4_blocked_lock->nbl_time timestamp is recorded in jiffies,
but then compared to a CLOCK_REALTIME timestamp later on, which makes
no sense.

For consistency with the other timestamps, change this to use a time_t.

This is a change in behavior, which may cause regressions, but the
current code is not sensible. On a system with CONFIG_HZ=1000,
the 'time_after((unsigned long)nbl->nbl_time, (unsigned long)cutoff))'
check is false for roughly the first 18 days of uptime and then true
for the next 49 days.

Fixes: 7919d0a27f1e ("nfsd: add a LRU list for blocked locks")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nfsd/nfs4state.c | 2 +-
 fs/nfsd/state.h     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 5cb0f774218a..9a063c4b4460 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6549,7 +6549,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	}
 
 	if (fl_flags & FL_SLEEP) {
-		nbl->nbl_time = jiffies;
+		nbl->nbl_time = get_seconds();
 		spin_lock(&nn->blocked_locks_lock);
 		list_add_tail(&nbl->nbl_list, &lock_sop->lo_blocked);
 		list_add_tail(&nbl->nbl_lru, &nn->blocked_locks_lru);
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 2b4165cd1d3e..03fc7b4380f9 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -606,7 +606,7 @@ static inline bool nfsd4_stateid_generation_after(stateid_t *a, stateid_t *b)
 struct nfsd4_blocked_lock {
 	struct list_head	nbl_list;
 	struct list_head	nbl_lru;
-	unsigned long		nbl_time;
+	time_t			nbl_time;
 	struct file_lock	nbl_lock;
 	struct knfsd_fh		nbl_fh;
 	struct nfsd4_callback	nbl_cb;
-- 
2.20.0

[PATCH v2 10/12] nfsd: use boottime for lease expiry alculation

[Arnd Bergmann]

A couple of time_t variables are only used to track the state of the
lease time and its expiration. The code correctly uses the 'time_after()'
macro to make this work on 32-bit architectures even beyond year 2038,
but the get_seconds() function and the time_t type itself are deprecated
as they behave inconsistently between 32-bit and 64-bit architectures
and often lead to code that is not y2038 safe.

As a minor issue, using get_seconds() leads to problems with concurrent
settimeofday() or clock_settime() calls, in the worst case timeout never
triggering after the time has been set backwards.

Change nfsd to use time64_t and ktime_get_boottime_seconds() here. This
is clearly excessive, as boottime by itself means we never go beyond 32
bits, but it does mean we handle this correctly and consistently without
having to worry about corner cases and should be no more expensive than
the previous implementation on 64-bit architectures.

The max_cb_time() function gets changed in order to avoid an expensive
64-bit division operation, but as the lease time is at most one hour,
there is no change in behavior.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nfsd/netns.h        |  4 ++--
 fs/nfsd/nfs4callback.c |  7 ++++++-
 fs/nfsd/nfs4state.c    | 41 +++++++++++++++++++----------------------
 fs/nfsd/nfsctl.c       |  6 +++---
 fs/nfsd/state.h        |  8 ++++----
 5 files changed, 34 insertions(+), 32 deletions(-)

diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 29bbe28eda53..2baf32311e00 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -92,8 +92,8 @@ struct nfsd_net {
 	bool in_grace;
 	const struct nfsd4_client_tracking_ops *client_tracking_ops;
 
-	time_t nfsd4_lease;
-	time_t nfsd4_grace;
+	time64_t nfsd4_lease;
+	time64_t nfsd4_grace;
 	bool somebody_reclaimed;
 
 	bool track_reclaim_completes;
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 24534db87e86..508d7c6c00b5 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -823,7 +823,12 @@ 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;
+
+	/* nfsd4_lease is set to at most one hour */
+	if (WARN_ON_ONCE(nn->nfsd4_lease > 3600))
+		return 360 * HZ;
+
+	return max(((u32)nn->nfsd4_lease)/10, 1u) * HZ;
 }
 
 static struct workqueue_struct *callback_wq;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9a063c4b4460..6afdef63f6d7 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -170,7 +170,7 @@ renew_client_locked(struct nfs4_client *clp)
 			clp->cl_clientid.cl_boot,
 			clp->cl_clientid.cl_id);
 	list_move_tail(&clp->cl_lru, &nn->client_lru);
-	clp->cl_time = get_seconds();
+	clp->cl_time = ktime_get_boottime_seconds();
 }
 
 static void put_client_renew_locked(struct nfs4_client *clp)
@@ -2612,7 +2612,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
 	gen_clid(clp, nn);
 	kref_init(&clp->cl_nfsdfs.cl_ref);
 	nfsd4_init_cb(&clp->cl_cb_null, clp, NULL, NFSPROC4_CLNT_CB_NULL);
-	clp->cl_time = get_seconds();
+	clp->cl_time = ktime_get_boottime_seconds();
 	clear_bit(0, &clp->cl_cb_slot_busy);
 	copy_verf(clp, verf);
 	memcpy(&clp->cl_addr, sa, sizeof(struct sockaddr_storage));
@@ -4282,7 +4282,7 @@ move_to_close_lru(struct nfs4_ol_stateid *s, struct net *net)
 	last = oo->oo_last_closed_stid;
 	oo->oo_last_closed_stid = s;
 	list_move_tail(&oo->oo_close_lru, &nn->close_lru);
-	oo->oo_time = get_seconds();
+	oo->oo_time = ktime_get_boottime_seconds();
 	spin_unlock(&nn->client_lock);
 	if (last)
 		nfs4_put_stid(&last->st_stid);
@@ -4377,7 +4377,7 @@ static void nfsd4_cb_recall_prepare(struct nfsd4_callback *cb)
 	 */
 	spin_lock(&state_lock);
 	if (dp->dl_time == 0) {
-		dp->dl_time = get_seconds();
+		dp->dl_time = ktime_get_boottime_seconds();
 		list_add_tail(&dp->dl_recall_lru, &nn->del_recall_lru);
 	}
 	spin_unlock(&state_lock);
@@ -5183,9 +5183,8 @@ nfsd4_end_grace(struct nfsd_net *nn)
  */
 static bool clients_still_reclaiming(struct nfsd_net *nn)
 {
-	unsigned long now = (unsigned long) ktime_get_real_seconds();
-	unsigned long double_grace_period_end = (unsigned long)nn->boot_time +
-					   2 * (unsigned long)nn->nfsd4_lease;
+	time64_t double_grace_period_end = nn->boot_time +
+					   2 * nn->nfsd4_lease;
 
 	if (nn->track_reclaim_completes &&
 			atomic_read(&nn->nr_reclaim_complete) ==
@@ -5198,12 +5197,12 @@ static bool clients_still_reclaiming(struct nfsd_net *nn)
 	 * If we've given them *two* lease times to reclaim, and they're
 	 * still not done, give up:
 	 */
-	if (time_after(now, double_grace_period_end))
+	if (ktime_get_boottime_seconds() > double_grace_period_end)
 		return false;
 	return true;
 }
 
-static time_t
+static time64_t
 nfs4_laundromat(struct nfsd_net *nn)
 {
 	struct nfs4_client *clp;
@@ -5212,8 +5211,8 @@ nfs4_laundromat(struct nfsd_net *nn)
 	struct nfs4_ol_stateid *stp;
 	struct nfsd4_blocked_lock *nbl;
 	struct list_head *pos, *next, reaplist;
-	time_t cutoff = get_seconds() - nn->nfsd4_lease;
-	time_t t, new_timeo = nn->nfsd4_lease;
+	time64_t cutoff = ktime_get_boottime_seconds() - nn->nfsd4_lease;
+	time64_t t, new_timeo = nn->nfsd4_lease;
 
 	dprintk("NFSD: laundromat service - starting\n");
 
@@ -5227,7 +5226,7 @@ nfs4_laundromat(struct nfsd_net *nn)
 	spin_lock(&nn->client_lock);
 	list_for_each_safe(pos, next, &nn->client_lru) {
 		clp = list_entry(pos, struct nfs4_client, cl_lru);
-		if (time_after((unsigned long)clp->cl_time, (unsigned long)cutoff)) {
+		if (clp->cl_time > cutoff) {
 			t = clp->cl_time - cutoff;
 			new_timeo = min(new_timeo, t);
 			break;
@@ -5250,7 +5249,7 @@ nfs4_laundromat(struct nfsd_net *nn)
 	spin_lock(&state_lock);
 	list_for_each_safe(pos, next, &nn->del_recall_lru) {
 		dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
-		if (time_after((unsigned long)dp->dl_time, (unsigned long)cutoff)) {
+		if (dp->dl_time > cutoff) {
 			t = dp->dl_time - cutoff;
 			new_timeo = min(new_timeo, t);
 			break;
@@ -5270,8 +5269,7 @@ nfs4_laundromat(struct nfsd_net *nn)
 	while (!list_empty(&nn->close_lru)) {
 		oo = list_first_entry(&nn->close_lru, struct nfs4_openowner,
 					oo_close_lru);
-		if (time_after((unsigned long)oo->oo_time,
-			       (unsigned long)cutoff)) {
+		if (oo->oo_time > cutoff) {
 			t = oo->oo_time - cutoff;
 			new_timeo = min(new_timeo, t);
 			break;
@@ -5301,8 +5299,7 @@ nfs4_laundromat(struct nfsd_net *nn)
 	while (!list_empty(&nn->blocked_locks_lru)) {
 		nbl = list_first_entry(&nn->blocked_locks_lru,
 					struct nfsd4_blocked_lock, nbl_lru);
-		if (time_after((unsigned long)nbl->nbl_time,
-			       (unsigned long)cutoff)) {
+		if (nbl->nbl_time > cutoff) {
 			t = nbl->nbl_time - cutoff;
 			new_timeo = min(new_timeo, t);
 			break;
@@ -5319,7 +5316,7 @@ nfs4_laundromat(struct nfsd_net *nn)
 		free_blocked_lock(nbl);
 	}
 out:
-	new_timeo = max_t(time_t, new_timeo, NFSD_LAUNDROMAT_MINTIMEOUT);
+	new_timeo = max_t(time64_t, new_timeo, NFSD_LAUNDROMAT_MINTIMEOUT);
 	return new_timeo;
 }
 
@@ -5329,13 +5326,13 @@ static void laundromat_main(struct work_struct *);
 static void
 laundromat_main(struct work_struct *laundry)
 {
-	time_t t;
+	time64_t t;
 	struct delayed_work *dwork = to_delayed_work(laundry);
 	struct nfsd_net *nn = container_of(dwork, struct nfsd_net,
 					   laundromat_work);
 
 	t = nfs4_laundromat(nn);
-	dprintk("NFSD: laundromat_main - sleeping for %ld seconds\n", t);
+	dprintk("NFSD: laundromat_main - sleeping for %lld seconds\n", t);
 	queue_delayed_work(laundry_wq, &nn->laundromat_work, t*HZ);
 }
 
@@ -6549,7 +6546,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	}
 
 	if (fl_flags & FL_SLEEP) {
-		nbl->nbl_time = get_seconds();
+		nbl->nbl_time = ktime_get_boottime_seconds();
 		spin_lock(&nn->blocked_locks_lock);
 		list_add_tail(&nbl->nbl_list, &lock_sop->lo_blocked);
 		list_add_tail(&nbl->nbl_lru, &nn->blocked_locks_lru);
@@ -7709,7 +7706,7 @@ nfs4_state_start_net(struct net *net)
 	nfsd4_client_tracking_init(net);
 	if (nn->track_reclaim_completes && nn->reclaim_str_hashtbl_size == 0)
 		goto skip_grace;
-	printk(KERN_INFO "NFSD: starting %ld-second grace period (net %x)\n",
+	printk(KERN_INFO "NFSD: starting %lld-second grace period (net %x)\n",
 	       nn->nfsd4_grace, net->ns.inum);
 	queue_delayed_work(laundry_wq, &nn->laundromat_work, nn->nfsd4_grace * HZ);
 	return 0;
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 11b42c523f04..aace740d5a92 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -956,7 +956,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)
+				  time64_t *time, struct nfsd_net *nn)
 {
 	char *mesg = buf;
 	int rv, i;
@@ -984,11 +984,11 @@ static ssize_t __nfsd4_write_time(struct file *file, char *buf, size_t size,
 		*time = i;
 	}
 
-	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%ld\n", *time);
+	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%lld\n", *time);
 }
 
 static ssize_t nfsd4_write_time(struct file *file, char *buf, size_t size,
-				time_t *time, struct nfsd_net *nn)
+				time64_t *time, struct nfsd_net *nn)
 {
 	ssize_t rv;
 
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 03fc7b4380f9..e426b22b5028 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -132,7 +132,7 @@ struct nfs4_delegation {
 	struct list_head	dl_recall_lru;  /* delegation recalled */
 	struct nfs4_clnt_odstate *dl_clnt_odstate;
 	u32			dl_type;
-	time_t			dl_time;
+	time64_t		dl_time;
 /* For recall: */
 	int			dl_retries;
 	struct nfsd4_callback	dl_recall;
@@ -310,7 +310,7 @@ struct nfs4_client {
 #endif
 	struct xdr_netobj	cl_name; 	/* id generated by client */
 	nfs4_verifier		cl_verifier; 	/* generated by client */
-	time_t                  cl_time;        /* time of last lease renewal */
+	time64_t		cl_time;	/* time of last lease renewal */
 	struct sockaddr_storage	cl_addr; 	/* client ipaddress */
 	bool			cl_mach_cred;	/* SP4_MACH_CRED in force */
 	struct svc_cred		cl_cred; 	/* setclientid principal */
@@ -449,7 +449,7 @@ struct nfs4_openowner {
 	 */
 	struct list_head	oo_close_lru;
 	struct nfs4_ol_stateid *oo_last_closed_stid;
-	time_t			oo_time; /* time of placement on so_close_lru */
+	time64_t		oo_time; /* time of placement on so_close_lru */
 #define NFS4_OO_CONFIRMED   1
 	unsigned char		oo_flags;
 };
@@ -606,7 +606,7 @@ static inline bool nfsd4_stateid_generation_after(stateid_t *a, stateid_t *b)
 struct nfsd4_blocked_lock {
 	struct list_head	nbl_list;
 	struct list_head	nbl_lru;
-	time_t			nbl_time;
+	time64_t		nbl_time;
 	struct file_lock	nbl_lock;
 	struct knfsd_fh		nbl_fh;
 	struct nfsd4_callback	nbl_cb;
-- 
2.20.0

[PATCH v2 11/12] nfsd: use ktime_get_real_seconds() in nfs4_verifier

[Arnd Bergmann]

gen_confirm() generates a unique identifier based on the current
time. This overflows in year 2038, but that is harmless since it
generally does not lead to duplicates, as long as the time has
been initialized by a real-time clock or NTP.

Using ktime_get_boottime_seconds() or ktime_get_seconds() would
avoid the overflow, but it would be more likely to result in
non-unique numbers.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nfsd/nfs4state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 6afdef63f6d7..efe3161f6cf7 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2215,7 +2215,7 @@ static void gen_confirm(struct nfs4_client *clp, struct nfsd_net *nn)
 	 * This is opaque to client, so no need to byte-swap. Use
 	 * __force to keep sparse happy
 	 */
-	verf[0] = (__force __be32)get_seconds();
+	verf[0] = (__force __be32)(u32)ktime_get_real_seconds();
 	verf[1] = (__force __be32)nn->clverifier_counter++;
 	memcpy(clp->cl_confirm.data, verf, sizeof(clp->cl_confirm.data));
 }
-- 
2.20.0

[PATCH v2 12/12] nfsd: remove nfs4_reset_lease() declarations

[Arnd Bergmann]

The function was removed a long time ago, but the declaration
and a dummy implementation are still there, referencing the
deprecated time_t type.

Remove both.

Fixes: f958a1320ff7 ("nfsd4: remove unnecessary lease-setting function")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nfsd/nfsd.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 57b93d95fa5c..31f152bbbb2f 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -142,7 +142,6 @@ int nfs4_state_start(void);
 int nfs4_state_start_net(struct net *net);
 void nfs4_state_shutdown(void);
 void nfs4_state_shutdown_net(struct net *net);
-void nfs4_reset_lease(time_t leasetime);
 int nfs4_reset_recoverydir(char *recdir);
 char * nfs4_recoverydir(void);
 bool nfsd4_spo_must_allow(struct svc_rqst *rqstp);
@@ -153,7 +152,6 @@ static inline int nfs4_state_start(void) { return 0; }
 static inline int nfs4_state_start_net(struct net *net) { return 0; }
 static inline void nfs4_state_shutdown(void) { }
 static inline void nfs4_state_shutdown_net(struct net *net) { }
-static inline void nfs4_reset_lease(time_t leasetime) { }
 static inline int nfs4_reset_recoverydir(char *recdir) { return 0; }
 static inline char * nfs4_recoverydir(void) {return NULL; }
 static inline bool nfsd4_spo_must_allow(struct svc_rqst *rqstp)
-- 
2.20.0

Re: [PATCH v2 10/12] nfsd: use boottime for lease expiry alculation

[Chuck Lever]

Hi Arnd-

> On Dec 13, 2019, at 9:10 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> A couple of time_t variables are only used to track the state of the
> lease time and its expiration. The code correctly uses the 'time_after()'
> macro to make this work on 32-bit architectures even beyond year 2038,
> but the get_seconds() function and the time_t type itself are deprecated
> as they behave inconsistently between 32-bit and 64-bit architectures
> and often lead to code that is not y2038 safe.
> 
> As a minor issue, using get_seconds() leads to problems with concurrent
> settimeofday() or clock_settime() calls, in the worst case timeout never
> triggering after the time has been set backwards.
> 
> Change nfsd to use time64_t and ktime_get_boottime_seconds() here. This
> is clearly excessive, as boottime by itself means we never go beyond 32
> bits, but it does mean we handle this correctly and consistently without
> having to worry about corner cases and should be no more expensive than
> the previous implementation on 64-bit architectures.
> 
> The max_cb_time() function gets changed in order to avoid an expensive
> 64-bit division operation, but as the lease time is at most one hour,
> there is no change in behavior.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> fs/nfsd/netns.h        |  4 ++--
> fs/nfsd/nfs4callback.c |  7 ++++++-
> fs/nfsd/nfs4state.c    | 41 +++++++++++++++++++----------------------
> fs/nfsd/nfsctl.c       |  6 +++---
> fs/nfsd/state.h        |  8 ++++----
> 5 files changed, 34 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index 29bbe28eda53..2baf32311e00 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -92,8 +92,8 @@ struct nfsd_net {
> 	bool in_grace;
> 	const struct nfsd4_client_tracking_ops *client_tracking_ops;
> 
> -	time_t nfsd4_lease;
> -	time_t nfsd4_grace;
> +	time64_t nfsd4_lease;
> +	time64_t nfsd4_grace;
> 	bool somebody_reclaimed;
> 
> 	bool track_reclaim_completes;
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 24534db87e86..508d7c6c00b5 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -823,7 +823,12 @@ 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;
> +
> +	/* nfsd4_lease is set to at most one hour */
> +	if (WARN_ON_ONCE(nn->nfsd4_lease > 3600))
> +		return 360 * HZ;

Why is the WARN_ON_ONCE added here? Is it really necessary?

(Otherwise these all LGTM).


> +
> +	return max(((u32)nn->nfsd4_lease)/10, 1u) * HZ;
> }
> 
> static struct workqueue_struct *callback_wq;
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 9a063c4b4460..6afdef63f6d7 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -170,7 +170,7 @@ renew_client_locked(struct nfs4_client *clp)
> 			clp->cl_clientid.cl_boot,
> 			clp->cl_clientid.cl_id);
> 	list_move_tail(&clp->cl_lru, &nn->client_lru);
> -	clp->cl_time = get_seconds();
> +	clp->cl_time = ktime_get_boottime_seconds();
> }
> 
> static void put_client_renew_locked(struct nfs4_client *clp)
> @@ -2612,7 +2612,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
> 	gen_clid(clp, nn);
> 	kref_init(&clp->cl_nfsdfs.cl_ref);
> 	nfsd4_init_cb(&clp->cl_cb_null, clp, NULL, NFSPROC4_CLNT_CB_NULL);
> -	clp->cl_time = get_seconds();
> +	clp->cl_time = ktime_get_boottime_seconds();
> 	clear_bit(0, &clp->cl_cb_slot_busy);
> 	copy_verf(clp, verf);
> 	memcpy(&clp->cl_addr, sa, sizeof(struct sockaddr_storage));
> @@ -4282,7 +4282,7 @@ move_to_close_lru(struct nfs4_ol_stateid *s, struct net *net)
> 	last = oo->oo_last_closed_stid;
> 	oo->oo_last_closed_stid = s;
> 	list_move_tail(&oo->oo_close_lru, &nn->close_lru);
> -	oo->oo_time = get_seconds();
> +	oo->oo_time = ktime_get_boottime_seconds();
> 	spin_unlock(&nn->client_lock);
> 	if (last)
> 		nfs4_put_stid(&last->st_stid);
> @@ -4377,7 +4377,7 @@ static void nfsd4_cb_recall_prepare(struct nfsd4_callback *cb)
> 	 */
> 	spin_lock(&state_lock);
> 	if (dp->dl_time == 0) {
> -		dp->dl_time = get_seconds();
> +		dp->dl_time = ktime_get_boottime_seconds();
> 		list_add_tail(&dp->dl_recall_lru, &nn->del_recall_lru);
> 	}
> 	spin_unlock(&state_lock);
> @@ -5183,9 +5183,8 @@ nfsd4_end_grace(struct nfsd_net *nn)
>  */
> static bool clients_still_reclaiming(struct nfsd_net *nn)
> {
> -	unsigned long now = (unsigned long) ktime_get_real_seconds();
> -	unsigned long double_grace_period_end = (unsigned long)nn->boot_time +
> -					   2 * (unsigned long)nn->nfsd4_lease;
> +	time64_t double_grace_period_end = nn->boot_time +
> +					   2 * nn->nfsd4_lease;
> 
> 	if (nn->track_reclaim_completes &&
> 			atomic_read(&nn->nr_reclaim_complete) ==
> @@ -5198,12 +5197,12 @@ static bool clients_still_reclaiming(struct nfsd_net *nn)
> 	 * If we've given them *two* lease times to reclaim, and they're
> 	 * still not done, give up:
> 	 */
> -	if (time_after(now, double_grace_period_end))
> +	if (ktime_get_boottime_seconds() > double_grace_period_end)
> 		return false;
> 	return true;
> }
> 
> -static time_t
> +static time64_t
> nfs4_laundromat(struct nfsd_net *nn)
> {
> 	struct nfs4_client *clp;
> @@ -5212,8 +5211,8 @@ nfs4_laundromat(struct nfsd_net *nn)
> 	struct nfs4_ol_stateid *stp;
> 	struct nfsd4_blocked_lock *nbl;
> 	struct list_head *pos, *next, reaplist;
> -	time_t cutoff = get_seconds() - nn->nfsd4_lease;
> -	time_t t, new_timeo = nn->nfsd4_lease;
> +	time64_t cutoff = ktime_get_boottime_seconds() - nn->nfsd4_lease;
> +	time64_t t, new_timeo = nn->nfsd4_lease;
> 
> 	dprintk("NFSD: laundromat service - starting\n");
> 
> @@ -5227,7 +5226,7 @@ nfs4_laundromat(struct nfsd_net *nn)
> 	spin_lock(&nn->client_lock);
> 	list_for_each_safe(pos, next, &nn->client_lru) {
> 		clp = list_entry(pos, struct nfs4_client, cl_lru);
> -		if (time_after((unsigned long)clp->cl_time, (unsigned long)cutoff)) {
> +		if (clp->cl_time > cutoff) {
> 			t = clp->cl_time - cutoff;
> 			new_timeo = min(new_timeo, t);
> 			break;
> @@ -5250,7 +5249,7 @@ nfs4_laundromat(struct nfsd_net *nn)
> 	spin_lock(&state_lock);
> 	list_for_each_safe(pos, next, &nn->del_recall_lru) {
> 		dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> -		if (time_after((unsigned long)dp->dl_time, (unsigned long)cutoff)) {
> +		if (dp->dl_time > cutoff) {
> 			t = dp->dl_time - cutoff;
> 			new_timeo = min(new_timeo, t);
> 			break;
> @@ -5270,8 +5269,7 @@ nfs4_laundromat(struct nfsd_net *nn)
> 	while (!list_empty(&nn->close_lru)) {
> 		oo = list_first_entry(&nn->close_lru, struct nfs4_openowner,
> 					oo_close_lru);
> -		if (time_after((unsigned long)oo->oo_time,
> -			       (unsigned long)cutoff)) {
> +		if (oo->oo_time > cutoff) {
> 			t = oo->oo_time - cutoff;
> 			new_timeo = min(new_timeo, t);
> 			break;
> @@ -5301,8 +5299,7 @@ nfs4_laundromat(struct nfsd_net *nn)
> 	while (!list_empty(&nn->blocked_locks_lru)) {
> 		nbl = list_first_entry(&nn->blocked_locks_lru,
> 					struct nfsd4_blocked_lock, nbl_lru);
> -		if (time_after((unsigned long)nbl->nbl_time,
> -			       (unsigned long)cutoff)) {
> +		if (nbl->nbl_time > cutoff) {
> 			t = nbl->nbl_time - cutoff;
> 			new_timeo = min(new_timeo, t);
> 			break;
> @@ -5319,7 +5316,7 @@ nfs4_laundromat(struct nfsd_net *nn)
> 		free_blocked_lock(nbl);
> 	}
> out:
> -	new_timeo = max_t(time_t, new_timeo, NFSD_LAUNDROMAT_MINTIMEOUT);
> +	new_timeo = max_t(time64_t, new_timeo, NFSD_LAUNDROMAT_MINTIMEOUT);
> 	return new_timeo;
> }
> 
> @@ -5329,13 +5326,13 @@ static void laundromat_main(struct work_struct *);
> static void
> laundromat_main(struct work_struct *laundry)
> {
> -	time_t t;
> +	time64_t t;
> 	struct delayed_work *dwork = to_delayed_work(laundry);
> 	struct nfsd_net *nn = container_of(dwork, struct nfsd_net,
> 					   laundromat_work);
> 
> 	t = nfs4_laundromat(nn);
> -	dprintk("NFSD: laundromat_main - sleeping for %ld seconds\n", t);
> +	dprintk("NFSD: laundromat_main - sleeping for %lld seconds\n", t);
> 	queue_delayed_work(laundry_wq, &nn->laundromat_work, t*HZ);
> }
> 
> @@ -6549,7 +6546,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 	}
> 
> 	if (fl_flags & FL_SLEEP) {
> -		nbl->nbl_time = get_seconds();
> +		nbl->nbl_time = ktime_get_boottime_seconds();
> 		spin_lock(&nn->blocked_locks_lock);
> 		list_add_tail(&nbl->nbl_list, &lock_sop->lo_blocked);
> 		list_add_tail(&nbl->nbl_lru, &nn->blocked_locks_lru);
> @@ -7709,7 +7706,7 @@ nfs4_state_start_net(struct net *net)
> 	nfsd4_client_tracking_init(net);
> 	if (nn->track_reclaim_completes && nn->reclaim_str_hashtbl_size == 0)
> 		goto skip_grace;
> -	printk(KERN_INFO "NFSD: starting %ld-second grace period (net %x)\n",
> +	printk(KERN_INFO "NFSD: starting %lld-second grace period (net %x)\n",
> 	       nn->nfsd4_grace, net->ns.inum);
> 	queue_delayed_work(laundry_wq, &nn->laundromat_work, nn->nfsd4_grace * HZ);
> 	return 0;
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 11b42c523f04..aace740d5a92 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -956,7 +956,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)
> +				  time64_t *time, struct nfsd_net *nn)
> {
> 	char *mesg = buf;
> 	int rv, i;
> @@ -984,11 +984,11 @@ static ssize_t __nfsd4_write_time(struct file *file, char *buf, size_t size,
> 		*time = i;
> 	}
> 
> -	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%ld\n", *time);
> +	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%lld\n", *time);
> }
> 
> static ssize_t nfsd4_write_time(struct file *file, char *buf, size_t size,
> -				time_t *time, struct nfsd_net *nn)
> +				time64_t *time, struct nfsd_net *nn)
> {
> 	ssize_t rv;
> 
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 03fc7b4380f9..e426b22b5028 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -132,7 +132,7 @@ struct nfs4_delegation {
> 	struct list_head	dl_recall_lru;  /* delegation recalled */
> 	struct nfs4_clnt_odstate *dl_clnt_odstate;
> 	u32			dl_type;
> -	time_t			dl_time;
> +	time64_t		dl_time;
> /* For recall: */
> 	int			dl_retries;
> 	struct nfsd4_callback	dl_recall;
> @@ -310,7 +310,7 @@ struct nfs4_client {
> #endif
> 	struct xdr_netobj	cl_name; 	/* id generated by client */
> 	nfs4_verifier		cl_verifier; 	/* generated by client */
> -	time_t                  cl_time;        /* time of last lease renewal */
> +	time64_t		cl_time;	/* time of last lease renewal */
> 	struct sockaddr_storage	cl_addr; 	/* client ipaddress */
> 	bool			cl_mach_cred;	/* SP4_MACH_CRED in force */
> 	struct svc_cred		cl_cred; 	/* setclientid principal */
> @@ -449,7 +449,7 @@ struct nfs4_openowner {
> 	 */
> 	struct list_head	oo_close_lru;
> 	struct nfs4_ol_stateid *oo_last_closed_stid;
> -	time_t			oo_time; /* time of placement on so_close_lru */
> +	time64_t		oo_time; /* time of placement on so_close_lru */
> #define NFS4_OO_CONFIRMED   1
> 	unsigned char		oo_flags;
> };
> @@ -606,7 +606,7 @@ static inline bool nfsd4_stateid_generation_after(stateid_t *a, stateid_t *b)
> struct nfsd4_blocked_lock {
> 	struct list_head	nbl_list;
> 	struct list_head	nbl_lru;
> -	time_t			nbl_time;
> +	time64_t		nbl_time;
> 	struct file_lock	nbl_lock;
> 	struct knfsd_fh		nbl_fh;
> 	struct nfsd4_callback	nbl_cb;
> -- 
> 2.20.0
> 

--
Chuck Lever

Re: [PATCH v2 10/12] nfsd: use boottime for lease expiry alculation

[Arnd Bergmann]

On Fri, Dec 13, 2019 at 5:26 PM Chuck Lever <chuck.lever@oracle.com> wrote:
> > On Dec 13, 2019, at 9:10 AM, Arnd Bergmann <arnd@arndb.de> wrote:

> > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> > index 24534db87e86..508d7c6c00b5 100644
> > --- a/fs/nfsd/nfs4callback.c
> > +++ b/fs/nfsd/nfs4callback.c
> > @@ -823,7 +823,12 @@ 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;
> > +
> > +     /* nfsd4_lease is set to at most one hour */
> > +     if (WARN_ON_ONCE(nn->nfsd4_lease > 3600))
> > +             return 360 * HZ;
>
> Why is the WARN_ON_ONCE added here? Is it really necessary?

This is to ensure the kernel doesn't change to a larger limit that
requires a 64-bit division on a 32-bit architecture.

With the old code, dividing by 10 was always fast as
nn->nfsd4_lease was the size of an integer register. Now it
is 64 bit wide, and I check that truncating it to 32 bit again
is safe.

> (Otherwise these all LGTM).

Thanks for taking a look.

      Arnd

Re: [PATCH v2 10/12] nfsd: use boottime for lease expiry alculation

[Chuck Lever]

> On Dec 13, 2019, at 11:40 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> On Fri, Dec 13, 2019 at 5:26 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>>> On Dec 13, 2019, at 9:10 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
>>> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
>>> index 24534db87e86..508d7c6c00b5 100644
>>> --- a/fs/nfsd/nfs4callback.c
>>> +++ b/fs/nfsd/nfs4callback.c
>>> @@ -823,7 +823,12 @@ 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;
>>> +
>>> +     /* nfsd4_lease is set to at most one hour */
>>> +     if (WARN_ON_ONCE(nn->nfsd4_lease > 3600))
>>> +             return 360 * HZ;
>> 
>> Why is the WARN_ON_ONCE added here? Is it really necessary?
> 
> This is to ensure the kernel doesn't change to a larger limit that
> requires a 64-bit division on a 32-bit architecture.
> 
> With the old code, dividing by 10 was always fast as
> nn->nfsd4_lease was the size of an integer register. Now it
> is 64 bit wide, and I check that truncating it to 32 bit again
> is safe.

OK. That comment should state this reason rather than just repeating
what the code does. ;-)


>> (Otherwise these all LGTM).
> 
> Thanks for taking a look.
> 
>      Arnd

--
Chuck Lever

Re: [PATCH v2 10/12] nfsd: use boottime for lease expiry alculation

[Arnd Bergmann]

On Fri, Dec 13, 2019 at 7:23 PM Chuck Lever <chuck.lever@oracle.com> wrote:
> > On Dec 13, 2019, at 11:40 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Fri, Dec 13, 2019 at 5:26 PM Chuck Lever <chuck.lever@oracle.com> wrote:
> >>> On Dec 13, 2019, at 9:10 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> >>> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> >>> index 24534db87e86..508d7c6c00b5 100644
> >
> > With the old code, dividing by 10 was always fast as
> > nn->nfsd4_lease was the size of an integer register. Now it
> > is 64 bit wide, and I check that truncating it to 32 bit again
> > is safe.
>
> OK. That comment should state this reason rather than just repeating
> what the code does. ;-)

I changed the comment now to:

+       /*
+        * nfsd4_lease is set to at most one hour in __nfsd4_write_time,
+        * so we can use 32-bit math on it. Warn if that assumption
+        * ever stops being true.
+        */

Modified branch pushed to
git://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git y2038-nfsd-v2

        Arnd

Re: [PATCH v2 10/12] nfsd: use boottime for lease expiry alculation

[Bruce Fields]

On Fri, Dec 13, 2019 at 01:23:08PM -0500, Chuck Lever wrote:
> 
> 
> > On Dec 13, 2019, at 11:40 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > 
> > On Fri, Dec 13, 2019 at 5:26 PM Chuck Lever <chuck.lever@oracle.com> wrote:
> >>> On Dec 13, 2019, at 9:10 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > 
> >>> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> >>> index 24534db87e86..508d7c6c00b5 100644
> >>> --- a/fs/nfsd/nfs4callback.c
> >>> +++ b/fs/nfsd/nfs4callback.c
> >>> @@ -823,7 +823,12 @@ 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;
> >>> +
> >>> +     /* nfsd4_lease is set to at most one hour */
> >>> +     if (WARN_ON_ONCE(nn->nfsd4_lease > 3600))
> >>> +             return 360 * HZ;
> >> 
> >> Why is the WARN_ON_ONCE added here? Is it really necessary?
> > 
> > This is to ensure the kernel doesn't change to a larger limit that
> > requires a 64-bit division on a 32-bit architecture.
> > 
> > With the old code, dividing by 10 was always fast as
> > nn->nfsd4_lease was the size of an integer register. Now it
> > is 64 bit wide, and I check that truncating it to 32 bit again
> > is safe.
> 
> OK. That comment should state this reason rather than just repeating
> what the code does. ;-)

Note that __nfsd4_write_time() already limits nfsd4_lease to 3600.

We could just use a smaller type for nfsd4_lease if that'd help.

--b.

Re: [PATCH v2 10/12] nfsd: use boottime for lease expiry alculation

[Arnd Bergmann]

On Fri, Dec 13, 2019 at 10:13 PM Bruce Fields <bfields@fieldses.org> wrote:
> On Fri, Dec 13, 2019 at 01:23:08PM -0500, Chuck Lever wrote:
> > > On Dec 13, 2019, at 11:40 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Fri, Dec 13, 2019 at 5:26 PM Chuck Lever <chuck.lever@oracle.com> wrote:
> > >>> +
> > >>> +     /* nfsd4_lease is set to at most one hour */
> > >>> +     if (WARN_ON_ONCE(nn->nfsd4_lease > 3600))
> > >>> +             return 360 * HZ;
> > >>
> > >> Why is the WARN_ON_ONCE added here? Is it really necessary?
> > >
> > > This is to ensure the kernel doesn't change to a larger limit that
> > > requires a 64-bit division on a 32-bit architecture.
> > >
> > > With the old code, dividing by 10 was always fast as
> > > nn->nfsd4_lease was the size of an integer register. Now it
> > > is 64 bit wide, and I check that truncating it to 32 bit again
> > > is safe.
> >
> > OK. That comment should state this reason rather than just repeating
> > what the code does. ;-)
>
> Note that __nfsd4_write_time() already limits nfsd4_lease to 3600.
>
> We could just use a smaller type for nfsd4_lease if that'd help.

I think it's generally clearer to have only one type to store the lease
time, and time64_t is the most sensible one, even if the range is a
bit excessive.

I've seen too many time related bugs from mixing integer types
incorrectly.

       Arnd

Re: [PATCH v2 00/12] nfsd: avoid 32-bit time_t

[Arnd Bergmann]

On Fri, Dec 13, 2019 at 3:12 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Hi Bruce, Chuck,
>
> NFSd is one of the last areas of the kernel that is not y2038 safe
> yet, this series addresses the remaining issues here.
>
> I did not get any comments for the first version I posted [1], and
> I hope this just means that everything was fine and you plan to
> merge this soon ;-)
>
> I uploaded a git branch to [2] for testing.
>
> Please review and merge for linux-5.6 so we can remove the 32-bit
> time handling from that release.

I've included the update y2038 nfsd branch from

git://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git y2038-nfsd-v2

in my y2038 branch now, so it should be part of linux-next from the coming
snapshot. My plan is to send a linux-5.6 pull request to the nfsd
maintainers for
this branch unless we find bugs in linux-next or I get more review comments.

        Arnd

Re: [PATCH v2 00/12] nfsd: avoid 32-bit time_t

[J. Bruce Fields]

On Wed, Dec 18, 2019 at 06:21:06PM +0100, Arnd Bergmann wrote:
> On Fri, Dec 13, 2019 at 3:12 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > Hi Bruce, Chuck,
> >
> > NFSd is one of the last areas of the kernel that is not y2038 safe
> > yet, this series addresses the remaining issues here.
> >
> > I did not get any comments for the first version I posted [1], and
> > I hope this just means that everything was fine and you plan to
> > merge this soon ;-)
> >
> > I uploaded a git branch to [2] for testing.
> >
> > Please review and merge for linux-5.6 so we can remove the 32-bit
> > time handling from that release.
> 
> I've included the update y2038 nfsd branch from
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git y2038-nfsd-v2
> 
> in my y2038 branch now, so it should be part of linux-next from the coming
> snapshot. My plan is to send a linux-5.6 pull request to the nfsd
> maintainers for
> this branch unless we find bugs in linux-next or I get more review comments.

Sorry for the silence.  The patches look fine to me, so I took them from
your git branch and applied them to my local tree (after fixing up some
minor conflicts with the copy patches) and then saw some
delegation-related test failures, which I haven't had the chance to
investigate yet.  Hopefully before the end of the week.

--b.

Re: [PATCH v2 00/12] nfsd: avoid 32-bit time_t

[Arnd Bergmann]

On Wed, Dec 18, 2019 at 6:25 PM J. Bruce Fields <bfields@fieldses.org> wrote:
> On Wed, Dec 18, 2019 at 06:21:06PM +0100, Arnd Bergmann wrote:
> > On Fri, Dec 13, 2019 at 3:12 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git y2038-nfsd-v2
> >
> > in my y2038 branch now, so it should be part of linux-next from the coming
> > snapshot. My plan is to send a linux-5.6 pull request to the nfsd
> > maintainers for
> > this branch unless we find bugs in linux-next or I get more review comments.
>
> Sorry for the silence.  The patches look fine to me, so I took them from
> your git branch and applied them to my local tree (after fixing up some
> minor conflicts with the copy patches) and then saw some
> delegation-related test failures, which I haven't had the chance to
> investigate yet.  Hopefully before the end of the week.

Ok, I've taken the nfsd changes out of my y2038 branch again then, to
make sure we don't get these regressions in linux-next.

Thanks for taking care of the patches. I will not be in the office from
Friday to the end of the year, so probably won't be able to respin the
series quickly once you find the bug(s), but I should be able to reply
to emails about this.

      Arnd

Re: [PATCH v2 10/12] nfsd: use boottime for lease expiry alculation

[J. Bruce Fields]

On Fri, Dec 13, 2019 at 03:10:44PM +0100, Arnd Bergmann wrote:
> @@ -5212,8 +5211,8 @@ nfs4_laundromat(struct nfsd_net *nn)
>  	struct nfs4_ol_stateid *stp;
>  	struct nfsd4_blocked_lock *nbl;
>  	struct list_head *pos, *next, reaplist;
> -	time_t cutoff = get_seconds() - nn->nfsd4_lease;
> -	time_t t, new_timeo = nn->nfsd4_lease;
> +	time64_t cutoff = ktime_get_boottime_seconds() - nn->nfsd4_lease;

For some reason the version I was testing still had this as
get_seconds(), which was what was causing test failures.  I'm not quite
sure what happened--I may have just typo'd something while fixing up a
conflict.

--b.

> +	time64_t t, new_timeo = nn->nfsd4_lease;
>  
>  	dprintk("NFSD: laundromat service - starting\n");
>  
> @@ -5227,7 +5226,7 @@ nfs4_laundromat(struct nfsd_net *nn)
>  	spin_lock(&nn->client_lock);
>  	list_for_each_safe(pos, next, &nn->client_lru) {
>  		clp = list_entry(pos, struct nfs4_client, cl_lru);
> -		if (time_after((unsigned long)clp->cl_time, (unsigned long)cutoff)) {
> +		if (clp->cl_time > cutoff) {
>  			t = clp->cl_time - cutoff;
>  			new_timeo = min(new_timeo, t);
>  			break;
> @@ -5250,7 +5249,7 @@ nfs4_laundromat(struct nfsd_net *nn)
>  	spin_lock(&state_lock);
>  	list_for_each_safe(pos, next, &nn->del_recall_lru) {
>  		dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> -		if (time_after((unsigned long)dp->dl_time, (unsigned long)cutoff)) {
> +		if (dp->dl_time > cutoff) {
>  			t = dp->dl_time - cutoff;
>  			new_timeo = min(new_timeo, t);
>  			break;
> @@ -5270,8 +5269,7 @@ nfs4_laundromat(struct nfsd_net *nn)
>  	while (!list_empty(&nn->close_lru)) {
>  		oo = list_first_entry(&nn->close_lru, struct nfs4_openowner,
>  					oo_close_lru);
> -		if (time_after((unsigned long)oo->oo_time,
> -			       (unsigned long)cutoff)) {
> +		if (oo->oo_time > cutoff) {
>  			t = oo->oo_time - cutoff;
>  			new_timeo = min(new_timeo, t);
>  			break;
> @@ -5301,8 +5299,7 @@ nfs4_laundromat(struct nfsd_net *nn)
>  	while (!list_empty(&nn->blocked_locks_lru)) {
>  		nbl = list_first_entry(&nn->blocked_locks_lru,
>  					struct nfsd4_blocked_lock, nbl_lru);
> -		if (time_after((unsigned long)nbl->nbl_time,
> -			       (unsigned long)cutoff)) {
> +		if (nbl->nbl_time > cutoff) {
>  			t = nbl->nbl_time - cutoff;
>  			new_timeo = min(new_timeo, t);
>  			break;
> @@ -5319,7 +5316,7 @@ nfs4_laundromat(struct nfsd_net *nn)
>  		free_blocked_lock(nbl);
>  	}
>  out:
> -	new_timeo = max_t(time_t, new_timeo, NFSD_LAUNDROMAT_MINTIMEOUT);
> +	new_timeo = max_t(time64_t, new_timeo, NFSD_LAUNDROMAT_MINTIMEOUT);
>  	return new_timeo;
>  }
>  
> @@ -5329,13 +5326,13 @@ static void laundromat_main(struct work_struct *);
>  static void
>  laundromat_main(struct work_struct *laundry)
>  {
> -	time_t t;
> +	time64_t t;
>  	struct delayed_work *dwork = to_delayed_work(laundry);
>  	struct nfsd_net *nn = container_of(dwork, struct nfsd_net,
>  					   laundromat_work);
>  
>  	t = nfs4_laundromat(nn);
> -	dprintk("NFSD: laundromat_main - sleeping for %ld seconds\n", t);
> +	dprintk("NFSD: laundromat_main - sleeping for %lld seconds\n", t);
>  	queue_delayed_work(laundry_wq, &nn->laundromat_work, t*HZ);
>  }
>  
> @@ -6549,7 +6546,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	}
>  
>  	if (fl_flags & FL_SLEEP) {
> -		nbl->nbl_time = get_seconds();
> +		nbl->nbl_time = ktime_get_boottime_seconds();
>  		spin_lock(&nn->blocked_locks_lock);
>  		list_add_tail(&nbl->nbl_list, &lock_sop->lo_blocked);
>  		list_add_tail(&nbl->nbl_lru, &nn->blocked_locks_lru);
> @@ -7709,7 +7706,7 @@ nfs4_state_start_net(struct net *net)
>  	nfsd4_client_tracking_init(net);
>  	if (nn->track_reclaim_completes && nn->reclaim_str_hashtbl_size == 0)
>  		goto skip_grace;
> -	printk(KERN_INFO "NFSD: starting %ld-second grace period (net %x)\n",
> +	printk(KERN_INFO "NFSD: starting %lld-second grace period (net %x)\n",
>  	       nn->nfsd4_grace, net->ns.inum);
>  	queue_delayed_work(laundry_wq, &nn->laundromat_work, nn->nfsd4_grace * HZ);
>  	return 0;
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 11b42c523f04..aace740d5a92 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -956,7 +956,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)
> +				  time64_t *time, struct nfsd_net *nn)
>  {
>  	char *mesg = buf;
>  	int rv, i;
> @@ -984,11 +984,11 @@ static ssize_t __nfsd4_write_time(struct file *file, char *buf, size_t size,
>  		*time = i;
>  	}
>  
> -	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%ld\n", *time);
> +	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%lld\n", *time);
>  }
>  
>  static ssize_t nfsd4_write_time(struct file *file, char *buf, size_t size,
> -				time_t *time, struct nfsd_net *nn)
> +				time64_t *time, struct nfsd_net *nn)
>  {
>  	ssize_t rv;
>  
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 03fc7b4380f9..e426b22b5028 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -132,7 +132,7 @@ struct nfs4_delegation {
>  	struct list_head	dl_recall_lru;  /* delegation recalled */
>  	struct nfs4_clnt_odstate *dl_clnt_odstate;
>  	u32			dl_type;
> -	time_t			dl_time;
> +	time64_t		dl_time;
>  /* For recall: */
>  	int			dl_retries;
>  	struct nfsd4_callback	dl_recall;
> @@ -310,7 +310,7 @@ struct nfs4_client {
>  #endif
>  	struct xdr_netobj	cl_name; 	/* id generated by client */
>  	nfs4_verifier		cl_verifier; 	/* generated by client */
> -	time_t                  cl_time;        /* time of last lease renewal */
> +	time64_t		cl_time;	/* time of last lease renewal */
>  	struct sockaddr_storage	cl_addr; 	/* client ipaddress */
>  	bool			cl_mach_cred;	/* SP4_MACH_CRED in force */
>  	struct svc_cred		cl_cred; 	/* setclientid principal */
> @@ -449,7 +449,7 @@ struct nfs4_openowner {
>  	 */
>  	struct list_head	oo_close_lru;
>  	struct nfs4_ol_stateid *oo_last_closed_stid;
> -	time_t			oo_time; /* time of placement on so_close_lru */
> +	time64_t		oo_time; /* time of placement on so_close_lru */
>  #define NFS4_OO_CONFIRMED   1
>  	unsigned char		oo_flags;
>  };
> @@ -606,7 +606,7 @@ static inline bool nfsd4_stateid_generation_after(stateid_t *a, stateid_t *b)
>  struct nfsd4_blocked_lock {
>  	struct list_head	nbl_list;
>  	struct list_head	nbl_lru;
> -	time_t			nbl_time;
> +	time64_t		nbl_time;
>  	struct file_lock	nbl_lock;
>  	struct knfsd_fh		nbl_fh;
>  	struct nfsd4_callback	nbl_cb;
> -- 
> 2.20.0