Hi Steve, On Thu, Sep 09, 2021 at 08:35:29AM -0400, Steven Rostedt wrote: > On Thu, 9 Sep 2021 12:57:34 +0100 > John Keeping <john@xxxxxxxxxxxx> wrote: > > > The tracing marker files are write-only streams with no meaningful > > concept of file position. Using stream_open() to mark them as > > stream-link indicates this and has the added advantage that a single > > file descriptor can now be used from multiple threads without contention > > thanks to clearing FMODE_ATOMIC_POS. > > > > Note that this has the potential to break existing userspace by since > > both lseek(2) and pwrite(2) will now return ESPIPE when previously lseek > > would have updated the stored offset and pwrite would have appended to > > the trace. A survey of libtracefs and several other projects found to > > use trace_marker(_raw) [1][2][3] suggests that everyone limits > > themselves to calling write(2) and close(2) on these file descriptors so > > there is a good chance this will go unnoticed and the benefits of > > reduced overhead and lock contention seem worth the risk. > > > > [1] https://github.com/google/perfetto > > [2] https://github.com/intel/media-driver/ > > [3] https://w1.fi/cgit/hostap/ > > > > Signed-off-by: John Keeping <john@xxxxxxxxxxxx> > > --- > > I'm marking this as RFC because it breaks the Prime Directive of Linux > > development, as explained above. But I'm hoping this is one of the > > The "Prime Directive of Linux development" is the tree falling in the > forest approach. If you break user space API but there's no user space > application around to notice the break, did you really break it? The answer > is "No". > > > cases where we get away with it because this is a huge improvement for > > multi-threaded programs when doing the simple thing of opening a single > > trace_marker FD at startup and just writing to it from any thread. > > I like the idea too. > > > > > After writing this, I realised that an alternative solution to my > > original problem would have been to use pwrite instead of write! But I > > still think fixing this at the source would be better. > > I'm fine with adding this. But I'm going to add it after the merge window > for the next release (5.16). > > If someone complains that it broke their application, we may need to revert > it, but I doubt that will happen. Were you expecting more input from me on this? The above sounded like "will be added for 5.16" but I don't see this change in v5.16-rc4 and the patch is still marked as "New" in patchwork [1] [1] https://patchwork.kernel.org/project/linux-trace-devel/patch/20210909115734.3818711-1-john@xxxxxxxxxxxx/ Thanks, John
|