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@xxxxxxxxxx>
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@xxxxxxxx>
SUSE Labs, CR
