On 10/2/24 14:10, Dan Carpenter wrote:
On Wed, Oct 02, 2024 at 01:49:48PM -0400, Waiman Long wrote:
- spin_unlock_irq(&ioc->lock);
+ spin_unlock(&ioc->lock);
return 0;
}
I would suggest adding a "lockdep_assert_irqs_disabled()" call before
spin_lock() to confirm that irq is indeed disabled just in case the callers
are changed in the future.
It's really hard to predict future bugs. I doubt we'll add new callers.
Outputting this information to a struct seq_file *sf is pretty specific.
If there were a bug related to this, then wouldn't it be caught by lockdep?
The other idea is that we could catch bugs like this using static analysis.
Like every time we take the &ioc->lock, either IRQs should already be disabled
or we disable it ourselves. I could write a Smatch check like this.
KTODO: add Smatch check to ensure IRQs are disabled for &ioc->lock
This is just a suggestion and it is fine if you don't think it is
necessary. The call can also serve as a comment that irq should have
been disabled at this point.
Cheers,
Longman
