Re: [PATCH] scsi: avoid to send scsi command with ->queue_limits lock held

[Damien Le Moal <dlemoal@kernel.org>]

On 1/6/25 10:02 AM, Ming Lei wrote:
> On Sat, Jan 04, 2025 at 04:17:47PM +0900, Damien Le Moal wrote:
>> On 12/31/24 13:22, Ming Lei wrote:
>>> Block request queue is often frozen before acquiring the queue
>>> ->limits_lock.
>>
>> "often" is rather vague. What cases are we talking about here beside the block
>> layer sysfs ->store() operations ? Fixing these is easy and does not need this
>> change.
> 
> Is it really necessary to make freeze lock to depend on ->limits_lock?

Yes, because you do not want to have requests in-flight when applying new limits.

> 
> sd_revalidate_disk() is really one special case, so I think this patch
> does correct thing.
> 
>>
>> Furthermore, this change almost feels like a layering violation as it replicates
>> most of the queue limits structure inside sd. This introducing a strong
>> dependency to the block layer internals which we should avoid.
> 
> No.
> 
> block layer is common library, which is storage abstraction, so it is
> pretty reasonable for storage drivers to depend block layer. You can
> look at it from another way, if any related queue limits change, the
> current storage driver need corresponding change too, with or without
> this change.

Of course block device driver layers like SCSI depend on the block layer. But
that dependency is at a high level API/function level. My concern is that your
patch mimics too much the block layer implementation internals instead of
relying on a high level API like we do now.

-- 
Damien Le Moal
Western Digital Research