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
