On Mon, Feb 10, 2025 at 8:52 AM Shinichiro Kawasaki <shinichiro.kawasaki@xxxxxxx> wrote: > > On Feb 09, 2025 / 20:20, Ming Lei wrote: > > All block internal state for dealing adding/removing debugfs entries > > have been removed, and debugfs can sync everything for us in fs level, > > so don't grab q->debugfs_mutex for adding/removing block internal debugfs > > entries. > > > > Now q->debugfs_mutex is only used for blktrace, meantime move creating > > queue debugfs dir code out of q->sysfs_lock. Both the two locks are > > connected with queue freeze IO lock. Then queue freeze IO lock chain > > with debugfs lock is cut. > > > > The following lockdep report can be fixed: > > > > https://lore.kernel.org/linux-block/ougniadskhks7uyxguxihgeuh2pv4yaqv4q3emo4gwuolgzdt6@brotly74p6bs/ > > > > Follows contexts which adds/removes debugfs entries: > > > > - update nr_hw_queues > > > > - add/remove disks > > > > - elevator switch > > > > - blktrace > > > > blktrace only adds entries under disk top directory, so we can ignore it, > > because it can only work iff disk is added. Also nothing overlapped with > > the other two contex, blktrace context is fine. > > > > Elevator switch is only allowed after disk is added, so there isn't race > > with add/remove disk. blk_mq_update_nr_hw_queues() always restores to > > previous elevator, so no race between these two. Elevator switch context > > is fine. > > > > So far blk_mq_update_nr_hw_queues() doesn't hold debugfs lock for > > adding/removing hctx entries, there might be race with add/remove disk, > > which is just fine in reality: > > > > - blk_mq_update_nr_hw_queues() is usually for error recovery, and disk > > won't be added/removed at the same time > > > > - even though there is race between the two contexts, it is just fine, > > since hctx won't be freed until queue is dead > > > > - we never see reports in this area without holding debugfs in > > blk_mq_update_nr_hw_queues() > > > > Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@xxxxxxx> > > Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx> > > Ming, thank you for this quick action. I applied this series on top of > v6.14-rc1 kernel and ran the block/002 test case. Unfortunately, still if fails > occasionally with the lockdep "WARNING: possible circular locking dependency > detected" below. Now debugfs_mutex is not reported as one of the dependent > locks, then I think this fix is working as expected. Instead, eq->sysfs_lock > creates similar dependency. My mere guess is that this patch avoids one > dependency, but still another dependency is left. Indeed, this patch cuts dependency on both q->sysfs_lock and q->debugfs_lock, but elevator ->sysfs_lock isn't covered, :-( Thanks, Ming
