Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3] trace-cmd: Stop recording when processes traced with -P exit
@ 2020-05-13 17:05 Bijan Tabatabai
  2020-05-13 20:16 ` Steven Rostedt
  0 siblings, 1 reply; 4+ messages in thread
From: Bijan Tabatabai @ 2020-05-13 17:05 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, Bijan Tabatabai

From: Bijan Tabatabai <bijan311@yahoo.com>

When the -F flag is used in trace-cmd record, trace-cmd stops recording
when the executable it is tracing terminates.
This patch makes the -P flag act similarly.

Signed-off-by: Bijan Tabatabai <bijan311@yahoo.com>
---
 This patch has changed to use the pidfd interface to determine if a process
 has ended.
 It also has changed to work with the patch "trace-cmd: Handle filtered PIDs per
 ftarce instance" (5502bcef0f962cda).
---
 tracecmd/include/trace-local.h |  2 +
 tracecmd/trace-record.c        | 84 ++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)

diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index 4c6a63d..5d58550 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -215,9 +215,11 @@ struct buffer_instance {
 
 	struct opt_list		*options;
 	struct filter_pids	*filter_pids;
+	struct filter_pids	*process_pids;
 	char			*common_pid_filter;
 	int			nr_filter_pids;
 	int			len_filter_pids;
+	int			nr_process_pids;
 	bool			ptrace_child;
 
 	int			have_set_event_pid;
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index d0619ba..51e9103 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -16,6 +16,7 @@
 #include <sys/time.h>
 #include <sys/wait.h>
 #include <sys/socket.h>
+#include <sys/syscall.h>
 #include <sys/utsname.h>
 #ifndef NO_PTRACE
 #include <sys/ptrace.h>
@@ -33,6 +34,7 @@
 #include <errno.h>
 #include <limits.h>
 #include <libgen.h>
+#include <poll.h>
 #include <pwd.h>
 #include <grp.h>
 #ifdef VSOCK
@@ -1230,6 +1232,81 @@ static pid_t trace_waitpid(enum trace_type type, pid_t pid, int *status, int opt
 			trace_stream_read(pids, recorder_threads, &tv);
 	} while (1);
 }
+
+#ifndef __NR_pidfd_open
+#define __NR_pidfd_open 434
+#endif
+
+static int pidfd_open(pid_t pid, unsigned int flags) {
+	return syscall(__NR_pidfd_open, pid, flags);
+}
+
+static int trace_waitpidfd(id_t pidfd) {
+	struct pollfd pollfd;
+
+	pollfd.fd = pidfd;
+	pollfd.events = POLLIN;
+
+	while (!finished) {
+		int ret = poll(&pollfd, 1, -1);
+		/* If waitid was interrupted, keep waiting */
+		if (ret < 0 && errno == EINTR)
+			continue;
+		else if (ret < 0)
+			return 1;
+		else
+			break;
+	}
+
+	return 0;
+}
+
+static int trace_wait_for_processes(struct buffer_instance *instance) {
+	int ret = 0;
+	int nr_fds = 0;
+	int i;
+	int *pidfds;
+	struct filter_pids *pid;
+
+	pidfds = malloc(sizeof(int) * instance->nr_process_pids);
+	if (!pidfds)
+		return 1;
+
+	for (pid = instance->process_pids;
+	     pid && instance->nr_process_pids;
+	     pid = pid->next) {
+		if (pid->exclude) {
+			instance->nr_process_pids--;
+			continue;
+		}
+		pidfds[nr_fds] = pidfd_open(pid->pid, 0);
+
+		/* If the pid doesn't exist, the process has probably exited */
+		if (pidfds[nr_fds] < 0 && errno == ESRCH) {
+			instance->nr_process_pids--;
+			continue;
+		} else if (pidfds[nr_fds] < 0) {
+			ret = 1;
+			goto out;
+		}
+
+		nr_fds++;
+		instance->nr_process_pids--;
+	}
+
+	for (i = 0; i < nr_fds; i++) {
+		if (trace_waitpidfd(pidfds[i])) {
+			ret = 1;
+			goto out;
+		}
+	}
+
+out:
+	for (i = 0; i < nr_fds; i++)
+		close(pidfds[i]);
+	free(pidfds);
+	return ret;
+}
 #ifndef NO_PTRACE
 
 /**
@@ -5866,7 +5943,9 @@ static void parse_record_options(int argc,
 				fpids_count += add_filter_pid(ctx->instance,
 							      atoi(pid), 0);
 				pid = strtok_r(NULL, ",", &sav);
+				ctx->instance->nr_process_pids++;
 			}
+			ctx->instance->process_pids = ctx->instance->filter_pids;
 			free(pids);
 			break;
 		case 'c':
@@ -6361,6 +6440,11 @@ static void record_trace(int argc, char **argv,
 		}
 		/* sleep till we are woken with Ctrl^C */
 		printf("Hit Ctrl^C to stop recording\n");
+		for_all_instances(instance) {
+			if (instance->nr_process_pids
+			    && !trace_wait_for_processes(instance))
+				finished = 1;
+		}
 		while (!finished)
 			trace_or_sleep(type, pwait);
 	}
-- 
2.25.1


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

* Re: [PATCH v3] trace-cmd: Stop recording when processes traced with -P exit
  2020-05-13 17:05 [PATCH v3] trace-cmd: Stop recording when processes traced with -P exit Bijan Tabatabai
@ 2020-05-13 20:16 ` Steven Rostedt
  2020-05-14  1:25   ` Bijan Tabatabai
  0 siblings, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2020-05-13 20:16 UTC (permalink / raw)
  To: Bijan Tabatabai; +Cc: linux-trace-devel, Bijan Tabatabai

On Wed, 13 May 2020 12:05:29 -0500
Bijan Tabatabai <bijan311@gmail.com> wrote:

> From: Bijan Tabatabai <bijan311@yahoo.com>
> 
> When the -F flag is used in trace-cmd record, trace-cmd stops recording
> when the executable it is tracing terminates.
> This patch makes the -P flag act similarly.
> 
> Signed-off-by: Bijan Tabatabai <bijan311@yahoo.com>
> ---
>  This patch has changed to use the pidfd interface to determine if a process
>  has ended.
>  It also has changed to work with the patch "trace-cmd: Handle filtered PIDs per
>  ftarce instance" (5502bcef0f962cda).
> ---
>  tracecmd/include/trace-local.h |  2 +
>  tracecmd/trace-record.c        | 84 ++++++++++++++++++++++++++++++++++
>  2 files changed, 86 insertions(+)
> 

Nice!

Although I just tried it out and noticed that if I run the following
command:

 # sleep 20& sleep 50&
[1] 2216
[2] 2218
 # trace-cmd record -e sched -P 2216 -B foo -e all -P 2218

It will exit out after the first sleep (20 second) and before the second is
done.

It appears to exit as soon as the first process is finished. We should want
to wait till the last one is finished.

-- Steve

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

* Re: [PATCH v3] trace-cmd: Stop recording when processes traced with -P exit
  2020-05-13 20:16 ` Steven Rostedt
@ 2020-05-14  1:25   ` Bijan Tabatabai
  2020-05-14  2:23     ` Steven Rostedt
  0 siblings, 1 reply; 4+ messages in thread
From: Bijan Tabatabai @ 2020-05-14  1:25 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-trace-devel, Bijan Tabatabai

I just tried that and discovered an interesting problem.
After the first sleep ended, my computer completely locked up, and
after a few minutes I decided to reset my computer.
Then after rebooting, I checked out the master branch, recompiled, and
tried running the same set of commands again.
This time, my computer locked up shortly after I pressed Ctrl+C after
both sleeps exited.
I am running Ubuntu 20.04 with Ubuntu's version of the 5.4 kernel.

Is this a bug that has been reported before? If not, what is the
process for "officially" reporting it?

Thank,
Bijan

On Wed, May 13, 2020 at 3:16 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 13 May 2020 12:05:29 -0500
> Bijan Tabatabai <bijan311@gmail.com> wrote:
>
> > From: Bijan Tabatabai <bijan311@yahoo.com>
> >
> > When the -F flag is used in trace-cmd record, trace-cmd stops recording
> > when the executable it is tracing terminates.
> > This patch makes the -P flag act similarly.
> >
> > Signed-off-by: Bijan Tabatabai <bijan311@yahoo.com>
> > ---
> >  This patch has changed to use the pidfd interface to determine if a process
> >  has ended.
> >  It also has changed to work with the patch "trace-cmd: Handle filtered PIDs per
> >  ftarce instance" (5502bcef0f962cda).
> > ---
> >  tracecmd/include/trace-local.h |  2 +
> >  tracecmd/trace-record.c        | 84 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 86 insertions(+)
> >
>
> Nice!
>
> Although I just tried it out and noticed that if I run the following
> command:
>
>  # sleep 20& sleep 50&
> [1] 2216
> [2] 2218
>  # trace-cmd record -e sched -P 2216 -B foo -e all -P 2218
>
> It will exit out after the first sleep (20 second) and before the second is
> done.
>
> It appears to exit as soon as the first process is finished. We should want
> to wait till the last one is finished.
>
> -- Steve

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

* Re: [PATCH v3] trace-cmd: Stop recording when processes traced with -P exit
  2020-05-14  1:25   ` Bijan Tabatabai
@ 2020-05-14  2:23     ` Steven Rostedt
  0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2020-05-14  2:23 UTC (permalink / raw)
  To: Bijan Tabatabai; +Cc: linux-trace-devel, Bijan Tabatabai

On Wed, 13 May 2020 20:25:46 -0500
Bijan Tabatabai <bijan311@gmail.com> wrote:

> I just tried that and discovered an interesting problem.
> After the first sleep ended, my computer completely locked up, and
> after a few minutes I decided to reset my computer.
> Then after rebooting, I checked out the master branch, recompiled, and
> tried running the same set of commands again.
> This time, my computer locked up shortly after I pressed Ctrl+C after
> both sleeps exited.
> I am running Ubuntu 20.04 with Ubuntu's version of the 5.4 kernel.
> 
> Is this a bug that has been reported before? If not, what is the
> process for "officially" reporting it?
> 

It's a kernel bug, and we just hit it a couple of weeks ago. The fix is here:

 https://lore.kernel.org/r/20200507173929.118079761@goodmis.org

It has been already added to Linus's tree and is filtering into the
stable branches (should soon be in your Ubuntu kernel).

The discussion around it is here (with the first attempted fix).

 https://lore.kernel.org/r/20200507173929.118079761@goodmis.org

The above is really just a work around, and the real fix is being
proposed and discussed here.

 https://lore.kernel.org/r/20200513152137.32426-1-joro@8bytes.org

-- Steve

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 17:05 [PATCH v3] trace-cmd: Stop recording when processes traced with -P exit Bijan Tabatabai
2020-05-13 20:16 ` Steven Rostedt
2020-05-14  1:25   ` Bijan Tabatabai
2020-05-14  2:23     ` Steven Rostedt

Linux-Trace-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-trace-devel/0 linux-trace-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-trace-devel linux-trace-devel/ https://lore.kernel.org/linux-trace-devel \
		linux-trace-devel@vger.kernel.org
	public-inbox-index linux-trace-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-trace-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git