On Tue, 2021-01-19 at 16:41 -0800, Jakub Kicinski wrote: > On Wed, 20 Jan 2021 00:12:26 +0000 Venkataramanan, Anirudh wrote: > > > > Attempt [0]: Enable the best-case scenario MSI-X vectors. > > > > > > > > Attempt [1]: Enable MSI-X vectors with the number of pf- > > > > > 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
