Re: [PATCH 1/5] fs: make syncfs(2) killable on s_umount

[Jan Kara]

On Fri 23-10-26 20:39:30, Josef Bacik wrote:
> A syncfs(2) which has to wait for s_umount sits in D state and can't be
> killed, for as long as whoever has the lock takes.
> 
> That can be very long.  Another syncfs(2) holds s_umount shared while it
> writes the filesystem out, which on a slow or dead device takes forever.
> That alone doesn't keep other readers out, but as soon as somebody wants
> the lock exclusively, a remount say, that writer queues up, and every
> reader which comes after it queues up behind the writer.
> 
> Taking the lock is the first thing syncfs(2) does and it can return an
> error, so use down_read_killable().
> 
> This only covers the wait for the lock.  Once syncfs(2) has it, it still
> waits for the writeback uninterruptibly.
> 
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/sync.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/sync.c b/fs/sync.c
> index 0681bfcb2da6..ea54af59cf4a 100644
> --- a/fs/sync.c
> +++ b/fs/sync.c
> @@ -156,7 +156,9 @@ SYSCALL_DEFINE1(syncfs, int, fd)
>  		return -EBADF;
>  	sb = fd_file(f)->f_path.dentry->d_sb;
>  
> -	down_read(&sb->s_umount);
> +	ret = down_read_killable(&sb->s_umount);
> +	if (ret)
> +		return ret;
>  	ret = sync_filesystem(sb);
>  	up_read(&sb->s_umount);
>  
> 
> -- 
> 2.55.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR