Re: [PATCH v5 01/17] filelock: make lease_alloc() take a flags argument

On Wed 05-11-25 11:53:47, Jeff Layton wrote:
> __break_lease() currently overrides the flc_flags field in the lease
> after allocating it. A forthcoming patch will add the ability to request
> a FL_DELEG type lease.
> 
> Instead of overriding the flags field, add a flags argument to
> lease_alloc() and lease_init() so it's set correctly after allocating.
> 
> Signed-off-by: Jeff Layton <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

								Honza

> ---
>  fs/locks.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index 04a3f0e2072461b6e2d3d1cd12f2b089d69a7db3..b33c327c21dcd49341fbeac47caeb72cdf7455db 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -585,7 +585,7 @@ static const struct lease_manager_operations lease_manager_ops = {
>  /*
>   * Initialize a lease, use the default lock manager operations
>   */
> -static int lease_init(struct file *filp, int type, struct file_lease *fl)
> +static int lease_init(struct file *filp, unsigned int flags, int type, struct file_lease *fl)
>  {
>  	if (assign_type(&fl->c, type) != 0)
>  		return -EINVAL;
> @@ -594,13 +594,13 @@ static int lease_init(struct file *filp, int type, struct file_lease *fl)
>  	fl->c.flc_pid = current->tgid;
>  
>  	fl->c.flc_file = filp;
> -	fl->c.flc_flags = FL_LEASE;
> +	fl->c.flc_flags = flags;
>  	fl->fl_lmops = &lease_manager_ops;
>  	return 0;
>  }
>  
>  /* Allocate a file_lock initialised to this type of lease */
> -static struct file_lease *lease_alloc(struct file *filp, int type)
> +static struct file_lease *lease_alloc(struct file *filp, unsigned int flags, int type)
>  {
>  	struct file_lease *fl = locks_alloc_lease();
>  	int error = -ENOMEM;
> @@ -608,7 +608,7 @@ static struct file_lease *lease_alloc(struct file *filp, int type)
>  	if (fl == NULL)
>  		return ERR_PTR(error);
>  
> -	error = lease_init(filp, type, fl);
> +	error = lease_init(filp, flags, type, fl);
>  	if (error) {
>  		locks_free_lease(fl);
>  		return ERR_PTR(error);
> @@ -1548,10 +1548,9 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
>  	int want_write = (mode & O_ACCMODE) != O_RDONLY;
>  	LIST_HEAD(dispose);
>  
> -	new_fl = lease_alloc(NULL, want_write ? F_WRLCK : F_RDLCK);
> +	new_fl = lease_alloc(NULL, type, want_write ? F_WRLCK : F_RDLCK);
>  	if (IS_ERR(new_fl))
>  		return PTR_ERR(new_fl);
> -	new_fl->c.flc_flags = type;
>  
>  	/* typically we will check that ctx is non-NULL before calling */
>  	ctx = locks_inode_context(inode);
> @@ -2033,7 +2032,7 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, int arg)
>  	struct fasync_struct *new;
>  	int error;
>  
> -	fl = lease_alloc(filp, arg);
> +	fl = lease_alloc(filp, FL_LEASE, arg);
>  	if (IS_ERR(fl))
>  		return PTR_ERR(fl);
>  
> 
> -- 
> 2.51.1
> 
-- 
Jan Kara <[email protected]>
SUSE Labs, CR