On Fri, 2018-12-14 at 19:39 +0800, Ming Lei wrote:
> static void print_stat(struct seq_file *m, struct blk_rq_stat *stat)
> {
> @@ -856,6 +857,15 @@ int blk_mq_debugfs_register(struct request_queue *q)
> goto err;
> }
>
> + if (q->rq_qos) {
> + struct rq_qos *rqos = q->rq_qos;
> +
> + while (rqos) {
> + blk_mq_debugfs_register_rqos(rqos);
> + rqos = rqos->next;
> + }
> + }
Have you considered to use a for-loop instead of a while loop? That would
allow to remove the if-statement and hence would make the code more compact.
> +int blk_mq_debugfs_register_rqos(struct rq_qos *rqos)
> +{
> + struct request_queue *q = rqos->q;
> + char *dir_name = rq_qos_id_to_name(rqos->id);
Please change "char *" into "const char *".
> enum rq_qos_id {
> RQ_QOS_WBT,
> RQ_QOS_CGROUP,
> @@ -22,6 +26,9 @@ struct rq_qos {
> struct request_queue *q;
> enum rq_qos_id id;
> struct rq_qos *next;
> +#ifdef CONFIG_BLK_DEBUG_FS
> + struct dentry *debugfs_dir;
> +#endif
> };
The indentation of "debugfs_dir" looks odd to me.
> +static inline char *rq_qos_id_to_name(enum rq_qos_id id)
> +{
> + switch (id) {
> + case RQ_QOS_WBT:
> + return "wbt";
> + case RQ_QOS_CGROUP:
> + return "cgroup";
> + }
> + return "unknown";
> +}
Same comment here as earlier: please use "const char *" instead of "char *"
for constant strings. Otherwise this patch looks fine to me.
Thanks,
Bart.
