tracing: Support poll on event hist file
Masami Hiramatsu (Google)
maintainerv4 cover tracing: Support poll on event hist file
Hi,
Here is the v4 patch to support polling on event 'hist' file. The previous version is here;
https://lore.kernel.org/all/172359427367.323666.6446548762874507863.stgit@devnote2/
This version uses getopt() in poll test command in [3/3] according to Shuah's comment in the previous thread.
Background
----------
There has been interest in allowing user programs to monitor kernel events in real time. Ftrace provides trace_pipe interface to wait on events in the ring buffer, but it is needed to wait until filling up a page with events in the ring buffer. We can also peek the trace file periodically, but that is inefficient way to monitor a randomely happening event.
Overview
--------
This patch set allows user to poll(or select, epoll) on event histogram interface. As you know each event has its own hist file which shows histograms generated by trigger action. So user can set a new hist trigger on any event you want to monitor, and poll on the hist file until it is updated.
There are 2 poll events are supported, POLLIN and POLLPRI. POLLIN means that there are any readable update on hist file and this event will be flashed only when you call read(). So, this is useful if you want to read the histogram periodically. The other POLLPRI event is for monitoring trace event. Like the POLLIN, this will be returned when the histogram is updated, but you don't need to read() the file and use poll() again.
Note that this waits for histogram update (not event arrival), thus you must set a histogram on the event at first.
Usage
-----
Here is an example usage:
----
TRACEFS=/sys/kernel/tracing
EVENT=$TRACEFS/events/sched/sched_process_free
# setup histogram trigger and enable event echo "hist:key=comm" >> $EVENT/trigger echo 1 > $EVENT/enable
# Wait for update
poll pri $EVENT/hist
# Event arrived.
echo "process free event is comming"
tail $TRACEFS/trace
----
The 'poll' command is in the selftest patch.
You can take this series also from here;
https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/log/?h=topic/event-hist-poll
Thank you,
---
Masami Hiramatsu (Google) (3):
tracing/hist: Add poll(POLLIN) support on hist file tracing/hist: Support POLLPRI event for poll on histogram selftests/tracing: Add hist poll() support test
include/linux/trace_events.h | 5 + kernel/trace/trace_events.c | 18 ++++ kernel/trace/trace_events_hist.c | 101 +++++++++++++++++++- tools/testing/selftests/ftrace/Makefile | 2 tools/testing/selftests/ftrace/poll.c | 74 +++++++++++++++ .../ftrace/test.d/trigger/trigger-hist-poll.tc | 74 +++++++++++++++ 6 files changed, 271 insertions(+), 3 deletions(-) create mode 100644 tools/testing/selftests/ftrace/poll.c create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-poll.tc
--
Masami Hiramatsu (Google) <[email protected]>
Masami Hiramatsu (Google)
maintainerPATCH v4 1/3 tracing/hist: Add poll(POLLIN) support on hist file
From: Masami Hiramatsu (Google) <[email protected]>
Add poll syscall support on the hist file. The Waiter will be waken up when the histogram is updated with POLLIN.
Currently, there is no way to wait for a specific event in userspace. So user needs to peek the trace periodicaly, or wait on trace_pipe. But that is not good idea to peek the trace for the event randomely happens. And trace_pipe is not coming back until a page is filled with events.
This allows user to wait for a specific events on hist file. User can set a histogram trigger on the event which they want to monitor. And poll() on its hist file. Since this poll() returns POLLIN, the next poll() will return soon unless you do read() on hist file.
NOTE: To read the hist file again, you must set the file offset to 0, but just for monitoring the event, you may not need to read the histogram.
--- include/linux/trace_events.h | 5 +++ kernel/trace/trace_events.c | 18 +++++++++ kernel/trace/trace_events_hist.c | 76 +++++++++++++++++++++++++++++++++++++- 3 files changed, 96 insertions(+), 3 deletions(-)
include/linux/trace_events.h +5 -0
@@ -663,6 +663,11 @@ struct trace_event_file {struct trace_subsystem_dir *system;struct list_head triggers; +#ifdef CONFIG_HIST_TRIGGERS +struct irq_work hist_work; +wait_queue_head_t hist_wq; +#endif +/* * 32 bit flags: * bit 0: enabled
kernel/trace/trace_events.c +18 -0
@@ -2972,6 +2972,20 @@ static bool event_in_systems(struct trace_event_call *call,return !*p ||isspace (*p) || *p ==',' ; } +#ifdef CONFIG_HIST_TRIGGERS +/* +* Wake up waiter on the hist_wq from irq_work because the hist trigger +* may happen in any context. +*/ +static void hist_event_irq_work (struct irq_work *work) +{ +struct trace_event_file *event_file; + + event_file = container_of(work, struct trace_event_file, hist_work); +wake_up_all (&event_file->hist_wq); +} +#endif +static struct trace_event_file *trace_create_new_event (struct trace_event_call *call,struct trace_array *tr)
@@ -3004,6 +3018,10 @@ trace_create_new_event(struct trace_event_call *call,INIT_LIST_HEAD (&file->triggers);list_add (&file->list, &tr->events);refcount_set (&file->ref,1 ); +#ifdef CONFIG_HIST_TRIGGERS +init_irq_work (&file->hist_work, hist_event_irq_work); +init_waitqueue_head (&file->hist_wq); +#endif return file; }
kernel/trace/trace_events_hist.c +73 -3
124 lines not on this page · raw message
Masami Hiramatsu (Google)
maintainerPATCH v4 2/3 tracing/hist: Support POLLPRI event for poll on histogram
In reply to Masami Hiramatsu (Google)From: Masami Hiramatsu (Google) <[email protected]>
Since POLLIN will not be flashed until read the hist file, user needs to repeat read() and poll() on hist for monitoring the event continuously. But the read() is somewhat redundant only for monitoring events.
This add POLLPRI poll event on hist, this event returns when a histogram is updated after open(), poll() or read(). Thus it is possible to wait next event without read().
--- kernel/trace/trace_events_hist.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)
kernel/trace/trace_events_hist.c +27 -2
@@ -5599,6 +5599,7 @@ static void hist_trigger_show(struct seq_file *m,struct hist_file_data {struct file *file;u64 last_read; +u64 last_act; };static u64 get_hist_hit_count (struct trace_event_file *event_file)
@@ -5636,6 +5637,11 @@ static int hist_show(struct seq_file *m, void *v)hist_trigger_show (m, data, n++); } hist_file->last_read =get_hist_hit_count (event_file); +/* +* Update last_act too so that poll()/POLLPRI can wait for the next +* event after any syscall on hist file. +*/ + hist_file->last_act = hist_file->last_read; out_unlock:mutex_unlock (&event_mutex);
@@ -5649,6 +5655,7 @@ static __poll_t event_hist_poll(struct file *file, struct poll_table_struct *waistruct seq_file *m = file->private_data;struct hist_file_data *hist_file = m->private;__poll_t ret =0 ; +u64 cnt;mutex_lock (&event_mutex);
@@ -5660,8 +5667,13 @@ static __poll_t event_hist_poll(struct file *file, struct poll_table_struct *waipoll_wait (file, &event_file->hist_wq, wait); -if (hist_file->last_read !=get_hist_hit_count (event_file)) - ret =EPOLLIN |EPOLLRDNORM ; + cnt =get_hist_hit_count (event_file); +if (hist_file->last_read != cnt) + ret |=EPOLLIN |EPOLLRDNORM ; +if (hist_file->last_act != cnt) { + hist_file->last_act = cnt; + ret |=EPOLLPRI ; + } out_unlock:mutex_unlock (&event_mutex);
@@ -5680,6 +5692,7 @@ static int event_hist_release(struct inode *inode, struct file *file)static int event_hist_open (struct inode *inode,struct file *file) { +struct trace_event_file *event_file;struct hist_file_data *hist_file;int ret;
@@ -5690,13 +5703,25 @@ static int event_hist_open(struct inode *inode, struct file *file) hist_file =kzalloc (sizeof (*hist_file),GFP_KERNEL );if (!hist_file)return -ENOMEM ; + +mutex_lock (&event_mutex); + event_file =event_file_data (file); +if (!event_file) { + ret = -ENODEV ; + goto out_unlock; + } + hist_file->file = file; + hist_file->last_act =get_hist_hit_count (event_file);/* Clear private_data to avoid warning in single_open() */ file->private_data =NULL ; ret =single_open (file, hist_show, hist_file); + +out_unlock:if (ret)kfree (hist_file); +mutex_unlock (&event_mutex);return ret; }
Masami Hiramatsu (Google)
maintainerPATCH v4 3/3 selftests/tracing: Add hist poll() support test
In reply to Masami Hiramatsu (Google)From: Masami Hiramatsu (Google) <[email protected]>
Add a testcase for poll() on hist file. This introduces a helper binary to the ftracetest, because there is no good way to reliably execute poll() on hist file.
---
Changes in v4:
- Use getopt() in poll.c (command options are changed)
- Update test code according to the new command options.
Changes in v2:
- Update poll command to support both of POLLIN and POLLPRI, and timeout.
- Identify unsupported stable kernel if poll-in returns soon.
- Test both of POLLIN and POLLPRI.
--- tools/testing/selftests/ftrace/Makefile | 2 + tools/testing/selftests/ftrace/poll.c | 74 ++++++++++++++++++++ .../ftrace/test.d/trigger/trigger-hist-poll.tc | 74 ++++++++++++++++++++ 3 files changed, 150 insertions(+) create mode 100644 tools/testing/selftests/ftrace/poll.c create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-poll.tc
tools/testing/selftests/ftrace/Makefile +2 -0
@@ -6,4 +6,6 @@ TEST_PROGS := ftracetest-ktap TEST_FILES := test.d settings EXTRA_CLEAN := $(OUTPUT)/logs/* +TEST_GEN_PROGS = poll + include ../lib.mk
tools/testing/selftests/ftrace/poll.c new file +74 -0
74 lines not on this page · raw message
tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-poll.tc new file +74 -0
74 lines not on this page · raw message
Re: [PATCH v4 3/3] selftests/tracing: Add hist poll() support test
Quoted 99 lines · Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) <[email protected]>
Add a testcase for
poll()on hist file. This introduces a helper binary to the ftracetest, because there is no good way to reliably executepoll()on hist file.Signed-off-by Masami Hiramatsu (Google) <[email protected]>---
Changes in v4:
- Usegetopt()inpoll.c(command options are changed)
- Update test code according to the new command options.
Changes in v2:
- Update poll command to support both of POLLIN and POLLPRI, and timeout.
- Identify unsupported stable kernel if poll-in returns soon.
- Test both of POLLIN and POLLPRI.--- tools/testing/selftests/ftrace/Makefile | 2 + tools/testing/selftests/ftrace/poll.c | 74 ++++++++++++++++++++ .../ftrace/test.d/trigger/trigger-hist-poll.tc | 74 ++++++++++++++++++++ 3 files changed, 150 insertions(+)create mode 100644
tools/testing/selftests/ftrace/poll.c
create mode 100644tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-poll.tctools/testing/selftests/ftrace/Makefile +2
-0@@ -6,4 +6,6 @@ TEST_PROGS := ftracetest-ktap TEST_FILES := test.d settings EXTRA_CLEAN := $(OUTPUT)/logs/* +TEST_GEN_PROGS = poll + include ../lib.mktools/testing/selftests/ftrace/poll.c new file +59
-0@@ -0,0 +1,74 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Simple poll on a file. +* +* Copyright (c) 2024 Google LLC. +*/ + +#include <errno.h> +#include <fcntl.h> +#include <poll.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> + +#define BUFSIZE 4096 + +/* +* Usage: +* poll [-I|-P] [-t timeout] FILE +*/ +int main (int argc,char *argv[]) +{ +struct pollfd pfd = {.events =POLLIN }; +char buf[BUFSIZE ]; +int timeout =-1 ; +int ret, opt; + +while ((opt =getopt (argc, argv,"IPt:" )) !=-1 ) { +switch (opt) { +case 'I' : + pfd.events =POLLIN ; +break ; +case 'P' : + pfd.events =POLLPRI ; +break ; +case 't' : + timeout =atoi (optarg); +break ; +default : +fprintf (stderr,"Usage: %s [-I|-P] [-t timeout] FILE\n" , + argv[0 ]); +return -1 ; + } + } +if (optind >= argc) { +fprintf (stderr,"Error: Polling file is not specified\n" ); +return -1 ; + } + + pfd.fd =open (argv[optind],O_RDONLY ); +if (pfd.fd <0 ) { +fprintf (stderr,"failed to open %s" , argv[optind]); +perror ("open" ); +return -1 ; + } + +/* Reset poll by read if POLLIN is specified. */ +if (pfd.events &POLLIN )
tools/testing/selftests/ftrace/poll.c line 69
Quoted 10 lines
tools/testing/selftests/ftrace/poll.c new file +10
-0+do {}while (read (pfd.fd, buf,BUFSIZE ) ==BUFSIZE ); + + ret =poll (&pfd,1 , timeout); +if (ret <0 && errno !=EINTR ) { +perror ("poll" ); +return -1 ; + } +close (pfd.fd); + +/* If timeout happned, return code is 0 */
Looks like you missed this one :) Otherwise looks good to me.
With this fixed
thanks,
-- Shuah
Masami Hiramatsu (Google)
maintainerRe: [PATCH v4 3/3] selftests/tracing: Add hist poll() support test
Quoted 104 lines · On Fri, 16 Aug 2024 08:04:41 -0600 Shuah Khan <[email protected]> wrote:
Quoted 103 lines · Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) <[email protected]>
Add a testcase for
poll()on hist file. This introduces a helper binary to the ftracetest, because there is no good way to reliably executepoll()on hist file.Signed-off-by Masami Hiramatsu (Google) <[email protected]>---
Changes in v4:
- Usegetopt()inpoll.c(command options are changed)
- Update test code according to the new command options.
Changes in v2:
- Update poll command to support both of POLLIN and POLLPRI, and timeout.
- Identify unsupported stable kernel if poll-in returns soon.
- Test both of POLLIN and POLLPRI.--- tools/testing/selftests/ftrace/Makefile | 2 + tools/testing/selftests/ftrace/poll.c | 74 ++++++++++++++++++++ .../ftrace/test.d/trigger/trigger-hist-poll.tc | 74 ++++++++++++++++++++ 3 files changed, 150 insertions(+)create mode 100644
tools/testing/selftests/ftrace/poll.c
create mode 100644tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-poll.tctools/testing/selftests/ftrace/Makefile +2
-0@@ -6,4 +6,6 @@ TEST_PROGS := ftracetest-ktap TEST_FILES := test.d settings EXTRA_CLEAN := $(OUTPUT)/logs/* +TEST_GEN_PROGS = poll + include ../lib.mktools/testing/selftests/ftrace/poll.c new file +63
-0@@ -0,0 +1,74 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Simple poll on a file. +* +* Copyright (c) 2024 Google LLC. +*/ + +#include <errno.h> +#include <fcntl.h> +#include <poll.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> + +#define BUFSIZE 4096 + +/* +* Usage: +* poll [-I|-P] [-t timeout] FILE +*/ +int main (int argc,char *argv[]) +{ +struct pollfd pfd = {.events =POLLIN }; +char buf[BUFSIZE ]; +int timeout =-1 ; +int ret, opt; + +while ((opt =getopt (argc, argv,"IPt:" )) !=-1 ) { +switch (opt) { +case 'I' : + pfd.events =POLLIN ; +break ; +case 'P' : + pfd.events =POLLPRI ; +break ; +case 't' : + timeout =atoi (optarg); +break ; +default : +fprintf (stderr,"Usage: %s [-I|-P] [-t timeout] FILE\n" , + argv[0 ]); +return -1 ; + } + } +if (optind >= argc) { +fprintf (stderr,"Error: Polling file is not specified\n" ); +return -1 ; + } + + pfd.fd =open (argv[optind],O_RDONLY ); +if (pfd.fd <0 ) { +fprintf (stderr,"failed to open %s" , argv[optind]); +perror ("open" ); +return -1 ; + } + +/* Reset poll by read if POLLIN is specified. */ +if (pfd.events &POLLIN ) +do {}while (read (pfd.fd, buf,BUFSIZE ) ==BUFSIZE ); + + ret =poll (&pfd,1 , timeout); +if (ret <0 && errno !=EINTR ) {
Quoted 9 lines
tools/testing/selftests/ftrace/poll.c line 69
Quoted 6 lines
tools/testing/selftests/ftrace/poll.c new file +6
-0+perror ("poll" ); +return -1 ; + } +close (pfd.fd); + +/* If timeout happned, return code is 0 */ Looks like you missed this one :) Otherwise looks good to me.
Oops, indeed.
Quoted 3 lines
With this fixed
Reviewed-by Shuah Khan <[email protected]>
Thanks!
Quoted 3 lines
thanks,
-- Shuah
Signature · 1 line
Masami Hiramatsu (Google) <[email protected]>
Masami Hiramatsu (Google)
maintainerRe: [PATCH v4 3/3] selftests/tracing: Add hist poll() support test
Quoted 116 lines · On Sun, 18 Aug 2024 00:06:20 +0900 Masami Hiramatsu (Google)…
Quoted 104 lines · On Fri, 16 Aug 2024 08:04:41 -0600 Shuah Khan <[email protected]> wrote:
Quoted 103 lines · Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) <[email protected]>
Add a testcase for
poll()on hist file. This introduces a helper binary to the ftracetest, because there is no good way to reliably executepoll()on hist file.Signed-off-by Masami Hiramatsu (Google) <[email protected]>---
Changes in v4:
- Usegetopt()inpoll.c(command options are changed)
- Update test code according to the new command options.
Changes in v2:
- Update poll command to support both of POLLIN and POLLPRI, and timeout.
- Identify unsupported stable kernel if poll-in returns soon.
- Test both of POLLIN and POLLPRI.--- tools/testing/selftests/ftrace/Makefile | 2 + tools/testing/selftests/ftrace/poll.c | 74 ++++++++++++++++++++ .../ftrace/test.d/trigger/trigger-hist-poll.tc | 74 ++++++++++++++++++++ 3 files changed, 150 insertions(+)create mode 100644
tools/testing/selftests/ftrace/poll.c
create mode 100644tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-poll.tctools/testing/selftests/ftrace/Makefile +2
-0@@ -6,4 +6,6 @@ TEST_PROGS := ftracetest-ktap TEST_FILES := test.d settings EXTRA_CLEAN := $(OUTPUT)/logs/* +TEST_GEN_PROGS = poll + include ../lib.mktools/testing/selftests/ftrace/poll.c new file +63
-0@@ -0,0 +1,74 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Simple poll on a file. +* +* Copyright (c) 2024 Google LLC. +*/ + +#include <errno.h> +#include <fcntl.h> +#include <poll.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> + +#define BUFSIZE 4096 + +/* +* Usage: +* poll [-I|-P] [-t timeout] FILE +*/ +int main (int argc,char *argv[]) +{ +struct pollfd pfd = {.events =POLLIN }; +char buf[BUFSIZE ]; +int timeout =-1 ; +int ret, opt; + +while ((opt =getopt (argc, argv,"IPt:" )) !=-1 ) { +switch (opt) { +case 'I' : + pfd.events =POLLIN ; +break ; +case 'P' : + pfd.events =POLLPRI ; +break ; +case 't' : + timeout =atoi (optarg); +break ; +default : +fprintf (stderr,"Usage: %s [-I|-P] [-t timeout] FILE\n" , + argv[0 ]); +return -1 ; + } + } +if (optind >= argc) { +fprintf (stderr,"Error: Polling file is not specified\n" ); +return -1 ; + } + + pfd.fd =open (argv[optind],O_RDONLY ); +if (pfd.fd <0 ) { +fprintf (stderr,"failed to open %s" , argv[optind]); +perror ("open" ); +return -1 ; + } + +/* Reset poll by read if POLLIN is specified. */ +if (pfd.events &POLLIN ) +do {}while (read (pfd.fd, buf,BUFSIZE ) ==BUFSIZE ); + + ret =poll (&pfd,1 , timeout); +if (ret <0 && errno !=EINTR ) {Quoted 9 lines
tools/testing/selftests/ftrace/poll.c line 69
Quoted 6 lines
tools/testing/selftests/ftrace/poll.c new file +6
-0+perror ("poll" ); +return -1 ; + } +close (pfd.fd); + +/* If timeout happned, return code is 0 */ Looks like you missed this one :) Otherwise looks good to me.
Quoted 1 line
Oops, indeed.
Hm, this comment is correct but misleadable. I meant that the poll() returns 0 if timeout happens.
-----
RETURN VALUE
On success, poll() returns a nonnegative value which is the number of elements in the pollfds whose revents fields have been set to a nonzero value (indicating an event or an error). A return value of zero indicates that the system call timed out before any file descriptors became read.
-----
But the comment should tell what this command does.
Thanks,
Quoted 14 lines
Quoted 4 lines
With this fixed
Reviewed-by Shuah Khan <[email protected]>Thanks!
Quoted 3 lines
thanks,
-- ShuahSignature · 1 line
Masami Hiramatsu (Google) <[email protected]>
Signature · 1 line
Masami Hiramatsu (Google) <[email protected]>