From: Daniel Bristot de Oliveira <bristot@kernel.org> Date: 2024-02-01 15:13:49
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.
Fixes: e88ed227f639 ("tracing/timerlat: Add user-space interface")
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
---
kernel/trace/trace_osnoise.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
@@ -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);
From: Steven Rostedt <rostedt@goodmis.org> Date: 2024-02-01 15:25:10
On Thu, 1 Feb 2024 16:13:39 +0100
Daniel Bristot de Oliveira [off-list ref] 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.
@@ -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);
From: Daniel Bristot de Oliveira <bristot@kernel.org> Date: 2024-02-01 15:27:26
On 2/1/24 16:25, Steven Rostedt wrote:
On Thu, 1 Feb 2024 16:13:39 +0100
Daniel Bristot de Oliveira [off-list ref] wrote:
quoted
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.
@@ -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);
On Thu, Feb 01, 2024 at 04:13:39PM +0100, Daniel Bristot de Oliveira 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.
What error exactly happens? Userspace, or the kernel crashes?
thanks,
greg k-h
From: Daniel Bristot de Oliveira <bristot@kernel.org> Date: 2024-02-01 16:02:59
On 2/1/24 16:44, Greg KH wrote:
On Thu, Feb 01, 2024 at 04:13:39PM +0100, Daniel Bristot de Oliveira wrote:
quoted
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.
What error exactly happens? Userspace, or the kernel crashes?
From: Steven Rostedt <rostedt@goodmis.org> Date: 2024-02-01 16:10:27
On Thu, 1 Feb 2024 17:02:56 +0100
Daniel Bristot de Oliveira [off-list ref] wrote:
On 2/1/24 16:44, Greg KH wrote:
quoted
On Thu, Feb 01, 2024 at 04:13:39PM +0100, Daniel Bristot de Oliveira wrote:
quoted
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.
What error exactly happens? Userspace, or the kernel crashes?
I usually include crash data like the above in the change log too. But
clean it up a little. At least strip the timestamps. (Note, I do that with
emacs, by cut and pasting the dump in a scratch buffer and then using
vertical cut with "Ctrl-X r-k").
-- Steve
On Thu, Feb 01, 2024 at 05:02:56PM +0100, Daniel Bristot de Oliveira wrote:
On 2/1/24 16:44, Greg KH wrote:
quoted
On Thu, Feb 01, 2024 at 04:13:39PM +0100, Daniel Bristot de Oliveira wrote:
quoted
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.
What error exactly happens? Userspace, or the kernel crashes?