Received-SPF: pass (google.com: domain of linux-kernel+bounces-48395-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Thu, 1 Feb 2024 10:25:26 -0500
From: Steven Rostedt <rostedt@goodmis.org>
To: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@efficios.com>, linux-kernel@vger.kernel.org,
 linux-trace-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] tracing/timerlat: Move hrtimer_init to timerlat_fd
 open()
Message-ID: <20240201102526.755de868@gandalf.local.home>
In-Reply-To: <7324dd3fc0035658c99b825204a66049389c56e3.1706798888.git.bristot@kernel.org>
References: <7324dd3fc0035658c99b825204a66049389c56e3.1706798888.git.bristot@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Thu,  1 Feb 2024 16:13:39 +0100
Daniel Bristot de Oliveira <bristot@kernel.org> wrote:

> Currently, the timerlat's hrtimer is initialized at the first read of
> timerlat_fd, and destroyed at close(). It works, but it causes an error
> if the user program open() and close() the file without reading.
> 
> Move hrtimer_init to timerlat_fd open() to avoid this problem.
> 
> No functional changes.

It can't be fixing something and not have any functional changes.

No functional changes means the code is restructured but the resulting
assembly would be the same.

Like moving functions around in a file so that you don't need extra
prototype declarations.

Please only add "No functional changes" if the function's assembly would be
the same.

> 
> Fixes: e88ed227f639 ("tracing/timerlat: Add user-space interface")

With a fixes tag, I'm assuming his should go into v6.8 with a Cc stable?

-- Steve


> Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
> ---
>  kernel/trace/trace_osnoise.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
> index bd0d01d00fb9..a8e28f9b9271 100644
> --- a/kernel/trace/trace_osnoise.c
> +++ b/kernel/trace/trace_osnoise.c
> @@ -2444,6 +2444,9 @@ static int timerlat_fd_open(struct inode *inode, struct file *file)
>  	tlat = this_cpu_tmr_var();
>  	tlat->count = 0;
>  
> +	hrtimer_init(&tlat->timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_PINNED_HARD);
> +	tlat->timer.function = timerlat_irq;
> +
>  	migrate_enable();
>  	return 0;
>  };
> @@ -2526,9 +2529,6 @@ timerlat_fd_read(struct file *file, char __user *ubuf, size_t count,
>  		tlat->tracing_thread = false;
>  		tlat->kthread = current;
>  
> -		hrtimer_init(&tlat->timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_PINNED_HARD);
> -		tlat->timer.function = timerlat_irq;
> -
>  		/* Annotate now to drift new period */
>  		tlat->abs_period = hrtimer_cb_get_time(&tlat->timer);
>