netdev - Re: [Intel-wired-lan] [PATCH iwl-next v1] ice: refactor the Tx scheduler feature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b5e34397-0b81-4132-86d0-498a111cc363@linux.intel.com>
Date: Tue, 4 Mar 2025 14:43:51 +0100
From: "Szapar-Mudlaw, Martyna" <martyna.szapar-mudlaw@...ux.intel.com>
To: Simon Horman <horms@...nel.org>,
 Mateusz Polchlopek <mateusz.polchlopek@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
 Marcin Szycik <marcin.szycik@...ux.intel.com>,
 Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
 Jedrzej Jagielski <jedrzej.jagielski@...el.com>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 Aleksandr Loktionov <aleksandr.loktionov@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1] ice: refactor the Tx
 scheduler feature

On 3/3/2025 10:54 AM, Simon Horman wrote:
> On Wed, Feb 26, 2025 at 12:33:56PM +0100, Mateusz Polchlopek wrote:
>> Embed ice_get_tx_topo_user_sel() inside the only caller:
>> ice_devlink_tx_sched_layers_get().
>> Instead of jump from the wrapper to the function that does "get" operation
>> it does "get" itself.
>>
>> Remove unnecessary comment and make usage of str_enabled_disabled()
>> in ice_init_tx_topology().
> 
> Hi Mateusz,
> 
> These changes seem reasonable to me.
> But I wonder if they could be motivated in the commit message.
> 
> What I mean is, the commit message explains what has been done.
> But I think it should explain why it has been done.

Hi Simon,

I'm replying on behalf of Mateusz since he's on leave, and we didn't 
want to keep this issue waiting too long.
Would such extended commit message make sense and address your concerns?

"Simplify the code by eliminating an unnecessary wrapper function. 
Previously, ice_devlink_tx_sched_layers_get() acted as a thin wrapper 
around ice_get_tx_topo_user_sel(), adding no real value but increasing 
code complexity. Since both functions were only used once, the wrapper 
was redundant and contributed approximately 20 lines of unnecessary 
code. By directly calling ice_get_tx_topo_user_sel(), improve 
readability and reduce function jumps, without altering functionality.
Also remove unnecessary comment and make usage of str_enabled_disabled() 
in ice_init_tx_topology()."

Thank you,
Martyna

> 
>> Suggested-by: Marcin Szycik <marcin.szycik@...ux.intel.com>
>> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
>> Reviewed-by: Jedrzej Jagielski <jedrzej.jagielski@...el.com>
>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
>> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@...el.com>
> 
> ...
>