| Message ID | 4650cbf668fd4853ad98dba834141e08@SIWEX5A.sing.micron.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Mon, 16 Apr 2018 14:31:49 +0000 "Bean Huo (beanhuo)" <beanhuo@micron.com> wrote: > Print the request tag along with other information > while tracing a command. > > Signed-off-by: Bean Huo <beanhuo@micron.com> > --- I don't see any issue with the tracing part. Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org> Others need to check the content. -- Steve
On Mon, Apr 16, 2018 at 8:28 AM, Steven Rostedt <rostedt@goodmis.org> wrote: > On Mon, 16 Apr 2018 14:31:49 +0000 > "Bean Huo (beanhuo)" <beanhuo@micron.com> wrote: > >> Print the request tag along with other information >> while tracing a command. >> >> Signed-off-by: Bean Huo <beanhuo@micron.com> Acked-by: Rajat Jain <rajatja@google.com> >> --- > > I don't see any issue with the tracing part. > > Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > > Others need to check the content. > > -- Steve
On Mon, 2018-04-16 at 09:41 -0700, Rajat Jain wrote: > On Mon, Apr 16, 2018 at 8:28 AM, Steven Rostedt <rostedt@goodmis.org> wrote: > > On Mon, 16 Apr 2018 14:31:49 +0000 > > "Bean Huo (beanhuo)" <beanhuo@micron.com> wrote: > > > > > Print the request tag along with other information > > > while tracing a command. > > > > > > Signed-off-by: Bean Huo <beanhuo@micron.com> > > Acked-by: Rajat Jain <rajatja@google.com> This patch is not acceptable because it adds support for tag tracing to the legacy block layer only. Any patch that adds a new feature to the legacy block layer must also add it to blk-mq. Bart.
On Mon, 2018-04-16 at 14:31 +0000, Bean Huo (beanhuo) wrote: > TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u prot_sgl=%u" \ > - " prot_op=%s cmnd=(%s %s raw=%s)", > + " prot_op=%s tag=%d cmnd=(%s %s raw=%s)", > > [ ... ] > TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u prot_sgl=%u" \ > - " prot_op=%s cmnd=(%s %s raw=%s) rtn=%d", > + " prot_op=%s tag=%d cmnd=(%s %s raw=%s) rtn=%d", > [ ... ] > TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u " \ > - "prot_sgl=%u prot_op=%s cmnd=(%s %s raw=%s) result=(driver=" \ > - "%s host=%s message=%s status=%s)", > + "prot_sgl=%u prot_op=%s tag=%d cmnd=(%s %s raw=%s) " \ > + "result=(driver=%s host=%s message=%s status=%s)", Which tools process these strings? Has it been verified whether or not the tools that process these strings still work fine with this patch applied? Thanks, Bart.
On Mon, 16 Apr 2018 20:49:12 +0000 Bart Van Assche <Bart.VanAssche@wdc.com> wrote: > Which tools process these strings? Has it been verified whether or not > the tools that process these strings still work fine with this patch > applied? Ideally, tools shouldn't process trace event strings, but I'm sure some do. :-/ Getting libtraceevent out as a library is a high priority of mine, and hopefully I should get something out in a couple of months. Ideally, tools should parse the raw data and then new fields will not affect them. -- Steve
On Mon, 2018-04-16 at 17:24 -0400, Steven Rostedt wrote: > On Mon, 16 Apr 2018 20:49:12 +0000 > Bart Van Assche <Bart.VanAssche@wdc.com> wrote: > > > Which tools process these strings? Has it been verified whether or not > > the tools that process these strings still work fine with this patch > > applied? > > Ideally, tools shouldn't process trace event strings, but I'm sure some > do. :-/ > > Getting libtraceevent out as a library is a high priority of mine, and > hopefully I should get something out in a couple of months. > > Ideally, tools should parse the raw data and then new fields will not > affect them. Hello Steve, The tool I'm most concerned about is blktrace. I'm not sure though how this tool receives event data from the block layer core. Thanks, Bart.
On Mon, 16 Apr 2018 21:30:54 +0000 Bart Van Assche <Bart.VanAssche@wdc.com> wrote: > Hello Steve, > > The tool I'm most concerned about is blktrace. I'm not sure though how this > tool receives event data from the block layer core. Yeah, blktrace is "special", it looks like it registers its callbacks from the tracepoints, and writes the data to its own relay buffer. As it's not relying on the output from the tracing directory, additional fields being added shouldn't affect it. Looking at the trace event "block_rq_requeue" we have in the blktrace kernel code: static void blk_register_tracepoints(void) { int ret; ret = register_trace_block_rq_insert(blk_add_trace_rq_insert, NULL); Where the callback blk_add_trace_rq_insert() gets called when the trace event is hit. static void blk_add_trace_rq_insert(void *ignore, struct request_queue *q, struct request *rq) { blk_add_trace_rq(rq, 0, blk_rq_bytes(rq), BLK_TA_INSERT, blk_trace_request_get_cgid(q, rq)); } Where: static void blk_add_trace_rq(struct request *rq, int error, unsigned int nr_bytes, u32 what, union kernfs_node_id *cgid) { calls __blk_add_trace(bt, blk_rq_trace_sector(rq), nr_bytes, req_op(rq), rq->cmd_flags, what, error, 0, NULL, cgid); Which calls either the ftrace tracing file or its own relay buffer. Looking at the code from git://git.kernel.org/pub/scm/linux/kernel/git/axboe/blktrace.git It appears that it does not rely on the ftrace ring buffers. So I'm guessing blktrace is not affected by this patch. -- Steve
diff --git a/include/trace/events/scsi.h b/include/trace/events/scsi.h index f624969..a4ada90 100644 --- a/include/trace/events/scsi.h +++ b/include/trace/events/scsi.h @@ -210,6 +210,7 @@ TRACE_EVENT(scsi_dispatch_cmd_start, __field( unsigned int, lun ) __field( unsigned int, opcode ) __field( unsigned int, cmd_len ) + __field( int, tag ) __field( unsigned int, data_sglen ) __field( unsigned int, prot_sglen ) __field( unsigned char, prot_op ) @@ -223,6 +224,7 @@ TRACE_EVENT(scsi_dispatch_cmd_start, __entry->lun = cmd->device->lun; __entry->opcode = cmd->cmnd[0]; __entry->cmd_len = cmd->cmd_len; + __entry->tag = cmd->request->tag; __entry->data_sglen = scsi_sg_count(cmd); __entry->prot_sglen = scsi_prot_sg_count(cmd); __entry->prot_op = scsi_get_prot_op(cmd); @@ -230,10 +232,10 @@ TRACE_EVENT(scsi_dispatch_cmd_start, ), TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u prot_sgl=%u" \ - " prot_op=%s cmnd=(%s %s raw=%s)", + " prot_op=%s tag=%d cmnd=(%s %s raw=%s)", __entry->host_no, __entry->channel, __entry->id, __entry->lun, __entry->data_sglen, __entry->prot_sglen, - show_prot_op_name(__entry->prot_op), + show_prot_op_name(__entry->prot_op), __entry->tag, show_opcode_name(__entry->opcode), __parse_cdb(__get_dynamic_array(cmnd), __entry->cmd_len), __print_hex(__get_dynamic_array(cmnd), __entry->cmd_len)) @@ -253,6 +255,7 @@ TRACE_EVENT(scsi_dispatch_cmd_error, __field( int, rtn ) __field( unsigned int, opcode ) __field( unsigned int, cmd_len ) + __field( int, tag ) __field( unsigned int, data_sglen ) __field( unsigned int, prot_sglen ) __field( unsigned char, prot_op ) @@ -267,6 +270,7 @@ TRACE_EVENT(scsi_dispatch_cmd_error, __entry->rtn = rtn; __entry->opcode = cmd->cmnd[0]; __entry->cmd_len = cmd->cmd_len; + __entry->tag = cmd->request->tag; __entry->data_sglen = scsi_sg_count(cmd); __entry->prot_sglen = scsi_prot_sg_count(cmd); __entry->prot_op = scsi_get_prot_op(cmd); @@ -274,10 +278,10 @@ TRACE_EVENT(scsi_dispatch_cmd_error, ), TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u prot_sgl=%u" \ - " prot_op=%s cmnd=(%s %s raw=%s) rtn=%d", + " prot_op=%s tag=%d cmnd=(%s %s raw=%s) rtn=%d", __entry->host_no, __entry->channel, __entry->id, __entry->lun, __entry->data_sglen, __entry->prot_sglen, - show_prot_op_name(__entry->prot_op), + show_prot_op_name(__entry->prot_op), __entry->tag, show_opcode_name(__entry->opcode), __parse_cdb(__get_dynamic_array(cmnd), __entry->cmd_len), __print_hex(__get_dynamic_array(cmnd), __entry->cmd_len), @@ -298,6 +302,7 @@ DECLARE_EVENT_CLASS(scsi_cmd_done_timeout_template, __field( int, result ) __field( unsigned int, opcode ) __field( unsigned int, cmd_len ) + __field( int, tag ) __field( unsigned int, data_sglen ) __field( unsigned int, prot_sglen ) __field( unsigned char, prot_op ) @@ -312,6 +317,7 @@ DECLARE_EVENT_CLASS(scsi_cmd_done_timeout_template, __entry->result = cmd->result; __entry->opcode = cmd->cmnd[0]; __entry->cmd_len = cmd->cmd_len; + __entry->tag = cmd->request->tag; __entry->data_sglen = scsi_sg_count(cmd); __entry->prot_sglen = scsi_prot_sg_count(cmd); __entry->prot_op = scsi_get_prot_op(cmd); @@ -319,11 +325,11 @@ DECLARE_EVENT_CLASS(scsi_cmd_done_timeout_template, ), TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u " \ - "prot_sgl=%u prot_op=%s cmnd=(%s %s raw=%s) result=(driver=" \ - "%s host=%s message=%s status=%s)", + "prot_sgl=%u prot_op=%s tag=%d cmnd=(%s %s raw=%s) " \ + "result=(driver=%s host=%s message=%s status=%s)", __entry->host_no, __entry->channel, __entry->id, __entry->lun, __entry->data_sglen, __entry->prot_sglen, - show_prot_op_name(__entry->prot_op), + show_prot_op_name(__entry->prot_op), __entry->tag, show_opcode_name(__entry->opcode), __parse_cdb(__get_dynamic_array(cmnd), __entry->cmd_len), __print_hex(__get_dynamic_array(cmnd), __entry->cmd_len),
Print the request tag along with other information while tracing a command. Signed-off-by: Bean Huo <beanhuo@micron.com> --- include/trace/events/scsi.h | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)