2022-02-10 23:49:57

by Yang Shi

[permalink] [raw]
Subject: [v8 PATCH] block: introduce block_rq_error tracepoint

Currently, rasdaemon uses the existing tracepoint block_rq_complete
and filters out non-error cases in order to capture block disk errors.

But there are a few problems with this approach:

1. Even kernel trace filter could do the filtering work, there is
still some overhead after we enable this tracepoint.

2. The filter is merely based on errno, which does not align with kernel
logic to check the errors for print_req_error().

3. block_rq_complete only provides dev major and minor to identify
the block device, it is not convenient to use in user-space.

So introduce a new tracepoint block_rq_error just for the error case.
With this patch, rasdaemon could switch to block_rq_error.

Since the new tracepoint has the similar implementation with
block_rq_complete, so move the existing code from TRACE_EVENT
block_rq_complete() into new event class block_rq_completion(). Then add
event for block_rq_complete and block_rq_err respectively from the newly
created event class per the suggestion from Chaitanya Kulkarni.

Cc: Jens Axboe <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Reviewed-by: Steven Rostedt <[email protected]>
Signed-off-by: Cong Wang <[email protected]>
Signed-off-by: Chaitanya Kulkarni <[email protected]>
Signed-off-by: Yang Shi <[email protected]>
---
The v3 patch was submitted in Feb 2020, and Steven reviewed the patch, but
it was not merged to upstream. See
https://lore.kernel.org/lkml/[email protected]/.

The problems fixed by that patch still exist and we do need it to make
disk error handling in rasdaemon easier. So this resurrected it and
continued the version number.

v7 --> v8:
* Combined two patches into one per Christoph Hellwig.
* Kept Steven's reviewed-by since there is no significant change for
tracepoint other than creating event class.
v6 --> v7:
* Prepared (two patches) by Chaitanya Kulkarni
* Created event class
v5 --> v6:
* Removed disk name per Christoph and Chaitanya
* Kept errno since I didn't find any other block tracepoints print blk
status code and userspace (i.e. rasdaemon) does expect errno.
v4 --> v5:
* Report the actual block layer status code instead of the errno per
Christoph Hellwig.
v3 --> v4:
* Rebased to v5.17-rc1.
* Collected reviewed-by tag from Steven.

block/blk-mq.c | 4 ++-
include/trace/events/block.h | 49 ++++++++++++++++++++++++++----------
2 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 1adfe4824ef5..b79a9b500105 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -789,8 +789,10 @@ bool blk_update_request(struct request *req, blk_status_t error,
#endif

if (unlikely(error && !blk_rq_is_passthrough(req) &&
- !(req->rq_flags & RQF_QUIET)))
+ !(req->rq_flags & RQF_QUIET))) {
blk_print_req_error(req, error);
+ trace_block_rq_error(req, error, nr_bytes);
+ }

blk_account_io_completion(req, nr_bytes);

diff --git a/include/trace/events/block.h b/include/trace/events/block.h
index 27170e40e8c9..7f4dfbdf12a6 100644
--- a/include/trace/events/block.h
+++ b/include/trace/events/block.h
@@ -100,19 +100,7 @@ TRACE_EVENT(block_rq_requeue,
__entry->nr_sector, 0)
);

-/**
- * block_rq_complete - block IO operation completed by device driver
- * @rq: block operations request
- * @error: status code
- * @nr_bytes: number of completed bytes
- *
- * The block_rq_complete tracepoint event indicates that some portion
- * of operation request has been completed by the device driver. If
- * the @rq->bio is %NULL, then there is absolutely no additional work to
- * do for the request. If @rq->bio is non-NULL then there is
- * additional work required to complete the request.
- */
-TRACE_EVENT(block_rq_complete,
+DECLARE_EVENT_CLASS(block_rq_completion,

TP_PROTO(struct request *rq, blk_status_t error, unsigned int nr_bytes),

@@ -144,6 +132,41 @@ TRACE_EVENT(block_rq_complete,
__entry->nr_sector, __entry->error)
);

+/**
+ * block_rq_complete - block IO operation completed by device driver
+ * @rq: block operations request
+ * @error: status code
+ * @nr_bytes: number of completed bytes
+ *
+ * The block_rq_complete tracepoint event indicates that some portion
+ * of operation request has been completed by the device driver. If
+ * the @rq->bio is %NULL, then there is absolutely no additional work to
+ * do for the request. If @rq->bio is non-NULL then there is
+ * additional work required to complete the request.
+ */
+DEFINE_EVENT(block_rq_completion, block_rq_complete,
+
+ TP_PROTO(struct request *rq, blk_status_t error, unsigned int nr_bytes),
+
+ TP_ARGS(rq, error, nr_bytes)
+);
+
+/**
+ * block_rq_error - block IO operation error reported by device driver
+ * @rq: block operations request
+ * @error: status code
+ * @nr_bytes: number of completed bytes
+ *
+ * The block_rq_error tracepoint event indicates that some portion
+ * of operation request has failed as reported by the device driver.
+ */
+DEFINE_EVENT(block_rq_completion, block_rq_error,
+
+ TP_PROTO(struct request *rq, blk_status_t error, unsigned int nr_bytes),
+
+ TP_ARGS(rq, error, nr_bytes)
+);
+
DECLARE_EVENT_CLASS(block_rq,

TP_PROTO(struct request *rq),
--
2.26.3



2022-02-11 18:26:55

by Jens Axboe

[permalink] [raw]
Subject: Re: [v8 PATCH] block: introduce block_rq_error tracepoint

On Thu, 10 Feb 2022 14:52:22 -0800, Yang Shi wrote:
> Currently, rasdaemon uses the existing tracepoint block_rq_complete
> and filters out non-error cases in order to capture block disk errors.
>
> But there are a few problems with this approach:
>
> 1. Even kernel trace filter could do the filtering work, there is
> still some overhead after we enable this tracepoint.
>
> [...]

Applied, thanks!

[1/1] block: introduce block_rq_error tracepoint
commit: d5869fdc189f0f12a954a48d58a48104a2f5d044

Best regards,
--
Jens Axboe


2022-02-12 11:36:30

by Yang Shi

[permalink] [raw]
Subject: Re: [v8 PATCH] block: introduce block_rq_error tracepoint

On Fri, Feb 11, 2022 at 9:00 AM Jens Axboe <[email protected]> wrote:
>
> On Thu, 10 Feb 2022 14:52:22 -0800, Yang Shi wrote:
> > Currently, rasdaemon uses the existing tracepoint block_rq_complete
> > and filters out non-error cases in order to capture block disk errors.
> >
> > But there are a few problems with this approach:
> >
> > 1. Even kernel trace filter could do the filtering work, there is
> > still some overhead after we enable this tracepoint.
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] block: introduce block_rq_error tracepoint
> commit: d5869fdc189f0f12a954a48d58a48104a2f5d044

Thank you!

>
> Best regards,
> --
> Jens Axboe
>
>

2022-02-12 13:56:42

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [v8 PATCH] block: introduce block_rq_error tracepoint

Looks good,

Reviewed-by: Christoph Hellwig <[email protected]>

2022-02-14 20:31:51

by Yang Shi

[permalink] [raw]
Subject: Re: [v8 PATCH] block: introduce block_rq_error tracepoint

On Thu, Feb 10, 2022 at 10:30 PM Christoph Hellwig <[email protected]> wrote:
>
> Looks good,
>
> Reviewed-by: Christoph Hellwig <[email protected]>

Thank you!