xfs
[Top] [All Lists]

Re: concurrent direct IO write in xfs

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: concurrent direct IO write in xfs
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 13 Feb 2012 12:48:06 -0500
Cc: Zheng Da <zhengda1936@xxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20120209060920.GF7479@dastard>
References: <CAFLer83FBZG9ZCrT2jUZBcTC2a2tx_CDmykyPF4cTP0dbHGw7Q@xxxxxxxxxxxxxx> <20120116232549.GC6922@dastard> <CAFLer81XkMTh_gxd95pzxCEs1yGRsTrZijX3c7ewgRzeA7DCSQ@xxxxxxxxxxxxxx> <20120123051155.GI15102@dastard> <CAFLer82QxfgXEx7ofzOHOK2YKiA+ab+_Aizd10SWHvnC-mVUHg@xxxxxxxxxxxxxx> <CAFLer81GWSCCCMppU=2dE+5KKqD-hYVKAA0hz9n-CBbxAs_xfw@xxxxxxxxxxxxxx> <20120124035431.GD6922@dastard> <20120209060920.GF7479@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Feb 09, 2012 at 05:09:20PM +1100, Dave Chinner wrote:
>       if (create) {
> -             lockmode = XFS_ILOCK_EXCL;
> +             /*
> +              * For direct IO, we lock in shared mode so that write
> +              * operations that don't require allocation can occur
> +              * concurrently. The ilock has to be dropped over the allocation
> +              * transaction reservation, so the only thing the ilock is
> +              * providing here is modification exclusion. i.e. there is no
> +              * need to hold the lock exclusive.
> +              *
> +              * For buffered IO, if we need to do delayed allocation then
> +              * hold the ilock exclusive so that the lookup and delalloc
> +              * reservation is atomic.
> +              */
> +             if (direct)
> +                     lockmode = XFS_ILOCK_SHARED;
> +             else
> +                     lockmode = XFS_ILOCK_EXCL;
>               xfs_ilock(ip, lockmode);
>       } else {
>               lockmode = xfs_ilock_map_shared(ip);

We'll actually need to use xfs_ilock_map_shared for the the direct
create case too, to make sure we have the exclusive lock when we first
read the extent list in.

Also xfs_qm_dqattach_locked really wants the inode locked exclusively,
which your current code doesn't handle.

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