On Thu, Feb 02, 2017 at 11:58:53AM +0100, Greg Kroah-Hartman wrote:

On Wed, Feb 01, 2017 at 12:31:15AM -0800, Omar Sandoval wrote:

quoted

On Wed, Feb 01, 2017 at 09:16:08AM +0100, Greg Kroah-Hartman wrote:

quoted

On Tue, Jan 31, 2017 at 02:53:16PM -0800, Omar Sandoval wrote:

quoted

From: Omar Sandoval <redacted>

When I moved the blk-mq debugging information to debugfs, I didn't
realize that blktrace also created directories in debugfs that
conflicted with the blk-mq directories. This series fixes that.

Patch 1 adds a new debugfs helper needed for patch 6. Greg, could I get
an ack on that if it makes sense? Jens and I went back and forth on this
for a little while, but patch 6 has more of the rationale on why we
decided that this approach was the cleanest.

I can't find patch 6, you only cc:ed me on the first patch :(

Care to bounce them all to me?

thanks,

greg k-h

Gah, I forgot --cc-cover to git-send-email. The series is all here:
http://marc.info/?l=linux-block&r=1&b=201701&w=2. I can also send the
patches directly to you if you prefer that.

I don't understand the problem here.  How do you not know if you have
created the debugfs file or not?  You have the structure, with the
correct name, how could it have been created?  Can't you save the dentry
to the debugfs file in the structure that has the name?

thanks,

greg k-h

Hey, Greg,

So here's the alternative to doing the lookup:

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 38052f625a0f..79ef6b9d1f96 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c

@@ -470,12 +470,15 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
 	if (!blk_debugfs_root)
 		goto err;
 
-	dir = debugfs_create_dir(buts->name, blk_debugfs_root);
-
+#ifdef CONFIG_BLK_DEBUG_FS
+	if (q->mq_ops && !bdev->bd_part.partno)
+		dir = q->debugfs_dir;
+#endif
+	if (!dir)
+		bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root);
 	if (!dir)
 		goto err;
 
-	bt->dir = dir;
 	bt->dev = dev;
 	atomic_set(&bt->dropped, 0);
 	INIT_LIST_HEAD(&bt->running_list);

So we could figure out if it exists, but it's very special-cased and
fragile. And then there's this further up:


	/*
	 * some device names have larger paths - convert the slashes
	 * to underscores for this to work as expected
	 */
	strreplace(buts->name, '/', '_');


which I'm not sure applies anymore but would also break this. Doing the
lookup is just more foolproof.

Thanks,
Omar