Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Zheng Yejian <zhengyejian1@huawei.com>
To:     <axboe@kernel.dk>, <rostedt@goodmis.org>, <mhiramat@kernel.org>
CC:     <linux-block@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-trace-kernel@vger.kernel.org>, <zhengyejian1@huawei.com>
Subject: [PATCH] blktrace: Gracefully return if fail in blk_register_tracepoints()
Date:   Tue, 28 Mar 2023 17:46:05 +0800
Message-ID: <20230328094605.480557-1-zhengyejian1@huawei.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7BIT
Content-Type:   text/plain; charset=US-ASCII
Precedence: bulk

Currently blk_register_tracepoints() WARN for every registration failure
of block tracepoint, however, blk_trace can still be setup-ed.

It seems better to cleanup registered tracepoints and return error when
a certain registration failed, even if it's a small probability, and
users can know the error and do blk_trace setup again.

Signed-off-by: Zheng Yejian <zhengyejian1@huawei.com>
---
 kernel/trace/blktrace.c | 113 +++++++++++++++++++++++++++++++---------
 1 file changed, 87 insertions(+), 26 deletions(-)

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index d5d94510afd3..62ac72866f02 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -60,7 +60,7 @@ static struct tracer_flags blk_tracer_flags = {
 static DEFINE_MUTEX(blk_probe_mutex);
 static int blk_probes_ref;
 
-static void blk_register_tracepoints(void);
+static int blk_register_tracepoints(void);
 static void blk_unregister_tracepoints(void);
 
 /*
@@ -330,12 +330,20 @@ static void blk_trace_free(struct request_queue *q, struct blk_trace *bt)
 	kfree(bt);
 }
 
-static void get_probe_ref(void)
+static int get_probe_ref(void)
 {
+	int ret = 0;
+
 	mutex_lock(&blk_probe_mutex);
-	if (++blk_probes_ref == 1)
-		blk_register_tracepoints();
+	if (++blk_probes_ref == 1) {
+		ret = blk_register_tracepoints();
+		if (ret) {
+			pr_warn("Could not register block tracepoints\n");
+			--blk_probes_ref;
+		}
+	}
 	mutex_unlock(&blk_probe_mutex);
+	return ret;
 }
 
 static void put_probe_ref(void)
@@ -608,13 +616,14 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
 	bt->pid = buts->pid;
 	bt->trace_state = Blktrace_setup;
 
+	ret = get_probe_ref();
+	if (ret)
+		goto err;
 	rcu_assign_pointer(q->blk_trace, bt);
-	get_probe_ref();
+	return 0;
 
-	ret = 0;
 err:
-	if (ret)
-		blk_trace_free(q, bt);
+	blk_trace_free(q, bt);
 	return ret;
 }
 
@@ -1088,42 +1097,92 @@ void blk_add_driver_data(struct request *rq, void *data, size_t len)
 }
 EXPORT_SYMBOL_GPL(blk_add_driver_data);
 
-static void blk_register_tracepoints(void)
+static int blk_register_tracepoints(void)
 {
 	int ret;
 
 	ret = register_trace_block_rq_insert(blk_add_trace_rq_insert, NULL);
-	WARN_ON(ret);
+	if (ret)
+		return ret;
 	ret = register_trace_block_rq_issue(blk_add_trace_rq_issue, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_rq_issue;
 	ret = register_trace_block_rq_merge(blk_add_trace_rq_merge, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_rq_merge;
 	ret = register_trace_block_rq_requeue(blk_add_trace_rq_requeue, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_rq_requeue;
 	ret = register_trace_block_rq_complete(blk_add_trace_rq_complete, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_rq_complete;
 	ret = register_trace_block_bio_bounce(blk_add_trace_bio_bounce, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_bio_bounce;
 	ret = register_trace_block_bio_complete(blk_add_trace_bio_complete, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_bio_complete;
 	ret = register_trace_block_bio_backmerge(blk_add_trace_bio_backmerge, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_bio_backmerge;
 	ret = register_trace_block_bio_frontmerge(blk_add_trace_bio_frontmerge, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_bio_frontmerge;
 	ret = register_trace_block_bio_queue(blk_add_trace_bio_queue, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_bio_queue;
 	ret = register_trace_block_getrq(blk_add_trace_getrq, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_getrq;
 	ret = register_trace_block_plug(blk_add_trace_plug, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_plug;
 	ret = register_trace_block_unplug(blk_add_trace_unplug, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_unplug;
 	ret = register_trace_block_split(blk_add_trace_split, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_split;
 	ret = register_trace_block_bio_remap(blk_add_trace_bio_remap, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_bio_remap;
 	ret = register_trace_block_rq_remap(blk_add_trace_rq_remap, NULL);
-	WARN_ON(ret);
+	if (ret)
+		goto err_block_rq_remap;
+	return 0;
+
+err_block_rq_remap:
+	unregister_trace_block_bio_remap(blk_add_trace_bio_remap, NULL);
+err_block_bio_remap:
+	unregister_trace_block_split(blk_add_trace_split, NULL);
+err_block_split:
+	unregister_trace_block_unplug(blk_add_trace_unplug, NULL);
+err_block_unplug:
+	unregister_trace_block_plug(blk_add_trace_plug, NULL);
+err_block_plug:
+	unregister_trace_block_getrq(blk_add_trace_getrq, NULL);
+err_block_getrq:
+	unregister_trace_block_bio_queue(blk_add_trace_bio_queue, NULL);
+err_block_bio_queue:
+	unregister_trace_block_bio_frontmerge(blk_add_trace_bio_frontmerge, NULL);
+err_block_bio_frontmerge:
+	unregister_trace_block_bio_backmerge(blk_add_trace_bio_backmerge, NULL);
+err_block_bio_backmerge:
+	unregister_trace_block_bio_complete(blk_add_trace_bio_complete, NULL);
+err_block_bio_complete:
+	unregister_trace_block_bio_bounce(blk_add_trace_bio_bounce, NULL);
+err_block_bio_bounce:
+	unregister_trace_block_rq_complete(blk_add_trace_rq_complete, NULL);
+err_block_rq_complete:
+	unregister_trace_block_rq_requeue(blk_add_trace_rq_requeue, NULL);
+err_block_rq_requeue:
+	unregister_trace_block_rq_merge(blk_add_trace_rq_merge, NULL);
+err_block_rq_merge:
+	unregister_trace_block_rq_issue(blk_add_trace_rq_issue, NULL);
+err_block_rq_issue:
+	unregister_trace_block_rq_insert(blk_add_trace_rq_insert, NULL);
+	tracepoint_synchronize_unregister();
+	return ret;
 }
 
 static void blk_unregister_tracepoints(void)
@@ -1643,8 +1702,10 @@ static int blk_trace_setup_queue(struct request_queue *q,
 
 	blk_trace_setup_lba(bt, bdev);
 
+	ret = get_probe_ref();
+	if (ret)
+		goto free_bt;
 	rcu_assign_pointer(q->blk_trace, bt);
-	get_probe_ref();
 	return 0;
 
 free_bt:
-- 
2.25.1