Re: [PATCH V2 6/6] block: split .sysfs_lock into two locks

[Bart Van Assche <bvanassche@acm.org>]

On 8/21/19 6:28 PM, Ming Lei wrote:
> On Wed, Aug 21, 2019 at 09:18:08AM -0700, Bart Van Assche wrote:
>> On 8/21/19 2:15 AM, Ming Lei wrote:
>>> diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
>>> index 31bbf10d8149..a4cc40ddda86 100644
>>> --- a/block/blk-mq-sysfs.c
>>> +++ b/block/blk-mq-sysfs.c
>>> @@ -247,7 +247,7 @@ void blk_mq_unregister_dev(struct device *dev, struct request_queue *q)
>>>    	struct blk_mq_hw_ctx *hctx;
>>>    	int i;
>>> -	lockdep_assert_held(&q->sysfs_lock);
>>> +	lockdep_assert_held(&q->sysfs_dir_lock);
>>>    	queue_for_each_hw_ctx(q, hctx, i)
>>>    		blk_mq_unregister_hctx(hctx);
>>> @@ -297,7 +297,7 @@ int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
>>>    	int ret, i;
>>>    	WARN_ON_ONCE(!q->kobj.parent);
>>> -	lockdep_assert_held(&q->sysfs_lock);
>>> +	lockdep_assert_held(&q->sysfs_dir_lock);
>>>    	ret = kobject_add(q->mq_kobj, kobject_get(&dev->kobj), "%s", "mq");
>>>    	if (ret < 0)
>>
>> blk_mq_unregister_dev and __blk_mq_register_dev() are only used by
>> blk_register_queue() and blk_unregister_queue(). It is the responsibility of
>> the callers of these function to serialize request queue registration and
>> unregistration. Is it really necessary to hold a mutex around the
>> blk_mq_unregister_dev and __blk_mq_register_dev() calls? Or in other words,
>> can it ever happen that multiple threads invoke one or both functions
>> concurrently?
> 
> hctx kobjects can be removed and re-added via blk_mq_update_nr_hw_queues()
> which may be called at the same time when queue is registering or
> un-registering.

Shouldn't blk_register_queue() and blk_unregister_queue() be serialized 
against blk_mq_update_nr_hw_queues()? Allowing these calls to proceed 
concurrently complicates the block layer and makes the block layer code 
harder to review than necessary. I don't think that it would help any 
block driver to allow these calls to proceed concurrently.

Thanks,

Bart.