On Tue, Jan 21, 2020 at 4:01 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > > On Mon, 20 Jan 2020 14:26:18 -0800 > Cong Wang <xiyou.wangcong@xxxxxxxxx> wrote: > > +/** > > + * 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(). > > > > + __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!
