Skip to contentSkip to conversation

knfsd change_attr_type & time_delta improvements

PATCH In review J. Bruce Fields 2 participants 4 messages Jun 14, 2018 – Jun 15, 2018

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

J. Bruce Fields

author
#0 lore raw

cover knfsd change_attr_type & time_delta improvements

From: "J. Bruce Fields" <[email protected]>

Implement change_attr_type, and fix time_delta.

I don't think the client currently uses either, I'm just assuming they'll be useful some day, especially now that (thanks to jlayton) the change attribute should be a real change attribute now in the IS_I_VERSION() case.

J. Bruce Fields (2):
nfsd: fix NFSv4 time_delta attribute
nfsd4: support change_attr_type attribute

 fs/nfsd/nfs4xdr.c    | 39 ++++++++++++++++++++++++++++++++++++---
 fs/nfsd/nfsd.h       |  1 +
 include/linux/nfs4.h |  8 ++++++++
 3 files changed, 45 insertions(+), 3 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH 1/2 nfsd: fix NFSv4 time_delta attribute

From: "J. Bruce Fields" <[email protected]>

Currently we return the worst-case value of 1 second in the time delta attribute. That's not terribly useful. Instead, return a value calculated from the time granularity supported by the filesystem and the system clock.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4xdr.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)
fs/nfsd/nfs4xdr.c +26 -3
index 59d471025949..4c5fa0ff2387 100644
@@ -2003,6 +2003,31 @@ static __be32 *encode_change(__be32 *p, struct kstat *stat, struct inode *inode,
 	return p;
 }
 
+/*
+ * ctime (in NFSv4, time_metadata) is not writeable, and the client
+ * doesn't really care what resolution could theoretically be stored by
+ * the filesystem.
+ *
+ * The client cares how close together changes can be while still
+ * guaranteeing ctime changes.  For most filesystems (which have
+ * timestamps with nanosecond fields) that is limited by the resolution
+ * of the time returned from current_time() (which I'm assuming to be
+ * 1/HZ).
+ */
+static __be32 *encode_time_delta(__be32 *p, struct inode *inode)
+{
+	struct timespec ts;
+	u32 ns;
+
+	ns = max(NSEC_PER_SEC/HZ, inode->i_sb->s_time_gran);
+	ts = ns_to_timespec(ns);
+
+	p = xdr_encode_hyper(p, ts.tv_sec);
+	*p++ = cpu_to_be32(ts.tv_nsec);
+
+	return p;
+}
+
 static __be32 *encode_cinfo(__be32 *p, struct nfsd4_change_info *c)
 {
 	*p++ = cpu_to_be32(c->atomic);

@@ -2794,9 +2819,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 		p = xdr_reserve_space(xdr, 12);
 		if (!p)
 			goto out_resource;
-		*p++ = cpu_to_be32(0);
-		*p++ = cpu_to_be32(1);
-		*p++ = cpu_to_be32(0);
+		p = encode_time_delta(p, d_inode(dentry));
 	}
 	if (bmval1 & FATTR4_WORD1_TIME_METADATA) {
 		p = xdr_reserve_space(xdr, 12);
Signature · 1 line
2.17.1
author
#2 lore raw

PATCH 2/2 nfsd4: support change_attr_type attribute

In reply to J. Bruce Fields

From: "J. Bruce Fields" <[email protected]>

The change attribute is what is used by clients to revalidate their caches. Our server may use i_version or ctime for that purpose. Those choices behave slightly differently, and it may be useful to the client to know which we're using. This attribute tells the client that. The Linux client doesn't yet use this attribute yet, though.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4xdr.c    | 10 ++++++++++
 fs/nfsd/nfsd.h       |  1 +
 include/linux/nfs4.h |  8 ++++++++
 3 files changed, 19 insertions(+)
fs/nfsd/nfs4xdr.c +10 -0
index 4c5fa0ff2387..eea31ff4a3ff 100644
@@ -2888,6 +2888,16 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 			goto out;
 	}
 
+	if (bmval2 & FATTR4_WORD2_CHANGE_ATTR_TYPE) {
+		p = xdr_reserve_space(xdr, 4);
+		if (!p)
+			goto out_resource;
+		if (IS_I_VERSION(d_inode(dentry)))
+			*p++ = cpu_to_be32(NFS4_CHANGE_TYPE_IS_MONOTONIC_INCR);
+		else
+			*p++ = cpu_to_be32(NFS4_CHANGE_TYPE_IS_TIME_METADATA);
+	}
+
 	if (bmval2 & FATTR4_WORD2_SECURITY_LABEL) {
 		status = nfsd4_encode_security_label(xdr, rqstp, context,
 								contextlen);
fs/nfsd/nfsd.h +1 -0
index 3fce905d0365..066899929863 100644
@@ -360,6 +360,7 @@ void		nfsd_lockd_shutdown(void);
 
 #define NFSD4_2_SUPPORTED_ATTRS_WORD2 \
 	(NFSD4_1_SUPPORTED_ATTRS_WORD2 | \
+	FATTR4_WORD2_CHANGE_ATTR_TYPE | \
 	FATTR4_WORD2_MODE_UMASK | \
 	NFSD4_2_SECURITY_ATTRS)
include/linux/nfs4.h +8 -0
index 57ffaa20d564..0877ed333733 100644
@@ -374,6 +374,13 @@ enum lock_type4 {
 	NFS4_WRITEW_LT = 4
 };
 
+enum change_attr_type4 {
+	NFS4_CHANGE_TYPE_IS_MONOTONIC_INCR = 0,
+	NFS4_CHANGE_TYPE_IS_VERSION_COUNTER = 1,
+	NFS4_CHANGE_TYPE_IS_VERSION_COUNTER_NOPNFS = 2,
+	NFS4_CHANGE_TYPE_IS_TIME_METADATA = 3,
+	NFS4_CHANGE_TYPE_IS_UNDEFINED = 4
+};
 
 /* Mandatory Attributes */
 #define FATTR4_WORD0_SUPPORTED_ATTRS    (1UL << 0)

@@ -441,6 +448,7 @@ enum lock_type4 {
 #define FATTR4_WORD2_LAYOUT_BLKSIZE     (1UL << 1)
 #define FATTR4_WORD2_MDSTHRESHOLD       (1UL << 4)
 #define FATTR4_WORD2_CLONE_BLKSIZE	(1UL << 13)
+#define FATTR4_WORD2_CHANGE_ATTR_TYPE	(1UL << 15)
 #define FATTR4_WORD2_SECURITY_LABEL     (1UL << 16)
 #define FATTR4_WORD2_MODE_UMASK		(1UL << 17)
Signature · 1 line
2.17.1
maintainer Reviewed-by on the v1 series
#3 lore raw
In reply to J. Bruce Fields
Quoted 10 lines · J. Bruce Fields

From: "J. Bruce Fields" <[email protected]>

Implement change_attr_type, and fix time_delta.

I don't think the client currently uses either, I'm just assuming they'll be useful some day, especially now that (thanks to jlayton) the change attribute should be a real change attribute now in the IS_I_VERSION() case.

Quoted 9 lines

J. Bruce Fields (2):
nfsd: fix NFSv4 time_delta attribute
nfsd4: support change_attr_type attribute

 fs/nfsd/nfs4xdr.c    | 39 ++++++++++++++++++++++++++++++++++++---
 fs/nfsd/nfsd.h       |  1 +
 include/linux/nfs4.h |  8 ++++++++
 3 files changed, 45 insertions(+), 3 deletions(-)

Both look good.

Reviewed-by Jeff Layton <[email protected]>