On Tue, Jan 21, 2020 at 4:01 PM Steven Rostedt [off-list ref] wrote:

On Mon, 20 Jan 2020 14:26:18 -0800
Cong Wang [off-list ref] wrote:

quoted

+/**
+ * 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.
+ */
+TRACE_EVENT(block_rq_error,
+
+     TP_PROTO(struct request *rq, int error, unsigned int nr_bytes),
+
+     TP_ARGS(rq, error, nr_bytes),
+
+     TP_STRUCT__entry(
+             __field(  dev_t,        dev                     )
+             __dynamic_array( char,  name,   DISK_NAME_LEN   )

Hmm, looks like I need to go and do a clean up of the kernel, and
educate people on how to use dynamic arrays :-/

Yeah.

The "len" field of a __dynamic_array() is to be a function to determine
the length needed for each instance of an event. By having a constant
there, it will be the same for every events, plus the meta data to hold
the "dynamic" part of the array. This would be much better to simple
use __array() instead.

But as you use "__assign_str()" below, then it's expected that name is
a nul terminated string. In which case, you want to define this as:

                __string( name, rq->rq_disk ? rq->rq_disk->disk_name : "?"  )

Ah, I wanted to use string() but all the existing users initialize/assign
them twice: once in TP_STRUCT__entry() and once in TP_fast_assign().

quoted

+             __field(  sector_t,     sector                  )
+             __field(  unsigned int, nr_sector               )
+             __field(  int,          error                   )
+             __array(  char,         rwbs,   RWBS_LEN        )
+             __dynamic_array( char,  cmd,    1               )

Not sure what you are doing with cmd. It appears to be always hard
coded as an empty string?

It is supposed to be a string of one-char commands. This is a
copy-n-paste from existing block_rq_requeue(). I don't know why
its length is 1, but yeah it looks wrong to me too.

Thanks!