Re: [PATCH 2/5] quota: make quotactl_fd(2) killable on s_umount

[Jan Kara]

On Fri 23-10-26 20:39:31, Josef Bacik wrote:
> quotactl_fd(2) takes s_umount of the filesystem the file descriptor is on,
> exclusively to turn quota on or off and shared for everything else.  If
> the lock is held for a long time, by a syncfs(2) on a device which doesn't
> answer say, the caller sits in D state and can't be killed.  For the
> commands which write it does so holding write access to the mount.
> 
> quotactl(2) with a device can already be killed at this point, it gets the
> lock from user_get_super().
> 
> All that has been done by the time the lock is taken is getting that write
> access.  Use the killable variants, and drop it on the way out.
> 
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/quota/quota.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index f906e4fd3f0f..94e6407734c7 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -997,9 +997,11 @@ SYSCALL_DEFINE4(quotactl_fd, unsigned int, fd, unsigned int, cmd,
>  
>  	sb = fd_file(f)->f_path.mnt->mnt_sb;
>  	if (quotactl_cmd_onoff(cmds))
> -		down_write(&sb->s_umount);
> +		ret = down_write_killable(&sb->s_umount);
>  	else
> -		down_read(&sb->s_umount);
> +		ret = down_read_killable(&sb->s_umount);
> +	if (ret)
> +		goto out;
>  
>  	ret = do_quotactl(sb, type, cmds, id, addr, ERR_PTR(-EINVAL));
>  
> @@ -1007,7 +1009,7 @@ SYSCALL_DEFINE4(quotactl_fd, unsigned int, fd, unsigned int, cmd,
>  		up_write(&sb->s_umount);
>  	else
>  		up_read(&sb->s_umount);
> -
> +out:
>  	if (quotactl_cmd_write(cmds))
>  		mnt_drop_write(fd_file(f)->f_path.mnt);
>  	return ret;
> 
> -- 
> 2.55.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR