On Sat, 2019-02-02 at 17:46 -0500, Chuck Lever wrote:
> > On Feb 1, 2019, at 9:30 PM, Tom Talpey <tom@xxxxxxxxxx> wrote:
> >
> > On 2/1/2019 2:57 PM, Chuck Lever wrote:
> > > Byte-swapping causes a CPU pipeline bubble on some processors.
> > > When
> > > a decoder is comparing an on-the-wire value for equality, byte-
> > > swapping can be avoided by comparing it directly to a pre-byte-
> > > swapped constant value.
> > > The current set of pre-xdr'd constants is missing some common
> > > values
> > > used in the RPC header. Fill those out.
> > > Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> > > ---
> > > include/linux/sunrpc/auth_gss.h | 5 ++-
> > > include/linux/sunrpc/xdr.h | 66 ++++++++++++++++++++++++-
> > > --------------
> > > 2 files changed, 45 insertions(+), 26 deletions(-)
> > > diff --git a/include/linux/sunrpc/auth_gss.h
> > > b/include/linux/sunrpc/auth_gss.h
> > > index 30427b7..adc4be2 100644
> > > --- a/include/linux/sunrpc/auth_gss.h
> > > +++ b/include/linux/sunrpc/auth_gss.h
> > > @@ -19,7 +19,10 @@
> > > #include <linux/sunrpc/svc.h>
> > > #include <linux/sunrpc/gss_api.h>
> > > -#define RPC_GSS_VERSION 1
> > > +enum {
> > > + RPC_GSS_VERSION = 1,
> > > + rpc_gss_version = cpu_to_be32(RPC_GSS_VERSION)
> > > +};
> > > #define MAXSEQ 0x80000000 /* maximum legal sequence number,
> > > from rfc 2203 */
> > > diff --git a/include/linux/sunrpc/xdr.h
> > > b/include/linux/sunrpc/xdr.h
> > > index 787939d..69161cb 100644
> > > --- a/include/linux/sunrpc/xdr.h
> > > +++ b/include/linux/sunrpc/xdr.h
> > > @@ -17,6 +17,7 @@
> > > #include <asm/byteorder.h>
> > > #include <asm/unaligned.h>
> > > #include <linux/scatterlist.h>
> > > +#include <linux/sunrpc/msg_prot.h>
> > > struct bio_vec;
> > > struct rpc_rqst;
> > > @@ -79,31 +80,46 @@ struct xdr_buf {
> > > buf->buflen = len;
> > > }
> > > -/*
> > > - * pre-xdr'ed macros.
> > > - */
> > > -
> > > -#define xdr_zero cpu_to_be32(0)
> > > -#define xdr_one cpu_to_be32(1)
> > > -#define xdr_two cpu_to_be32(2)
> > > -
> > > -#define rpc_success cpu_to_be32(RPC_SUCCESS)
> > > -#define rpc_prog_unavail cpu_to_be32(RPC_PROG_UNAVAIL)
> > > -#define rpc_prog_mismatch cpu_to_be32(RPC_PROG_MISMATCH)
> > > -#define rpc_proc_unavail cpu_to_be32(RPC_PROC_UNAVAIL)
> > > -#define rpc_garbage_args cpu_to_be32(RPC_GARBAGE_ARGS)
> > > -#define rpc_system_err cpu_to_be32(RPC_SYSTEM_ER
> > > R)
> > > -#define rpc_drop_reply cpu_to_be32(RPC_DROP_REPL
> > > Y)
> > > -
> > > -#define rpc_auth_ok cpu_to_be32(RPC_AUTH_OK)
> > > -#define rpc_autherr_badcred cpu_to_be32(RPC_AUTH_BADCRED)
> > > -#define rpc_autherr_rejectedcred
> > > cpu_to_be32(RPC_AUTH_REJECTEDCRED)
> > > -#define rpc_autherr_badverf cpu_to_be32(RPC_AUTH_BADVERF)
> > > -#define rpc_autherr_rejectedverf
> > > cpu_to_be32(RPC_AUTH_REJECTEDVERF)
> > > -#define rpc_autherr_tooweak cpu_to_be32(RPC_AUTH_TOOWEAK)
> > > -#define rpcsec_gsserr_credproblem cpu_to_be32(RPCSEC_GSS_CR
> > > EDPROBLEM)
> > > -#define rpcsec_gsserr_ctxproblem cpu_to_be32(RPCSEC_GSS_CT
> > > XPROBLEM)
> > > -#define rpc_autherr_oldseqnum cpu_to_be32(101)
> > > +enum xdr_be32_equivalents {
> > > + xdr_zero = cpu_to_be32(0),
> > > + xdr_one = cpu_to_be32(1),
> > > + xdr_two = cpu_to_be32(2),
> >
> > It is clever to use an enum to pre-compute these values, but
>
> Perhaps not clever; it is a current Linux kernel coding
> practice to use an enum in favor of a C macro for constants.
Sure, but won't that confuse 'sparse' static checking? These constants
will no longer appear as being big endian.
Doesn't gcc's __builtin_bswap32() already compute the result at compile
time when you feed it a constant value? AFAICS it is supposed to, which
is why we use it directly in include/uapi/linux/swab.h instead of using
a special cased __builtin_constant_p().
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
