Messages in this thread | | | From | linke <> | Subject | Re: [PATCH] ring-buffer: use READ_ONCE() to read cpu_buffer->commit_page in concurrent environment | Date | Thu, 29 Feb 2024 20:32:26 +0800 |
| |
Hi Steven, sorry for the late reply.
> > Now the reason for the above READ_ONCE() is because the variables *are* > going to be used again. We do *not* want the compiler to play any games > with that. >
I don't think it is because the variables are going to be used again. Compiler optimizations barely do bad things in single thread programs. It is because cpu_buffer->commit_page may change concurrently and should be accessed atomically.
/* Make sure commit page didn't change */ curr_commit_page = READ_ONCE(cpu_buffer->commit_page); curr_commit_ts = READ_ONCE(curr_commit_page->page->time_stamp);
/* If the commit page changed, then there's more data */ if (curr_commit_page != commit_page || curr_commit_ts != commit_ts) return 0;
This code read cpu_buffer->commit_page and time_stamp again to check whether commit page changed. It shows that cpu_buffer->commit_page and time_stamp may be changed by other threads.
commit_page = cpu_buffer->commit_page; commit_ts = commit_page->page->time_stamp;
So the commit_page and time_stamp above is read while other threads may change it. I think it is a data race if it is not atomic. Thus it is necessary to use READ_ONCE() here.
| |