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

[Ahmed Zaki <ahmed.zaki@intel.com>]

On 2023-08-24 14:01, Przemek Kitszel wrote:
> On 8/24/23 17:46, Ahmed Zaki wrote:
>>
>> On 2023-08-24 08:49, Alexander Lobakin wrote:
>>> 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.
>>
>> OK, I will add a bloat-o-meter in v2
>>
>>
>>>
>>>> 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).
>>
>>   ./scripts/checkpatch.pl --strict not showing any errors. No idea why. 
>
> I'm using this thanks to Marcin:
> ./scripts/checkpatch.pl --show-types --codespell --mailback --ignore 
> LONG_LINE_STRING --max-line-length=80 --patch $1

yep, that is doing it. I guess "strict" is not that strict after all :)

Thanks,

Ahmed


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