linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v3] tracing: add testable specifications for event_enable_write/read
@ 2025-06-30 14:20 Gabriele Paoloni
  2025-07-02 16:16 ` Gabriele Paoloni
  0 siblings, 1 reply; 2+ messages in thread
From: Gabriele Paoloni @ 2025-06-30 14:20 UTC (permalink / raw)
  To: rostedt, mhiramat, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel
  Cc: Gabriele Paoloni

This patch implements the documentation of event_enable_write and
event_enable_read in the form of testable function's expectations.

Signed-off-by: Gabriele Paoloni <gpaoloni@redhat.com>
---
Following the discussion in v1 (see [1]), v3 adds improvements with
the "Context" specifications and the definition of the assumptions
of use for the correct invocation of the documented functions.

[1] https://lore.kernel.org/linux-trace-kernel/20250612104349.5047-1-gpaoloni@redhat.com/

v3 has been sent as in v2 there is a mistake in the return value
specifications of event_enable_read
---
 kernel/trace/trace_events.c | 80 +++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 120531268abf..aa9992b0f43c 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1771,6 +1771,51 @@ static void p_stop(struct seq_file *m, void *p)
 	mutex_unlock(&event_mutex);
 }
 
+/**
+ * event_enable_read - read from a trace event file to retrieve its status.
+ * @filp: file pointer associated with the target trace event;
+ * @ubuf: user space buffer where the event status is copied to;
+ * @cnt: number of bytes to be copied to the user space buffer;
+ * @ppos: the current position in the buffer.
+ *
+ * This is a way for user space executables to retrieve the status of a
+ * specific event
+ *
+ * Function's expectations:
+ * - This function shall lock the global event_mutex before performing any
+ *   operation on the target event file and unlock it after all operations on
+ *   the target event file have completed;
+ * - This function shall retrieve the status flags from the file associated
+ *   with the target event;
+ * - This function shall format the string to report the event status to user
+ *   space:
+ *   - The first character of the string to be copied to user space shall be
+ *     set to "1" if the enabled flag is set AND the soft_disabled flag is not
+ *     set, else it shall be set to "0";
+ *   - The second character of the string to be copied to user space shall be
+ *     set to "*" if either the soft_disabled flag or the soft_mode flag is
+ *     set, else it shall be set to "\n";
+ *   - The third character of the string to be copied to user space shall b
+ *     set to "\n" if either the soft_disabled flag or the soft_mode flag is
+ *     set, else it shall be set to "0";
+ *   - Any other character in the string to be copied to user space shall be
+ *     set to "0";
+ * - This function shall invoke simple_read_from_buffer() to perform the copy
+ *   of the kernel space string to ubuf.
+ *
+ * Assumptions of Use:
+ * - The caller shall  pass cnt equal or greater than the length of the string
+ *   to be copied to user space;
+ * - Any read operation on a file descriptor, unless it is the first operation
+ *   following a trace event file open, shall be preceded by an lseek
+ *   invocation to reposition the file offset to zero.
+ *
+ * Context: process context, locks and unlocks event_mutex.
+ *
+ * Return: the number of copied bytes on success, -ENODEV if the event file
+ * cannot be retrieved from the input filp, any error returned by
+ * simple_read_from_buffer.
+ */
 static ssize_t
 event_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
 		  loff_t *ppos)
@@ -1801,6 +1846,41 @@ event_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
 	return simple_read_from_buffer(ubuf, cnt, ppos, buf, strlen(buf));
 }
 
+/**
+ * event_enable_write - write to a trace event file to enable/disable it.
+ * @filp: file pointer associated with the target trace event;
+ * @ubuf: user space buffer where the enable/disable value is copied from;
+ * @cnt: number of bytes to be copied from the user space buffer;
+ * @ppos: the current position in the buffer.
+ *
+ * This is a way for user space executables to enable or disable event
+ * recording.
+ *
+ * Function's expectations:
+ * - This function shall copy cnt bytes from the input ubuf buffer to a kernel
+ *   space buffer and shall convert the string within the kernel space buffer
+ *   into a decimal base format number;
+ * - This function shall lock the global event_mutex before performing any
+ *   operation on the target event file and unlock it after all operations on
+ *   the target event file have completed;
+ * - This function shall check the size of the per-cpu ring-buffers used for
+ *   the event trace data and, if smaller than TRACE_BUF_SIZE_DEFAULT, expand
+ *   them to TRACE_BUF_SIZE_DEFAULT bytes (sizes larger than
+ *   TRACE_BUF_SIZE_DEFAULT are not allowed);
+ * - This function shall invoke ftrace_event_enable_disable to enable or
+ *   disable the target trace event according to the value read from user space
+ *   (0 - disable, 1 - enable);
+ * - This function shall increase the file position pointed by ppos by the
+ *   number of bytes specified by cnt;
+ *
+ * Context: process context, locks and unlocks event_mutex.
+ *
+ * Return: the number of written bytes on success, any error returned by
+ * kstrtoul_from_user, -ENODEV if the event file cannot be retrieved from the
+ * input filp, any error returned by tracing_update_buffers, any error returned
+ * by ftrace_event_enable_disable, -EINVAL if the value copied from the user
+ * space ubuf is different from 0 or 1.
+ */
 static ssize_t
 event_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
 		   loff_t *ppos)
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [RFC PATCH v3] tracing: add testable specifications for event_enable_write/read
  2025-06-30 14:20 [RFC PATCH v3] tracing: add testable specifications for event_enable_write/read Gabriele Paoloni
@ 2025-07-02 16:16 ` Gabriele Paoloni
  0 siblings, 0 replies; 2+ messages in thread
