On Wed, 23 Jun 2021 16:02:00 +0300
Yordan Karadzhov <y.karadz@xxxxxxxxx> wrote:
> > I'm thinking we should invert the above. That is, have a stop instead of
> > "keep_going", where it is false by default (when the instance is created).
> >
> > That's because, if we start here, and the SIGINT comes in before we get
> > here, it keep_going might get set to false, and missed.
>
> I intentionally tried to avoid having any dependency of the initialization value of the "keep_going" flag. We have to
> consider the case when the user creates one instance and then calls this function multiple times in a row.
If anything then, make it the first thing that gets done, and not just
before the loop.
That is:
+int tracefs_trace_pipe_stream(int fd, struct tracefs_instance *instance,
+ int flags)
+{
+ int *keep_going = instance ? &instance->pipe_keep_going :
+ &top_pipe_keep_going;
+ const char *file = "trace_pipe";
+ int brass[2], in_fd, ret = -1;
+ off_t data_size;
*keep_going = true;
+
+ in_fd = tracefs_instance_file_open(instance, file, O_RDONLY);
+ if (in_fd < 0) {
+ tracefs_warning("Failed to open 'trace_pipe'.");
+ return ret;
+ }
+
+ if(pipe(brass) < 0) {
+ tracefs_warning("Failed to open pipe.");
+ goto close_file;
+ }
+
+ data_size = fcntl(brass[0], F_GETPIPE_SZ);
+ if (data_size <= 0) {
+ tracefs_warning("Failed to open pipe (size=0).");
+ goto close_all;
+ }
+
+ *keep_going = true;
And not here.
+ while (*keep_going) {
+ ret = splice(in_fd, NULL,
+ brass[1], NULL,
+ data_size, flags);
+ if (ret < 0)
+ break;
+
+ ret = splice(brass[0], NULL,
+ fd, NULL,
+ data_size, flags);
+ if (ret < 0)
+ break;
+ }
And I think we need to make it volatile, otherwise the compiler is free to
ignore it. Because the compiler does not need to know about threads.
*keep_going = true;
while (*keep_going) {
[ do something ]
}
To the compiler that is the same as:
while (1) {
[ do something ]
}
And is free to make that change when optimizing.
What needs to be done is:
(*(volatile bool *)keep_going) = true;
and
while (*(volatile bool *)keep_going) {
That way the compiler knows that the value can change from outside its
knowledge.
-- Steve
|