Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <59534604ed620c0b7e9ec565a4f1505ca0910420.camel@kernel.org>
Subject: Re: [PATCH v1 1/8] NFSD: Optimize nfsd4_encode_operation()
From:   Jeff Layton <jlayton@kernel.org>
To:     Chuck Lever III <chuck.lever@oracle.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Date:   Mon, 25 Jul 2022 11:36:12 -0400
In-Reply-To: <A87D312E-1042-451A-A1B7-438351BCE368@oracle.com>
References: <165852051841.11198.2929614302983292322.stgit@manet.1015granger.net>
         <A87D312E-1042-451A-A1B7-438351BCE368@oracle.com>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.3 (3.44.3-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Fri, 2022-07-22 at 20:12 +0000, Chuck Lever III wrote:
>=20
> > On Jul 22, 2022, at 4:08 PM, Chuck Lever <chuck.lever@oracle.com> wrote=
:
> >=20
> > write_bytes_to_xdr_buf() is a generic way to place a variable-length
> > data item in an already-reserved spot in the encoding buffer.
> > However, it is costly, and here, it is unnecessary because the
> > data item is fixed in size, the buffer destination address is
> > always word-aligned, and the destination location is already in
> > @p.
> >=20
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> > fs/nfsd/nfs4xdr.c |    3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >=20
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index 2acea7792bb2..b52ea7d8313a 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -5373,8 +5373,7 @@ nfsd4_encode_operation(struct nfsd4_compoundres *=
resp, struct nfsd4_op *op)
> > 						so->so_replay.rp_buf, len);
> > 	}
> > status:
> > -	/* Note that op->status is already in network byte order: */
> > -	write_bytes_to_xdr_buf(xdr->buf, post_err_offset - 4, &op->status, 4)=
;
> > +	*p =3D op->status;
> > }
> >=20
> > /*=20
>=20
> I hit "Send" too soon. Here's the cover letter for this series.
>=20
> write_bytes_to_xdr_buf() is a generic mechanism by which to push
> an arbitrary set of bytes to an arbitrary alignment within an
> xdr_buf. It's expensive to use, though.
>=20
> I found several hot paths in the server's NFSv4 reply encoders
> that write a 4-byte XDR data item on a 4-byte alignment, and
> thus can use the classic "*p =3D cpu_to_be32(yada)" form instead
> of write_bytes_to_xdr_buf().
>=20
> This series replaces some write_bytes_to_xdr_buf() call sites.
>=20
>=20
> --
> Chuck Lever
>=20
>=20
>=20

Nice cleanup. This series looks good to me (and it's a net-negative LoC
too).

Reviewed-by: Jeff Layton <jlayton@kernel.org>