linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* a few iomap / bmap cleanups v2
@ 2019-10-30 18:04 Christoph Hellwig
  2019-10-30 18:04 ` [PATCH 1/9] xfs: simplify xfs_iomap_eof_align_last_fsb Christoph Hellwig
                   ` (8 more replies)
  0 siblings, 9 replies; 13+ messages in thread
From: Christoph Hellwig @ 2019-10-30 18:04 UTC (permalink / raw)
  To: linux-xfs

Hi all,

this series cleans up a few lose ends through the iomap and bmap
write path.

Changes since v1:
 - improvements to an assert in the bmap code

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/9] xfs: simplify xfs_iomap_eof_align_last_fsb
  2019-10-30 18:04 a few iomap / bmap cleanups v2 Christoph Hellwig
@ 2019-10-30 18:04 ` Christoph Hellwig
  2019-10-30 18:04 ` [PATCH 2/9] xfs: mark xfs_eof_alignment static Christoph Hellwig
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2019-10-30 18:04 UTC (permalink / raw)
  To: linux-xfs; +Cc: Darrick J . Wong

By open coding xfs_bmap_last_extent instead of calling it through a
double indirection we don't need to handle an error return that
can't happen given that we are guaranteed to have the extent list in
memory already.  Also simplify the calling conventions a little and
move the extent list assert from the only caller into the function.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_bmap_util.c | 23 ---------------------
 fs/xfs/xfs_bmap_util.h |  2 --
 fs/xfs/xfs_iomap.c     | 47 ++++++++++++++++++++----------------------
 3 files changed, 22 insertions(+), 50 deletions(-)

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index fb31d7d6701e..be606f4e3a74 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -179,29 +179,6 @@ xfs_bmap_rtalloc(
 }
 #endif /* CONFIG_XFS_RT */
 
-/*
- * Check if the endoff is outside the last extent. If so the caller will grow
- * the allocation to a stripe unit boundary.  All offsets are considered outside
- * the end of file for an empty fork, so 1 is returned in *eof in that case.
- */
-int
-xfs_bmap_eof(
-	struct xfs_inode	*ip,
-	xfs_fileoff_t		endoff,
-	int			whichfork,
-	int			*eof)
-{
-	struct xfs_bmbt_irec	rec;
-	int			error;
-
-	error = xfs_bmap_last_extent(NULL, ip, whichfork, &rec, eof);
-	if (error || *eof)
-		return error;
-
-	*eof = endoff >= rec.br_startoff + rec.br_blockcount;
-	return 0;
-}
-
 /*
  * Extent tree block counting routines.
  */
diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
index 3e0fa0d363d1..9f993168b55b 100644
--- a/fs/xfs/xfs_bmap_util.h
+++ b/fs/xfs/xfs_bmap_util.h
@@ -30,8 +30,6 @@ xfs_bmap_rtalloc(struct xfs_bmalloca *ap)
 }
 #endif /* CONFIG_XFS_RT */
 
-int	xfs_bmap_eof(struct xfs_inode *ip, xfs_fileoff_t endoff,
-		     int whichfork, int *eof);
 int	xfs_bmap_punch_delalloc_range(struct xfs_inode *ip,
 		xfs_fileoff_t start_fsb, xfs_fileoff_t length);
 
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 8317b936d3c1..4209c32db96d 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -156,25 +156,33 @@ xfs_eof_alignment(
 	return align;
 }
 
-STATIC int
+/*
+ * Check if last_fsb is outside the last extent, and if so grow it to the next
+ * stripe unit boundary.
+ */
+static xfs_fileoff_t
 xfs_iomap_eof_align_last_fsb(
 	struct xfs_inode	*ip,
-	xfs_extlen_t		extsize,
-	xfs_fileoff_t		*last_fsb)
+	xfs_fileoff_t		end_fsb)
 {
-	xfs_extlen_t		align = xfs_eof_alignment(ip, extsize);
+	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
+	xfs_extlen_t		extsz = xfs_get_extsz_hint(ip);
+	xfs_extlen_t		align = xfs_eof_alignment(ip, extsz);
+	struct xfs_bmbt_irec	irec;
+	struct xfs_iext_cursor	icur;
+
+	ASSERT(ifp->if_flags & XFS_IFEXTENTS);
 
 	if (align) {
-		xfs_fileoff_t	new_last_fsb = roundup_64(*last_fsb, align);
-		int		eof, error;
+		xfs_fileoff_t	aligned_end_fsb = roundup_64(end_fsb, align);
 
-		error = xfs_bmap_eof(ip, new_last_fsb, XFS_DATA_FORK, &eof);
-		if (error)
-			return error;
-		if (eof)
-			*last_fsb = new_last_fsb;
+		xfs_iext_last(ifp, &icur);
+		if (!xfs_iext_get_extent(ifp, &icur, &irec) ||
+		    aligned_end_fsb >= irec.br_startoff + irec.br_blockcount)
+			return aligned_end_fsb;
 	}
-	return 0;
+
+	return end_fsb;
 }
 
 int
