Re: When to lock pipe->rd_wait.lock?

[Christian Brauner <brauner@kernel.org>]

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.