Re: [PATCH net-next 1/1] ice: Improve MSI-X vector enablement fallback logic

On Tue, 2021-01-19 at 16:41 -0800, Jakub Kicinski wrote:

On Wed, 20 Jan 2021 00:12:26 +0000 Venkataramanan, Anirudh wrote:

quoted

quoted

quoted

Attempt [0]: Enable the best-case scenario MSI-X vectors.

Attempt [1]: Enable MSI-X vectors with the number of pf-  

quoted

num_lan_msix  

reduced by a factor of 2 from the previous attempt (i.e.
num_online_cpus() / 2).

Attempt [2]: Same as attempt [1], except reduce by a factor of
4.

Attempt [3]: Enable the bare-minimum MSI-X vectors.

Also, if the adjusted_base_msix ever hits the minimum required
for
LAN,
then just set the needed MSI-X for that feature to the minimum
(similar to attempt [3]).  

I don't really get why you switch to this manual "exponential
back-
off"
rather than continuing to use pci_enable_msix_range(), but fixing
the
capping to ICE_MIN_LAN_VECS.  

As per the current logic, if the driver does not get the number of
MSI-
X vectors it needs, it will immediately drop to "Do I have at least
two
(ICE_MIN_LAN_VECS) MSI-X vectors?". If yes, the driver will enable
a
single Tx/Rx traffic queue pair, bound to one of the two MSI-X
vectors.

This is a bit of an all-or-nothing type approach. There's a mid-
ground
that can allow more queues to be enabled (ex. driver asked for 300
vectors, but got 68 vectors, so enabled 64 data queues) and this
patch
implements the mid-ground logic. 

This mid-ground logic can also be implemented based on the return
value
of pci_enable_msix_range() but IMHO the implementation in this
patch
using pci_enable_msix_exact is better because it's always only
enabling/reserving as many MSI-X vectors as required, not more, not
less.

What do you mean by "required" in the last sentence? 

.. as "required" in that particular iteration of the loop.

The driver
requests num_online_cpus()-worth of IRQs, so it must work with any
number of IRQs. Why is num_cpus() / 1,2,4,8 "required"?

Let me back up a bit here. 

Ultimately, the issue we are trying to solve here is "what happens when
the driver doesn't get as many MSI-X vectors as it needs, and how it's
interpreted by the end user"

Let's say there are these two systems, each with 256 cores but the
response to pci_enable_msix_range() is different:

System 1: 256 cores, pci_enable_msix_range returns 75 vectors
System 2: 256 cores, pci_enable_msix_range returns 220 vectors 

In this case, the number of queues the user would see enabled on each
of these systems would be very different (73 on system 1 and 218 on
system 2). This variabilty makes it difficult to define what the
expected behavior should be, because it's not exactly obvious to the
user how many free MSI-X vectors a given system has. Instead, if the
driver reduced it's demand for vectors in a well defined manner
(num_cpus() / 1,2,4,8), the user visible difference between the two
systems wouldn't be so drastic.

If this is plain wrong or if there's a preferred approach, I'd be happy
to discuss further.

Ani