xfs
[Top] [All Lists]

Re: [PATCH 4/9] [PATCH 4/9] xfs: dont serialise direct IO reads on page

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 4/9] [PATCH 4/9] xfs: dont serialise direct IO reads on page cache
From: Greg KH <greg@xxxxxxxxx>
Date: Tue, 22 Nov 2011 13:34:25 -0800
Cc: stable@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, Dave Chinner <dchinner@xxxxxxxxxx>, Alex Elder <aelder@xxxxxxx>
Dkim-signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=date:from:to:cc:subject:message-id :references:mime-version:content-type:in-reply-to; s=smtpout; bh=5V4nkBjRSrdwBjoUKrIjwlURSaI=; b=WQ0jFh3OzniwDIs5shzOh7wphu54 /aXCixFmhycmvtrbwD+AMy8FiOVz/ck7yzN587vcOPVhBzMpj0u49Gdl/QDxI0ND xgBnhfSmN/ctZAd8VRfitXqYzwYHBdS0G7kJ3ec7x4sNefGomURlg1qHSukXUHUw OEwFzRnjAuq32MM=
In-reply-to: <20111119181544.111984285@xxxxxxxxxxxxxxxxxxxxxx>
References: <20111119181336.964593075@xxxxxxxxxxxxxxxxxxxxxx> <20111119181544.111984285@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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

<Prev in Thread] Current Thread [Next in Thread>