On Tue, Jun 09, 2020 at 10:32:18AM -0700, Christoph Hellwig wrote:

On Tue, Jun 09, 2020 at 05:29:22PM +0000, Luis Chamberlain wrote:

quoted

Is scsi-generic is the only unwanted ugly child blktrace has to deal
with? For some reason I thought drivers/md/md.c was one but it seems
like it is not. Do we have an easy way to search for these? I think
this would just affect how we express the comment only.

grep for blk_trace_setup.  For all blk devices that setup comes in
through the block device ioctl path, and that relies on having a
struct block_device and queue.  sg on the other hand calls
blk_trace_setup directly with a NULL bdev argument.

Alright, then we should be good.

quoted

quoted

 		 */
-		dir = q->sg_debugfs_dir;
+		dir = debugfs_create_dir(buts->name, blk_debugfs_root);
+		bt->dir = dir;

The other chicken and egg problem to consider at least in the comments
is that the debugfs directory for these types of devices *have* an
exposed path, but the data structure is rather opaque to the device and
even blktrace.  Fortunately given the recent set of changes around the
q->blk_trace and clarifications around its use we have made it clear now
that so long as hold the q->blk_trace_mutex *and* check q->blk_trace we
*should* not race against two separate creations of debugfs directories,
so I think this is safe, so long as these indpendent drivers don't end
up re-using the same path for some other things later in the future, and
since we have control over what goes under debugfsroot block / I think
we should be good.

But I think that the concern for race on names may still be worth
explaining a bit here.

Feel free to add more comments, but please try to keep them short
and crisp.  At the some point long comments really distract from what
is going on.

Sure.

Come to think of it, given the above, I think we can also do way with
the the partition stuff too, and rely on the buts->name too. I'll try
this out, and test it.

  Luis