Re: [PATCH V2 2/2] block: fix "Directory XXXXX with parent 'block' already present!"

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Sun, Apr 24, 2022 at 08:04:59PM +0800, Ming Lei wrote:
> On Sun, Apr 24, 2022 at 01:51:45PM +0200, Hannes Reinecke wrote:
> > On 4/24/22 11:28, Ming Lei wrote:
> > > On Sun, Apr 24, 2022 at 10:53:29AM +0200, Hannes Reinecke wrote:
> > > > On 4/23/22 16:39, Ming Lei wrote:
> > > > > q->debugfs_dir is used by blk-mq debugfs and blktrace. The dentry is
> > > > > created when adding disk, and removed when releasing request queue.
> > > > > 
> > > > > There is small window between releasing disk and releasing request
> > > > > queue, and during the period, one disk with same name may be created
> > > > > and added, so debugfs_create_dir() may complain with "Directory XXXXX
> > > > > with parent 'block' already present!"
> > > > > 
> > > > > Fixes the issue by moving debugfs_create_dir() into blk_alloc_queue(),
> > > > > and the dir name is named with q->id from beginning, and switched to
> > > > > disk name when adding disk, and finally changed to q->id in disk_release().
> > > > > 
> > > > > Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> > > > > Reported-by: Dan Williams <dan.j.williams@intel.com>
> > > > > Cc: yukuai (C) <yukuai3@huawei.com>
> > > > > Cc: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> > > > > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > > > > ---
> > > > >    block/blk-core.c  | 4 ++++
> > > > >    block/blk-sysfs.c | 4 ++--
> > > > >    block/genhd.c     | 8 ++++++++
> > > > >    3 files changed, 14 insertions(+), 2 deletions(-)
> > > > > 
> > > > Errm.
> > > > 
> > > > Isn't this superfluous now that Jens merged Yu Kuais patch?
> > > 
> > > Jens has dropped Yu Kuai's patch which caused kernel panic.
> > > 
> > Right.
> > But still, this patch looks really odd.
> > How is userspace supposed to use the directories prior to the renaming?
> 
> That doesn't make any difference for current uses, but we may extend it
> to support debugfs for non-blk request queue in future by exporting q->id
> somewhere. Even though now the interested q->id can be figured out
> easily by very simple ebpf trace prog.
> 
> > 
> > And as you already have identified the places where we can safely create
> > (and remove) the debugfs directories, why can't we move the call to create
> > and remove the debugfs directories to those locations and do away with the
> > renaming?
> 
> First it needs more change to fix the kernel panic.
> 
> Second removing debugfs dir in del_gendisk will break blktests block/002.

Then fix the test?  debugfs interactions that cause kernel bugs should
be ok to change the functionality of.  Remember, this is for
debugging...

thanks,

greg k-h