From: Gabriele Paoloni @ 2025-07-02 16:16 UTC (permalink / raw)
  To: rostedt, mhiramat, mathieu.desnoyers, linux-kernel,
	linux-trace-kernel

On Mon, Jun 30, 2025 at 4:20 PM Gabriele Paoloni <gpaoloni@redhat.com> wrote:
>
> This patch implements the documentation of event_enable_write and
> event_enable_read in the form of testable function's expectations.
>
> Signed-off-by: Gabriele Paoloni <gpaoloni@redhat.com>
> ---
> Following the discussion in v1 (see [1]), v3 adds improvements with
> the "Context" specifications and the definition of the assumptions
> of use for the correct invocation of the documented functions.
>
> [1] https://lore.kernel.org/linux-trace-kernel/20250612104349.5047-1-gpaoloni@redhat.com/
>
> v3 has been sent as in v2 there is a mistake in the return value
> specifications of event_enable_read

Following the detailed review and suggestions from Steven here [ref]
the review of this patch can probably be skipped and I'll work directly
on v4

[ref]: https://lore.kernel.org/linux-trace-kernel/CA+wEVJbjhLmA4ZR5w7s6QDCfjET=Pf2J9PsFhC2wdO1nQ5YY+A@mail.gmail.com/T/#ma46663274411b08c14f380a69363dc6225b09a45

Thanks
Gab



