| Message ID | 165852051841.11198.2929614302983292322.stgit@manet.1015granger.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/8] NFSD: Optimize nfsd4_encode_operation() | expand |
> 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
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>
> 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
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; } /*
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(-)