@@ -206,19 +214,8 @@ xfs_iomap_write_direct(
 
 	ASSERT(xfs_isilocked(ip, lockmode));
 
-	if ((offset + count) > XFS_ISIZE(ip)) {
-		/*
-		 * Assert that the in-core extent list is present since this can
-		 * call xfs_iread_extents() and we only have the ilock shared.
-		 * This should be safe because the lock was held around a bmapi
-		 * call in the caller and we only need it to access the in-core
-		 * list.
-		 */
-		ASSERT(XFS_IFORK_PTR(ip, XFS_DATA_FORK)->if_flags &
-								XFS_IFEXTENTS);
-		error = xfs_iomap_eof_align_last_fsb(ip, extsz, &last_fsb);
-		if (error)
-			goto out_unlock;
+	if (offset + count > XFS_ISIZE(ip)) {
+		last_fsb = xfs_iomap_eof_align_last_fsb(ip, last_fsb);
 	} else {
 		if (nmaps && (imap->br_startblock == HOLESTARTBLOCK))
 			last_fsb = min(last_fsb, (xfs_fileoff_t)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/9] xfs: mark xfs_eof_alignment static
  2019-10-30 18:04 a few iomap / bmap cleanups v2 Christoph Hellwig
  2019-10-30 18:04 ` [PATCH 1/9] xfs: simplify xfs_iomap_eof_align_last_fsb Christoph Hellwig
@ 2019-10-30 18:04 ` Christoph Hellwig
  2019-10-30 18:04 ` [PATCH 3/9] xfs: remove the extsize argument to xfs_eof_alignment Christoph Hellwig
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2019-10-30 18:04 UTC (permalink / raw)
  To: linux-xfs; +Cc: Darrick J . Wong

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_iomap.c | 2 +-
 fs/xfs/xfs_iomap.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 4209c32db96d..02526cffc5a3 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -116,7 +116,7 @@ xfs_iomap_end_fsb(
 		   XFS_B_TO_FSB(mp, mp->m_super->s_maxbytes));
 }
 
-xfs_extlen_t
+static xfs_extlen_t
 xfs_eof_alignment(
 	struct xfs_inode	*ip,
 	xfs_extlen_t		extsize)
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index 7aed28275089..d5b292bdaf82 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -17,7 +17,6 @@ int xfs_iomap_write_unwritten(struct xfs_inode *, xfs_off_t, xfs_off_t, bool);
 
 int xfs_bmbt_to_iomap(struct xfs_inode *, struct iomap *,
 		struct xfs_bmbt_irec *, u16);
-xfs_extlen_t xfs_eof_alignment(struct xfs_inode *ip, xfs_extlen_t extsize);
 
 static inline xfs_filblks_t
 xfs_aligned_fsb_count(
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/9] xfs: remove the extsize argument to xfs_eof_alignment
  2019-10-30 18:04 a few iomap / bmap cleanups v2 Christoph Hellwig
  2019-10-30 18:04 ` [PATCH 1/9] xfs: simplify xfs_iomap_eof_align_last_fsb Christoph Hellwig
  2019-10-30 18:04 ` [PATCH 2/9] xfs: mark xfs_eof_alignment static Christoph Hellwig
@ 2019-10-30 18:04 ` Christoph Hellwig
  2019-10-30 19:36   ` Darrick J. Wong
  2019-10-30 18:04 ` [PATCH 4/9] xfs: slightly tweak an assert in xfs_fs_map_blocks Christoph Hellwig
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2019-10-30 18:04 UTC (permalink / raw)
  To: linux-xfs

And move the code dependent on it to the one caller that cares
instead.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_iomap.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 02526cffc5a3..c21c4f7a7389 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -118,8 +118,7 @@ xfs_iomap_end_fsb(
 
 static xfs_extlen_t
 xfs_eof_alignment(
-	struct xfs_inode	*ip,
-	xfs_extlen_t		extsize)
+	struct xfs_inode	*ip)
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	xfs_extlen_t		align = 0;
@@ -142,17 +141,6 @@ xfs_eof_alignment(
 			align = 0;
 	}
 
-	/*
-	 * Always round up the allocation request to an extent boundary
-	 * (when file on a real-time subvolume or has di_extsize hint).
-	 */
-	if (extsize) {
-		if (align)
-			align = roundup_64(align, extsize);
-		else
-			align = extsize;
-	}
-
 	return align;
 }
 
@@ -167,12 +155,22 @@ xfs_iomap_eof_align_last_fsb(
 {
 	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
 	xfs_extlen_t		extsz = xfs_get_extsz_hint(ip);
-	xfs_extlen_t		align = xfs_eof_alignment(ip, extsz);
+	xfs_extlen_t		align = xfs_eof_alignment(ip);
 	struct xfs_bmbt_irec	irec;
 	struct xfs_iext_cursor	icur;
 
 	ASSERT(ifp->if_flags & XFS_IFEXTENTS);
 
+	/*
+	 * Always round up the allocation request to the extent hint boundary.
+	 */
+	if (extsz) {
+		if (align)
+			align = roundup_64(align, extsz);
+		else
+			align = extsz;
+	}
+
 	if (align) {
 		xfs_fileoff_t	aligned_end_fsb = roundup_64(end_fsb, align);
 
@@ -992,7 +990,7 @@ xfs_buffered_write_iomap_begin(
 			p_end_fsb = XFS_B_TO_FSBT(mp, end_offset) +
 					prealloc_blocks;
 
-			align = xfs_eof_alignment(ip, 0);
+			align = xfs_eof_alignment(ip);
 			if (align)
 				p_end_fsb = roundup_64(p_end_fsb, align);
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/9] xfs: slightly tweak an assert in xfs_fs_map_blocks
  2019-10-30 18:04 a few iomap / bmap cleanups v2 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2019-10-30 18:04 ` [PATCH 3/9] xfs: remove the extsize argument to xfs_eof_alignment Christoph Hellwig
@ 2019-10-30 18:04 ` Christoph Hellwig
  2019-10-30 19:26   ` Darrick J. Wong
  2019-10-30 18:04 ` [PATCH 5/9] xfs: don't log the inode in xfs_fs_map_blocks if it wasn't modified Christoph Hellwig
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2019-10-30 18:04 UTC (permalink / raw)
  To: linux-xfs

We should never see delalloc blocks for a pNFS layout, write or not.
Adjust the assert to check for that.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_pnfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index c637f0192976..3634ffff3b07 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -148,11 +148,11 @@ xfs_fs_map_blocks(
 	if (error)
 		goto out_unlock;
 
+	ASSERT(!nimaps || imap.br_startblock != DELAYSTARTBLOCK);
+
 	if (write) {
 		enum xfs_prealloc_flags	flags = 0;
 
-		ASSERT(imap.br_startblock != DELAYSTARTBLOCK);
-
 		if (!nimaps || imap.br_startblock == HOLESTARTBLOCK) {
 			/*
 			 * xfs_iomap_write_direct() expects to take ownership of
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/9] xfs: don't log the inode in xfs_fs_map_blocks if it wasn't modified
  2019-10-30 18:04 a few iomap / bmap cleanups v2 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2019-10-30 18:04 ` [PATCH 4/9] xfs: slightly tweak an assert in xfs_fs_map_blocks Christoph Hellwig
@ 2019-10-30 18:04 ` Christoph Hellwig
  2019-10-30 19:31   ` Darrick J. Wong
  2019-10-30 18:04 ` [PATCH 6/9] xfs: simplify the xfs_iomap_write_direct calling conventions Christoph Hellwig
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2019-10-30 18:04 UTC (permalink / raw)
  To: linux-xfs

Even if we are asked for a write layout there is no point in logging
the inode unless we actually modified it in some way.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_pnfs.c | 42 ++++++++++++++++++------------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index 3634ffff3b07..ada46e9f5ff1 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -150,30 +150,24 @@ xfs_fs_map_blocks(
 
 	ASSERT(!nimaps || imap.br_startblock != DELAYSTARTBLOCK);
 
-	if (write) {
-		enum xfs_prealloc_flags	flags = 0;
-
-		if (!nimaps || imap.br_startblock == HOLESTARTBLOCK) {
-			/*
-			 * xfs_iomap_write_direct() expects to take ownership of
-			 * the shared ilock.
-			 */
-			xfs_ilock(ip, XFS_ILOCK_SHARED);
-			error = xfs_iomap_write_direct(ip, offset, length,
-						       &imap, nimaps);
-			if (error)
-				goto out_unlock;
-
-			/*
-			 * Ensure the next transaction is committed
-			 * synchronously so that the blocks allocated and
-			 * handed out to the client are guaranteed to be
-			 * present even after a server crash.
-			 */
-			flags |= XFS_PREALLOC_SET | XFS_PREALLOC_SYNC;
-		}
-
-		error = xfs_update_prealloc_flags(ip, flags);
+	if (write && (!nimaps || imap.br_startblock == HOLESTARTBLOCK)) {
+		/*
+		 * xfs_iomap_write_direct() expects to take ownership of the
+		 * shared ilock.
+		 */
+		xfs_ilock(ip, XFS_ILOCK_SHARED);
+		error = xfs_iomap_write_direct(ip, offset, length, &imap,
+					       nimaps);
+		if (error)
+			goto out_unlock;
+
+		/*
+		 * Ensure the next transaction is committed synchronously so
+		 * that the blocks allocated and handed out to the client are
+		 * guaranteed to be present even after a server crash.
+		 */
+		error = xfs_update_prealloc_flags(ip,
+				XFS_PREALLOC_SET | XFS_PREALLOC_SYNC);
 		if (error)
 			goto out_unlock;
 	}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/9] xfs: simplify the xfs_iomap_write_direct calling conventions
  2019-10-30 18:04 a few iomap / bmap cleanups v2 Christoph Hellwig
                   ` (4 preceding siblings ...)
  2019-10-30 18:04 ` [PATCH 5/9] xfs: don't log the inode in xfs_fs_map_blocks if it wasn't modified Christoph Hellwig
@ 2019-10-30 18:04 ` Christoph Hellwig
  2019-10-30 18:04 ` [PATCH 7/9] xfs: refactor xfs_bmapi_allocate Christoph Hellwig
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2019-10-30 18:04 UTC (permalink / raw)
  To: linux-xfs; +Cc: Darrick J . Wong

Move the EOF alignment and checking for the next allocated extent into
the callers to avoid the need to pass the byte based offset and count
as well as looking at the incoming imap.  The added benefit is that
the caller can unlock the incoming ilock and the function doesn't have
funny unbalanced locking contexts.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_iomap.c | 82 ++++++++++++++++------------------------------
 fs/xfs/xfs_iomap.h |  6 ++--
 fs/xfs/xfs_pnfs.c  | 25 +++++++-------
 3 files changed, 46 insertions(+), 67 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index c21c4f7a7389..217eb44c1159 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -148,7 +148,7 @@ xfs_eof_alignment(
  * Check if last_fsb is outside the last extent, and if so grow it to the next
  * stripe unit boundary.
  */
-static xfs_fileoff_t
+xfs_fileoff_t
 xfs_iomap_eof_align_last_fsb(
 	struct xfs_inode	*ip,
 	xfs_fileoff_t		end_fsb)
@@ -185,61 +185,36 @@ xfs_iomap_eof_align_last_fsb(
 
 int
 xfs_iomap_write_direct(
-	xfs_inode_t	*ip,
-	xfs_off_t	offset,
-	size_t		count,
-	xfs_bmbt_irec_t *imap,
-	int		nmaps)
+	struct xfs_inode	*ip,
+	xfs_fileoff_t		offset_fsb,
+	xfs_fileoff_t		count_fsb,
+	struct xfs_bmbt_irec	*imap)
 {
-	xfs_mount_t	*mp = ip->i_mount;
-	xfs_fileoff_t	offset_fsb = XFS_B_TO_FSBT(mp, offset);
-	xfs_fileoff_t	last_fsb = xfs_iomap_end_fsb(mp, offset, count);
-	xfs_filblks_t	count_fsb, resaligned;
-	xfs_extlen_t	extsz;
-	int		nimaps;
-	int		quota_flag;
-	int		rt;
-	xfs_trans_t	*tp;
-	uint		qblocks, resblks, resrtextents;
-	int		error;
-	int		lockmode;
-	int		bmapi_flags = XFS_BMAPI_PREALLOC;
-	uint		tflags = 0;
-
-	rt = XFS_IS_REALTIME_INODE(ip);
-	extsz = xfs_get_extsz_hint(ip);
-	lockmode = XFS_ILOCK_SHARED;	/* locked by caller */
-
-	ASSERT(xfs_isilocked(ip, lockmode));
+	struct xfs_mount	*mp = ip->i_mount;
+	struct xfs_trans	*tp;
+	xfs_filblks_t		resaligned;
+	int			nimaps;
+	int			quota_flag;
+	uint			qblocks, resblks;
+	unsigned int		resrtextents = 0;
+	int			error;
+	int			bmapi_flags = XFS_BMAPI_PREALLOC;
+	uint			tflags = 0;
 
-	if (offset + count > XFS_ISIZE(ip)) {
-		last_fsb = xfs_iomap_eof_align_last_fsb(ip, last_fsb);
-	} else {
-		if (nmaps && (imap->br_startblock == HOLESTARTBLOCK))
-			last_fsb = min(last_fsb, (xfs_fileoff_t)
-					imap->br_blockcount +
-					imap->br_startoff);
-	}
-	count_fsb = last_fsb - offset_fsb;
 	ASSERT(count_fsb > 0);
-	resaligned = xfs_aligned_fsb_count(offset_fsb, count_fsb, extsz);
 
-	if (unlikely(rt)) {
+	resaligned = xfs_aligned_fsb_count(offset_fsb, count_fsb,
+					   xfs_get_extsz_hint(ip));
+	if (unlikely(XFS_IS_REALTIME_INODE(ip))) {
 		resrtextents = qblocks = resaligned;
 		resrtextents /= mp->m_sb.sb_rextsize;
 		resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
 		quota_flag = XFS_QMOPT_RES_RTBLKS;
 	} else {
-		resrtextents = 0;
 		resblks = qblocks = XFS_DIOSTRAT_SPACE_RES(mp, resaligned);
 		quota_flag = XFS_QMOPT_RES_REGBLKS;
 	}
 
-	/*
-	 * Drop the shared lock acquired by the caller, attach the dquot if
-	 * necessary and move on to transaction setup.
-	 */
-	xfs_iunlock(ip, lockmode);
 	error = xfs_qm_dqattach(ip);
 	if (error)
 		return error;
@@ -269,8 +244,7 @@ xfs_iomap_write_direct(
 	if (error)
 		return error;
 
-	lockmode = XFS_ILOCK_EXCL;
-	xfs_ilock(ip, lockmode);
+	xfs_ilock(ip, XFS_ILOCK_EXCL);
 
 	error = xfs_trans_reserve_quota_nblks(tp, ip, qblocks, 0, quota_flag);
 	if (error)
@@ -307,7 +281,7 @@ xfs_iomap_write_direct(
 		error = xfs_alert_fsblock_zero(ip, imap);
 
 out_unlock:
-	xfs_iunlock(ip, lockmode);
+	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	return error;
 
 out_res_cancel:
@@ -807,14 +781,16 @@ xfs_direct_write_iomap_begin(
 	 * lower level functions are updated.
 	 */
 	length = min_t(loff_t, length, 1024 * PAGE_SIZE);
+	end_fsb = xfs_iomap_end_fsb(mp, offset, length);
 
-	/*
-	 * xfs_iomap_write_direct() expects the shared lock. It is unlocked on
-	 * return.
-	 */
-	if (lockmode == XFS_ILOCK_EXCL)
-		xfs_ilock_demote(ip, lockmode);
-	error = xfs_iomap_write_direct(ip, offset, length, &imap, nimaps);
+	if (offset + length > XFS_ISIZE(ip))
+		end_fsb = xfs_iomap_eof_align_last_fsb(ip, end_fsb);
+	else if (nimaps && imap.br_startblock == HOLESTARTBLOCK)
+		end_fsb = min(end_fsb, imap.br_startoff + imap.br_blockcount);
+	xfs_iunlock(ip, lockmode);
+
+	error = xfs_iomap_write_direct(ip, offset_fsb, end_fsb - offset_fsb,
+			&imap);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index d5b292bdaf82..7d3703556d0e 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -11,9 +11,11 @@
 struct xfs_inode;
 struct xfs_bmbt_irec;
 
-int xfs_iomap_write_direct(struct xfs_inode *, xfs_off_t, size_t,
-			struct xfs_bmbt_irec *, int);
+int xfs_iomap_write_direct(struct xfs_inode *ip, xfs_fileoff_t offset_fsb,
+		xfs_fileoff_t count_fsb, struct xfs_bmbt_irec *imap);
 int xfs_iomap_write_unwritten(struct xfs_inode *, xfs_off_t, xfs_off_t, bool);
+xfs_fileoff_t xfs_iomap_eof_align_last_fsb(struct xfs_inode *ip,
+		xfs_fileoff_t end_fsb);
 
 int xfs_bmbt_to_iomap(struct xfs_inode *, struct iomap *,
 		struct xfs_bmbt_irec *, u16);
diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index ada46e9f5ff1..ae3f00cb6a43 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -143,21 +143,20 @@ xfs_fs_map_blocks(
 	lock_flags = xfs_ilock_data_map_shared(ip);
 	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
 				&imap, &nimaps, bmapi_flags);
-	xfs_iunlock(ip, lock_flags);
-
-	if (error)
-		goto out_unlock;
 
 	ASSERT(!nimaps || imap.br_startblock != DELAYSTARTBLOCK);
 
-	if (write && (!nimaps || imap.br_startblock == HOLESTARTBLOCK)) {
-		/*
-		 * xfs_iomap_write_direct() expects to take ownership of the
-		 * shared ilock.
-		 */
-		xfs_ilock(ip, XFS_ILOCK_SHARED);
-		error = xfs_iomap_write_direct(ip, offset, length, &imap,
-					       nimaps);
+	if (!error && write &&
+	    (!nimaps || imap.br_startblock == HOLESTARTBLOCK)) {
+		if (offset + length > XFS_ISIZE(ip))
+			end_fsb = xfs_iomap_eof_align_last_fsb(ip, end_fsb);
+		else if (nimaps && imap.br_startblock == HOLESTARTBLOCK)
+			end_fsb = min(end_fsb, imap.br_startoff +
+					       imap.br_blockcount);
+		xfs_iunlock(ip, lock_flags);
+
+		error = xfs_iomap_write_direct(ip, offset_fsb,
+				end_fsb - offset_fsb, &imap);
 		if (error)
 			goto out_unlock;
 
@@ -170,6 +169,8 @@ xfs_fs_map_blocks(
 				XFS_PREALLOC_SET | XFS_PREALLOC_SYNC);
 		if (error)
 			goto out_unlock;
+	} else {
+		xfs_iunlock(ip, lock_flags);
 	}
 	xfs_iunlock(ip, XFS_IOLOCK_EXCL);
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 7/9] xfs: refactor xfs_bmapi_allocate
  2019-10-30 18:04 a few iomap / bmap cleanups v2 Christoph Hellwig
                   ` (5 preceding siblings ...)
  2019-10-30 18:04 ` [PATCH 6/9] xfs: simplify the xfs_iomap_write_direct calling conventions Christoph Hellwig
@ 2019-10-30 18:04 ` Christoph Hellwig
  2019-10-30 18:04 ` [PATCH 8/9] xfs: move extent zeroing to xfs_bmapi_allocate Christoph Hellwig
  2019-10-30 18:04 ` [PATCH 9/9] xfs: cleanup use of the XFS_ALLOC_ flags Christoph Hellwig
  8 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2019-10-30 18:04 UTC (permalink / raw)
  To: linux-xfs; +Cc: Darrick J . Wong

Avoid duplicate userdata and data fork checks by restructuring the code
so we only have a helper for userdata allocations that combines these
checks in a straight foward way.  That also helps to obsoletes the
comments explaining what the code does as it is now clearly obvious.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 93 +++++++++++++++++++---------------------
 1 file changed, 45 insertions(+), 48 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 392a809c13e8..1e319a4830be 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3641,20 +3641,6 @@ xfs_bmap_btalloc(
 	return 0;
 }
 
-/*
- * xfs_bmap_alloc is called by xfs_bmapi to allocate an extent for a file.
- * It figures out where to ask the underlying allocator to put the new extent.
- */
-STATIC int
-xfs_bmap_alloc(
-	struct xfs_bmalloca	*ap)	/* bmap alloc argument struct */
-{
-	if (XFS_IS_REALTIME_INODE(ap->ip) &&
-	    xfs_alloc_is_userdata(ap->datatype))
-		return xfs_bmap_rtalloc(ap);
-	return xfs_bmap_btalloc(ap);
-}
-
 /* Trim extent to fit a logical block range. */
 void
 xfs_trim_extent(
@@ -4010,6 +3996,42 @@ xfs_bmapi_reserve_delalloc(
 	return error;
 }
 
+static int
+xfs_bmap_alloc_userdata(
+	struct xfs_bmalloca	*bma)
+{
+	struct xfs_mount	*mp = bma->ip->i_mount;
+	int			whichfork = xfs_bmapi_whichfork(bma->flags);
+	int			error;
+
+	/*
+	 * Set the data type being allocated. For the data fork, the first data
+	 * in the file is treated differently to all other allocations. For the
+	 * attribute fork, we only need to ensure the allocated range is not on
+	 * the busy list.
+	 */
+	bma->datatype = XFS_ALLOC_NOBUSY;
+	if (bma->flags & XFS_BMAPI_ZERO)
+		bma->datatype |= XFS_ALLOC_USERDATA_ZERO;
+	if (whichfork == XFS_DATA_FORK) {
+		if (bma->offset == 0)
+			bma->datatype |= XFS_ALLOC_INITIAL_USER_DATA;
+		else
+			bma->datatype |= XFS_ALLOC_USERDATA;
+
+		if (mp->m_dalign && bma->length >= mp->m_dalign) {
+			error = xfs_bmap_isaeof(bma, whichfork);
+			if (error)
+				return error;
+		}
+
+		if (XFS_IS_REALTIME_INODE(bma->ip))
+			return xfs_bmap_rtalloc(bma);
+	}
+
+	return xfs_bmap_btalloc(bma);
+}
+
 static int
 xfs_bmapi_allocate(
 	struct xfs_bmalloca	*bma)
@@ -4037,43 +4059,18 @@ xfs_bmapi_allocate(
 					bma->got.br_startoff - bma->offset);
 	}
 
-	/*
-	 * Set the data type being allocated. For the data fork, the first data
-	 * in the file is treated differently to all other allocations. For the
-	 * attribute fork, we only need to ensure the allocated range is not on
-	 * the busy list.
-	 */
-	if (!(bma->flags & XFS_BMAPI_METADATA)) {
-		bma->datatype = XFS_ALLOC_NOBUSY;
-		if (whichfork == XFS_DATA_FORK) {
-			if (bma->offset == 0)
-				bma->datatype |= XFS_ALLOC_INITIAL_USER_DATA;
-			else
-				bma->datatype |= XFS_ALLOC_USERDATA;
-		}
-		if (bma->flags & XFS_BMAPI_ZERO)
-			bma->datatype |= XFS_ALLOC_USERDATA_ZERO;
-	}
-
-	bma->minlen = (bma->flags & XFS_BMAPI_CONTIG) ? bma->length : 1;
-
-	/*
-	 * Only want to do the alignment at the eof if it is userdata and
-	 * allocation length is larger than a stripe unit.
-	 */
-	if (mp->m_dalign && bma->length >= mp->m_dalign &&
-	    !(bma->flags & XFS_BMAPI_METADATA) && whichfork == XFS_DATA_FORK) {
-		error = xfs_bmap_isaeof(bma, whichfork);
-		if (error)
-			return error;
-	}
+	if (bma->flags & XFS_BMAPI_CONTIG)
+		bma->minlen = bma->length;
+	else
+		bma->minlen = 1;
 
-	error = xfs_bmap_alloc(bma);
-	if (error)
+	if (bma->flags & XFS_BMAPI_METADATA)
+		error = xfs_bmap_btalloc(bma);
+	else
+		error = xfs_bmap_alloc_userdata(bma);
+	if (error || bma->blkno == NULLFSBLOCK)
 		return error;
 
-	if (bma->blkno == NULLFSBLOCK)
-		return 0;
 	if ((ifp->if_flags & XFS_IFBROOT) && !bma->cur)
 		bma->cur = xfs_bmbt_init_cursor(mp, bma->tp, bma->ip, whichfork);
 	/*
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 8/9] xfs: move extent zeroing to xfs_bmapi_allocate
  2019-10-30 18:04 a few iomap / bmap cleanups v2 Christoph Hellwig
                   ` (6 preceding siblings ...)
  2019-10-30 18:04 ` [PATCH 7/9] xfs: refactor xfs_bmapi_allocate Christoph Hellwig
@ 2019-10-30 18:04 ` Christoph Hellwig
  2019-10-30 18:04 ` [PATCH 9/9] xfs: cleanup use of the XFS_ALLOC_ flags Christoph Hellwig
  8 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2019-10-30 18:04 UTC (permalink / raw)
  To: linux-xfs; +Cc: Darrick J . Wong

Move the extent zeroing case there for the XFS_BMAPI_ZERO flag outside
the low-level allocator and into xfs_bmapi_allocate, where is still
is in transaction context, but outside the very lowlevel code where
it doesn't belong.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_alloc.c |  7 -------
 fs/xfs/libxfs/xfs_alloc.h |  4 +---
 fs/xfs/libxfs/xfs_bmap.c  | 10 ++++++----
 fs/xfs/xfs_bmap_util.c    |  7 -------
 4 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 84866c195bfc..0539d61ff12c 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -3084,13 +3084,6 @@ xfs_alloc_vextent(
 			args->len);
 #endif
 
-		/* Zero the extent if we were asked to do so */
-		if (args->datatype & XFS_ALLOC_USERDATA_ZERO) {
-			error = xfs_zero_extent(args->ip, args->fsbno, args->len);
-			if (error)
-				goto error0;
-		}
-
 	}
 	xfs_perag_put(args->pag);
 	return 0;
diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
index d6ed5d2c07c2..626384d75c9c 100644
--- a/fs/xfs/libxfs/xfs_alloc.h
+++ b/fs/xfs/libxfs/xfs_alloc.h
@@ -54,7 +54,6 @@ typedef struct xfs_alloc_arg {
 	struct xfs_mount *mp;		/* file system mount point */
 	struct xfs_buf	*agbp;		/* buffer for a.g. freelist header */
 	struct xfs_perag *pag;		/* per-ag struct for this agno */
-	struct xfs_inode *ip;		/* for userdata zeroing method */
 	xfs_fsblock_t	fsbno;		/* file system block number */
 	xfs_agnumber_t	agno;		/* allocation group number */
 	xfs_agblock_t	agbno;		/* allocation group-relative block # */
@@ -83,8 +82,7 @@ typedef struct xfs_alloc_arg {
  */
 #define XFS_ALLOC_USERDATA		(1 << 0)/* allocation is for user data*/
 #define XFS_ALLOC_INITIAL_USER_DATA	(1 << 1)/* special case start of file */
-#define XFS_ALLOC_USERDATA_ZERO		(1 << 2)/* zero extent on allocation */
-#define XFS_ALLOC_NOBUSY		(1 << 3)/* Busy extents not allowed */
+#define XFS_ALLOC_NOBUSY		(1 << 2)/* Busy extents not allowed */
 
 static inline bool
 xfs_alloc_is_userdata(int datatype)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 1e319a4830be..ec8ecfd46a48 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3555,8 +3555,6 @@ xfs_bmap_btalloc(
 	args.wasdel = ap->wasdel;
 	args.resv = XFS_AG_RESV_NONE;
 	args.datatype = ap->datatype;
-	if (ap->datatype & XFS_ALLOC_USERDATA_ZERO)
-		args.ip = ap->ip;
 
 	error = xfs_alloc_vextent(&args);
 	if (error)
@@ -4011,8 +4009,6 @@ xfs_bmap_alloc_userdata(
 	 * the busy list.
 	 */
 	bma->datatype = XFS_ALLOC_NOBUSY;
-	if (bma->flags & XFS_BMAPI_ZERO)
-		bma->datatype |= XFS_ALLOC_USERDATA_ZERO;
 	if (whichfork == XFS_DATA_FORK) {
 		if (bma->offset == 0)
 			bma->datatype |= XFS_ALLOC_INITIAL_USER_DATA;
@@ -4071,6 +4067,12 @@ xfs_bmapi_allocate(
 	if (error || bma->blkno == NULLFSBLOCK)
 		return error;
 
+	if (bma->flags & XFS_BMAPI_ZERO) {
+		error = xfs_zero_extent(bma->ip, bma->blkno, bma->length);
+		if (error)
+			return error;
+	}
+
 	if ((ifp->if_flags & XFS_IFBROOT) && !bma->cur)
 		bma->cur = xfs_bmbt_init_cursor(mp, bma->tp, bma->ip, whichfork);
 	/*
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index be606f4e3a74..e953e7bda712 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -165,13 +165,6 @@ xfs_bmap_rtalloc(
 		xfs_trans_mod_dquot_byino(ap->tp, ap->ip,
 			ap->wasdel ? XFS_TRANS_DQ_DELRTBCOUNT :
 					XFS_TRANS_DQ_RTBCOUNT, (long) ralen);
-
-		/* Zero the extent if we were asked to do so */
-		if (ap->datatype & XFS_ALLOC_USERDATA_ZERO) {
-			error = xfs_zero_extent(ap->ip, ap->blkno, ap->length);
-			if (error)
-				return error;
-		}
 	} else {
 		ap->length = 0;
 	}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 9/9] xfs: cleanup use of the XFS_ALLOC_ flags
  2019-10-30 18:04 a few iomap / bmap cleanups v2 Christoph Hellwig
                   ` (7 preceding siblings ...)
  2019-10-30 18:04 ` [PATCH 8/9] xfs: move extent zeroing to xfs_bmapi_allocate Christoph Hellwig
@ 2019-10-30 18:04 ` Christoph Hellwig
  8 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2019-10-30 18:04 UTC (permalink / raw)
  To: linux-xfs; +Cc: Darrick J . Wong

Always set XFS_ALLOC_USERDATA for data fork allocations, and check it
in xfs_alloc_is_userdata instead of the current obsfucated check.
Also remove the xfs_alloc_is_userdata and xfs_alloc_allow_busy_reuse
helpers to make the code a little easier to understand.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_alloc.c |  8 ++++----
 fs/xfs/libxfs/xfs_alloc.h | 12 ------------
 fs/xfs/libxfs/xfs_bmap.c  | 11 +++++------
 fs/xfs/xfs_extent_busy.c  |  2 +-
 fs/xfs/xfs_filestream.c   |  2 +-
 5 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 0539d61ff12c..b8d48d5fa6a5 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -331,7 +331,7 @@ xfs_alloc_compute_diff(
 	xfs_extlen_t	newlen1=0;	/* length with newbno1 */
 	xfs_extlen_t	newlen2=0;	/* length with newbno2 */
 	xfs_agblock_t	wantend;	/* end of target extent */
-	bool		userdata = xfs_alloc_is_userdata(datatype);
+	bool		userdata = datatype & XFS_ALLOC_USERDATA;
 
 	ASSERT(freelen >= wantlen);
 	freeend = freebno + freelen;
@@ -1041,9 +1041,9 @@ xfs_alloc_ag_vextent_small(
 		goto out;
 
 	xfs_extent_busy_reuse(args->mp, args->agno, fbno, 1,
-			      xfs_alloc_allow_busy_reuse(args->datatype));
+			      (args->datatype & XFS_ALLOC_NOBUSY));
 
-	if (xfs_alloc_is_userdata(args->datatype)) {
+	if (args->datatype & XFS_ALLOC_USERDATA) {
 		struct xfs_buf	*bp;
 
 		bp = xfs_btree_get_bufs(args->mp, args->tp, args->agno, fbno);
@@ -2381,7 +2381,7 @@ xfs_alloc_fix_freelist(
 	 * somewhere else if we are not being asked to try harder at this
 	 * point
 	 */
-	if (pag->pagf_metadata && xfs_alloc_is_userdata(args->datatype) &&
+	if (pag->pagf_metadata && (args->datatype & XFS_ALLOC_USERDATA) &&
 	    (flags & XFS_ALLOC_FLAG_TRYLOCK)) {
 		ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));
 		goto out_agbp_relse;
diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
index 626384d75c9c..7380fbe4a3ff 100644
--- a/fs/xfs/libxfs/xfs_alloc.h
+++ b/fs/xfs/libxfs/xfs_alloc.h
@@ -84,18 +84,6 @@ typedef struct xfs_alloc_arg {
 #define XFS_ALLOC_INITIAL_USER_DATA	(1 << 1)/* special case start of file */
 #define XFS_ALLOC_NOBUSY		(1 << 2)/* Busy extents not allowed */
 
-static inline bool
-xfs_alloc_is_userdata(int datatype)
-{
-	return (datatype & ~XFS_ALLOC_NOBUSY) != 0;
-}
-
-static inline bool
-xfs_alloc_allow_busy_reuse(int datatype)
-{
-	return (datatype & XFS_ALLOC_NOBUSY) == 0;
-}
-
 /* freespace limit calculations */
 #define XFS_ALLOC_AGFL_RESERVE	4
 unsigned int xfs_alloc_set_aside(struct xfs_mount *mp);
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index ec8ecfd46a48..11ec3e0f3d59 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3059,7 +3059,7 @@ xfs_bmap_adjacent(
 	mp = ap->ip->i_mount;
 	nullfb = ap->tp->t_firstblock == NULLFSBLOCK;
 	rt = XFS_IS_REALTIME_INODE(ap->ip) &&
-		xfs_alloc_is_userdata(ap->datatype);
+		(ap->datatype & XFS_ALLOC_USERDATA);
 	fb_agno = nullfb ? NULLAGNUMBER : XFS_FSB_TO_AGNO(mp,
 							ap->tp->t_firstblock);
 	/*
@@ -3412,7 +3412,7 @@ xfs_bmap_btalloc(
 
 	if (ap->flags & XFS_BMAPI_COWFORK)
 		align = xfs_get_cowextsz_hint(ap->ip);
-	else if (xfs_alloc_is_userdata(ap->datatype))
+	else if (ap->datatype & XFS_ALLOC_USERDATA)
 		align = xfs_get_extsz_hint(ap->ip);
 	if (align) {
 		error = xfs_bmap_extsize_align(mp, &ap->got, &ap->prev,
@@ -3427,7 +3427,7 @@ xfs_bmap_btalloc(
 	fb_agno = nullfb ? NULLAGNUMBER : XFS_FSB_TO_AGNO(mp,
 							ap->tp->t_firstblock);
 	if (nullfb) {
-		if (xfs_alloc_is_userdata(ap->datatype) &&
+		if ((ap->datatype & XFS_ALLOC_USERDATA) &&
 		    xfs_inode_is_filestream(ap->ip)) {
 			ag = xfs_filestream_lookup_ag(ap->ip);
 			ag = (ag != NULLAGNUMBER) ? ag : 0;
@@ -3467,7 +3467,7 @@ xfs_bmap_btalloc(
 		 * enough for the request.  If one isn't found, then adjust
 		 * the minimum allocation size to the largest space found.
 		 */
-		if (xfs_alloc_is_userdata(ap->datatype) &&
+		if ((ap->datatype & XFS_ALLOC_USERDATA) &&
 		    xfs_inode_is_filestream(ap->ip))
 			error = xfs_bmap_btalloc_filestreams(ap, &args, &blen);
 		else
@@ -4010,10 +4010,9 @@ xfs_bmap_alloc_userdata(
 	 */
 	bma->datatype = XFS_ALLOC_NOBUSY;
 	if (whichfork == XFS_DATA_FORK) {
+		bma->datatype |= XFS_ALLOC_USERDATA;
 		if (bma->offset == 0)
 			bma->datatype |= XFS_ALLOC_INITIAL_USER_DATA;
-		else
-			bma->datatype |= XFS_ALLOC_USERDATA;
 
 		if (mp->m_dalign && bma->length >= mp->m_dalign) {
 			error = xfs_bmap_isaeof(bma, whichfork);
diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c
index 2183d87be4cf..3991e59cfd18 100644
--- a/fs/xfs/xfs_extent_busy.c
+++ b/fs/xfs/xfs_extent_busy.c
@@ -367,7 +367,7 @@ xfs_extent_busy_trim(
 		 * If this is a metadata allocation, try to reuse the busy
 		 * extent instead of trimming the allocation.
 		 */
-		if (!xfs_alloc_is_userdata(args->datatype) &&
+		if (!(args->datatype & XFS_ALLOC_USERDATA) &&
 		    !(busyp->flags & XFS_EXTENT_BUSY_DISCARDED)) {
 			if (!xfs_extent_busy_update_extent(args->mp, args->pag,
 							  busyp, fbno, flen,
diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
index 574a7a8b4736..2ae356775f63 100644
--- a/fs/xfs/xfs_filestream.c
+++ b/fs/xfs/xfs_filestream.c
@@ -374,7 +374,7 @@ xfs_filestream_new_ag(
 		startag = (item->ag + 1) % mp->m_sb.sb_agcount;
 	}
 
-	if (xfs_alloc_is_userdata(ap->datatype))
+	if (ap->datatype & XFS_ALLOC_USERDATA)
 		flags |= XFS_PICK_USERDATA;
 	if (ap->tp->t_flags & XFS_TRANS_LOWMODE)
 		flags |= XFS_PICK_LOWSPACE;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/9] xfs: slightly tweak an assert in xfs_fs_map_blocks
  2019-10-30 18:04 ` [PATCH 4/9] xfs: slightly tweak an assert in xfs_fs_map_blocks Christoph Hellwig
@ 2019-10-30 19:26   ` Darrick J. Wong
  0 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2019-10-30 19:26 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Wed, Oct 30, 2019 at 11:04:14AM -0700, Christoph Hellwig wrote:
> We should never see delalloc blocks for a pNFS layout, write or not.
> Adjust the assert to check for that.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_pnfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
> index c637f0192976..3634ffff3b07 100644
> --- a/fs/xfs/xfs_pnfs.c
> +++ b/fs/xfs/xfs_pnfs.c
> @@ -148,11 +148,11 @@ xfs_fs_map_blocks(
>  	if (error)
>  		goto out_unlock;
>  
> +	ASSERT(!nimaps || imap.br_startblock != DELAYSTARTBLOCK);
> +
>  	if (write) {
>  		enum xfs_prealloc_flags	flags = 0;
>  
> -		ASSERT(imap.br_startblock != DELAYSTARTBLOCK);
> -
>  		if (!nimaps || imap.br_startblock == HOLESTARTBLOCK) {
>  			/*
>  			 * xfs_iomap_write_direct() expects to take ownership of
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/9] xfs: don't log the inode in xfs_fs_map_blocks if it wasn't modified
  2019-10-30 18:04 ` [PATCH 5/9] xfs: don't log the inode in xfs_fs_map_blocks if it wasn't modified Christoph Hellwig
@ 2019-10-30 19:31   ` Darrick J. Wong
  0 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2019-10-30 19:31 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Wed, Oct 30, 2019 at 11:04:15AM -0700, Christoph Hellwig wrote:
> Even if we are asked for a write layout there is no point in logging
> the inode unless we actually modified it in some way.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks fine...

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_pnfs.c | 42 ++++++++++++++++++------------------------
>  1 file changed, 18 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
> index 3634ffff3b07..ada46e9f5ff1 100644
> --- a/fs/xfs/xfs_pnfs.c
> +++ b/fs/xfs/xfs_pnfs.c
> @@ -150,30 +150,24 @@ xfs_fs_map_blocks(
>  
>  	ASSERT(!nimaps || imap.br_startblock != DELAYSTARTBLOCK);
>  
> -	if (write) {
> -		enum xfs_prealloc_flags	flags = 0;
> -
> -		if (!nimaps || imap.br_startblock == HOLESTARTBLOCK) {
> -			/*
> -			 * xfs_iomap_write_direct() expects to take ownership of
> -			 * the shared ilock.
> -			 */
> -			xfs_ilock(ip, XFS_ILOCK_SHARED);
> -			error = xfs_iomap_write_direct(ip, offset, length,
> -						       &imap, nimaps);
> -			if (error)
> -				goto out_unlock;
> -
> -			/*
> -			 * Ensure the next transaction is committed
> -			 * synchronously so that the blocks allocated and
> -			 * handed out to the client are guaranteed to be
> -			 * present even after a server crash.
> -			 */
> -			flags |= XFS_PREALLOC_SET | XFS_PREALLOC_SYNC;
> -		}
> -
> -		error = xfs_update_prealloc_flags(ip, flags);
> +	if (write && (!nimaps || imap.br_startblock == HOLESTARTBLOCK)) {
> +		/*
> +		 * xfs_iomap_write_direct() expects to take ownership of the
> +		 * shared ilock.
> +		 */
> +		xfs_ilock(ip, XFS_ILOCK_SHARED);
> +		error = xfs_iomap_write_direct(ip, offset, length, &imap,
> +					       nimaps);
> +		if (error)
> +			goto out_unlock;
> +
> +		/*
> +		 * Ensure the next transaction is committed synchronously so
> +		 * that the blocks allocated and handed out to the client are
> +		 * guaranteed to be present even after a server crash.
> +		 */
> +		error = xfs_update_prealloc_flags(ip,
> +				XFS_PREALLOC_SET | XFS_PREALLOC_SYNC);
>  		if (error)
>  			goto out_unlock;
>  	}
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/9] xfs: remove the extsize argument to xfs_eof_alignment
  2019-10-30 18:04 ` [PATCH 3/9] xfs: remove the extsize argument to xfs_eof_alignment Christoph Hellwig
@ 2019-10-30 19:36   ` Darrick J. Wong
  0 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2019-10-30 19:36 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Wed, Oct 30, 2019 at 11:04:13AM -0700, Christoph Hellwig wrote:
> And move the code dependent on it to the one caller that cares
> instead.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_iomap.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 02526cffc5a3..c21c4f7a7389 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -118,8 +118,7 @@ xfs_iomap_end_fsb(
>  
>  static xfs_extlen_t
>  xfs_eof_alignment(
> -	struct xfs_inode	*ip,
> -	xfs_extlen_t		extsize)
> +	struct xfs_inode	*ip)
>  {
>  	struct xfs_mount	*mp = ip->i_mount;
>  	xfs_extlen_t		align = 0;
> @@ -142,17 +141,6 @@ xfs_eof_alignment(
>  			align = 0;
>  	}
>  
> -	/*
> -	 * Always round up the allocation request to an extent boundary
> -	 * (when file on a real-time subvolume or has di_extsize hint).
> -	 */
> -	if (extsize) {
> -		if (align)
> -			align = roundup_64(align, extsize);
> -		else
> -			align = extsize;
> -	}
> -
>  	return align;
>  }
>  
> @@ -167,12 +155,22 @@ xfs_iomap_eof_align_last_fsb(
>  {
>  	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
>  	xfs_extlen_t		extsz = xfs_get_extsz_hint(ip);
> -	xfs_extlen_t		align = xfs_eof_alignment(ip, extsz);
> +	xfs_extlen_t		align = xfs_eof_alignment(ip);
>  	struct xfs_bmbt_irec	irec;
>  	struct xfs_iext_cursor	icur;
>  
>  	ASSERT(ifp->if_flags & XFS_IFEXTENTS);
>  
> +	/*
> +	 * Always round up the allocation request to the extent hint boundary.
> +	 */
> +	if (extsz) {
> +		if (align)
> +			align = roundup_64(align, extsz);
> +		else
> +			align = extsz;
> +	}
> +
>  	if (align) {
>  		xfs_fileoff_t	aligned_end_fsb = roundup_64(end_fsb, align);
>  
> @@ -992,7 +990,7 @@ xfs_buffered_write_iomap_begin(
>  			p_end_fsb = XFS_B_TO_FSBT(mp, end_offset) +
>  					prealloc_blocks;
>  
> -			align = xfs_eof_alignment(ip, 0);
> +			align = xfs_eof_alignment(ip);
>  			if (align)
>  				p_end_fsb = roundup_64(p_end_fsb, align);
>  
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-10-30 19:36 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-30 18:04 a few iomap / bmap cleanups v2 Christoph Hellwig
2019-10-30 18:04 ` [PATCH 1/9] xfs: simplify xfs_iomap_eof_align_last_fsb Christoph Hellwig
2019-10-30 18:04 ` [PATCH 2/9] xfs: mark xfs_eof_alignment static Christoph Hellwig
2019-10-30 18:04 ` [PATCH 3/9] xfs: remove the extsize argument to xfs_eof_alignment Christoph Hellwig
2019-10-30 19:36   ` Darrick J. Wong
2019-10-30 18:04 ` [PATCH 4/9] xfs: slightly tweak an assert in xfs_fs_map_blocks Christoph Hellwig
2019-10-30 19:26   ` Darrick J. Wong
2019-10-30 18:04 ` [PATCH 5/9] xfs: don't log the inode in xfs_fs_map_blocks if it wasn't modified Christoph Hellwig
2019-10-30 19:31   ` Darrick J. Wong
2019-10-30 18:04 ` [PATCH 6/9] xfs: simplify the xfs_iomap_write_direct calling conventions Christoph Hellwig
2019-10-30 18:04 ` [PATCH 7/9] xfs: refactor xfs_bmapi_allocate Christoph Hellwig
2019-10-30 18:04 ` [PATCH 8/9] xfs: move extent zeroing to xfs_bmapi_allocate Christoph Hellwig
2019-10-30 18:04 ` [PATCH 9/9] xfs: cleanup use of the XFS_ALLOC_ flags Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).