Re: [PATCH RFC 19/20] pipe: use f_pipe

[Jan Kara <jack@suse.cz>]

On Fri 30-08-24 15:05:00, Christian Brauner wrote:
> Pipes use f_version to defer poll notifications until a write has been
> observed. Since multiple file's refer to the same struct pipe_inode_info
> in their ->private_data moving it into their isn't feasible since we
> would need to introduce an additional pointer indirection.
> 
> However, since pipes don't require f_pos_lock we placed a new f_pipe
> member into a union with f_pos_lock that pipes can use. This is similar
> to what we already do for struct inode where we have additional fields
> per file type. This will allow us to fully remove f_version in the next
> step.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/pipe.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 7dff2aa50a6d..b8f1943c57b9 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -686,7 +686,7 @@ pipe_poll(struct file *filp, poll_table *wait)
>  	if (filp->f_mode & FMODE_READ) {
>  		if (!pipe_empty(head, tail))
>  			mask |= EPOLLIN | EPOLLRDNORM;
> -		if (!pipe->writers && filp->f_version != pipe->w_counter)
> +		if (!pipe->writers && filp->f_pipe != pipe->w_counter)
>  			mask |= EPOLLHUP;
>  	}
>  
> @@ -1108,7 +1108,7 @@ static int fifo_open(struct inode *inode, struct file *filp)
>  	bool is_pipe = inode->i_sb->s_magic == PIPEFS_MAGIC;
>  	int ret;
>  
> -	filp->f_version = 0;
> +	filp->f_pipe = 0;
>  
>  	spin_lock(&inode->i_lock);
>  	if (inode->i_pipe) {
> @@ -1155,7 +1155,7 @@ static int fifo_open(struct inode *inode, struct file *filp)
>  			if ((filp->f_flags & O_NONBLOCK)) {
>  				/* suppress EPOLLHUP until we have
>  				 * seen a writer */
> -				filp->f_version = pipe->w_counter;
> +				filp->f_pipe = pipe->w_counter;
>  			} else {
>  				if (wait_for_partner(pipe, &pipe->w_counter))
>  					goto err_rd;
> 
> -- 
> 2.45.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR