Oh good, this commit was never pushed to Linus, so I can just fold it all together. Phew!
[ sorry for the noise ] -- Steve On Thu, 10 Sep 2026 21:40:08 -0400 Steven Rostedt <[email protected]> wrote: > From: Steven Rostedt <[email protected]> > > The commit 5ad289336148 ("tracing: Use a single array to represent tracer > options files") simplified the array that stores the individual tracer > options within the trace_array from being a multi array where each tracer > had its own array for its options to a single array where all the tracers > options were in a single array of the trace_array. > > Testing has shown why it was done with multiple arrays in the first place. > The single array use krealloc() to increase the size of the array when new > tracers are added. A krealloc can change the location of the array if it > needs more memory than the current location can provide. The files > associated with the arrays passed in the address of these elements, and > when the array changed, the elements became stale and no longer pointed to > the element that represented the option. > > Luckily, the reason this was done was also to check on open if the element > still existed or if it was removed when an instance directory was removed. > On opening a file with a stale element due to the array moving from a > krealloc(), it would not find a corresponding trace_array and the open > would return -ENODEV. > > Revert the code back to multi-array where only the outer array gets > resized and its elements point to the allocated arrays for each tracer. > > Update the trace_array_trace_options_get() function to search each tracer > array of the main array of the trace_array using a new helper function > tr_option_match(). This preserves the logic to find an existing > trace_array corresponding to the trace options descriptor and doesn't > suffer from krealloc() moving the elements around. > > Fixes: e8ea6a874574 ("tracing: Take trace_array reference when opening a > tracer options file") > Signed-off-by: Steven Rostedt <[email protected]> > --- > kernel/trace/trace.c | 63 +++++++++++++++++++++++++++++--------------- > kernel/trace/trace.h | 22 ++++++++++------ > 2 files changed, 56 insertions(+), 29 deletions(-) > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index 7cc3fac70216..e4a490d3d08c 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -7678,7 +7678,7 @@ static ssize_t > trace_options_read(struct file *filp, char __user *ubuf, size_t cnt, > loff_t *ppos) > { > - struct trace_options *topt = filp->private_data; > + struct trace_option_dentry *topt = filp->private_data; > char *buf; > > if (topt->flags->val & topt->opt->bit) > @@ -7693,7 +7693,7 @@ static ssize_t > trace_options_write(struct file *filp, const char __user *ubuf, size_t cnt, > loff_t *ppos) > { > - struct trace_options *topt = filp->private_data; > + struct trace_option_dentry *topt = filp->private_data; > unsigned long val; > int ret; > > @@ -7717,6 +7717,18 @@ trace_options_write(struct file *filp, const char > __user *ubuf, size_t cnt, > return cnt; > } > > +static bool tr_option_match(struct trace_array *tr, void *topt) > +{ > + for (int i = 0; i < tr->nr_topts; i++) { > + struct trace_options *tr_topts = &tr->topts[i]; > + > + if (topt >= (void *)&tr_topts->topts[0] && > + topt < (void *)&tr_topts->topts[tr_topts->nr_topts]) > + return true; > + } > + return false; > +} > + > /* > * The topt is the address of a trace_array->topts[] element that holds the > * the tracer options descriptor. But since the trace_array reference has not > @@ -7742,8 +7754,7 @@ static int trace_array_tracer_options_get(void *topt) > > guard(mutex)(&trace_types_lock); > list_for_each_entry(tr, &ftrace_trace_arrays, list) { > - if (topt >= (void *)&tr->topts[0] && > - topt < (void *)&tr->topts[tr->nr_topts]) > + if (tr_option_match(tr, topt)) > return __trace_array_get(tr); > } > return -ENODEV; > @@ -7751,7 +7762,7 @@ static int trace_array_tracer_options_get(void *topt) > > static int tracing_open_options(struct inode *inode, struct file *filp) > { > - struct trace_options *topt = inode->i_private; > + struct trace_option_dentry *topt = inode->i_private; > int ret; > > ret = trace_array_tracer_options_get(topt); > @@ -7764,7 +7775,7 @@ static int tracing_open_options(struct inode *inode, > struct file *filp) > > static int tracing_release_options(struct inode *inode, struct file *file) > { > - struct trace_options *topt = file->private_data; > + struct trace_option_dentry *topt = file->private_data; > > trace_array_put(topt->tr); > return 0; > @@ -7967,9 +7978,8 @@ static struct dentry *trace_options_init_dentry(struct > trace_array *tr) > > static void > create_trace_option_file(struct trace_array *tr, > - struct trace_options *topt, > + struct trace_option_dentry *topt, > struct tracer_flags *flags, > - struct tracer *tracer, > struct tracer_opt *opt) > { > struct dentry *t_options; > @@ -7981,7 +7991,6 @@ create_trace_option_file(struct trace_array *tr, > topt->flags = flags; > topt->opt = opt; > topt->tr = tr; > - topt->tracer = tracer; > > topt->entry = trace_create_file(opt->name, TRACE_MODE_WRITE, > t_options, topt, &trace_options_fops); > @@ -7991,9 +8000,10 @@ static int > create_trace_option_files(struct trace_array *tr, struct tracer *tracer, > struct tracer_flags *flags) > { > - struct trace_options *topts; > + struct trace_option_dentry *topts; > + struct trace_options *tr_topts; > struct tracer_opt *opts; > - int i, cnt; > + int cnt; > > if (!flags || !flags->opts) > return 0; > @@ -8003,22 +8013,30 @@ create_trace_option_files(struct trace_array *tr, > struct tracer *tracer, > for (cnt = 0; opts[cnt].name; cnt++) > ; > > - topts = krealloc_array(tr->topts, tr->nr_topts + cnt, > sizeof(*tr->topts), > - GFP_KERNEL); > + topts = kzalloc_objs(*topts, cnt + 1); > if (!topts) > + return 0; > + > + tr_topts = krealloc_array(tr->topts, tr->nr_topts + 1, > sizeof(*tr->topts), > + GFP_KERNEL); > + if (!tr_topts) { > + kfree(topts); > return -ENOMEM; > + } > > - tr->topts = topts; > + tr->topts = tr_topts; > + tr->topts[tr->nr_topts].tracer = tracer; > + tr->topts[tr->nr_topts].topts = topts; > + tr->topts[tr->nr_topts].nr_topts = cnt; > + tr->nr_topts++; > > - for (topts += tr->nr_topts, i = 0; i < cnt; topts++, i++) { > - create_trace_option_file(tr, topts, flags, tracer, > - &opts[i]); > - MEM_FAIL(topts->entry == NULL, > + for (cnt = 0; opts[cnt].name; cnt++) { > + create_trace_option_file(tr, &topts[cnt], flags, > + &opts[cnt]); > + MEM_FAIL(topts[cnt].entry == NULL, > "Failed to create trace option: %s", > - opts[i].name); > + opts[cnt].name); > } > - > - tr->nr_topts += cnt; > return 0; > } > > @@ -8880,6 +8898,9 @@ static int __remove_instance(struct trace_array *tr) > if (tr->flags & TRACE_ARRAY_FL_VMALLOC) > vfree((void *)tr->range_addr_start); > > + for (i = 0; i < tr->nr_topts; i++) { > + kfree(tr->topts[i].topts); > + } > kfree(tr->topts); > > free_cpumask_var(tr->pipe_cpumask); > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h > index fd0919a2c375..bd3c8f80300f 100644 > --- a/kernel/trace/trace.h > +++ b/kernel/trace/trace.h > @@ -224,6 +224,12 @@ struct array_buffer { > > #define TRACE_FLAGS_MAX_SIZE 64 > > +struct trace_options { > + struct tracer *tracer; > + struct trace_option_dentry *topts; > + int nr_topts; > +}; > + > struct trace_pid_list *trace_pid_list_alloc(void); > void trace_pid_list_free(struct trace_pid_list *pid_list); > bool trace_pid_list_is_set(struct trace_pid_list *pid_list, unsigned int > pid); > @@ -320,14 +326,6 @@ struct trace_module_delta { > long delta[]; > }; > > -struct trace_options { > - struct tracer_opt *opt; > - struct tracer_flags *flags; > - struct trace_array *tr; > - struct tracer *tracer; > - struct dentry *entry; > -}; > - > /* > * The trace array - an array of per-CPU trace arrays. This is the > * highest level data structure that individual tracers deal with. > @@ -608,6 +606,14 @@ struct tracer_flags { > /* Makes more easy to define a tracer opt */ > #define TRACER_OPT(s, b) .name = #s, .bit = b > > + > +struct trace_option_dentry { > + struct tracer_opt *opt; > + struct tracer_flags *flags; > + struct trace_array *tr; > + struct dentry *entry; > +}; > + > /** > * struct tracer - a specific tracer and its callbacks to interact with > tracefs > * @name: the name chosen to select it on the available_tracers file
