Re: [RFC PATCH v1 06/18] xfs: add iomap async buffered write support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 28, 2022 at 01:03:49PM -0700, Stefan Roesch wrote:
> 
> 
> On 4/26/22 3:54 PM, Dave Chinner wrote:
> > On Tue, Apr 26, 2022 at 10:43:23AM -0700, Stefan Roesch wrote:
> >> This adds the async buffered write support to the iomap layer of XFS. If
> >> a lock cannot be acquired or additional reads need to be performed, the
> >> request will return -EAGAIN in case this is an async buffered write request.
> >>
> >> Signed-off-by: Stefan Roesch <shr@xxxxxx>
> >> ---
> >>  fs/xfs/xfs_iomap.c | 33 +++++++++++++++++++++++++++++++--
> >>  1 file changed, 31 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> >> index e552ce541ec2..80b6c48e88af 100644
> >> --- a/fs/xfs/xfs_iomap.c
> >> +++ b/fs/xfs/xfs_iomap.c
> >> @@ -881,18 +881,28 @@ xfs_buffered_write_iomap_begin(
> >>  	bool			eof = false, cow_eof = false, shared = false;
> >>  	int			allocfork = XFS_DATA_FORK;
> >>  	int			error = 0;
> >> +	bool			no_wait = (flags & IOMAP_NOWAIT);
> >>  
> >>  	if (xfs_is_shutdown(mp))
> >>  		return -EIO;
> >>  
> >>  	/* we can't use delayed allocations when using extent size hints */
> >> -	if (xfs_get_extsz_hint(ip))
> >> +	if (xfs_get_extsz_hint(ip)) {
> >> +		if (no_wait)
> >> +			return -EAGAIN;
> >> +
> >>  		return xfs_direct_write_iomap_begin(inode, offset, count,
> >>  				flags, iomap, srcmap);
> > 
> > Why can't we do IOMAP_NOWAIT allocation here?
> > xfs_direct_write_iomap_begin() supports IOMAP_NOWAIT semantics just
> > fine - that's what the direct IO path uses...
> 
> I'll make that change in the next version.
> > 
> >> +	}
> >>  
> >>  	ASSERT(!XFS_IS_REALTIME_INODE(ip));
> >>  
> >> -	xfs_ilock(ip, XFS_ILOCK_EXCL);
> >> +	if (no_wait) {
> >> +		if (!xfs_ilock_nowait(ip, XFS_ILOCK_EXCL))
> >> +			return -EAGAIN;
> >> +	} else {
> >> +		xfs_ilock(ip, XFS_ILOCK_EXCL);
> >> +	}
> > 
> > handled by xfs_ilock_for_iomap().
> 
> The helper xfs_ilock_for_iomap cannot be used for this purpose. The function
> xfs_ilock_for_iomap tries to deduce the lock mode. For the function xfs_file_buffered_write()
> the lock mode must be exclusive. However this is not always the case when the
> helper xfs_ilock_for_iomap is used. There are cases where xfs_is_cow_inode(() is not
> true.

That's trivially fixed by having the caller initialise the lock_mode
to what they want, rather than have the function set the initial
type to SHARED....

> I'll add a new helper that will be used here.

... so I don't think a new helper is needed.

--Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux