diff mbox series

[02/11] xfs: make xfs_file_aio_write_checks IOCB_NOWAIT-aware

Message ID 20210118193516.2915706-3-hch@lst.de (mailing list archive)
State Superseded
Headers show
Series [01/11] xfs: factor out a xfs_ilock_iocb helper | expand

Commit Message

Christoph Hellwig Jan. 18, 2021, 7:35 p.m. UTC
Ensure we don't block on the iolock, or waiting for I/O in
xfs_file_aio_write_checks if the caller asked to avoid that.

Fixes: 29a5d29ec181 ("xfs: nowait aio support")
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_file.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

Comments

Christoph Hellwig Jan. 20, 2021, 4:28 p.m. UTC | #1
On Tue, Jan 19, 2021 at 09:33:37AM -0300, Raphael Carvalho wrote:
> >          * No fallback to buffered IO after short writes for XFS, direct
> > I/O
> > @@ -632,7 +648,8 @@ xfs_file_dax_write(
> >                 error = xfs_setfilesize(ip, pos, ret);
> >         }
> >  out:
> > -       xfs_iunlock(ip, iolock);
> > +       if (iolock)
> > +               xfs_iunlock(ip, iolock);
> >
> 
> Not familiar with the code but looks like you're setting *iolock to zero on
> error and perhaps you want to dereference it here instead

In this function iolock is a scalar value, not a pointer.
xfs_file_aio_write_checks gets it passed by reference and clears it,
and here we check that the iolock is locked at all.
Darrick J. Wong Jan. 20, 2021, 6:42 p.m. UTC | #2
On Mon, Jan 18, 2021 at 08:35:07PM +0100, Christoph Hellwig wrote:
> Ensure we don't block on the iolock, or waiting for I/O in
> xfs_file_aio_write_checks if the caller asked to avoid that.
> 
> Fixes: 29a5d29ec181 ("xfs: nowait aio support")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Brian Foster <bfoster@redhat.com>

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_file.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index c441cddfa4acbc..fb4e6f2852bb8b 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -335,7 +335,14 @@ xfs_file_aio_write_checks(
>  	if (error <= 0)
>  		return error;
>  
> -	error = xfs_break_layouts(inode, iolock, BREAK_WRITE);
> +	if (iocb->ki_flags & IOCB_NOWAIT) {
> +		error = break_layout(inode, false);
> +		if (error == -EWOULDBLOCK)
> +			error = -EAGAIN;
> +	} else {
> +		error = xfs_break_layouts(inode, iolock, BREAK_WRITE);
> +	}
> +
>  	if (error)
>  		return error;
>  
> @@ -346,7 +353,11 @@ xfs_file_aio_write_checks(
>  	if (*iolock == XFS_IOLOCK_SHARED && !IS_NOSEC(inode)) {
>  		xfs_iunlock(ip, *iolock);
>  		*iolock = XFS_IOLOCK_EXCL;
> -		xfs_ilock(ip, *iolock);
> +		error = xfs_ilock_iocb(iocb, *iolock);
> +		if (error) {
> +			*iolock = 0;
> +			return error;
> +		}
>  		goto restart;
>  	}
>  	/*
> @@ -368,6 +379,10 @@ xfs_file_aio_write_checks(
>  	isize = i_size_read(inode);
>  	if (iocb->ki_pos > isize) {
>  		spin_unlock(&ip->i_flags_lock);
> +
> +		if (iocb->ki_flags & IOCB_NOWAIT)
> +			return -EAGAIN;
> +
>  		if (!drained_dio) {
>  			if (*iolock == XFS_IOLOCK_SHARED) {
>  				xfs_iunlock(ip, *iolock);
> @@ -593,7 +608,8 @@ xfs_file_dio_aio_write(
>  			   &xfs_dio_write_ops,
>  			   is_sync_kiocb(iocb) || unaligned_io);
>  out:
> -	xfs_iunlock(ip, iolock);
> +	if (iolock)
> +		xfs_iunlock(ip, iolock);
>  
>  	/*
>  	 * No fallback to buffered IO after short writes for XFS, direct I/O
> @@ -632,7 +648,8 @@ xfs_file_dax_write(
>  		error = xfs_setfilesize(ip, pos, ret);
>  	}
>  out:
> -	xfs_iunlock(ip, iolock);
> +	if (iolock)
> +		xfs_iunlock(ip, iolock);
>  	if (error)
>  		return error;
>  
> -- 
> 2.29.2
>
diff mbox series

Patch

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index c441cddfa4acbc..fb4e6f2852bb8b 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -335,7 +335,14 @@  xfs_file_aio_write_checks(
 	if (error <= 0)
 		return error;
 
-	error = xfs_break_layouts(inode, iolock, BREAK_WRITE);
+	if (iocb->ki_flags & IOCB_NOWAIT) {
+		error = break_layout(inode, false);
+		if (error == -EWOULDBLOCK)
+			error = -EAGAIN;
+	} else {
+		error = xfs_break_layouts(inode, iolock, BREAK_WRITE);
+	}
+
 	if (error)
 		return error;
 
@@ -346,7 +353,11 @@  xfs_file_aio_write_checks(
 	if (*iolock == XFS_IOLOCK_SHARED && !IS_NOSEC(inode)) {
 		xfs_iunlock(ip, *iolock);
 		*iolock = XFS_IOLOCK_EXCL;
-		xfs_ilock(ip, *iolock);
+		error = xfs_ilock_iocb(iocb, *iolock);
+		if (error) {
+			*iolock = 0;
+			return error;
+		}
 		goto restart;
 	}
 	/*
@@ -368,6 +379,10 @@  xfs_file_aio_write_checks(
 	isize = i_size_read(inode);
 	if (iocb->ki_pos > isize) {
 		spin_unlock(&ip->i_flags_lock);
+
+		if (iocb->ki_flags & IOCB_NOWAIT)
+			return -EAGAIN;
+
 		if (!drained_dio) {
 			if (*iolock == XFS_IOLOCK_SHARED) {
 				xfs_iunlock(ip, *iolock);
@@ -593,7 +608,8 @@  xfs_file_dio_aio_write(
 			   &xfs_dio_write_ops,
 			   is_sync_kiocb(iocb) || unaligned_io);
 out:
-	xfs_iunlock(ip, iolock);
+	if (iolock)
+		xfs_iunlock(ip, iolock);
 
 	/*
 	 * No fallback to buffered IO after short writes for XFS, direct I/O
@@ -632,7 +648,8 @@  xfs_file_dax_write(
 		error = xfs_setfilesize(ip, pos, ret);
 	}
 out:
-	xfs_iunlock(ip, iolock);
+	if (iolock)
+		xfs_iunlock(ip, iolock);
 	if (error)
 		return error;