On Fri, 28 Feb 2025 17:47:12 -0500 Steven Rostedt <[email protected]> wrote:
> On Thu, 27 Feb 2025 09:45:50 +0900 > "Masami Hiramatsu (Google)" <[email protected]> wrote: > > > From: Masami Hiramatsu (Google) <[email protected]> > > > > Add comments about entry data storing code to __store_entry_arg() and > > traceprobe_get_entry_data_size(). These are a bit complicated because of > > building the entry data storing code and scanning it. > > > > This just add comments, no behavior change. > > > > Reported-by: Steven Rostedt <[email protected]> > > Closes: > > https://lore.kernel.org/all/[email protected]/ > > Signed-off-by: Masami Hiramatsu (Google) <[email protected]> > > --- > > kernel/trace/trace_probe.c | 28 ++++++++++++++++++++++++++++ > > 1 file changed, 28 insertions(+) > > > > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c > > index 8f58ee1e8858..2eeecb6c95ee 100644 > > --- a/kernel/trace/trace_probe.c > > +++ b/kernel/trace/trace_probe.c > > @@ -770,6 +770,10 @@ static int check_prepare_btf_string_fetch(char > > *typename, > > > > #ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API > > > > +/* > > + * Add the entry code to store the 'argnum'th parameter and return the > > offset > > + * in the entry data buffer where the data will be stored. > > + */ > > static int __store_entry_arg(struct trace_probe *tp, int argnum) > > { > > struct probe_entry_arg *earg = tp->entry_arg; > > @@ -793,6 +797,20 @@ static int __store_entry_arg(struct trace_probe *tp, > > int argnum) > > tp->entry_arg = earg; > > } > > > > + /* > > + * The entry code array is repeating the pair of > > + * [FETCH_OP_ARG(argnum)][FETCH_OP_ST_EDATA(offset of entry data > > buffer)] > > + * and the rest of entries are filled with [FETCH_OP_END]. > > + * > > + * To reduce the redundant function parameter fetching, we scan the > > entry > > + * code array to find the FETCH_OP_ARG which already fetches the > > 'argnum' > > + * parameter. If it doesn't match, update 'offset' to find the last > > + * offset. > > + * If we find the FETCH_OP_END without matching FETCH_OP_ARG entry, we > > + * will save the entry with FETCH_OP_ARG and FETCH_OP_ST_EDATA, and > > + * return data offset so that caller can find the data offset in the > > entry > > + * data buffer. > > + */ > > So I'm still not 100% sure what is happening here, but at least these > comments can help me figure it out if I spend some time gawking at the code > a bit more ;-) I think the code is a bit complicated too. I will rewrite it a bit. > > Reviewed-by: Steven Rostedt (Google) <[email protected]> Thanks you! > > -- Steve > > > > offset = 0; > > for (i = 0; i < earg->size - 1; i++) { > > switch (earg->code[i].op) { > > @@ -826,6 +844,16 @@ int traceprobe_get_entry_data_size(struct trace_probe > > *tp) > > if (!earg) > > return 0; > > > > + /* > > + * earg->code[] array has an operation sequence which is run in > > + * the entry handler. > > + * The sequence stopped by FETCH_OP_END and each data stored in > > + * the entry data buffer by FETCH_OP_ST_EDATA. The FETCH_OP_ST_EDATA > > + * stores the data at the data buffer + its offset, and all data are > > + * "unsigned long" size. The offset must be increased when a data is > > + * stored. Thus we need to find the last FETCH_OP_ST_EDATA in the > > + * code array. > > + */ > > for (i = 0; i < earg->size; i++) { > > switch (earg->code[i].op) { > > case FETCH_OP_END: > -- Masami Hiramatsu (Google) <[email protected]>
