Subject: Re: [PATCH net-next v2 3/7] ibmvnic: Reduce memcpys in tx descriptor generation

From: Simon Horman <[email-masked]>
Date: Wed, 7 Aug 2024 14:16:39 +0100

On Tue, Aug 06, 2024 at 02:37:02PM -0500, Nick Child wrote:
> Previously when creating the header descriptors, the driver would:
> 1. allocate a temporary buffer on the stack (in build_hdr_descs_arr)
> 2. memcpy the header info into the temporary buffer (in build_hdr_data)
> 3. memcpy the temp buffer into a local variable (in create_hdr_descs)
> 4. copy the local variable into the return buffer (in create_hdr_descs)
> 
> Since, there is no opportunity for errors during this process, the temp
> buffer is not needed and work can be done on the return buffer directly.
> 
> Repurpose build_hdr_data() to only calculate the header lengths. Rename
> it to get_hdr_lens().
> Edit create_hdr_descs() to read from the skb directly and copy directly
> into the returned useful buffer.
> 
> The process now involves less memory and write operations while
> also being more readable.
> 
> Signed-off-by: Nick Child <[email-masked]>
> ---
>  drivers/net/ethernet/ibm/ibmvnic.c | 80 +++++++++++++-----------------
>  1 file changed, 34 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index 7d552d4bbe15..4fe2c8c17b05 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -2150,46 +2150,38 @@ static int ibmvnic_close(struct net_device *netdev)
>   * Builds a buffer containing these headers.  Saves individual header
>   * lengths and total buffer length to be used to build descriptors.
>   */
> -static int build_hdr_data(u8 hdr_field, struct sk_buff *skb,
> -			  int *hdr_len, u8 *hdr_data)
> +static int get_hdr_lens(u8 hdr_field, struct sk_buff *skb,
> +			int *hdr_len)

nit: The Kernel doc immediately above this function should be updated to
     reflect the new name of the function and removal of one parameter.

     Also, although not strictly related to this patch, ideally it should
     include a "Return:" or "Returns:" section.

Flagged by W=1 builds and ./scripts/kernel-doc -none -Wall

...