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;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "Chuck Lever" <chuck.lever@oracle.com>
Cc:     linux-nfs@vger.kernel.org
Subject: Re: [PATCH v1 2/5] SUNRPC: Optimize xdr_reserve_space()
In-reply-to: <165445911199.1664.12318094116152634589.stgit@bazille.1015granger.net>
References: <165445865736.1664.4497130284832282034.stgit@bazille.1015granger.net>,
 <165445911199.1664.12318094116152634589.stgit@bazille.1015granger.net>
Date:   Tue, 07 Jun 2022 11:03:16 +1000
Message-id: <165456379661.22243.4266686429763691053@noble.neil.brown.name>
Precedence: bulk

On Mon, 06 Jun 2022, Chuck Lever wrote:
> The xdr_get_next_encode_buffer() function is called infrequently.
> On a typical build/test workload, it's called about 1 time in 400
> calls to xdr_reserve_space() (measured on NFSD).
> 
> Force the compiler to remove it from xdr_reserve_space(), which is
> a hot path. This change reduces the size of xdr_reserve_space() from
> 10 cache lines to 4 on my test system.

Does this really help at all?  Are the instructions that are executed in
the common case distributed over those 10 cache line, or are they all in
the first 4?

I would have thought the "unlikely" in xdr_reserve_space() would have
pushed all the code from xdr_get_next_encode_buffer() to the end of the
function leaving the remainder in a small contiguous chunk.

NeilBrown


> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  net/sunrpc/xdr.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
> index b57cf9df4de8..08a85375b311 100644
> --- a/net/sunrpc/xdr.c
> +++ b/net/sunrpc/xdr.c
> @@ -945,8 +945,13 @@ inline void xdr_commit_encode(struct xdr_stream *xdr)
>  }
>  EXPORT_SYMBOL_GPL(xdr_commit_encode);
>  
> -static __be32 *xdr_get_next_encode_buffer(struct xdr_stream *xdr,
> -		size_t nbytes)
> +/*
> + * The buffer space to be reserved crosses the boundary between
> + * xdr->buf->head and xdr->buf->pages, or between two pages
> + * in xdr->buf->pages.
> + */
> +static noinline __be32 *xdr_get_next_encode_buffer(struct xdr_stream *xdr,
> +						   size_t nbytes)
>  {
>  	__be32 *p;
>  	int space_left;
> 
> 
>