I can't say I'm a fan of all these long backtraces in commit logs.. > +static struct dentry *blk_debugfs_dir_register(const char *name) > +{ > + return debugfs_create_dir(name, blk_debugfs_root); > +} I don't think we really need this helper. > +void blk_part_debugfs_unregister(struct hd_struct *p) > +{ > + debugfs_remove_recursive(p->debugfs_dir); > + p->debugfs_dir = NULL; > +} Why do we need to clear the pointer here? > +#ifdef CONFIG_DEBUG_FS > + /* Currently only used by kernel/trace/blktrace.c */ > + struct dentry *debugfs_dir; > +#endif Does that comment really add value? > +static struct dentry *blk_trace_debugfs_dir(struct block_device *bdev, > + struct request_queue *q) > { > + struct hd_struct *p = NULL; > > + * Some drivers like scsi-generic use a NULL block device. For > + * other drivers when bdev != bdev->bd_contain we are doing a blktrace > + * on a parition, otherwise we know we are working on the whole > + * disk, and for that the request_queue already has its own debugfs_dir. > + * which we have been using for other things other than blktrace. > + */ > + if (bdev && bdev != bdev->bd_contains) > + p = bdev->bd_part; > > + if (p) > + return p->debugfs_dir; > + > + return q->debugfs_dir; This could be simplified down to: if (bdev && bdev != bdev->bd_contains) return bdev->bd_part->debugfs_dir; return q->debugfs_dir; Given that bd_part is in __blkdev_get very near bd_contains. Also given that this patch completely rewrites blk_trace_debugfs_dir is there any point in the previous patch? > @@ -491,6 +500,7 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, > struct dentry *dir = NULL; > int ret; > > + > if (!buts->buf_size || !buts->buf_nr) > return -EINVAL; > Spurious whitespace change.