mbox series

[net-next,v5,0/7] introduce DEFINE_FLEX() macro

Message ID 20230912115937.1645707-1-przemyslaw.kitszel@intel.com (mailing list archive)
Headers show
Series introduce DEFINE_FLEX() macro | expand

Message

Przemek Kitszel Sept. 12, 2023, 11:59 a.m. UTC
Add DEFINE_FLEX() macro, that helps on-stack allocation of structures
with trailing flex array member.
Expose __struct_size() macro which reads size of data allocated
by DEFINE_FLEX().

Accompany new macros introduction with actual usage,
in the ice driver - hence targeting for netdev tree.

Obvious benefits include simpler resulting code, less heap usage,
less error checking. Less obvious is the fact that compiler has
more room to optimize, and as a whole, even with more stuff on the stack,
we end up with overall better (smaller) report from bloat-o-meter:
add/remove: 8/6 grow/shrink: 7/18 up/down: 2211/-2270 (-59)
(individual results in each patch).

v5: same as v4, just not RFC
v4: _Static_assert() to ensure compiletime const count param
v3: tidy up 1st patch
v2: Kees: reusing __struct_size() instead of doubling it as a new macro

Przemek Kitszel (7):
  overflow: add DEFINE_FLEX() for on-stack allocs
  ice: ice_sched_remove_elems: replace 1 elem array param by u32
  ice: drop two params of ice_aq_move_sched_elems()
  ice: make use of DEFINE_FLEX() in ice_ddp.c
  ice: make use of DEFINE_FLEX() for struct ice_aqc_add_tx_qgrp
  ice: make use of DEFINE_FLEX() for struct ice_aqc_dis_txq_item
  ice: make use of DEFINE_FLEX() in ice_switch.c

 drivers/net/ethernet/intel/ice/ice_common.c | 20 ++-----
 drivers/net/ethernet/intel/ice/ice_ddp.c    | 39 ++++---------
 drivers/net/ethernet/intel/ice/ice_lag.c    | 48 ++++------------
 drivers/net/ethernet/intel/ice/ice_lib.c    | 23 ++------
 drivers/net/ethernet/intel/ice/ice_sched.c  | 56 ++++++------------
 drivers/net/ethernet/intel/ice/ice_sched.h  |  6 +-
 drivers/net/ethernet/intel/ice/ice_switch.c | 63 +++++----------------
 drivers/net/ethernet/intel/ice/ice_xsk.c    | 22 +++----
 include/linux/compiler_types.h              | 32 +++++++----
 include/linux/fortify-string.h              |  4 --
 include/linux/overflow.h                    | 35 ++++++++++++
 11 files changed, 130 insertions(+), 218 deletions(-)


base-commit: 73be7fb14e83d24383f840a22f24d3ed222ca319

Comments

Kees Cook Sept. 12, 2023, 4:16 p.m. UTC | #1
On Tue, Sep 12, 2023 at 07:59:30AM -0400, Przemek Kitszel wrote:
> Add DEFINE_FLEX() macro, that helps on-stack allocation of structures
> with trailing flex array member.
> Expose __struct_size() macro which reads size of data allocated
> by DEFINE_FLEX().
> 
> Accompany new macros introduction with actual usage,
> in the ice driver - hence targeting for netdev tree.
> 
> Obvious benefits include simpler resulting code, less heap usage,
> less error checking. Less obvious is the fact that compiler has
> more room to optimize, and as a whole, even with more stuff on the stack,
> we end up with overall better (smaller) report from bloat-o-meter:
> add/remove: 8/6 grow/shrink: 7/18 up/down: 2211/-2270 (-59)
> (individual results in each patch).
> 
> v5: same as v4, just not RFC
> v4: _Static_assert() to ensure compiletime const count param
> v3: tidy up 1st patch
> v2: Kees: reusing __struct_size() instead of doubling it as a new macro
> 
> Przemek Kitszel (7):
>   overflow: add DEFINE_FLEX() for on-stack allocs
>   ice: ice_sched_remove_elems: replace 1 elem array param by u32
>   ice: drop two params of ice_aq_move_sched_elems()
>   ice: make use of DEFINE_FLEX() in ice_ddp.c
>   ice: make use of DEFINE_FLEX() for struct ice_aqc_add_tx_qgrp
>   ice: make use of DEFINE_FLEX() for struct ice_aqc_dis_txq_item
>   ice: make use of DEFINE_FLEX() in ice_switch.c

Looks good to me! Feel free to pick up via netdev.

-Kees
Przemek Kitszel Sept. 19, 2023, 11:10 a.m. UTC | #2
On 9/12/23 18:16, Kees Cook wrote:
> On Tue, Sep 12, 2023 at 07:59:30AM -0400, Przemek Kitszel wrote:
>> Add DEFINE_FLEX() macro, that helps on-stack allocation of structures
>> with trailing flex array member.
>> Expose __struct_size() macro which reads size of data allocated
>> by DEFINE_FLEX().
>>
>> Accompany new macros introduction with actual usage,
>> in the ice driver - hence targeting for netdev tree.
>>
>> Obvious benefits include simpler resulting code, less heap usage,
>> less error checking. Less obvious is the fact that compiler has
>> more room to optimize, and as a whole, even with more stuff on the stack,
>> we end up with overall better (smaller) report from bloat-o-meter:
>> add/remove: 8/6 grow/shrink: 7/18 up/down: 2211/-2270 (-59)
>> (individual results in each patch).
>>
>> v5: same as v4, just not RFC
>> v4: _Static_assert() to ensure compiletime const count param
>> v3: tidy up 1st patch
>> v2: Kees: reusing __struct_size() instead of doubling it as a new macro
>>
>> Przemek Kitszel (7):
>>    overflow: add DEFINE_FLEX() for on-stack allocs
>>    ice: ice_sched_remove_elems: replace 1 elem array param by u32
>>    ice: drop two params of ice_aq_move_sched_elems()
>>    ice: make use of DEFINE_FLEX() in ice_ddp.c
>>    ice: make use of DEFINE_FLEX() for struct ice_aqc_add_tx_qgrp
>>    ice: make use of DEFINE_FLEX() for struct ice_aqc_dis_txq_item
>>    ice: make use of DEFINE_FLEX() in ice_switch.c
> 
> Looks good to me! Feel free to pick up via netdev.
> 
> -Kees
> 

