This series restructures the server's fattr4 encoder. It is largely
a maintenance improvement (ie, only 2nd-order benefits). There are
no new features or performance benefits, and I hope there will be no
changes in behavior.
The goals:
* Better alignment with spec
* Easier to read and audit
* Less brittle
* Some code de-duplication
This series applies to v6.6-rc2. Minor adjustment will be needed to
apply it to nfsd-next. I apologize for the number of patches, but
each of them (with only a couple of exceptions) is small and
mechanical, and therefore easily digested.
A branch containing these patches is available in this repo:
https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
See the "nfsd4-encoder-overhaul" branch.
---
Chuck Lever (52):
NFSD: Add simple u32, u64, and bool encoders
NFSD: Rename nfsd4_encode_bitmap()
NFSD: Clean up nfsd4_encode_setattr()
NFSD: Add struct nfsd4_fattr_args
NFSD: Add nfsd4_encode_fattr4__true()
NFSD: Add nfsd4_encode_fattr4__false()
NFSD: Add nfsd4_encode_fattr4_supported_attrs()
NFSD: Add nfsd4_encode_fattr4_type()
NFSD: Add nfsd4_encode_fattr4_fh_expire_type()
NFSD: Add nfsd4_encode_fattr4_change()
NFSD: Add nfsd4_encode_fattr4_size()
NFSD: Add nfsd4_encode_fattr4_fsid()
NFSD: Add nfsd4_encode_fattr4_lease_time()
NFSD: Add nfsd4_encode_fattr4_rdattr_error()
NFSD: Add nfsd4_encode_fattr4_aclsupport()
NFSD: Add nfsd4_encode_nfsace4()
NFSD: Add nfsd4_encode_fattr4_acl()
NFSD: Add nfsd4_encode_fattr4_filehandle()
NFSD: Add nfsd4_encode_fattr4_fileid()
NFSD: Add nfsd4_encode_fattr4_files_avail()
NFSD: Add nfsd4_encode_fattr4_files_free()
NFSD: Add nfsd4_encode_fattr4_files_total()
NFSD: Add nfsd4_encode_fattr4_fs_locations()
NFSD: Add nfsd4_encode_fattr4_maxfilesize()
NFSD: Add nfsd4_encode_fattr4_maxlink()
NFSD: Add nfsd4_encode_fattr4_maxname()
NFSD: Add nfsd4_encode_fattr4_maxread()
NFSD: Add nfsd4_encode_fattr4_maxwrite()
NFSD: Add nfsd4_encode_fattr4_mode()
NFSD: Add nfsd4_encode_fattr4_numlinks()
NFSD: Add nfsd4_encode_fattr4_owner()
NFSD: Add nfsd4_encode_fattr4_owner_group()
NFSD: Add nfsd4_encode_fattr4_rawdev()
NFSD: Add nfsd4_encode_fattr4_space_avail()
NFSD: Add nfsd4_encode_fattr4_space_free()
NFSD: Add nfsd4_encode_fattr4_space_total()
NFSD: Add nfsd4_encode_fattr4_space_used()
NFSD: Add nfsd4_encode_fattr4_time_access()
NFSD: Add nfsd4_encode_fattr4_time_create()
NFSD: Add nfsd4_encode_fattr4_time_delta()
NFSD: Add nfsd4_encode_fattr4_time_metadata()
NFSD: Add nfsd4_encode_fattr4_time_modify()
NFSD: Add nfsd4_encode_fattr4_mounted_on_fileid()
NFSD: Add nfsd4_encode_fattr4_fs_layout_types()
NFSD: Add nfsd4_encode_fattr4_layout_types()
NFSD: Add nfsd4_encode_fattr4_layout_blksize()
NFSD: Add nfsd4_encode_fattr4_suppattr_exclcreat()
NFSD: Add nfsd4_encode_fattr4_sec_label()
NFSD: Add nfsd4_encode_fattr4_xattr_support()
NFSD: Copy FATTR4 bit number definitions from RFCs
NFSD: Use a bitmask loop to encode FATTR4 results
NFSD: Rename nfsd4_encode_fattr()
fs/nfsd/nfs4xdr.c | 1419 +++++++++++++++++++++-----------------
fs/nfsd/nfsfh.c | 2 +-
fs/nfsd/nfsfh.h | 3 +-
fs/nfsd/xdr4.h | 119 ++++
include/linux/iversion.h | 2 +-
include/linux/nfs4.h | 260 +++++--
6 files changed, 1085 insertions(+), 720 deletions(-)
--
Chuck Lever
From: Chuck Lever <[email protected]>
Refactor the encoder for FATTR4_TIME_DELTA into a helper. In a
subsequent patch, this helper will be called from a bitmask loop.
fattr4_time_delta is specified as an nfstime4, so de-duplicate this
encoder.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4xdr.c | 53 ++++++++++++++++++++++++-----------------------------
1 file changed, 24 insertions(+), 29 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 5a98dc3910ef..28eb777f82d2 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2561,31 +2561,6 @@ static __be32 nfsd4_encode_specdata4(struct xdr_stream *xdr,
return nfsd4_encode_uint32_t(xdr, minor);
}
-/*
- * 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 timespec64 ts;
- u32 ns;
-
- ns = max_t(u32, NSEC_PER_SEC/HZ, inode->i_sb->s_time_gran);
- ts = ns_to_timespec64(ns);
-
- p = xdr_encode_hyper(p, ts.tv_sec);
- *p++ = cpu_to_be32(ts.tv_nsec);
-
- return p;
-}
-
static __be32
nfsd4_encode_change_info4(struct xdr_stream *xdr, const struct nfsd4_change_info *c)
{
@@ -3266,6 +3241,27 @@ static __be32 nfsd4_encode_fattr4_time_create(struct xdr_stream *xdr,
return nfsd4_encode_nfstime4(xdr, &args->stat.btime);
}
+/*
+ * 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 nfsd4_encode_fattr4_time_delta(struct xdr_stream *xdr,
+ const struct nfsd4_fattr_args *args)
+{
+ const struct inode *inode = d_inode(args->dentry);
+ u32 ns = max_t(u32, NSEC_PER_SEC/HZ, inode->i_sb->s_time_gran);
+ struct timespec64 ts = ns_to_timespec64(ns);
+
+ return nfsd4_encode_nfstime4(xdr, &ts);
+}
+
/*
* Note: @fhp can be NULL; in this case, we might have to compose the filehandle
* ourselves.
@@ -3599,10 +3595,9 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
goto out;
}
if (bmval1 & FATTR4_WORD1_TIME_DELTA) {
- p = xdr_reserve_space(xdr, 12);
- if (!p)
- goto out_resource;
- p = encode_time_delta(p, d_inode(dentry));
+ status = nfsd4_encode_fattr4_time_delta(xdr, &args);
+ if (status != nfs_ok)
+ goto out;
}
if (bmval1 & FATTR4_WORD1_TIME_METADATA) {
status = nfsd4_encode_nfstime4(xdr, &args.stat.ctime);
From: Chuck Lever <[email protected]>
Refactor the encoder for FATTR4_SPACE_USED into a helper. In a
subsequent patch, this helper will be called from a bitmask loop.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4xdr.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 963fa2ccadd7..80be20217402 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3248,6 +3248,12 @@ static __be32 nfsd4_encode_fattr4_space_total(struct xdr_stream *xdr,
return nfsd4_encode_uint64_t(xdr, total);
}
+static __be32 nfsd4_encode_fattr4_space_used(struct xdr_stream *xdr,
+ const struct nfsd4_fattr_args *args)
+{
+ return nfsd4_encode_uint64_t(xdr, (u64)args->stat.blocks << 9);
+}
+
/*
* Note: @fhp can be NULL; in this case, we might have to compose the filehandle
* ourselves.
@@ -3266,7 +3272,6 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
__be32 *p, *attrlen_p;
int starting_len = xdr->buf->len;
int attrlen_offset;
- u64 dummy64;
__be32 status;
int err;
#ifdef CONFIG_NFSD_V4_SECURITY_LABEL
@@ -3567,11 +3572,9 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
goto out;
}
if (bmval1 & FATTR4_WORD1_SPACE_USED) {
- p = xdr_reserve_space(xdr, 8);
- if (!p)
- goto out_resource;
- dummy64 = (u64)args.stat.blocks << 9;
- p = xdr_encode_hyper(p, dummy64);
+ status = nfsd4_encode_fattr4_space_used(xdr, &args);
+ if (status != nfs_ok)
+ goto out;
}
if (bmval1 & FATTR4_WORD1_TIME_ACCESS) {
status = nfsd4_encode_nfstime4(xdr, &args.stat.atime);
From: Chuck Lever <[email protected]>
Refactor the encoder for FATTR4_SIZE into a helper. In a subsequent
patch, this helper will be called from a bitmask loop.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4xdr.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 3149cfcdac35..601b3a0e61d4 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3024,6 +3024,12 @@ static __be32 nfsd4_encode_fattr4_change(struct xdr_stream *xdr,
return nfsd4_encode_changeid4(xdr, c);
}
+static __be32 nfsd4_encode_fattr4_size(struct xdr_stream *xdr,
+ const struct nfsd4_fattr_args *args)
+{
+ return nfsd4_encode_uint64_t(xdr, args->stat.size);
+}
+
/*
* Note: @fhp can be NULL; in this case, we might have to compose the filehandle
* ourselves.
@@ -3169,10 +3175,9 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
goto out;
}
if (bmval0 & FATTR4_WORD0_SIZE) {
- p = xdr_reserve_space(xdr, 8);
- if (!p)
- goto out_resource;
- p = xdr_encode_hyper(p, args.stat.size);
+ status = nfsd4_encode_fattr4_size(xdr, &args);
+ if (status != nfs_ok)
+ goto out;
}
if (bmval0 & FATTR4_WORD0_LINK_SUPPORT) {
status = nfsd4_encode_fattr4__true(xdr, &args);
From: Chuck Lever <[email protected]>
Refactor the encoder for FATTR4_TYPE into a helper. In a subsequent
patch, this helper will be called from a bitmask loop.
In addition, restructure the code so that byte-swapping is done on
constant values rather than at run time. Run-time swapping can be
costly on some platforms, and "type" is a frequently-requested
attribute.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4xdr.c | 63 ++++++++++++++++++++++++++++++++++-------------------
1 file changed, 40 insertions(+), 23 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index da5df33cac04..c1dc6810f043 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2792,20 +2792,6 @@ static __be32 nfsd4_encode_fs_locations(struct xdr_stream *xdr,
return 0;
}
-static u32 nfs4_file_type(umode_t mode)
-{
- switch (mode & S_IFMT) {
- case S_IFIFO: return NF4FIFO;
- case S_IFCHR: return NF4CHR;
- case S_IFDIR: return NF4DIR;
- case S_IFBLK: return NF4BLK;
- case S_IFLNK: return NF4LNK;
- case S_IFREG: return NF4REG;
- case S_IFSOCK: return NF4SOCK;
- default: return NF4BAD;
- }
-}
-
static inline __be32
nfsd4_encode_aclname(struct xdr_stream *xdr, struct svc_rqst *rqstp,
struct nfs4_ace *ace)
@@ -2977,6 +2963,44 @@ static __be32 nfsd4_encode_fattr4_supported_attrs(struct xdr_stream *xdr,
return nfsd4_encode_bitmap4(xdr, supp[0], supp[1], supp[2]);
}
+static __be32 nfsd4_encode_fattr4_type(struct xdr_stream *xdr,
+ const struct nfsd4_fattr_args *args)
+{
+ __be32 *p;
+
+ p = xdr_reserve_space(xdr, XDR_UNIT);
+ if (!p)
+ return nfserr_resource;
+
+ switch (args->stat.mode & S_IFMT) {
+ case S_IFIFO:
+ *p = cpu_to_be32(NF4FIFO);
+ break;
+ case S_IFCHR:
+ *p = cpu_to_be32(NF4CHR);
+ break;
+ case S_IFDIR:
+ *p = cpu_to_be32(NF4DIR);
+ break;
+ case S_IFBLK:
+ *p = cpu_to_be32(NF4BLK);
+ break;
+ case S_IFLNK:
+ *p = cpu_to_be32(NF4LNK);
+ break;
+ case S_IFREG:
+ *p = cpu_to_be32(NF4REG);
+ break;
+ case S_IFSOCK:
+ *p = cpu_to_be32(NF4SOCK);
+ break;
+ default:
+ return nfserr_serverfault;
+ }
+
+ return nfs_ok;
+}
+
/*
* Note: @fhp can be NULL; in this case, we might have to compose the filehandle
* ourselves.
@@ -2995,7 +3019,6 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
__be32 *p, *attrlen_p;
int starting_len = xdr->buf->len;
int attrlen_offset;
- u32 dummy;
u64 dummy64;
__be32 status;
int err;
@@ -3107,15 +3130,9 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
goto out;
}
if (bmval0 & FATTR4_WORD0_TYPE) {
- p = xdr_reserve_space(xdr, 4);
- if (!p)
- goto out_resource;
- dummy = nfs4_file_type(args.stat.mode);
- if (dummy == NF4BAD) {
- status = nfserr_serverfault;
+ status = nfsd4_encode_fattr4_type(xdr, &args);
+ if (status != nfs_ok)
goto out;
- }
- *p++ = cpu_to_be32(dummy);
}
if (bmval0 & FATTR4_WORD0_FH_EXPIRE_TYPE) {
p = xdr_reserve_space(xdr, 4);
On Mon, 2023-09-18 at 09:56 -0400, Chuck Lever wrote:
> This series restructures the server's fattr4 encoder. It is largely
> a maintenance improvement (ie, only 2nd-order benefits). There are
> no new features or performance benefits, and I hope there will be no
> changes in behavior.
>
> The goals:
> * Better alignment with spec
> * Easier to read and audit
> * Less brittle
> * Some code de-duplication
>
> This series applies to v6.6-rc2. Minor adjustment will be needed to
> apply it to nfsd-next. I apologize for the number of patches, but
> each of them (with only a couple of exceptions) is small and
> mechanical, and therefore easily digested.
>
> A branch containing these patches is available in this repo:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>
> See the "nfsd4-encoder-overhaul" branch.
>
> ---
>
> Chuck Lever (52):
> NFSD: Add simple u32, u64, and bool encoders
> NFSD: Rename nfsd4_encode_bitmap()
> NFSD: Clean up nfsd4_encode_setattr()
> NFSD: Add struct nfsd4_fattr_args
> NFSD: Add nfsd4_encode_fattr4__true()
> NFSD: Add nfsd4_encode_fattr4__false()
> NFSD: Add nfsd4_encode_fattr4_supported_attrs()
> NFSD: Add nfsd4_encode_fattr4_type()
> NFSD: Add nfsd4_encode_fattr4_fh_expire_type()
> NFSD: Add nfsd4_encode_fattr4_change()
> NFSD: Add nfsd4_encode_fattr4_size()
> NFSD: Add nfsd4_encode_fattr4_fsid()
> NFSD: Add nfsd4_encode_fattr4_lease_time()
> NFSD: Add nfsd4_encode_fattr4_rdattr_error()
> NFSD: Add nfsd4_encode_fattr4_aclsupport()
> NFSD: Add nfsd4_encode_nfsace4()
> NFSD: Add nfsd4_encode_fattr4_acl()
> NFSD: Add nfsd4_encode_fattr4_filehandle()
> NFSD: Add nfsd4_encode_fattr4_fileid()
> NFSD: Add nfsd4_encode_fattr4_files_avail()
> NFSD: Add nfsd4_encode_fattr4_files_free()
> NFSD: Add nfsd4_encode_fattr4_files_total()
> NFSD: Add nfsd4_encode_fattr4_fs_locations()
> NFSD: Add nfsd4_encode_fattr4_maxfilesize()
> NFSD: Add nfsd4_encode_fattr4_maxlink()
> NFSD: Add nfsd4_encode_fattr4_maxname()
> NFSD: Add nfsd4_encode_fattr4_maxread()
> NFSD: Add nfsd4_encode_fattr4_maxwrite()
> NFSD: Add nfsd4_encode_fattr4_mode()
> NFSD: Add nfsd4_encode_fattr4_numlinks()
> NFSD: Add nfsd4_encode_fattr4_owner()
> NFSD: Add nfsd4_encode_fattr4_owner_group()
> NFSD: Add nfsd4_encode_fattr4_rawdev()
> NFSD: Add nfsd4_encode_fattr4_space_avail()
> NFSD: Add nfsd4_encode_fattr4_space_free()
> NFSD: Add nfsd4_encode_fattr4_space_total()
> NFSD: Add nfsd4_encode_fattr4_space_used()
> NFSD: Add nfsd4_encode_fattr4_time_access()
> NFSD: Add nfsd4_encode_fattr4_time_create()
> NFSD: Add nfsd4_encode_fattr4_time_delta()
> NFSD: Add nfsd4_encode_fattr4_time_metadata()
> NFSD: Add nfsd4_encode_fattr4_time_modify()
> NFSD: Add nfsd4_encode_fattr4_mounted_on_fileid()
> NFSD: Add nfsd4_encode_fattr4_fs_layout_types()
> NFSD: Add nfsd4_encode_fattr4_layout_types()
> NFSD: Add nfsd4_encode_fattr4_layout_blksize()
> NFSD: Add nfsd4_encode_fattr4_suppattr_exclcreat()
> NFSD: Add nfsd4_encode_fattr4_sec_label()
> NFSD: Add nfsd4_encode_fattr4_xattr_support()
> NFSD: Copy FATTR4 bit number definitions from RFCs
> NFSD: Use a bitmask loop to encode FATTR4 results
> NFSD: Rename nfsd4_encode_fattr()
>
>
> fs/nfsd/nfs4xdr.c | 1419 +++++++++++++++++++++-----------------
> fs/nfsd/nfsfh.c | 2 +-
> fs/nfsd/nfsfh.h | 3 +-
> fs/nfsd/xdr4.h | 119 ++++
> include/linux/iversion.h | 2 +-
> include/linux/nfs4.h | 260 +++++--
> 6 files changed, 1085 insertions(+), 720 deletions(-)
>
> --
> Chuck Lever
>
Large set, but the change is fairly mechanical overall, and the result
is much more readable.
Reviewed-by: Jeff Layton <[email protected]>