Re: [PATCH] [v2]aacraid: Reply queue mapping to CPUs based on IRQ affinity

[John Meneghini <jmeneghi@redhat.com>]

> From: Martin K. Petersen
> Sent: Wednesday, February 12, 2025 6:56 PM

> [You don't often get email from "martin.petersen@oracle.comjames.bottomley@hansenpartnership.comjmeneghi"@redhat.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

Appears to still be a problem. I'll work with Sagar and see if we can clean this up.

>> Add a new modparam "aac_cpu_offline_feature" to control CPU offlining. 
>> By default, it's disabled (0), but can be enabled during driver load
> 
>> with:
>>         insmod ./aacraid.ko aac_cpu_offline_feature=1
> 
> We are very hesitant when it comes to adding new module parameters. And
> why wouldn't you want offlining to just work? Is the performance penalty
> really substantial enough that we have to introduce an explicit "don't
> be broken" option?

Yes, this is something that we debated about internally, before asking Sagar to send this patch.

I agree that it would be much better if we simply fix the driver and make offline_cpu suport work.

The modparam was added as a compromise, to allow current users and customers who do NOT care about
cpu_offline support to keep the increased performance they want.  People generally complain any
time there is a performance regression.

The current upstream driver is more or less unchanged when the mod param is of off, which is the default.
So upstream users will see no performance regression... but don't try to offline a cpu or you will see
a panic. This is the state of the current upstream driver.

> Thank you for your time to review and giving your valuable opinion.
> There are two reasons why I chose the modparam way
> 1) As you rightly guessed - the performance penalty is high when it comes to few RAID level configurations - which is not desired
> 2) Not a lot of people would use CPU offlining feature as part of their regular usage. This is mostly for admin purposes.
> 
> These two reasons made me opt for the modparam.
> We and our folks at RedHat did venture into trying few other options - but this seemed like a nice fit.

Another option we thought about was making this a kconfig option. We have a patch that replaces the modparam with
a Kconfig option.

However, I agree it would be better to just fix the driver, performance impact notwithstanding, and ship it. For
my part I'd rather have a correctly functioning driver, that's slower, but doesn't panic.

It's really up to the upstream community.  We need to understand what they want.

/John