Re: [PATCH v2] blk_iocost: remove some duplicate irq disable/enables

[Waiman Long]

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