Re: [PATCH RFC 18/20] fs: add f_pipe

[Jan Kara <jack@suse.cz>]

On Fri 30-08-24 15:04:59, Christian Brauner wrote:
> Only regular files with FMODE_ATOMIC_POS and directories need
> f_pos_lock. Place a new f_pipe member in a union with f_pos_lock
> that they can use and make them stop abusing f_version in follow-up
> patches.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

What makes me a bit uneasy is that we do mutex_init() on the space in
struct file and then pipe_open() will clobber it. And then we eventually
call mutex_destroy() on the clobbered mutex. I think so far this does no
harm but mostly by luck. I think we need to make sure that when f_pos_lock
is unused, we don't even call mutex_init() / mutex_destroy() on it.

								Honza

> ---
>  include/linux/fs.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 3e6b3c1afb31..ca4925008244 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1001,6 +1001,7 @@ static inline int ra_has_index(struct file_ra_state *ra, pgoff_t index)
>   * @f_cred: stashed credentials of creator/opener
>   * @f_path: path of the file
>   * @f_pos_lock: lock protecting file position
> + * @f_pipe: specific to pipes
>   * @f_pos: file position
>   * @f_version: file version
>   * @f_security: LSM security context of this file
> @@ -1026,7 +1027,12 @@ struct file {
>  	const struct cred		*f_cred;
>  	/* --- cacheline 1 boundary (64 bytes) --- */
>  	struct path			f_path;
> -	struct mutex			f_pos_lock;
> +	union {
> +		/* regular files (with FMODE_ATOMIC_POS) and directories */
> +		struct mutex		f_pos_lock;
> +		/* pipes */
> +		u64			f_pipe;
> +	};
>  	loff_t				f_pos;
>  	u64				f_version;
>  	/* --- cacheline 2 boundary (128 bytes) --- */
> 
> -- 
> 2.45.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR