Re: [PATCH] block: don't acquire .sysfs_lock before removing mq & iosched kobjects

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

On 8/16/19 6:55 AM, Ming Lei wrote:
> The kernfs built-in lock of 'kn->count' is held in sysfs .show/.store
> path. Meantime, inside block's .show/.store callback, q->sysfs_lock is
> required.
> 
> However, when mq & iosched kobjects are removed via
> blk_mq_unregister_dev() & elv_unregister_queue(), q->sysfs_lock is held
> too. This way causes AB-BA lock because the kernfs built-in lock of
> 'kn-count' is required inside kobject_del() too, see the lockdep warning[1].
> 
> On the other hand, it isn't necessary to acquire q->sysfs_lock for
> both blk_mq_unregister_dev() & elv_unregister_queue() because
> clearing REGISTERED flag prevents storing to 'queue/scheduler'
> from being happened. Also sysfs write(store) is exclusive, so no
> necessary to hold the lock for elv_unregister_queue() when it is
> called in switching elevator path.
> 
> Fixes the issue by not holding the q->sysfs_lock for blk_mq_unregister_dev() &
> elv_unregister_queue().

Have you considered to split sysfs_lock into multiple mutexes? Today it 
is very hard to verify the correctness of block layer code that uses 
sysfs_lock because it has not been documented anywhere what that mutex 
protects. I think that mutex should be split into at least two mutexes: 
one that protects switching I/O schedulers and another one that protects 
hctx->tags and hctx->sched_tags.

Thanks,

Bart.