On Wed, May 27, 2020 at 03:12:02AM +0000, Luis Chamberlain wrote: > You forgot to deal with partitions. Putting similar lipstick on the pig, > this is what I end up with, let me know if this seems agreeable: So even with the partition stuff in place, this approach still don't allow multiple uses of blktrace against a scsi-generic device and its backend real block device, say TYPE_DISK. A simple example is a scsi drive hooked up used to allow users to do blktrace /dev/sda *and* blktrace /dev/sg0, but with the proposed change /dev/sg0 no longer works beacuse the dentry pertains to the '/dev/sda' name, not '/dev/sg0'. We can shoehorn in a solution following the style proposed as follows. We can keep this only slightly cleaner if we don't care about the extra dentry even if a user disables CONFIG_CHR_DEV_SG. The cost would just be an extra dentry on the request_queue. I'll run this through 0-day and then post a new hopefully final series, but if you don't think this or would prefer something lease please let me know. diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index 86c107de2836..f46bdc7f6509 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -920,6 +920,9 @@ static void blk_release_queue(struct kobject *kobj) blk_trace_shutdown(q); debugfs_remove_recursive(q->debugfs_dir); +#if defined(CONFIG_CHR_DEV_SG) || defined(CONFIG_CHR_DEV_SG_MODULE) + debugfs_remove_recursive(q->sg_debugfs_dir); +#endif if (queue_is_mq(q)) blk_mq_debugfs_unregister(q); @@ -939,6 +942,21 @@ struct kobj_type blk_queue_ktype = { .release = blk_release_queue, }; +#if defined(CONFIG_CHR_DEV_SG) || defined(CONFIG_CHR_DEV_SG_MODULE) +/** + * blk_sg_debugfs_init - initialize debugs for scsi-generic + * @q: the associated queue + * @name: name of the scsi-generic device + * + * To be used by scsi-generic for allowing it to use blktrace. + */ +void blk_sg_debugfs_init(struct request_queue *q, const char *name) +{ + q->sg_debugfs_dir = debugfs_create_dir(name, blk_debugfs_root); +} +EXPORT_SYMBOL_GPL(blk_sg_debugfs_init); +#endif + /** * blk_register_queue - register a block layer queue with sysfs * @disk: Disk of which the request queue should be registered with sysfs. diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 20472aaaf630..c87fe1923f3d 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -1519,6 +1519,7 @@ static int sg_add_device(struct device *cl_dev, struct class_interface *cl_intf) { struct scsi_device *scsidp = to_scsi_device(cl_dev->parent); + struct request_queue *q = scsidp->request_queue; struct gendisk *disk; Sg_device *sdp = NULL; struct cdev * cdev = NULL; @@ -1573,6 +1574,7 @@ sg_add_device(struct device *cl_dev, struct class_interface *cl_intf) } else pr_warn("%s: sg_sys Invalid\n", __func__); + blk_sg_debugfs_init(q, disk->disk_name); sdev_printk(KERN_NOTICE, scsidp, "Attached scsi generic sg%d " "type %d\n", sdp->index, scsidp->type); diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 5877b03b8117..be5a40d59f60 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -575,6 +575,9 @@ struct request_queue { struct bio_set bio_split; struct dentry *debugfs_dir; +#if defined(CONFIG_CHR_DEV_SG) || defined(CONFIG_CHR_DEV_SG_MODULE) + struct dentry *sg_debugfs_dir; +#endif #ifdef CONFIG_BLK_DEBUG_FS struct dentry *sched_debugfs_dir; struct dentry *rqos_debugfs_dir; @@ -858,6 +861,14 @@ static inline void rq_flush_dcache_pages(struct request *rq) extern int blk_register_queue(struct gendisk *disk); extern void blk_unregister_queue(struct gendisk *disk); +#if defined(CONFIG_CHR_DEV_SG) || defined(CONFIG_CHR_DEV_SG_MODULE) +extern void blk_sg_debugfs_init(struct request_queue *q, const char *name); +#else +static inline void blk_sg_debugfs_init(struct request_queue *q, + const char *name) +{ +} +#endif extern blk_qc_t generic_make_request(struct bio *bio); extern blk_qc_t direct_make_request(struct bio *bio); extern void blk_rq_init(struct request_queue *q, struct request *rq); diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c index a55cbfd060f5..5b0310f38e11 100644 --- a/kernel/trace/blktrace.c +++ b/kernel/trace/blktrace.c @@ -511,6 +511,11 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, */ if (bdev && bdev != bdev->bd_contains) { dir = bdev->bd_part->debugfs_dir; + } else if (q->sg_debugfs_dir && + strlen(buts->name) == strlen(q->sg_debugfs_dir->d_name.name) + && strcmp(buts->name, q->sg_debugfs_dir->d_name.name) == 0) { + /* scsi-generic requires use of its own directory */ + dir = q->sg_debugfs_dir; } else { /* * For queues that do not have a gendisk attached to them, that