Hi Chuck,
A general question: what's the rule for deciding whether to allocate
statically or dynamically? I thought that "small" structures it's
better to preallocate (statically) for performance reasons. Or is the
idea here that copy_notify/copy are rare operations that instead they
should be allocated dynamically and so that other operations doesn't
consume more memory in nfsd4_op structure?
On Fri, Jul 22, 2022 at 4:35 PM Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
>
> struct nfsd4_copy_notify is part of struct nfsd4_op, which resides
> in an 8-element array.
>
> sizeof(struct nfsd4_op):
> Before: /* size: 2208, cachelines: 35, members: 5 */
> After: /* size: 1696, cachelines: 27, members: 5 */
>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
> fs/nfsd/nfs4proc.c | 4 ++--
> fs/nfsd/nfs4xdr.c | 12 ++++++++++--
> fs/nfsd/xdr4.h | 4 ++--
> 3 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 3895eb52d2b1..22c5ccb83d20 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1953,9 +1953,9 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> /* For now, only return one server address in cpn_src, the
> * address used by the client to connect to this server.
> */
> - cn->cpn_src.nl4_type = NL4_NETADDR;
> + cn->cpn_src->nl4_type = NL4_NETADDR;
> status = nfsd4_set_netaddr((struct sockaddr *)&rqstp->rq_daddr,
> - &cn->cpn_src.u.nl4_addr);
> + &cn->cpn_src->u.nl4_addr);
> WARN_ON_ONCE(status);
> if (status) {
> nfs4_put_cpntf_state(nn, cps);
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 358b3338c4cc..335431199077 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1952,10 +1952,17 @@ nfsd4_decode_copy_notify(struct nfsd4_compoundargs *argp,
> {
> __be32 status;
>
> + cn->cpn_src = svcxdr_tmpalloc(argp, sizeof(*cn->cpn_src));
> + if (cn->cpn_src == NULL)
> + return nfserrno(-ENOMEM); /* XXX: jukebox? */
> + cn->cpn_dst = svcxdr_tmpalloc(argp, sizeof(*cn->cpn_dst));
> + if (cn->cpn_dst == NULL)
> + return nfserrno(-ENOMEM); /* XXX: jukebox? */
> +
> status = nfsd4_decode_stateid4(argp, &cn->cpn_src_stateid);
> if (status)
> return status;
> - return nfsd4_decode_nl4_server(argp, &cn->cpn_dst);
> + return nfsd4_decode_nl4_server(argp, cn->cpn_dst);
> }
>
> static __be32
> @@ -4898,7 +4905,8 @@ nfsd4_encode_copy_notify(struct nfsd4_compoundres *resp, __be32 nfserr,
>
> *p++ = cpu_to_be32(1);
>
> - return nfsd42_encode_nl4_server(resp, &cn->cpn_src);
> + nfserr = nfsd42_encode_nl4_server(resp, cn->cpn_src);
> + return nfserr;
> }
>
> static __be32
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 6e6a89008ce1..f253fc3f4708 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -595,13 +595,13 @@ struct nfsd4_offload_status {
> struct nfsd4_copy_notify {
> /* request */
> stateid_t cpn_src_stateid;
> - struct nl4_server cpn_dst;
> + struct nl4_server *cpn_dst;
>
> /* response */
> stateid_t cpn_cnr_stateid;
> u64 cpn_sec;
> u32 cpn_nsec;
> - struct nl4_server cpn_src;
> + struct nl4_server *cpn_src;
> };
>
> struct nfsd4_op {
>
>
