lkml.org 
[lkml]   [2020]   [Feb]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Patch v3] block: introduce block_rq_error tracepoint
    On Sun,  2 Feb 2020 21:36:50 -0800
    Cong Wang <xiyou.wangcong@gmail.com> wrote:

    > Cc: Jens Axboe <axboe@kernel.dk>
    > Cc: Steven Rostedt <rostedt@goodmis.org>
    > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
    > ---
    > block/blk-core.c | 4 +++-
    > include/trace/events/block.h | 41 ++++++++++++++++++++++++++++++++++++
    > 2 files changed, 44 insertions(+), 1 deletion(-)
    >
    > diff --git a/block/blk-core.c b/block/blk-core.c
    > index 089e890ab208..0c7ad70d06be 100644
    > --- a/block/blk-core.c
    > +++ b/block/blk-core.c
    > @@ -1450,8 +1450,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))) {
    > + trace_block_rq_error(req, blk_status_to_errno(error), nr_bytes);

    I'm curious to why you don't just pass error into the trace event.
    Looks like blk_status_to_errno() is a function call and that injects
    code at the location of the call. Note, it is not a big deal as I
    believe (haven't looked at the objdump of it), the call may be placed
    in the nop portion of the code, and not hit when the trace point is not
    enabled. But moving the blk_status_to_errno() call to the
    TP_fast_assign() will move it to another section entirely.

    I did see trace_blk_rq_complete() does the same thing, so perhaps that
    could just be a clean up change after this on both trace events.



    > +
    > + TP_printk("%d,%d %s %s %llu + %u [%d]",
    > + MAJOR(__entry->dev), MINOR(__entry->dev),
    > + __get_str(name), __entry->rwbs,
    > + (unsigned long long)__entry->sector,
    > + __entry->nr_sector, __entry->error)
    > +);
    > +

    Other than my comment above, for the trace event correctness point of view:

    Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

    -- Steve

    \
     
     \ /
      Last update: 2020-02-03 19:26    [W:4.167 / U:0.640 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site