Re: [PATCH v2 16/16] fs: create {sb,file}_write_not_started() helpers

[Jan Kara <jack@suse.cz>]

On Wed 22-11-23 14:27:15, Amir Goldstein wrote:
> Create new helpers {sb,file}_write_not_started() that can be used
> to assert that sb_start_write() is not held.
> 
> This is needed for fanotify "pre content" events.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

I'm not against this but I'm somewhat wondering, where exactly do you plan
to use this :) (does not seem to be in this patch set). Because one easily
forgets about the subtle implementation details and uses
!sb_write_started() instead of sb_write_not_started()...

								Honza

> ---
>  include/linux/fs.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 05780d993c7d..c085172f832f 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1669,6 +1669,17 @@ static inline bool sb_write_started(const struct super_block *sb)
>  	return __sb_write_started(sb, SB_FREEZE_WRITE);
>  }
>  
> +/**
> + * sb_write_not_started - check if SB_FREEZE_WRITE is not held
> + * @sb: the super we write to
> + *
> + * May be false positive with !CONFIG_LOCKDEP/LOCK_STATE_UNKNOWN.
> + */
> +static inline bool sb_write_not_started(const struct super_block *sb)
> +{
> +	return __sb_write_started(sb, SB_FREEZE_WRITE) <= 0;
> +}
> +
>  /**
>   * file_write_started - check if SB_FREEZE_WRITE is held
>   * @file: the file we write to
> @@ -1684,6 +1695,21 @@ static inline bool file_write_started(const struct file *file)
>  	return sb_write_started(file_inode(file)->i_sb);
>  }
>  
> +/**
> + * file_write_not_started - check if SB_FREEZE_WRITE is not held
> + * @file: the file we write to
> + *
> + * May be false positive with !CONFIG_LOCKDEP/LOCK_STATE_UNKNOWN.
> + * May be false positive with !S_ISREG, because file_start_write() has
> + * no effect on !S_ISREG.
> + */
> +static inline bool file_write_not_started(const struct file *file)
> +{
> +	if (!S_ISREG(file_inode(file)->i_mode))
> +		return true;
> +	return sb_write_not_started(file_inode(file)->i_sb);
> +}
> +
>  /**
>   * sb_end_write - drop write access to a superblock
>   * @sb: the super we wrote to
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR