Re: [for-next][PATCH 05/10] tracing: Use hashtable.h for event_hash

[Steven Rostedt <rostedt@goodmis.org>]

When I ran my scripts to merge all my topic branches, this one has a
slight conflict with the ftrace branch that adds function arguments to
the function and function graph tracer. As this is mostly a clean up,
I'm going to remove this patch and save it for the next merge window.

Sasha, could you just rebase this on my for-next branch for the repo in
the MAINTAINERS file?

-- Steve


On Sun, 23 Mar 2025 08:29:38 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Sasha Levin <sashal@kernel.org>
> 
> Convert the event_hash array in trace_output.c to use the generic
> hashtable implementation from hashtable.h instead of the manually
> implemented hash table.
> 
> This simplifies the code and makes it more maintainable by using the
> standard hashtable API defined in hashtable.h.
> 
> Rename EVENT_HASHSIZE to EVENT_HASH_BITS to properly reflect its new
> meaning as the number of bits for the hashtable size.
> 
> Link: https://lore.kernel.org/20250319190545.3058319-1-sashal@kernel.org
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  kernel/trace/trace_output.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index 03d56f711ad1..3144d020e6c6 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -12,15 +12,16 @@
>  #include <linux/sched/clock.h>
>  #include <linux/sched/mm.h>
>  #include <linux/idr.h>
> +#include <linux/hashtable.h>
>  
>  #include "trace_output.h"
>  
> -/* must be a power of 2 */
> -#define EVENT_HASHSIZE	128
> +/* 2^7 = 128 */
> +#define EVENT_HASH_BITS 7
>  
>  DECLARE_RWSEM(trace_event_sem);
>  
> -static struct hlist_head event_hash[EVENT_HASHSIZE] __read_mostly;
> +static DEFINE_HASHTABLE(event_hash, EVENT_HASH_BITS);
>  
>  enum print_line_t trace_print_bputs_msg_only(struct trace_iterator *iter)
>  {
> @@ -694,11 +695,8 @@ int trace_print_lat_context(struct trace_iterator *iter)
>  struct trace_event *ftrace_find_event(int type)
>  {
>  	struct trace_event *event;
> -	unsigned key;
>  
> -	key = type & (EVENT_HASHSIZE - 1);
> -
> -	hlist_for_each_entry(event, &event_hash[key], node) {
> +	hash_for_each_possible(event_hash, event, node, type) {
>  		if (event->type == type)
>  			return event;
>  	}
> @@ -753,7 +751,6 @@ void trace_event_read_unlock(void)
>   */
>  int register_trace_event(struct trace_event *event)
>  {
> -	unsigned key;
>  	int ret = 0;
>  
>  	down_write(&trace_event_sem);
> @@ -786,9 +783,7 @@ int register_trace_event(struct trace_event *event)
>  	if (event->funcs->binary == NULL)
>  		event->funcs->binary = trace_nop_print;
>  
> -	key = event->type & (EVENT_HASHSIZE - 1);
> -
> -	hlist_add_head(&event->node, &event_hash[key]);
> +	hash_add(event_hash, &event->node, event->type);
>  
>  	ret = event->type;
>   out:
> @@ -803,7 +798,7 @@ EXPORT_SYMBOL_GPL(register_trace_event);
>   */
>  int __unregister_trace_event(struct trace_event *event)
>  {
> -	hlist_del(&event->node);
> +	hash_del(&event->node);
>  	free_trace_event_type(event->type);
>  	return 0;
>  }