Re: [PATCH v1 1/8] NFSD: Optimize nfsd4_encode_operation()

[Chuck Lever III <chuck.lever@oracle.com>]

> On Jul 25, 2022, at 11:36 AM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> On Fri, 2022-07-22 at 20:12 +0000, Chuck Lever III wrote:
>> 
>>> On Jul 22, 2022, at 4:08 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
>>> 
>>> 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.
>>> 
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> ---
>>> fs/nfsd/nfs4xdr.c |    3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>> 
>>> 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 = op->status;
>>> }
>>> 
>>> /* 
>> 
>> I hit "Send" too soon. Here's the cover letter for this series.
>> 
>> 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.
>> 
>> 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 = cpu_to_be32(yada)" form instead
>> of write_bytes_to_xdr_buf().
>> 
>> This series replaces some write_bytes_to_xdr_buf() call sites.
>> 
>> 
>> --
>> Chuck Lever
>> 
>> 
>> 
> 
> Nice cleanup. This series looks good to me (and it's a net-negative LoC
> too).
> 
> Reviewed-by: Jeff Layton <jlayton@kernel.org>

Thanks, Jeff!


--
Chuck Lever