On Sat, Nov 19, 2011 at 01:13:40PM -0500, Christoph Hellwig wrote:
> There is no need to grab the i_mutex of the IO lock in exclusive
> mode if we don't need to invalidate the page cache. Taking these
> locks on every direct IO effective serialises them as taking the IO
> lock in exclusive mode has to wait for all shared holders to drop
> the lock. That only happens when IO is complete, so effective it
> prevents dispatch of concurrent direct IO reads to the same inode.
>
> Fix this by taking the IO lock shared to check the page cache state,
> and only then drop it and take the IO lock exclusively if there is
> work to be done. Hence for the normal direct IO case, no exclusive
> locking will occur.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Tested-by: Joern Engel <joern@xxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Alex Elder <aelder@xxxxxxx>
What is the git commit id that matches this patch in Linus's tree?
thanks,
greg k-h
> ---
> fs/xfs/linux-2.6/xfs_file.c | 17 ++++++++++++++---
> 1 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/fs/xfs/linux-2.6/xfs_file.c b/fs/xfs/linux-2.6/xfs_file.c
> index 7f782af2..93cc02d 100644
> --- a/fs/xfs/linux-2.6/xfs_file.c
> +++ b/fs/xfs/linux-2.6/xfs_file.c
> @@ -309,7 +309,19 @@ xfs_file_aio_read(
> if (XFS_FORCED_SHUTDOWN(mp))
> return -EIO;
>
> - if (unlikely(ioflags & IO_ISDIRECT)) {
> + /*
> + * Locking is a bit tricky here. If we take an exclusive lock
> + * for direct IO, we effectively serialise all new concurrent
> + * read IO to this file and block it behind IO that is currently in
> + * progress because IO in progress holds the IO lock shared. We only
> + * need to hold the lock exclusive to blow away the page cache, so
> + * only take lock exclusively if the page cache needs invalidation.
> + * This allows the normal direct IO case of no page cache pages to
> + * proceeed concurrently without serialisation.
> + */
> + xfs_rw_ilock(ip, XFS_IOLOCK_SHARED);
> + if ((ioflags & IO_ISDIRECT) && inode->i_mapping->nrpages) {
> + xfs_rw_iunlock(ip, XFS_IOLOCK_SHARED);
> xfs_rw_ilock(ip, XFS_IOLOCK_EXCL);
>
> if (inode->i_mapping->nrpages) {
> @@ -322,8 +334,7 @@ xfs_file_aio_read(
> }
> }
> xfs_rw_ilock_demote(ip, XFS_IOLOCK_EXCL);
> - } else
> - xfs_rw_ilock(ip, XFS_IOLOCK_SHARED);
> + }
>
> trace_xfs_file_read(ip, size, iocb->ki_pos, ioflags);
>
> --
> 1.7.7
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
