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

From: Hannes Reinecke <hare@suse.de>
To: Ming Lei <ming.lei@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org, Christoph Hellwig <hch@lst.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>,
	Dan Williams <dan.j.williams@intel.com>,
	yukuai <yukuai3@huawei.com>
Subject: Re: [PATCH V2 2/2] block: fix "Directory XXXXX with parent 'block' already present!"
Date: Sun, 24 Apr 2022 13:51:45 +0200	[thread overview]
Message-ID: <682a215d-de50-40f1-b6f8-48801617bcad@suse.de> (raw)
In-Reply-To: <YmUX/Q9o08rOSTaQ@T590>

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?

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?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer