[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b1faad7b-531b-429f-97a4-aa93a160569c@intel.com>
Date: Fri, 17 Oct 2025 16:35:18 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
CC: Przemek Kitszel <przemyslaw.kitszel@...el.com>,
<intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>, Jacob Keller
<jacob.e.keller@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1] ice: lower default
irq/queue counts on high-core systems
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
Date: Fri, 17 Oct 2025 09:30:44 +0200
> On Fri, Oct 17, 2025 at 07:03:31AM +0200, Przemek Kitszel wrote:
>> On 10/16/25 17:36, Alexander Lobakin wrote:
>>> From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
>>> Date: Thu, 16 Oct 2025 08:22:50 +0200
>>>
>>>> On some high-core systems loading ice driver with default values can
>>>> lead to queue/irq exhaustion. It will result in no additional resources
>>>> for SR-IOV.
>>>>
>>>> In most cases there is no performance reason for more than 64 queues.
>>>> Limit the default value to 64. Still, using ethtool the number of
>>>> queues can be changed up to num_online_cpus().
>>>>
>>>> This change affects only the default queue amount on systems with more
>>>> than 64 cores.
>>>>
>>>> Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
>>>> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
>>>> ---
>>>> drivers/net/ethernet/intel/ice/ice.h | 20 ++++++++++++++++++++
>>>> drivers/net/ethernet/intel/ice/ice_irq.c | 6 ++++--
>>>> drivers/net/ethernet/intel/ice/ice_lib.c | 8 ++++----
>>>> 3 files changed, 28 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
>>>> index 3d4d8b88631b..354ec2950ff3 100644
>>>> --- a/drivers/net/ethernet/intel/ice/ice.h
>>>> +++ b/drivers/net/ethernet/intel/ice/ice.h
>>>> @@ -1133,4 +1133,24 @@ static inline struct ice_hw *ice_get_primary_hw(struct ice_pf *pf)
>>>> else
>>>> return &pf->adapter->ctrl_pf->hw;
>>>> }
>>>> +
>>>> +/**
>>>> + * ice_capped_num_cpus - normalize the number of CPUs to a reasonable limit
>>>> + *
>>>> + * This function returns the number of online CPUs, but caps it at suitable
>>>> + * default to prevent excessive resource allocation on systems with very high
>>>> + * CPU counts.
>>>> + *
>>>> + * Note: suitable default is currently at 64, which is reflected in default_cpus
>>>> + * constant. In most cases there is no much benefit for more than 64 and it is a
>>>> + * power of 2 number.
>>>> + *
>>>> + * Return: number of online CPUs, capped at suitable default.
>>>> + */
>>>> +static inline u16 ice_capped_num_cpus(void)
>>>> +{
>>>> + const int default_cpus = 64;
>>>
>>> Maybe we should just use netif_get_num_default_rss_queues() like I did
>>> in idpf?
>>>
>>> Or it still can be too high e.g. on clusters with > 256 CPUs?
>>
>> good point,
>> perhaps we should both use it and change the (kernel) func to cap at 64
>>
>
> Sounds good, thanks for pointing the function.
>
> Do you think it is ok to cap the generic function? Maybe other vendors
> want more default queues.
Nah I don't think it's a good idea to hardcode any numbers in the
generic function.
>
> What about capping netif_get_num_default_rss_queues() at 64 just for
> ice?
netif_get_num_default_rss_queues() returns *half* of the number of
*physical* cores. I.e. it will return something bigger than 64 only in
case of > 256 threads in the system (considering SMT).
Do we need to still cap this to 64 in ice at all?
Thanks,
Olek
Powered by blists - more mailing lists