On Wed, Sep 20, 2023 at 02:34:51PM +0200, Max Kellermann wrote: > Hi, > > I'm trying to understand the code that allocates a new buffer from a > pipe's buffer ring. I'd like to write a patch that eliminates some > duplicate code, to make it less error prone. > > In fs/pipe.c, pipe_write() locks pipe->rd_wait.lock while the pipe's > head gets evaluated and incremented (all while pipe->mutex is locked). > My limited understand is that holding this spinlock is important > because it protects the head/tail accesses in pipe_readable() which is > gets called by wait_event while the spinlock is held, but without > pipe->mutex. > > However in fs/splice.c, splice_pipe_to_pipe() contains very similar > code; a new buffer gets allocated, head gets incremented - but without > caring for pipe->rd_wait.lock. > Please help me understand the point of locking pipe->rd_wait.lock, and > why it's necessary in pipe_write() but not in splice_pipe_to_pipe(). > Is that a bug or am I missing something? Afaict, the mutex is sufficient protection unless you're using watchqueues which use post_one_notification() that cannot acquire the pipe mutex. Since splice operations aren't supported on such kernel notification pipes - see get_pipe_info() - it should be unproblematic.
