linux-kernel - Re: [PATCH v3] scsi: aacraid: Fix reply queue mapping to CPUs based on IRQ affinity

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] [day] [month] [year] [list]

Message-ID: <ce2ccc7d-143b-4c9f-99c6-05d68166e9b4@oracle.com>
Date: Thu, 10 Jul 2025 09:12:36 +0100
From: John Garry <john.g.garry@...cle.com>
To: Sagar.Biradar@...rochip.com, jmeneghi@...hat.com,
        James.Bottomley@...senPartnership.com, martin.petersen@...cle.com,
        linux-scsi@...r.kernel.org, aacraid@...rosemi.com, corbet@....net,
        hch@....de, dwagner@...e.de
Cc: linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, thenzl@...hat.com,
        Scott.Benesh@...rochip.com, Don.Brace@...rochip.com,
        Tom.White@...rochip.com, Abhinav.Kuchibhotla@...rochip.com,
        mpatalan@...hat.com, Raja.VS@...rochip.com
Subject: Re: [PATCH v3] scsi: aacraid: Fix reply queue mapping to CPUs based
 on IRQ affinity

On 10/07/2025 07:49, Sagar.Biradar@...rochip.com wrote:

+ Daniel, hch

>>> This patch fixes a bug in the original path that caused I/O hangs. The
>>> I/O hangs were because of an MSIx vector not having a mapped online CPU
>>> upon receiving completion.
>>>
>>> This patch enables Multi-Q support in the aacriad driver. Multi-Q support
>>> in the driver is needed to support CPU offlining.
>> I assume that you mean "safe" CPU offlining.
>>
>> It seems to me that in all cases we use queue interrupt affinity
>> spreading and managed interrupts for MSIX, right?
>>
>> See aac_define_int_mode() -> pci_alloc_irq_vectors(..., PCI_IRQ_MSIX |
>> PCI_IRQ_AFFINITY);
>>
>> But then for this non- Multi-Q support, the queue seems to be chosen
>> based on a round-robin approach in the driver. That round-robin comes
>> from how fib.vector_no is assigned in aac_fib_vector_assign(). If this
>> is the case, then why are managed interrupts being used for this non-
>> Multi-Q support at all?
>>
>> I may be wrong about this. That driver is hard to understand with so
>> many knobs.
>>
> 
> Thank you very much for raising this — you're right that using PCI_IRQ_AFFINITY in non-MultiQ mode doesn’t offer real value,
 > since the driver doesn’t utilize the affinity mapping.> That said, 
the current implementation is functionally correct and consistent with 
the driver’s historical behavior.

For some time now this driver has been having issues in this area, so it 
is strange to say that behavior is correct.

Indeed, this patch is trying to fix the broken behaviour Re. CPU 
hotplug, right?

> To keep the patch focused and avoid scope creep, I’d prefer to leave the affinity flag logic as is for now.

To me, first it would be good to stop using PCI_IRQ_AFFINITY - that 
should fix any broken behaviour regarding CPU hotplug.

Then, if you still want to use PCI_IRQ_AFFINITY, then do it like it is 
done in this patch.

> 
> I’d be happy to follow up with a small cleanup patch, sometime in future, to improve this if you think it would help.

Daniel (cc'ed) is working on a method to isolate CPUs so that CPUs using 
for queue mapping can be configured for tuning performance, so that you 
don't need Kconfig options like in this patch.