| Message ID | 20250407180745.42848-1-andybnac@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ftrace: properly merge notrace hash | expand |
On Tue, 8 Apr 2025 02:07:44 +0800 Andy Chiu <andybnac@gmail.com> wrote: > The global notrace hash should be jointly decided by the intersection of > each subops's notrace hash, but not the filter hash. > > Fixes: 5fccc7552ccb ("ftrace: Add subops logic to allow one ops to manage many") > Signed-off-by: Andy Chiu <andybnac@gmail.com> > --- > kernel/trace/ftrace.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 1a48aedb5255..ee662f380b61 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -3526,18 +3526,17 @@ int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int > ftrace_hash_empty(subops->func_hash->notrace_hash)) { > notrace_hash = EMPTY_HASH; > } else { > - size_bits = max(ops->func_hash->filter_hash->size_bits, > - subops->func_hash->filter_hash->size_bits); > + size_bits = max(ops->func_hash->notrace_hash->size_bits, > + subops->func_hash->notrace_hash->size_bits); > notrace_hash = alloc_ftrace_hash(size_bits); > if (!notrace_hash) { > - free_ftrace_hash(filter_hash); > + free_ftrace_hash(notrace_hash); > return -ENOMEM; > } > > - ret = intersect_hash(¬race_hash, ops->func_hash->filter_hash, > - subops->func_hash->filter_hash); > + ret = intersect_hash(¬race_hash, ops->func_hash->notrace_hash, > + subops->func_hash->notrace_hash); Thanks for catching this. > if (ret < 0) { > - free_ftrace_hash(filter_hash); The filter_hash still needs to be freed, as it could have been allocated in the previous if statement and never used (both the filter_hash and notrace_hash get used at the end of the function via ftrace_update_ops(). Care to send a v2? -- Steve > free_ftrace_hash(notrace_hash); > return ret; > }
Steven Rostedt <rostedt@goodmis.org> 於 2025年4月8日 週二 上午4:08寫道： > > On Tue, 8 Apr 2025 02:07:44 +0800 > Andy Chiu <andybnac@gmail.com> wrote: > > > The global notrace hash should be jointly decided by the intersection of > > each subops's notrace hash, but not the filter hash. > > > > Fixes: 5fccc7552ccb ("ftrace: Add subops logic to allow one ops to manage many") > > Signed-off-by: Andy Chiu <andybnac@gmail.com> > > --- > > kernel/trace/ftrace.c | 11 +++++------ > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > > index 1a48aedb5255..ee662f380b61 100644 > > --- a/kernel/trace/ftrace.c > > +++ b/kernel/trace/ftrace.c > > @@ -3526,18 +3526,17 @@ int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int > > ftrace_hash_empty(subops->func_hash->notrace_hash)) { > > notrace_hash = EMPTY_HASH; > > } else { > > - size_bits = max(ops->func_hash->filter_hash->size_bits, > > - subops->func_hash->filter_hash->size_bits); > > + size_bits = max(ops->func_hash->notrace_hash->size_bits, > > + subops->func_hash->notrace_hash->size_bits); > > notrace_hash = alloc_ftrace_hash(size_bits); > > if (!notrace_hash) { > > - free_ftrace_hash(filter_hash); > > + free_ftrace_hash(notrace_hash); > > return -ENOMEM; > > } > > > > - ret = intersect_hash(¬race_hash, ops->func_hash->filter_hash, > > - subops->func_hash->filter_hash); > > + ret = intersect_hash(¬race_hash, ops->func_hash->notrace_hash, > > + subops->func_hash->notrace_hash); > > Thanks for catching this. > > > > if (ret < 0) { > > - free_ftrace_hash(filter_hash); > > The filter_hash still needs to be freed, as it could have been allocated in > the previous if statement and never used (both the filter_hash and > notrace_hash get used at the end of the function via ftrace_update_ops(). > > Care to send a v2? Yes, thanks for reminding! Let me send a v2 with filter_hash freed on this condition. Regards, Andy
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 1a48aedb5255..ee662f380b61 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -3526,18 +3526,17 @@ int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int ftrace_hash_empty(subops->func_hash->notrace_hash)) { notrace_hash = EMPTY_HASH; } else { - size_bits = max(ops->func_hash->filter_hash->size_bits, - subops->func_hash->filter_hash->size_bits); + size_bits = max(ops->func_hash->notrace_hash->size_bits, + subops->func_hash->notrace_hash->size_bits); notrace_hash = alloc_ftrace_hash(size_bits); if (!notrace_hash) { - free_ftrace_hash(filter_hash); + free_ftrace_hash(notrace_hash); return -ENOMEM; } - ret = intersect_hash(¬race_hash, ops->func_hash->filter_hash, - subops->func_hash->filter_hash); + ret = intersect_hash(¬race_hash, ops->func_hash->notrace_hash, + subops->func_hash->notrace_hash); if (ret < 0) { - free_ftrace_hash(filter_hash); free_ftrace_hash(notrace_hash); return ret; }
The global notrace hash should be jointly decided by the intersection of each subops's notrace hash, but not the filter hash. Fixes: 5fccc7552ccb ("ftrace: Add subops logic to allow one ops to manage many") Signed-off-by: Andy Chiu <andybnac@gmail.com> --- kernel/trace/ftrace.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)