linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ftrace: properly merge notrace hash
@ 2025-04-07 18:07 Andy Chiu
  2025-04-07 20:09 ` Steven Rostedt
  0 siblings, 1 reply; 3+ messages in thread
From: Andy Chiu @ 2025-04-07 18:07 UTC (permalink / raw)
  To: rostedt, linux-kernel, linux-trace-kernel
  Cc: mark.rutland, mhiramat, mathieu.desnoyers, Andy Chiu, bjorn,
	puranjay12, alexghiti, paul.walmsley, greentime.hu, nick.hu,
	nylon.chen, eric.lin, vicent.chen, zong.li, yongxuan.wang,
	samuel.holland, olivia.chu, c2232430

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(&notrace_hash, ops->func_hash->filter_hash,
-				     subops->func_hash->filter_hash);
+		ret = intersect_hash(&notrace_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;
 		}
-- 
2.39.3 (Apple Git-145)


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] ftrace: properly merge notrace hash
  2025-04-07 18:07 [PATCH] ftrace: properly merge notrace hash Andy Chiu
@ 2025-04-07 20:09 ` Steven Rostedt
  2025-04-08 15:55   ` Andy Chiu
  0 siblings, 1 reply; 3+ messages in thread
From: Steven Rostedt @ 2025-04-07 20:09 UTC (permalink / raw)
  To: Andy Chiu
  Cc: linux-kernel, linux-trace-kernel, mark.rutland, mhiramat,
	mathieu.desnoyers, bjorn, puranjay12, alexghiti, paul.walmsley,
	greentime.hu, nick.hu, nylon.chen, eric.lin, vicent.chen, zong.li,
	yongxuan.wang, samuel.holland, olivia.chu, c2232430

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(&notrace_hash, ops->func_hash->filter_hash,
> -				     subops->func_hash->filter_hash);
> +		ret = intersect_hash(&notrace_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;
>  		}


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] ftrace: properly merge notrace hash
  2025-04-07 20:09 ` Steven Rostedt
@ 2025-04-08 15:55   ` Andy Chiu
  0 siblings, 0 replies; 3+ messages in thread
From: Andy Chiu @ 2025-04-08 15:55 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, linux-trace-kernel, mark.rutland, mhiramat,
	mathieu.desnoyers, bjorn, puranjay12, alexghiti, paul.walmsley,
	greentime.hu, nick.hu, nylon.chen, eric.lin, zong.li,
	yongxuan.wang, samuel.holland, olivia.chu, c2232430, vincent.chen

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(&notrace_hash, ops->func_hash->filter_hash,
> > -                                  subops->func_hash->filter_hash);
> > +             ret = intersect_hash(&notrace_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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-04-08 15:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-07 18:07 [PATCH] ftrace: properly merge notrace hash Andy Chiu
2025-04-07 20:09 ` Steven Rostedt
2025-04-08 15:55   ` Andy Chiu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).