On 10/23/20 4:58 AM, Peter Zijlstra wrote: > On Thu, Oct 22, 2020 at 01:47:14PM -0400, Nitesh Narayan Lal wrote: > >> Hi Peter, >> >> So based on the suggestions from you and Thomas, I think something like the >> following should do the job within pci_alloc_irq_vectors_affinity(): >> >> + if (!pci_is_managed(dev) && (hk_cpus < num_online_cpus())) >> + max_vecs = clamp(hk_cpus, min_vecs, max_vecs); >> >> I do know that you didn't like the usage of "hk_cpus < num_online_cpus()" >> and to an extent I agree that it does degrade the code clarity. > It's not just code clarity; I simply don't understand it. It feels like > a band-aid that breaks thing. > > At the very least it needs a ginormous (and coherent) comment that > explains: > > - the interface > - the usage > - this hack That make sense. > >> However, since there is a certain inconsistency in the number of vectors >> that drivers request through this API IMHO we will need this, otherwise >> we could cause an impact on the drivers even in setups that doesn't >> have any isolated CPUs. > So shouldn't we then fix the drivers / interface first, to get rid of > this inconsistency? > Considering we agree that excess vector is a problem that needs to be solved across all the drivers and that you are comfortable with the other three patches in the set. If I may suggest the following: - We can pick those three patches for now, as that will atleast fix a driver that is currently impacting RT workloads. Is that a fair expectation? - In the meanwhile, I will start looking into individual drivers that consume this API to find out if there is a co-relation that can be derived between the max_vecs and number of CPUs. If that exists then I can go ahead and tweak the API's max_vecs accordingly. However, if this is absolutely random then I can come up with a sane comment before this check that covers the list of items you suggested. - I also want to explore the comments made by Thomas which may take some time. -- Thanks Nitesh
Attachment:
signature.asc
Description: OpenPGP digital signature
