On 2/18/25 2:51 PM, Christoph Hellwig wrote: > The mix of blk-sysfs and block in the subject lines is a bit odd. > Maybe just use the block prefix everywhere? > Okay I will update subject line of each patch to have "block" prefix everywhere. > Also q->sysfs_lock is almost unused now and we should probably look > into killing it entirely. > Yes that's the eventual goal and I'd work towards it. > blk_mq_hw_sysfs_show takes it around the ->show methods which > looks pretty useless. The debugfs code takes it for a few undocumented > things, which are worth digging into and if needed split into a separate > lock. > > The concurrent ranges code takes it - I think that is because it does > register a complex sysfs hierarchy from something that could race with > add_disk / del_gendisk. Damien, can you help with your thoughts? > (sd.c also has a comment reference it and the removed sysfs_dir_lock > which needs fixing anyway). > > blk_register_queue still takes it around a pretty random range of code > including nesting with other locks. I can't see what it protects > against, but it could use a careful look. > > blk_unregister_queue takes it just to clear QUEUE_FLAG_REGISTERED, > which by definition can't really protect against anything. Yes and also as clearing QUEUE_FLAG_REGISTERED uses atomic bitops, we don't need to acquire q->sysfs_lock here. > > Also the sysfs_lock in the elevator_queue should probably go away or > be replaced with the new elevator_lock for the non-show/store path > for the same reasons as outlined in this series. yes agreed. Once this patch series is approved, I'd work further to eliminate the remaining use of q->sysfs_lock in block layer. At some places we may be able to just straight away remove it and other places we may replace it with appropriate lock. Thanks, --Nilay
