Re: [Intel-wired-lan] [PATCH iwl-next] iavf: remove "inline" functions from iavf_txrx.c

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Ahmed Zaki <ahmed.zaki@intel.com>
Date: Thu, 24 Aug 2023 06:23:37 -0600

> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> The iAVF txrx hotpath code has several functions that are marked as
> "static inline" in the iavf_txrx.c file. This use of inline is frowned
> upon in the netdev community and explicitly marked as something to avoid
> in the Linux coding-style document (section 15).
> 
> Even though these functions are only used once, it is expected that GCC
> is smart enough to decide when to perform function inlining where
> appropriate without the "hint".

The compilers sometimes do unexpected things. I wouldn't blindly hope.
This means, I'd like to have some objdiff or at least bloat-o-meter
output here to prove this commit doesn't hurt.
If there are changes in the object code -- then some perf tests as well.

> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
> ---
>  drivers/net/ethernet/intel/iavf/iavf_txrx.c | 43 ++++++++++-----------
>  1 file changed, 20 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_txrx.c b/drivers/net/ethernet/intel/iavf/iavf_txrx.c
> index 8c5f6096b002..562dafb89f1d 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_txrx.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_txrx.c
> @@ -7,8 +7,7 @@
>  #include "iavf_trace.h"
>  #include "iavf_prototype.h"
>  
> -static inline __le64 build_ctob(u32 td_cmd, u32 td_offset, unsigned int size,
> -				u32 td_tag)
> +static __le64 build_ctob(u32 td_cmd, u32 td_offset, unsigned int size, u32 td_tag)
>  {
>  	return cpu_to_le64(IAVF_TX_DESC_DTYPE_DATA |
>  			   ((u64)td_cmd  << IAVF_TXD_QW1_CMD_SHIFT) |
> @@ -370,8 +369,7 @@ static void iavf_enable_wb_on_itr(struct iavf_vsi *vsi,
>  	q_vector->arm_wb_state = true;
>  }
>  
> -static inline bool iavf_container_is_rx(struct iavf_q_vector *q_vector,
> -					struct iavf_ring_container *rc)
> +static bool iavf_container_is_rx(struct iavf_q_vector *q_vector, struct iavf_ring_container *rc)

Over 80 chars, as well as in a couple of other places below, please
recheck the patch (you're trying to fix checkpatch issues and at the
same time introducing new? :D).

>  {
>  	return &q_vector->rx == rc;
>  }

[...]

Thanks,
Olek
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan