Re: [PATCH] pipe_read: don't wake up the writer if the pipe is still full

[Oleg Nesterov <oleg@redhat.com>]

Hi!

On 03/03, Sapkal, Swapnil wrote:
>
> >but if you have time, could you check if this patch (with or without the
> >previous debugging patch) makes any difference? Just to be sure.
>
> Sure, I will give this a try.

Forget ;)

[...snip...]

> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -417,9 +417,19 @@ static inline int is_packetized(struct file *file)
>  /* Done while waiting without holding the pipe lock - thus the READ_ONCE() */
>  static inline bool pipe_writable(const struct pipe_inode_info *pipe)
>  {
> -	unsigned int head = READ_ONCE(pipe->head);
> -	unsigned int tail = READ_ONCE(pipe->tail);
>  	unsigned int max_usage = READ_ONCE(pipe->max_usage);
> +	unsigned int head, tail;
> +
> +	tail = READ_ONCE(pipe->tail);
> +	/*
> +	 * Since the unsigned arithmetic in this lockless preemptible context
> +	 * relies on the fact that the tail can never be ahead of head, read
> +	 * the head after the tail to ensure we've not missed any updates to
> +	 * the head. Reordering the reads can cause wraparounds and give the
> +	 * illusion that the pipe is full.
> +	 */
> +	smp_rmb();
> +	head = READ_ONCE(pipe->head);
>  	return !pipe_full(head, tail, max_usage) ||
>  		!READ_ONCE(pipe->readers);

Ooh, thanks!!!

And sorry, can't work today. To be honest, I have some concerns, but probably
I am wrong... I'll return tomorrow.

In any case, finally we have a hint. Thank you both!

(btw, please look at pipe_poll).

Oleg.