> ---
>  kernel/trace/trace_events.c | 80 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 120531268abf..aa9992b0f43c 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1771,6 +1771,51 @@ static void p_stop(struct seq_file *m, void *p)
>         mutex_unlock(&event_mutex);
>  }
>
> +/**
> + * event_enable_read - read from a trace event file to retrieve its status.
> + * @filp: file pointer associated with the target trace event;
> + * @ubuf: user space buffer where the event status is copied to;
> + * @cnt: number of bytes to be copied to the user space buffer;
> + * @ppos: the current position in the buffer.
> + *
> + * This is a way for user space executables to retrieve the status of a
> + * specific event
> + *
> + * Function's expectations:
> + * - This function shall lock the global event_mutex before performing any
> + *   operation on the target event file and unlock it after all operations on
> + *   the target event file have completed;
> + * - This function shall retrieve the status flags from the file associated
> + *   with the target event;
> + * - This function shall format the string to report the event status to user
> + *   space:
> + *   - The first character of the string to be copied to user space shall be
> + *     set to "1" if the enabled flag is set AND the soft_disabled flag is not
> + *     set, else it shall be set to "0";
> + *   - The second character of the string to be copied to user space shall be
> + *     set to "*" if either the soft_disabled flag or the soft_mode flag is
> + *     set, else it shall be set to "\n";
> + *   - The third character of the string to be copied to user space shall b
> + *     set to "\n" if either the soft_disabled flag or the soft_mode flag is
> + *     set, else it shall be set to "0";
> + *   - Any other character in the string to be copied to user space shall be
> + *     set to "0";
> + * - This function shall invoke simple_read_from_buffer() to perform the copy
> + *   of the kernel space string to ubuf.
> + *
> + * Assumptions of Use:
> + * - The caller shall  pass cnt equal or greater than the length of the string
> + *   to be copied to user space;
> + * - Any read operation on a file descriptor, unless it is the first operation
> + *   following a trace event file open, shall be preceded by an lseek
> + *   invocation to reposition the file offset to zero.
> + *
> + * Context: process context, locks and unlocks event_mutex.
> + *
> + * Return: the number of copied bytes on success, -ENODEV if the event file
> + * cannot be retrieved from the input filp, any error returned by
> + * simple_read_from_buffer.
> + */
>  static ssize_t
>  event_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
>                   loff_t *ppos)
> @@ -1801,6 +1846,41 @@ event_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
>         return simple_read_from_buffer(ubuf, cnt, ppos, buf, strlen(buf));
>  }
>
> +/**
> + * event_enable_write - write to a trace event file to enable/disable it.
> + * @filp: file pointer associated with the target trace event;
> + * @ubuf: user space buffer where the enable/disable value is copied from;
> + * @cnt: number of bytes to be copied from the user space buffer;
> + * @ppos: the current position in the buffer.
> + *
> + * This is a way for user space executables to enable or disable event
> + * recording.
> + *
> + * Function's expectations:
> + * - This function shall copy cnt bytes from the input ubuf buffer to a kernel
> + *   space buffer and shall convert the string within the kernel space buffer
> + *   into a decimal base format number;
> + * - This function shall lock the global event_mutex before performing any
> + *   operation on the target event file and unlock it after all operations on
> + *   the target event file have completed;
> + * - This function shall check the size of the per-cpu ring-buffers used for
> + *   the event trace data and, if smaller than TRACE_BUF_SIZE_DEFAULT, expand
> + *   them to TRACE_BUF_SIZE_DEFAULT bytes (sizes larger than
> + *   TRACE_BUF_SIZE_DEFAULT are not allowed);
> + * - This function shall invoke ftrace_event_enable_disable to enable or
> + *   disable the target trace event according to the value read from user space
> + *   (0 - disable, 1 - enable);
> + * - This function shall increase the file position pointed by ppos by the
> + *   number of bytes specified by cnt;
> + *
> + * Context: process context, locks and unlocks event_mutex.
> + *
> + * Return: the number of written bytes on success, any error returned by
> + * kstrtoul_from_user, -ENODEV if the event file cannot be retrieved from the
> + * input filp, any error returned by tracing_update_buffers, any error returned
> + * by ftrace_event_enable_disable, -EINVAL if the value copied from the user
> + * space ubuf is different from 0 or 1.
> + */
>  static ssize_t
>  event_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
>                    loff_t *ppos)
> --
> 2.48.1
>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-07-02 16:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-30 14:20 [RFC PATCH v3] tracing: add testable specifications for event_enable_write/read Gabriele Paoloni
2025-07-02 16:16 ` Gabriele Paoloni

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).