| Message ID | 20190909182722.16783-20-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [01/19] iomap: better document the IOMAP_F_* flags | expand |
On Mon, Sep 09, 2019 at 08:27:22PM +0200, Christoph Hellwig wrote: > Only bail out once we know that a COW allocation is actually required, > similar to how we handle normal data fork allocations. > > Signed-off-by: Christoph Hellwig <hch@lst.de> /me is still a little fuzzy about what NOWAIT is actually supposed to mean (no waiting for fs metadata to load? no waiting on other io to the same device? no waiting for anything, period?) but afaict this doesn't seem to change the behavior much... Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/xfs_iomap.c | 23 +++++------------------ > 1 file changed, 5 insertions(+), 18 deletions(-) > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c > index e4e79aa5b695..9e1b4b94acac 100644 > --- a/fs/xfs/xfs_iomap.c > +++ b/fs/xfs/xfs_iomap.c > @@ -693,15 +693,8 @@ xfs_ilock_for_iomap( > * COW writes may allocate delalloc space or convert unwritten COW > * extents, so we need to make sure to take the lock exclusively here. > */ > - if (xfs_is_cow_inode(ip) && is_write) { > - /* > - * FIXME: It could still overwrite on unshared extents and not > - * need allocation. > - */ > - if (flags & IOMAP_NOWAIT) > - return -EAGAIN; > + if (xfs_is_cow_inode(ip) && is_write) > mode = XFS_ILOCK_EXCL; > - } > > /* > * Extents not yet cached requires exclusive access, don't block. This > @@ -760,12 +753,6 @@ xfs_direct_write_iomap_begin( > if (XFS_FORCED_SHUTDOWN(mp)) > return -EIO; > > - /* > - * Lock the inode in the manner required for the specified operation and > - * check for as many conditions that would result in blocking as > - * possible. This removes most of the non-blocking checks from the > - * mapping code below. > - */ > error = xfs_ilock_for_iomap(ip, flags, &lockmode); > if (error) > return error; > @@ -775,11 +762,11 @@ xfs_direct_write_iomap_begin( > if (error) > goto out_unlock; > > - /* > - * Break shared extents if necessary. Checks for non-blocking IO have > - * been done up front, so we don't need to do them here. > - */ > if (imap_needs_cow(ip, &imap, flags, nimaps)) { > + error = -EAGAIN; > + if (flags & IOMAP_NOWAIT) > + goto out_unlock; > + > /* may drop and re-acquire the ilock */ > error = xfs_reflink_allocate_cow(ip, &imap, &cmap, &shared, > &lockmode, flags & IOMAP_DIRECT); > -- > 2.20.1 >
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c index e4e79aa5b695..9e1b4b94acac 100644 --- a/fs/xfs/xfs_iomap.c +++ b/fs/xfs/xfs_iomap.c @@ -693,15 +693,8 @@ xfs_ilock_for_iomap( * COW writes may allocate delalloc space or convert unwritten COW * extents, so we need to make sure to take the lock exclusively here. */ - if (xfs_is_cow_inode(ip) && is_write) { - /* - * FIXME: It could still overwrite on unshared extents and not - * need allocation. - */ - if (flags & IOMAP_NOWAIT) - return -EAGAIN; + if (xfs_is_cow_inode(ip) && is_write) mode = XFS_ILOCK_EXCL; - } /* * Extents not yet cached requires exclusive access, don't block. This @@ -760,12 +753,6 @@ xfs_direct_write_iomap_begin( if (XFS_FORCED_SHUTDOWN(mp)) return -EIO; - /* - * Lock the inode in the manner required for the specified operation and - * check for as many conditions that would result in blocking as - * possible. This removes most of the non-blocking checks from the - * mapping code below. - */ error = xfs_ilock_for_iomap(ip, flags, &lockmode); if (error) return error; @@ -775,11 +762,11 @@ xfs_direct_write_iomap_begin( if (error) goto out_unlock; - /* - * Break shared extents if necessary. Checks for non-blocking IO have - * been done up front, so we don't need to do them here. - */ if (imap_needs_cow(ip, &imap, flags, nimaps)) { + error = -EAGAIN; + if (flags & IOMAP_NOWAIT) + goto out_unlock; + /* may drop and re-acquire the ilock */ error = xfs_reflink_allocate_cow(ip, &imap, &cmap, &shared, &lockmode, flags & IOMAP_DIRECT);
Only bail out once we know that a COW allocation is actually required, similar to how we handle normal data fork allocations. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_iomap.c | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-)