Messages in this thread | | | Date | Mon, 24 Oct 2022 12:19:26 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH 2/2] clk: Add trace events for rate requests |
| |
On Tue, 18 Oct 2022 15:56:42 +0200 Maxime Ripard <maxime@cerno.tech> wrote:
I know I reviewed this but looking at this again, I noticed:
> +DECLARE_EVENT_CLASS(clk_rate_request, > + > + TP_PROTO(struct clk_rate_request *req), > + > + TP_ARGS(req), > + > + TP_STRUCT__entry( > + __string( name, req->core ? req->core->name : "none") > + __field(unsigned long, min ) > + __field(unsigned long, max ) > + __string( pname, req->best_parent_hw ? clk_hw_get_name(req->best_parent_hw) : "none" )
It may be best to move the two __string() declarations together. The reason is that dynamic strings (which __string() is) uses 4 bytes embedded in the first part of the event. Two bytes for offset, where the dynamic string exists, and two bytes for the strings length.
On 64 bit machines the above has:
__string() 4 bytes __field(unsigned long) 8 bytes __field(unsigned long) 8 bytes __string() 4 bytes
and then another unsigned long field below, which is another 8 bytes.
> + __field(unsigned long, prate ) > + ), > + > +
As compilers tend to use word alignment, the above turns into:
__string() 4 bytes
__PADDING__ 4 bytes
__field(unsigned long) 8 bytes __field(unsigned long) 8 bytes __string() 4 bytes
__PADDING__ 4 bytes
__field(unsigned long) 8 bytes
Where there will be 8 bytes of padding in that event that wastes precious ring buffer space. By changing the event to:
TP_STRUCT__entry( __string( name, req->core ? req->core->name : "none") __string( pname, req->best_parent_hw ? clk_hw_get_name(req->best_parent_hw) : "none" ) __field(unsigned long, min ) __field(unsigned long, max ) __field(unsigned long, prate ) ),
It will turn the size into:
__string() 4 bytes __string() 4 bytes __field(unsigned long) 8 bytes __field(unsigned long) 8 bytes __field(unsigned long) 8 bytes
With no padding and no wasted space.
I would suggest changing this.
-- Steve
| |