Thanks!

Patchwork [1] says it's "Awaiting Upstream", which is the same for most 
of the "to: IWL" patches. That means it's delegated to Tony?

By any means, minimizing "usage examples" to just ice driver makes it 
easy to merge via Tony's tree.

[1] 
https://patchwork.kernel.org/project/netdevbpf/patch/20230912115937.1645707-2-przemyslaw.kitszel@intel.com/
Tony Nguyen Sept. 27, 2023, 6:53 p.m. UTC | #3
On 9/19/2023 4:10 AM, Przemek Kitszel wrote:
> On 9/12/23 18:16, Kees Cook wrote:
>> On Tue, Sep 12, 2023 at 07:59:30AM -0400, Przemek Kitszel wrote:
>>> Add DEFINE_FLEX() macro, that helps on-stack allocation of structures
>>> with trailing flex array member.
>>> Expose __struct_size() macro which reads size of data allocated
>>> by DEFINE_FLEX().
>>>
>>> Accompany new macros introduction with actual usage,
>>> in the ice driver - hence targeting for netdev tree.
>>>
>>> Obvious benefits include simpler resulting code, less heap usage,
>>> less error checking. Less obvious is the fact that compiler has
>>> more room to optimize, and as a whole, even with more stuff on the 
>>> stack,
>>> we end up with overall better (smaller) report from bloat-o-meter:
>>> add/remove: 8/6 grow/shrink: 7/18 up/down: 2211/-2270 (-59)
>>> (individual results in each patch).
>>>
>>> v5: same as v4, just not RFC
>>> v4: _Static_assert() to ensure compiletime const count param
>>> v3: tidy up 1st patch
>>> v2: Kees: reusing __struct_size() instead of doubling it as a new macro
>>>
>>> Przemek Kitszel (7):
>>>    overflow: add DEFINE_FLEX() for on-stack allocs
>>>    ice: ice_sched_remove_elems: replace 1 elem array param by u32
>>>    ice: drop two params of ice_aq_move_sched_elems()
>>>    ice: make use of DEFINE_FLEX() in ice_ddp.c
>>>    ice: make use of DEFINE_FLEX() for struct ice_aqc_add_tx_qgrp
>>>    ice: make use of DEFINE_FLEX() for struct ice_aqc_dis_txq_item
>>>    ice: make use of DEFINE_FLEX() in ice_switch.c
>>
>> Looks good to me! Feel free to pick up via netdev.
>>
>> -Kees
>>
> 
> Thanks!
> 
> Patchwork [1] says it's "Awaiting Upstream", which is the same for most 
> of the "to: IWL" patches. That means it's delegated to Tony?

netdev maintainers,

As this has non-Intel changes and is marked for 'net-next', do you want 
to take this or prefer me to take via IWL and send as PR?

Thanks,
Tony

> By any means, minimizing "usage examples" to just ice driver makes it 
> easy to merge via Tony's tree.
> 
> [1] 
> https://patchwork.kernel.org/project/netdevbpf/patch/20230912115937.1645707-2-przemyslaw.kitszel@intel.com/
patchwork-bot+netdevbpf@kernel.org Oct. 3, 2023, 9:10 p.m. UTC | #4
Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 12 Sep 2023 07:59:30 -0400 you wrote:
> Add DEFINE_FLEX() macro, that helps on-stack allocation of structures
> with trailing flex array member.
> Expose __struct_size() macro which reads size of data allocated
> by DEFINE_FLEX().
> 
> Accompany new macros introduction with actual usage,
> in the ice driver - hence targeting for netdev tree.
> 
> [...]

Here is the summary with links:
  - [net-next,v5,1/7] overflow: add DEFINE_FLEX() for on-stack allocs
    https://git.kernel.org/netdev/net-next/c/26dd68d293fd
  - [net-next,v5,2/7] ice: ice_sched_remove_elems: replace 1 elem array param by u32
    https://git.kernel.org/netdev/net-next/c/ece285af77d0
  - [net-next,v5,3/7] ice: drop two params of ice_aq_move_sched_elems()
    https://git.kernel.org/netdev/net-next/c/a034fcdbeaf7
  - [net-next,v5,4/7] ice: make use of DEFINE_FLEX() in ice_ddp.c
    https://git.kernel.org/netdev/net-next/c/230064baa43d
  - [net-next,v5,5/7] ice: make use of DEFINE_FLEX() for struct ice_aqc_add_tx_qgrp
    https://git.kernel.org/netdev/net-next/c/43bba3b1664d
  - [net-next,v5,6/7] ice: make use of DEFINE_FLEX() for struct ice_aqc_dis_txq_item
    https://git.kernel.org/netdev/net-next/c/11dee3d611dd
  - [net-next,v5,7/7] ice: make use of DEFINE_FLEX() in ice_switch.c
    https://git.kernel.org/netdev/net-next/c/e268b9722705

You are awesome, thank you!