> Moreover, q->sysfs_dir_lock is also used at few other callsites along > with q->sysfs_lock for protecting the addition/deletion of kojects. > One such example is when we register with sysfs a set of independent > access ranges for a disk. Here as well we could get rid off q->sysfs_ > dir_lock and only use q->sysfs_lock. > > The only variable which q->sysfs_dir_lock appears to protect is q-> > mq_sysfs_init_done which is set/unset while registering/unregistering > blk-mq with sysfs. But this variable could be easily converted to an > atomic type and used safely without using q->sysfs_dir_lock. > So sysfs_dir_lock absolutely should go. OTOH relying more on sysfs_lock is a bad idea, as that is also serialied with the attributes, which again on a pure sysfs basis isn't needed. Given that you don't add new critical sections for it this should be fine, though. > So with this patch we remove q->sysfs_dir_lock from each callsite > and also make q->mq_sysfs_init_done an atomic variable. Using an atomic_t for a single variable is usually not a good idea. Let's take a step back and look at what mq_sysfs_init_done is trying to do. It is set by blk_mq_sysfs_register, which is called by blk_register_queue. before marking the queue registered and setting the disk live. It is cleared in blk_mq_sysfs_unregister called from blk_unregister_queue just after clearing the queue registered bit. So maybe we could do something with the queue registered bit, although eventually I'd like to kill that as well, but either way we need to explain how the flag prevents the nr_hw_queues update racing with disk addition/removal. AFAICS we're not completely getting this right even right now. We'd probably need to hold tag_list_lock over registering and unregistering the hctx sysfs files.
