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: > > 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. > > > */ > > > - 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
