xfs: Block size > PAGE_SIZE support

From: Dave Chinner <[email protected]>

->writepage is only used in one place - single page writeback from memory reclaim. We only allow such writeback from kswapd, not from direct memory reclaim, and so it is rarely used. When it comes from kswapd, it is effectively random dirty page shoot-down, which is horrible for IO patterns. We will already have background writeback trying to clean all the dirty pages in memory as efficiently as possible, so having kswapd interrupt our well formed IO stream only slows things down. So get rid of xfs_vm_writepage() completely.

[adding linux-mm to the CC list]

Quoted 10 lines · Dave Chinner

From: Dave Chinner <[email protected]>

->writepage is only used in one place - single page writeback from memory reclaim. We only allow such writeback from kswapd, not from direct memory reclaim, and so it is rarely used. When it comes from kswapd, it is effectively random dirty page shoot-down, which is horrible for IO patterns. We will already have background writeback trying to clean all the dirty pages in memory as efficiently as possible, so having kswapd interrupt our well formed IO stream only slows things down. So get rid of xfs_vm_writepage() completely.

Interesting. IFF we can pull this off it would simplify a lot of things, so I'm generally in favor of it.

Quoted 14 lines · Christoph Hellwig

[adding linux-mm to the CC list]

Quoted 10 lines · Dave Chinner

From: Dave Chinner <[email protected]>

->writepage is only used in one place - single page writeback from memory reclaim. We only allow such writeback from kswapd, not from direct memory reclaim, and so it is rarely used. When it comes from kswapd, it is effectively random dirty page shoot-down, which is horrible for IO patterns. We will already have background writeback trying to clean all the dirty pages in memory as efficiently as possible, so having kswapd interrupt our well formed IO stream only slows things down. So get rid of xfs_vm_writepage() completely.

Quoted 2 lines

Interesting. IFF we can pull this off it would simplify a lot of things, so I'm generally in favor of it.

Over the past few days of hammeringon this, the only thing I've noticed is that page reclaim hangs up less, but it's also putting a bit more pressure on the shrinkers. Filesystem intensive workloads that drive the machine into reclaim via the page cache seem to hit breakdown conditions slightly earlier and the impact is that the shrinkers are run harder. Mostly I see this as the XFS buffer cache having a much harder time keeping a working set active.

However, while the workloads hit the working set cache, writeback performance does seem to be slightly higher. It is, however, being offset by the deeper lows that come from the cache being turned over.

So there's a bit of rebalancing to be done here as a followup, but I've been unable to drive the system into unexepected OOM kills or other bad behaviour as a result of removing ->writepage.

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

From: Dave Chinner <[email protected]>

The last AG may be very small comapred to all other AGs, and hence AG reservations based on the superblock AG size may actually consume more space than the AG actually has. This results on assert failures like:

XFS: Assertion failed: xfs_perag_resv(pag, XFS_AG_RESV_METADATA)->ar_reserved + xfs_perag_resv(pag, XFS_AG_RESV_RMAPBT)->ar_reserved <= pag->pagf_freeblks + pag->pagf_flcount, file: fs/xfs/libxfs/xfs_ag_resv.c, line: 319

[   48.932891]  xfs_ag_resv_init+0x1bd/0x1d0
[   48.933853]  xfs_fs_reserve_ag_blocks+0x37/0xb0
[   48.934939]  xfs_mountfs+0x5b3/0x920
[   48.935804]  xfs_fs_fill_super+0x462/0x640
[   48.936784]  ? xfs_test_remount_options+0x60/0x60
[   48.937908]  mount_bdev+0x178/0x1b0
[   48.938751]  mount_fs+0x36/0x170
[   48.939533]  vfs_kern_mount.part.43+0x54/0x130
[   48.940596]  do_mount+0x20e/0xcb0
[   48.941396]  ? memdup_user+0x3e/0x70
[   48.942249]  ksys_mount+0xba/0xd0
[   48.943046]  __x64_sys_mount+0x21/0x30
[   48.943953]  do_syscall_64+0x54/0x170
[   48.944835]  entry_SYSCALL_64_after_hwframe+0x49/0xbe

Hence we need to ensure the finobt per-ag space reservations take into account the size of the last AG rather than treat it like all the other full size AGs.

Note that both refcountbt and rmapbt already take the size of the AG into account via reading the AGF length directly.

Signed-off-by Dave Chinner <[email protected]>

---
 fs/xfs/libxfs/xfs_ialloc_btree.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

fs/xfs/libxfs/xfs_ialloc_btree.c +3 -1

index 86c50208a143..62014780d5e4 100644
@@ -538,15 +538,24 @@ xfs_inobt_rec_check_count(
 
 static xfs_extlen_t
 xfs_inobt_max_size(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	xfs_agnumber_t		agno)
 {
+	uint32_t		agblocks = mp->m_sb.sb_agblocks;

fs/xfs/libxfs/xfs_ialloc_btree.c +4 -3

+
 	return xfs_btree_calc_size(mp->m_inobt_mnr,
-		(uint64_t)mp->m_sb.sb_agblocks * mp->m_sb.sb_inopblock /
-				XFS_INODES_PER_CHUNK);
+				(uint64_t)agblocks * mp->m_sb.sb_inopblock /
+					XFS_INODES_PER_CHUNK);
 }
 
 static int

@@ -594,7 +603,7 @@ xfs_finobt_calc_reserves(
 	if (error)
 		return error;
 
-	*ask += xfs_inobt_max_size(mp);
+	*ask += xfs_inobt_max_size(mp, agno);
 	*used += tree_len;
 	return 0;
 }

Signature · 1 line

2.19.1

Quoted 53 lines · Dave Chinner

From: Dave Chinner <[email protected]>

The last AG may be very small comapred to all other AGs, and hence AG reservations based on the superblock AG size may actually consume more space than the AG actually has. This results on assert failures like:

XFS: Assertion failed: xfs_perag_resv(pag, XFS_AG_RESV_METADATA)->ar_reserved + xfs_perag_resv(pag, XFS_AG_RESV_RMAPBT)->ar_reserved <= pag->pagf_freeblks + pag->pagf_flcount, file: fs/xfs/libxfs/xfs_ag_resv.c, line: 319

[   48.932891]  xfs_ag_resv_init+0x1bd/0x1d0
[   48.933853]  xfs_fs_reserve_ag_blocks+0x37/0xb0
[   48.934939]  xfs_mountfs+0x5b3/0x920
[   48.935804]  xfs_fs_fill_super+0x462/0x640
[   48.936784]  ? xfs_test_remount_options+0x60/0x60
[   48.937908]  mount_bdev+0x178/0x1b0
[   48.938751]  mount_fs+0x36/0x170
[   48.939533]  vfs_kern_mount.part.43+0x54/0x130
[   48.940596]  do_mount+0x20e/0xcb0
[   48.941396]  ? memdup_user+0x3e/0x70
[   48.942249]  ksys_mount+0xba/0xd0
[   48.943046]  __x64_sys_mount+0x21/0x30
[   48.943953]  do_syscall_64+0x54/0x170
[   48.944835]  entry_SYSCALL_64_after_hwframe+0x49/0xbe

Hence we need to ensure the finobt per-ag space reservations take into account the size of the last AG rather than treat it like all the other full size AGs.

Note that both refcountbt and rmapbt already take the size of the AG into account via reading the AGF length directly.

Signed-off-by Dave Chinner <[email protected]>

---
 fs/xfs/libxfs/xfs_ialloc_btree.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

fs/xfs/libxfs/xfs_ialloc_btree.c +4 -1

index 86c50208a143..62014780d5e4 100644
@@ -538,15 +538,24 @@ xfs_inobt_rec_check_count(
 
 static xfs_extlen_t
 xfs_inobt_max_size(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	xfs_agnumber_t		agno)
 {
+	uint32_t		agblocks = mp->m_sb.sb_agblocks;
+
 	/* Bail out if we're uninitialized, which can happen in mkfs. */
 	if (mp->m_inobt_mxr[0] == 0)
 		return 0;
 

fs/xfs/libxfs/xfs_ialloc_btree.c xfs_inobt_rec_check_count( · line 554

Quoted 5 lines

fs/xfs/libxfs/xfs_ialloc_btree.c +5 -0

+	/* last AG can be a runt */
+	if (agno == mp->m_sb.sb_agcount - 1) {
+		div_u64_rem(mp->m_sb.sb_dblocks, mp->m_sb.sb_agblocks,
+				&agblocks);
+	}

Why not: agblocks = xfs_ag_block_count(mp, agno); ?

From: Dave Chinner <[email protected]>

If we are soing sub-block dio that extends EOF, we need to zero the unused tail of the block to initialise the data in it it. If we do not zero the tail of the block, then an immediate mmap read of the EOF block will expose stale data beyond EOF to userspace. Found with fsx running sub-block DIO sizes vs MAPREAD/MAPWRITE operations.

Fix this by detecting if the end of the DIO write is beyond EOF and zeroing the tail if necessary.

Signed-off-by Dave Chinner <[email protected]>

---
 fs/iomap.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

fs/iomap.c +0 -0

index 64ce240217a1..16d16596b00f 100644

From: Dave Chinner <[email protected]>

iomap_dio_rw() has all the infrastructure in place to support block size > page size filesystems because it is essentially just sub-block DIO. It needs help, however, with the sub-block zeroing code (needs multi-page IOs) page cache invalidation over the block being written.

Signed-off-by Dave Chinner <[email protected]>

---
 fs/iomap.c | 65 ++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 49 insertions(+), 16 deletions(-)

fs/iomap.c +0 -0

index 16d16596b00f..8878b1f1f9c7 100644

fs/iomap.c +44 -16

 static blk_qc_t
 iomap_dio_zero(struct iomap_dio *dio, struct iomap *iomap, loff_t pos,
 		unsigned len)
 {
 	struct page *page = ZERO_PAGE(0);
 	struct bio *bio;
+	int npages = howmany(len, PAGE_SIZE);
+
+	WARN_ON_ONCE(npages > 16);
 
-	bio = bio_alloc(GFP_KERNEL, 1);
+	bio = bio_alloc(GFP_KERNEL, npages);
 	bio_set_dev(bio, iomap->bdev);
 	bio->bi_iter.bi_sector = iomap_sector(iomap, pos);
 	bio->bi_private = dio;
 	bio->bi_end_io = iomap_dio_bio_end_io;
 
-	get_page(page);
-	__bio_add_page(bio, page, len, 0);
+	while (npages-- > 0) {
+		unsigned plen = min_t(unsigned, PAGE_SIZE, len);
+		get_page(page);
+		__bio_add_page(bio, page, plen, 0);
+		len -= plen;
+	}
+	WARN_ON(len != 0);
 	bio_set_op_attrs(bio, REQ_OP_WRITE, REQ_SYNC | REQ_IDLE);
 
 	atomic_inc(&dio->ref);

@@ -1752,6 +1765,38 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
 	}
 }
 
+/*
+ * This is lifted almost straight from xfs_flush_unmap_range(). Need a generic
+ * version of the block size rounding for these purposes.
+ */
+static int
+iomap_flush_unmap_range(struct file *f, loff_t offset, loff_t len)
+{
+	struct inode *inode = file_inode(f);
+	loff_t rounding, start, end;
+	int ret;
+
+	rounding = max_t(loff_t, i_blocksize(inode), PAGE_SIZE);
+	start = round_down(offset, rounding);
+	end = round_up(offset + len, rounding) - 1;
+
+	ret = filemap_write_and_wait_range(inode->i_mapping, start, end);
+	if (ret)
+		return ret;
+
+	/*
+	 * Try to invalidate cache pages for the range we're direct
+	 * writing.  If this invalidation fails, tough, the write will
+	 * still work, but racing two incompatible write paths is a
+	 * pretty crazy thing to do, so we don't support it 100%.
+	 */
+	ret = invalidate_inode_pages2_range(inode->i_mapping,
+			start >> PAGE_SHIFT, end >> PAGE_SHIFT);
+	if (ret)
+		dio_warn_stale_pagecache(f);
+	return 0;
+}
+
 /*
  * iomap_dio_rw() always completes O_[D]SYNC writes regardless of whether the IO
  * is being issued as AIO or not.  This allows us to optimise pure data writes

@@ -1829,22 +1874,10 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 		flags |= IOMAP_NOWAIT;
 	}
 
-	ret = filemap_write_and_wait_range(mapping, start, end);
+	ret = iomap_flush_unmap_range(iocb->ki_filp, start, end);
 	if (ret)
 		goto out_free_dio;
 
-	/*
-	 * Try to invalidate cache pages for the range we're direct
-	 * writing.  If this invalidation fails, tough, the write will
-	 * still work, but racing two incompatible write paths is a
-	 * pretty crazy thing to do, so we don't support it 100%.
-	 */
-	ret = invalidate_inode_pages2_range(mapping,
-			start >> PAGE_SHIFT, end >> PAGE_SHIFT);
-	if (ret)
-		dio_warn_stale_pagecache(iocb->ki_filp);
-	ret = 0;
-
 	if (iov_iter_rw(iter) == WRITE && !dio->wait_for_completion &&
 	    !inode->i_sb->s_dio_done_wq) {
 		ret = sb_init_dio_done_wq(inode->i_sb);

+/*
+ * This is lifted almost straight from xfs_flush_unmap_range(). Need a generic
+ * version of the block size rounding for these purposes.
+ */

fs/iomap.c static void iomap_dio_bio_end_io(struct bio *bio) · line 1564

Quoted 9 lines

static blk_qc_t
iomap_dio_zero(struct iomap_dio *dio, struct iomap *iomap, loff_t pos,

fs/iomap.c +3 -0

 		unsigned len)
 {
 	struct page *page = ZERO_PAGE(0);
 	struct bio *bio;
+	int npages = howmany(len, PAGE_SIZE);
+
+	WARN_ON_ONCE(npages > 16);

Where does this magic 16 come from?

fs/iomap.c static void iomap_dio_bio_end_io(struct bio *bio) · line 1578

Quoted 1 line

fs/iomap.c +1 -0

+	WARN_ON(len != 0);

WARN_ON_ONCE please to avoid making the log unreadable if it ever triggers.

Quoted 4 lines

+/*
+ * This is lifted almost straight from xfs_flush_unmap_range(). Need a generic
+ * version of the block size rounding for these purposes.
+ */

Can you just add a generic version of this in a separate patch and also switch XFS over to it?

include/linux/iomap.h static inline struct iomap_page *to_iomap_page(struct page *page) · line 128

Quoted 7 lines

include/linux/iomap.h +7 -0

+/*
+ * Return the block size we should use for page cache based operations.
+ * This will return the inode block size for block size < PAGE_SIZE,
+ * otherwise it will return PAGE_SIZE.
+ */
+static inline unsigned
+iomap_pageblock_size(struct inode *inode)

Maybe cal this a chunk size instead?

fs/xfs/xfs_aops.c +3 -0

+	unsigned		bsize =	i_blocksize(mapping->host);
 
 	/*
 	 * Refuse to write pages out if we are called from reclaim context.

@@ -922,6 +923,19 @@ xfs_vm_writepages(
 	if (WARN_ON_ONCE(current->flags & PF_MEMALLOC_NOFS))
 		return 0;
 
+	/*
+	 * If the block size is larger than page size, extent the incoming write

Quoted 8 lines

fs/xfs/xfs_aops.c +1 -0

+	unsigned		bsize =	i_blocksize(mapping->host);
 
 	/*
 	 * Refuse to write pages out if we are called from reclaim context.

@@ -922,6 +923,19 @@ xfs_vm_writepages(
 	if (WARN_ON_ONCE(current->flags & PF_MEMALLOC_NOFS))
 		return 0;
 

fs/xfs/xfs_aops.c xfs_vm_writepages( · line 937

Quoted 12 lines

fs/xfs/xfs_aops.c +12 -0

+	/*
+	 * If the block size is larger than page size, extent the incoming write
+	 * request to fsb granularity and alignment. This is a requirement for
+	 * data integrity operations and it doesn't hurt for other write
+	 * operations, so do it unconditionally.
+	 */
+	if (wbc->range_start)
+		wbc->range_start = round_down(wbc->range_start, bsize);
+	if (wbc->range_end != LLONG_MAX)
+		wbc->range_end = round_up(wbc->range_end, bsize);
+	if (wbc->nr_to_write < wbc->range_end - wbc->range_start)
+		wbc->nr_to_write = round_up(wbc->nr_to_write, bsize);

This looks fine to me, but I'd be much more comfortable it we had it in the common writeback code instead of inside XFS.

Quoted 9 lines

fs/xfs/xfs_aops.c xfs_vm_writepages( · line 937

Quoted 6 lines

fs/xfs/xfs_aops.c +6 -0

+	if (wbc->range_start)
+		wbc->range_start = round_down(wbc->range_start, bsize);
+	if (wbc->range_end != LLONG_MAX)
+		wbc->range_end = round_up(wbc->range_end, bsize);
+	if (wbc->nr_to_write < wbc->range_end - wbc->range_start)
+		wbc->nr_to_write = round_up(wbc->nr_to_write, bsize);

This looks fine to me, but I'd be much more comfortable it we had it in the common writeback code instead of inside XFS.

Where in the common code? there's quite a few places that can call ->writepages...

From: Dave Chinner <[email protected]>

For data integrity purposes, we need to write back the entire filesystem block when asked to sync a sub-block range of the file. When the filesystem block size is larger than the page size, this means we need to convert single page integrity writes into whole block integrity writes. We do this by extending the writepage range to filesystem block granularity and alignment.

Signed-off-by Dave Chinner <[email protected]>

---
 fs/xfs/xfs_aops.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

fs/xfs/xfs_aops.c +4 -0

index f6ef9e0a7312..5334f16be166 100644
@@ -900,6 +900,7 @@ xfs_vm_writepages(
 		.io_type = XFS_IO_HOLE,
 	};
 	int			ret;
+	unsigned		bsize =	i_blocksize(mapping->host);
 
 	/*
 	 * Refuse to write pages out if we are called from reclaim context.

@@ -922,6 +923,19 @@ xfs_vm_writepages(
 	if (WARN_ON_ONCE(current->flags & PF_MEMALLOC_NOFS))
 		return 0;
 
+	/*
+	 * If the block size is larger than page size, extent the incoming write
+	 * request to fsb granularity and alignment. This is a requirement for

fs/xfs/xfs_aops.c +0 -0

 	xfs_iflags_clear(XFS_I(mapping->host), XFS_ITRUNCATED);
 	ret = write_cache_pages(mapping, wbc, xfs_do_writepage, &wpc);
 	if (wpc.ioend)

Signature · 1 line

2.19.1

Quoted 30 lines · Dave Chinner

From: Dave Chinner <[email protected]>

For data integrity purposes, we need to write back the entire filesystem block when asked to sync a sub-block range of the file. When the filesystem block size is larger than the page size, this means we need to convert single page integrity writes into whole block integrity writes. We do this by extending the writepage range to filesystem block granularity and alignment.

Signed-off-by Dave Chinner <[email protected]>

---
 fs/xfs/xfs_aops.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

fs/xfs/xfs_aops.c +1 -0

index f6ef9e0a7312..5334f16be166 100644
@@ -900,6 +900,7 @@ xfs_vm_writepages(
 		.io_type = XFS_IO_HOLE,
 	};
 	int			ret;
+	unsigned		bsize =	i_blocksize(mapping->host);
 
 	/*
 	 * Refuse to write pages out if we are called from reclaim context.

@@ -922,6 +923,19 @@ xfs_vm_writepages(
 	if (WARN_ON_ONCE(current->flags & PF_MEMALLOC_NOFS))
 		return 0;
 

fs/xfs/xfs_aops.c xfs_vm_writepages( · line 938

Quoted 13 lines

fs/xfs/xfs_aops.c +13 -0

+	/*
+	 * If the block size is larger than page size, extent the incoming write
+	 * request to fsb granularity and alignment. This is a requirement for
+	 * data integrity operations and it doesn't hurt for other write
+	 * operations, so do it unconditionally.
+	 */
+	if (wbc->range_start)
+		wbc->range_start = round_down(wbc->range_start, bsize);
+	if (wbc->range_end != LLONG_MAX)
+		wbc->range_end = round_up(wbc->range_end, bsize);
+	if (wbc->nr_to_write < wbc->range_end - wbc->range_start)
+		wbc->nr_to_write = round_up(wbc->nr_to_write, bsize);
+

This latter bit causes endless writeback loops in tests such as generic/475 (I think I reproduced it with xfs/141 as well). The

writeback infrastructure samples ->nr_to_write before and after ->writepages() calls to identify progress. Unconditionally bumping it to something larger than the original value can lead to an underflow in the writeback code that seems to throw things off. E.g., see the following wb tracepoints (w/ 4k block and page size):

   kworker/u8:13-189   [003] ...1   317.968147: writeback_single_inode_start: bdi 253:9: ino=8389005 state=I_DIRTY_PAGES|I_SYNC dirtied_when=4294773087 age=211 index=0 to_write=1024 wrote=0 cgroup_ino=4294967295
   kworker/u8:13-189   [003] ...1   317.968150: writeback_single_inode: bdi 253:9: ino=8389005 state=I_DIRTY_PAGES|I_SYNC dirtied_when=4294773087 age=211 index=0 to_write=1024 wrote=18446744073709548544 cgroup_ino=4294967295

The wrote value goes from 0 to garbage and writeback_sb_inodes() uses the same basic calculation for 'wrote.'

BTW, I haven't gone through the broader set, but just looking at this bit what's the purpose of rounding ->nr_to_write (which is a page count) to a block size in the first place?

Quoted 31 lines · Brian Foster

Quoted 30 lines · Dave Chinner

From: Dave Chinner <[email protected]>

For data integrity purposes, we need to write back the entire filesystem block when asked to sync a sub-block range of the file. When the filesystem block size is larger than the page size, this means we need to convert single page integrity writes into whole block integrity writes. We do this by extending the writepage range to filesystem block granularity and alignment.

Signed-off-by Dave Chinner <[email protected]>

---
 fs/xfs/xfs_aops.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

fs/xfs/xfs_aops.c +1 -0

index f6ef9e0a7312..5334f16be166 100644
@@ -900,6 +900,7 @@ xfs_vm_writepages(
 		.io_type = XFS_IO_HOLE,
 	};
 	int			ret;
+	unsigned		bsize =	i_blocksize(mapping->host);
 
 	/*
 	 * Refuse to write pages out if we are called from reclaim context.

@@ -922,6 +923,19 @@ xfs_vm_writepages(
 	if (WARN_ON_ONCE(current->flags & PF_MEMALLOC_NOFS))
 		return 0;
 

Quoted 16 lines

fs/xfs/xfs_aops.c xfs_vm_writepages( · line 938

Quoted 13 lines

fs/xfs/xfs_aops.c +13 -0

+	/*
+	 * If the block size is larger than page size, extent the incoming write
+	 * request to fsb granularity and alignment. This is a requirement for
+	 * data integrity operations and it doesn't hurt for other write
+	 * operations, so do it unconditionally.
+	 */
+	if (wbc->range_start)
+		wbc->range_start = round_down(wbc->range_start, bsize);
+	if (wbc->range_end != LLONG_MAX)
+		wbc->range_end = round_up(wbc->range_end, bsize);
+	if (wbc->nr_to_write < wbc->range_end - wbc->range_start)
+		wbc->nr_to_write = round_up(wbc->nr_to_write, bsize);
+

This latter bit causes endless writeback loops in tests such as generic/475 (I think I reproduced it with xfs/141 as well). The

Yup, I've seen that, but haven't fixed it yet because I still haven't climbed out of the dedupe/clone/copy file range data corruption hole that fsx pulled the lid of.

Basically, I can't get back to working on bs > ps until I get the stuff we actually support working correctly first...

Quoted 11 lines

writeback infrastructure samples ->nr_to_write before and after ->writepages() calls to identify progress. Unconditionally bumping it to something larger than the original value can lead to an underflow in the writeback code that seems to throw things off. E.g., see the following wb tracepoints (w/ 4k block and page size):

   kworker/u8:13-189   [003] ...1   317.968147: writeback_single_inode_start: bdi 253:9: ino=8389005 state=I_DIRTY_PAGES|I_SYNC dirtied_when=4294773087 age=211 index=0 to_write=1024 wrote=0 cgroup_ino=4294967295
   kworker/u8:13-189   [003] ...1   317.968150: writeback_single_inode: bdi 253:9: ino=8389005 state=I_DIRTY_PAGES|I_SYNC dirtied_when=4294773087 age=211 index=0 to_write=1024 wrote=18446744073709548544 cgroup_ino=4294967295

The wrote value goes from 0 to garbage and writeback_sb_inodes() uses the same basic calculation for 'wrote.'

Easy enough to fix, just stash the originals and restore them once done.

Quoted 4 lines

BTW, I haven't gone through the broader set, but just looking at this bit what's the purpose of rounding ->nr_to_write (which is a page count) to a block size in the first place?

fsync on a single page range.

We write that page, allocate the block (which spans 16 pages), and then return from writeback leaving 15/16 pages on that block still dirty in memory. Then we force the log, pushing the allocation and metadata to disk. Crash.

On recovery, we expose 15/16 pages of stale data because we only wrote one of the pages over the block during fsync.

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

Quoted 83 lines · Dave Chinner

Quoted 31 lines · Brian Foster

Quoted 30 lines · Dave Chinner

From: Dave Chinner <[email protected]>

For data integrity purposes, we need to write back the entire filesystem block when asked to sync a sub-block range of the file. When the filesystem block size is larger than the page size, this means we need to convert single page integrity writes into whole block integrity writes. We do this by extending the writepage range to filesystem block granularity and alignment.

Signed-off-by Dave Chinner <[email protected]>

---
 fs/xfs/xfs_aops.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

fs/xfs/xfs_aops.c +1 -0

index f6ef9e0a7312..5334f16be166 100644
@@ -900,6 +900,7 @@ xfs_vm_writepages(
 		.io_type = XFS_IO_HOLE,
 	};
 	int			ret;
+	unsigned		bsize =	i_blocksize(mapping->host);
 
 	/*
 	 * Refuse to write pages out if we are called from reclaim context.

@@ -922,6 +923,19 @@ xfs_vm_writepages(
 	if (WARN_ON_ONCE(current->flags & PF_MEMALLOC_NOFS))
 		return 0;
 

Quoted 16 lines

fs/xfs/xfs_aops.c xfs_vm_writepages( · line 938

Quoted 13 lines

fs/xfs/xfs_aops.c +13 -0

+	/*
+	 * If the block size is larger than page size, extent the incoming write
+	 * request to fsb granularity and alignment. This is a requirement for
+	 * data integrity operations and it doesn't hurt for other write
+	 * operations, so do it unconditionally.
+	 */
+	if (wbc->range_start)
+		wbc->range_start = round_down(wbc->range_start, bsize);
+	if (wbc->range_end != LLONG_MAX)
+		wbc->range_end = round_up(wbc->range_end, bsize);
+	if (wbc->nr_to_write < wbc->range_end - wbc->range_start)
+		wbc->nr_to_write = round_up(wbc->nr_to_write, bsize);
+

This latter bit causes endless writeback loops in tests such as generic/475 (I think I reproduced it with xfs/141 as well). The

Yup, I've seen that, but haven't fixed it yet because I still haven't climbed out of the dedupe/clone/copy file range data corruption hole that fsx pulled the lid of.

Basically, I can't get back to working on bs > ps until I get the stuff we actually support working correctly first...

Quoted 11 lines

writeback infrastructure samples ->nr_to_write before and after ->writepages() calls to identify progress. Unconditionally bumping it to something larger than the original value can lead to an underflow in the writeback code that seems to throw things off. E.g., see the following wb tracepoints (w/ 4k block and page size):

   kworker/u8:13-189   [003] ...1   317.968147: writeback_single_inode_start: bdi 253:9: ino=8389005 state=I_DIRTY_PAGES|I_SYNC dirtied_when=4294773087 age=211 index=0 to_write=1024 wrote=0 cgroup_ino=4294967295
   kworker/u8:13-189   [003] ...1   317.968150: writeback_single_inode: bdi 253:9: ino=8389005 state=I_DIRTY_PAGES|I_SYNC dirtied_when=4294773087 age=211 index=0 to_write=1024 wrote=18446744073709548544 cgroup_ino=4294967295

The wrote value goes from 0 to garbage and writeback_sb_inodes() uses the same basic calculation for 'wrote.'

Easy enough to fix, just stash the originals and restore them once done.

Quoted 4 lines

BTW, I haven't gone through the broader set, but just looking at this bit what's the purpose of rounding ->nr_to_write (which is a page count) to a block size in the first place?

fsync on a single page range.

We write that page, allocate the block (which spans 16 pages), and then return from writeback leaving 15/16 pages on that block still dirty in memory. Then we force the log, pushing the allocation and metadata to disk. Crash.

Quoted 3 lines

On recovery, we expose 15/16 pages of stale data because we only wrote one of the pages over the block during fsync.

But why the block size and not something that represents pages per block? Note again that ->nr_to_write is a page count and you're comparing and aligning it with fields that are byte offsets.

IOW, an
integrity write isn't limited by page count since it's critical to flush the specified range. Unless I'm misunderstanding how this field is used in the writeback code, ISTM that there shouldn't be any need to muck with nr_to_write here for the purposes of fsync.

Quoted 84 lines · Brian Foster

Quoted 83 lines · Dave Chinner

Quoted 31 lines · Brian Foster

Quoted 30 lines · Dave Chinner

From: Dave Chinner <[email protected]>

For data integrity purposes, we need to write back the entire filesystem block when asked to sync a sub-block range of the file. When the filesystem block size is larger than the page size, this means we need to convert single page integrity writes into whole block integrity writes. We do this by extending the writepage range to filesystem block granularity and alignment.

Signed-off-by Dave Chinner <[email protected]>

---
 fs/xfs/xfs_aops.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

fs/xfs/xfs_aops.c +1 -0

index f6ef9e0a7312..5334f16be166 100644
@@ -900,6 +900,7 @@ xfs_vm_writepages(
 		.io_type = XFS_IO_HOLE,
 	};
 	int			ret;
+	unsigned		bsize =	i_blocksize(mapping->host);
 
 	/*
 	 * Refuse to write pages out if we are called from reclaim context.

@@ -922,6 +923,19 @@ xfs_vm_writepages(
 	if (WARN_ON_ONCE(current->flags & PF_MEMALLOC_NOFS))
 		return 0;
 

Quoted 16 lines

fs/xfs/xfs_aops.c xfs_vm_writepages( · line 938

Quoted 13 lines

fs/xfs/xfs_aops.c +13 -0

+	/*
+	 * If the block size is larger than page size, extent the incoming write
+	 * request to fsb granularity and alignment. This is a requirement for
+	 * data integrity operations and it doesn't hurt for other write
+	 * operations, so do it unconditionally.
+	 */
+	if (wbc->range_start)
+		wbc->range_start = round_down(wbc->range_start, bsize);
+	if (wbc->range_end != LLONG_MAX)
+		wbc->range_end = round_up(wbc->range_end, bsize);
+	if (wbc->nr_to_write < wbc->range_end - wbc->range_start)
+		wbc->nr_to_write = round_up(wbc->nr_to_write, bsize);
+

This latter bit causes endless writeback loops in tests such as generic/475 (I think I reproduced it with xfs/141 as well). The

Yup, I've seen that, but haven't fixed it yet because I still haven't climbed out of the dedupe/clone/copy file range data corruption hole that fsx pulled the lid of.

Basically, I can't get back to working on bs > ps until I get the stuff we actually support working correctly first...

Quoted 11 lines

writeback infrastructure samples ->nr_to_write before and after ->writepages() calls to identify progress. Unconditionally bumping it to something larger than the original value can lead to an underflow in the writeback code that seems to throw things off. E.g., see the following wb tracepoints (w/ 4k block and page size):

   kworker/u8:13-189   [003] ...1   317.968147: writeback_single_inode_start: bdi 253:9: ino=8389005 state=I_DIRTY_PAGES|I_SYNC dirtied_when=4294773087 age=211 index=0 to_write=1024 wrote=0 cgroup_ino=4294967295
   kworker/u8:13-189   [003] ...1   317.968150: writeback_single_inode: bdi 253:9: ino=8389005 state=I_DIRTY_PAGES|I_SYNC dirtied_when=4294773087 age=211 index=0 to_write=1024 wrote=18446744073709548544 cgroup_ino=4294967295

The wrote value goes from 0 to garbage and writeback_sb_inodes() uses the same basic calculation for 'wrote.'

Easy enough to fix, just stash the originals and restore them once done.

Quoted 4 lines

BTW, I haven't gone through the broader set, but just looking at this bit what's the purpose of rounding ->nr_to_write (which is a page count) to a block size in the first place?

fsync on a single page range.

We write that page, allocate the block (which spans 16 pages), and then return from writeback leaving 15/16 pages on that block still dirty in memory. Then we force the log, pushing the allocation and metadata to disk. Crash.

Quoted 7 lines

Quoted 3 lines

On recovery, we expose 15/16 pages of stale data because we only wrote one of the pages over the block during fsync.

But why the block size and not something that represents pages per block? Note again that ->nr_to_write is a page count and you're comparing and aligning it with fields that are byte offsets.

Not sure I follow you - block size is directly related to the number of pages per block. You can't calculate one without the other...

/me goes back an looks at the code, finally it dawns on him that the bug is he's rounding to block size, not (block size >> PAGE_SHIFT). I'll fix that.

Quoted 5 lines

IOW, an
integrity write isn't limited by page count since it's critical to flush the specified range. Unless I'm misunderstanding how this field is used in the writeback code, ISTM that there shouldn't be any need to muck with nr_to_write here for the purposes of fsync.

Sorry, I thought you were asking about why were were rounding the range in general, not nr_to_write specifically.

We don't need to modify nr_to_write for WB_SYNC_ALL, write_cache_pages() will not terminate on a nr_to_write expiry. So, unlike the range rounding, the nr_to_write rounding is really only for WB_SYNC_NONE.

i.e. we round WB_SYNC_NONE to try to get whole blocks written so we don't end up with partially written blocks on disk for extended periods of time (e.g. between background writeback periods). It doesn't matter for WB_SYNC_ALL, but it will reduce the potential for stale data beign exposed when crashes occur and random pages in blocks have't been written back. i.e. it's to help iprevent re-exposing the problematic cases that we added the "NULL files after crash" workarounds for.

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

Quoted 9 lines · Dave Chinner

i.e. we round WB_SYNC_NONE to try to get whole blocks written so we don't end up with partially written blocks on disk for extended periods of time (e.g. between background writeback periods). It doesn't matter for WB_SYNC_ALL, but it will reduce the potential for stale data beign exposed when crashes occur and random pages in blocks have't been written back. i.e. it's to help iprevent re-exposing the problematic cases that we added the "NULL files after crash" workarounds for.

Ok. I see that there are earlier patches to do zero-around on sub-block writes, so the idea makes a bit more sense with that in mind. That said, I still don't grok how messing with nr_to_write is effective.

For background writeback (WB_SYNC_NONE), the range fields are clamped out (0-LONG_MAX) since the location of pages to write is not really a concern. In that case, ->nr_to_write is set based on some bandwidth heuristic and the only change we make here is to round it. If we consider the fact that any mapping itself may consist of some combination of zeroed-around delalloc blocks (covered by an aligned number of dirty pages) and already allocated/overwrite blocks (covered by any number of dirty pages), how does a rounded ->nr_to_write actually help us at all? Can't the magic ->nr_to_write value that prevents stopping at a partially written sub-block page be unaligned to the block size?

Given the above, I don't see how tweaking ->nr_to_write really helps at all even as an optimization. Unless I'm missing something else in the earlier patches that facilitate this, ISTM that something more explicit is required if you want to increase the odds that zeroed-around blocks are written together.

Hi folks,

We've had a fair number of problems reported on 64k block size filesystems of late, but none of the XFS developers have Power or ARM machines handy to reproduce them or even really test the fixes.

The iomap infrastructure we introduced a while back was designed with the capabity of block size > page size support in mind, but we hadn't tried to implement it.

So after another 64k block size bug report late last week I said to Darrick "How hard could it be"?

About 6 billion (yes, B) fsx ops later, I have most of the XFS functionality working on 64k block sizes on x86_64. Buffered read/write, mmap read/write and direct IO read/write all work. All the fallocate() operations work correctly, as does truncate. xfsdump and xfs_restore are happy with it, as is xfs_repair. xfs-scrub needed some help, but I've tested Darrick's fixes for that quite a bit over the past few days.

It passes most of xfstests - there's some test failures that I have to determine whether they are code bugs or test problems (i.e. some tests don't deal with 64k block sizes correctly or assume block size <= page size).

What I haven't tested yet is shared extents - the COW path, clone_file_range and dedupe_file_range. I discovered earlier today that fsx doesn't support copy/clone/dedupe_file_operations operations, so before I go any further I need to enxpahnce fsx. Then

fix all the bugs it uncovers on block size <= page size filesystems. And then I'll move onto adding the rest of the functionality this patch set requires.

The main addition to the iomap code to support this functionality is the "zero-around" capability. When the filesystem is mapping a new block, a delalloc range or an unwritten extent, it sets the IOMAP_F_ZERO_AROUND flag in the iomap it returns. This tells the iomap code that it needs to expand the range of the operation being performed to cover entire blocks. i.e. if the data being written doesn't span the filesystem block, it needs to instantiate and zero pages in the page cache to cover the portions of the block the data doesn't cover.

Because the page cache may already hold data for the regions (e.g. read over a hole/unwritten extent) the zero-around code does not zero pages that are already marked up to date. It is assumed that whatever put those pages into the page cache has already done the right thing with them.

Yes, this means the unit of page cache IO is still individual pages. There are no mm/ changes at all, no page cache changes, nothing. That all still just operates on individual pages and is oblivious to the fact the filesystem/iomap codeis now processing gangs of pages at a time instead of just one.

Actually, I stretch the truth there a little - there is one change to XFS that is important to note here. I removed ->writepage from XFS (patches 1 and 2). We can't use it for large block sizes because we want to write whole blocks at a time if they are newly allocated or unwritten. And really, it's just a nasty hack that gets in the way of background writeback doing an efficient job of cleaning dirty pages. So I killed it.

We also need to expose the large block size to stat(2). If we don't, the applications that use stat.st_bsize for operations that require block size alignment (e.g. various fallocate ops) then fail because 4k is not the block size of a 64k block size filesystem.

A number of latent bugs in existing code were uncovered as I worked through this - patches 3-5 fix bugs in XFS and iomap that can be triggered on existing systems but it's somewhat hard to expose them.

Patches 6-12 introduce all the iomap infrastructure needed to support block size > page size.

Patches 13-16 introduce the necessary functionality to trigger the iompa infrastructure, tell userspace the right thing, make sub-block fsync ranges do the right thing and finally remove the code that prevents large block size filesystems from mounting on small page size machines.

It works, it seems pretty robust and runs enough of fstests that I've already used it to find, fix and test a 64k block size bug in XFS:

https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?h=for-next&id=837514f7a4ca4aca06aec5caa5ff56d33ef06976

I think this is the last of the XFS Irix features we haven't implemented in Linux XFS - it's only taken us 20 years to get here, but the end of the tunnel is in sight.

Nah, it's probably a train. Or maybe a flame. :)

Anyway, I'm interested to see what people think of the approach.

Cheers,

Dave.

Quoted 6 lines · Dave Chinner

Hi folks,

We've had a fair number of problems reported on 64k block size filesystems of late, but none of the XFS developers have Power or ARM machines handy to reproduce them or even really test the fixes.

Quoted 6 lines

The iomap infrastructure we introduced a while back was designed with the capabity of block size > page size support in mind, but we hadn't tried to implement it.

So after another 64k block size bug report late last week I said to Darrick "How hard could it be"?

"Nothing is ever simple" :)

Quoted 13 lines

About 6 billion (yes, B) fsx ops later, I have most of the XFS functionality working on 64k block sizes on x86_64. Buffered read/write, mmap read/write and direct IO read/write all work. All the fallocate() operations work correctly, as does truncate. xfsdump and xfs_restore are happy with it, as is xfs_repair. xfs-scrub needed some help, but I've tested Darrick's fixes for that quite a bit over the past few days.

It passes most of xfstests - there's some test failures that I have to determine whether they are code bugs or test problems (i.e. some tests don't deal with 64k block sizes correctly or assume block size <= page size).

Quoted 4 lines

What I haven't tested yet is shared extents - the COW path, clone_file_range and dedupe_file_range. I discovered earlier today that fsx doesn't support copy/clone/dedupe_file_operations operations, so before I go any further I need to enxpahnce fsx. Then

I assume that means you only tested this on reflink=0 filesystems?

Looking at fsstress, it looks like we don't test copy_file_range either. I can try adding the missing clone/dedupe/copy to both programs, but maybe you've already done that while I was asleep?

Quoted 7 lines · Darrick J. Wong

Quoted 6 lines · Dave Chinner

Hi folks,

We've had a fair number of problems reported on 64k block size filesystems of late, but none of the XFS developers have Power or ARM machines handy to reproduce them or even really test the fixes.

Quoted 8 lines

Quoted 6 lines

The iomap infrastructure we introduced a while back was designed with the capabity of block size > page size support in mind, but we hadn't tried to implement it.

So after another 64k block size bug report late last week I said to Darrick "How hard could it be"?

"Nothing is ever simple" :)

"It'll only take a couple of minutes!"

Quoted 13 lines

Quoted 13 lines

About 6 billion (yes, B) fsx ops later, I have most of the XFS functionality working on 64k block sizes on x86_64. Buffered read/write, mmap read/write and direct IO read/write all work. All the fallocate() operations work correctly, as does truncate. xfsdump and xfs_restore are happy with it, as is xfs_repair. xfs-scrub needed some help, but I've tested Darrick's fixes for that quite a bit over the past few days.

It passes most of xfstests - there's some test failures that I have to determine whether they are code bugs or test problems (i.e. some tests don't deal with 64k block sizes correctly or assume block size <= page size).

Quoted 6 lines

Quoted 4 lines

What I haven't tested yet is shared extents - the COW path, clone_file_range and dedupe_file_range. I discovered earlier today that fsx doesn't support copy/clone/dedupe_file_operations operations, so before I go any further I need to enxpahnce fsx. Then

I assume that means you only tested this on reflink=0 filesystems?

Correct.

Quoted 3 lines

Looking at fsstress, it looks like we don't test copy_file_range either. I can try adding the missing clone/dedupe/copy to both programs, but maybe you've already done that while I was asleep?

No, I haven't started on this yet. I've been sleeping. :P

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

Quoted 45 lines · Dave Chinner

Quoted 7 lines · Darrick J. Wong

Quoted 6 lines · Dave Chinner

Hi folks,

We've had a fair number of problems reported on 64k block size filesystems of late, but none of the XFS developers have Power or ARM machines handy to reproduce them or even really test the fixes.

Quoted 8 lines

Quoted 6 lines

The iomap infrastructure we introduced a while back was designed with the capabity of block size > page size support in mind, but we hadn't tried to implement it.

So after another 64k block size bug report late last week I said to Darrick "How hard could it be"?

"Nothing is ever simple" :)

"It'll only take a couple of minutes!"

Quoted 13 lines

Quoted 13 lines

About 6 billion (yes, B) fsx ops later, I have most of the XFS functionality working on 64k block sizes on x86_64. Buffered read/write, mmap read/write and direct IO read/write all work. All the fallocate() operations work correctly, as does truncate. xfsdump and xfs_restore are happy with it, as is xfs_repair. xfs-scrub needed some help, but I've tested Darrick's fixes for that quite a bit over the past few days.

It passes most of xfstests - there's some test failures that I have to determine whether they are code bugs or test problems (i.e. some tests don't deal with 64k block sizes correctly or assume block size <= page size).

Quoted 6 lines

Quoted 4 lines

What I haven't tested yet is shared extents - the COW path, clone_file_range and dedupe_file_range. I discovered earlier today that fsx doesn't support copy/clone/dedupe_file_operations operations, so before I go any further I need to enxpahnce fsx. Then

I assume that means you only tested this on reflink=0 filesystems?

Correct.

Quoted 3 lines

Looking at fsstress, it looks like we don't test copy_file_range either. I can try adding the missing clone/dedupe/copy to both programs, but maybe you've already done that while I was asleep?

Quoted 1 line

No, I haven't started on this yet. I've been sleeping. :P

I started wondering if we were missing anything from not having fsx support clone/dedupe and ended up with:

https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/log/?h=fsstress-clone

Quoted 46 lines · Darrick J. Wong

Quoted 45 lines · Dave Chinner

Quoted 7 lines · Darrick J. Wong

Quoted 6 lines · Dave Chinner

Hi folks,

We've had a fair number of problems reported on 64k block size filesystems of late, but none of the XFS developers have Power or ARM machines handy to reproduce them or even really test the fixes.

Quoted 8 lines

Quoted 6 lines

The iomap infrastructure we introduced a while back was designed with the capabity of block size > page size support in mind, but we hadn't tried to implement it.

So after another 64k block size bug report late last week I said to Darrick "How hard could it be"?

"Nothing is ever simple" :)

"It'll only take a couple of minutes!"

Quoted 13 lines

Quoted 13 lines

About 6 billion (yes, B) fsx ops later, I have most of the XFS functionality working on 64k block sizes on x86_64. Buffered read/write, mmap read/write and direct IO read/write all work. All the fallocate() operations work correctly, as does truncate. xfsdump and xfs_restore are happy with it, as is xfs_repair. xfs-scrub needed some help, but I've tested Darrick's fixes for that quite a bit over the past few days.

It passes most of xfstests - there's some test failures that I have to determine whether they are code bugs or test problems (i.e. some tests don't deal with 64k block sizes correctly or assume block size <= page size).

Quoted 6 lines

Quoted 4 lines

What I haven't tested yet is shared extents - the COW path, clone_file_range and dedupe_file_range. I discovered earlier today that fsx doesn't support copy/clone/dedupe_file_operations operations, so before I go any further I need to enxpahnce fsx. Then

I assume that means you only tested this on reflink=0 filesystems?

Correct.

Quoted 3 lines

Looking at fsstress, it looks like we don't test copy_file_range either. I can try adding the missing clone/dedupe/copy to both programs, but maybe you've already done that while I was asleep?

Quoted 6 lines

Quoted 1 line

No, I haven't started on this yet. I've been sleeping. :P

I started wondering if we were missing anything from not having fsx support clone/dedupe and ended up with:

https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/log/?h=fsstress-clone

Some fixes to that below.

I haven't got to testing dedupe or clone - copy_file_range explodes in under 40 operations in on generic/263. do_splice_direct() looks to be broken in several different waysat this point.

Cheers,

Dave.

Signature · 19 lines

Dave Chinner
[email protected]

fsx: clean up copy/dedupe file range support.

From: Dave Chinner <[email protected]>

copy_file_range() needs to obey read/write constraints otherwise is
blows up when direct IO is used.

FIDEDUPERANGE has a completely screwed up API for error reporting.
The ioctl succeeds even if dedupe fails, so you have to check
every individual dedupe operations for failure. Without this, dedupe
"succeeds" on kernels filesystems that don't even support dedupe...

Signed-off-by: Dave Chinner <[email protected]>
---
 ltp/fsx.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

ltp/fsx.c +5 -1

index fad50e0022af..b51910b8b2e1 100644
@@ -1382,7 +1382,11 @@ do_dedupe_range(unsigned offset, unsigned length, unsigned dest)
 	fdr->info[0].dest_fd = fd;
 	fdr->info[0].dest_offset = dest;
 
-	if (ioctl(fd, FIDEDUPERANGE, fdr) == -1) {
+	if (ioctl(fd, FIDEDUPERANGE, fdr) == -1 ||
+	    fdr->info[0].status < 0) {
+		if (fdr->info[0].status < 0)
+			errno = -fdr->info[0].status;
+
 		if (errno == EOPNOTSUPP || errno == ENOTTY) {
 			if (!quiet && testcalls > simulatedopcount)
 				prt("skipping unsupported dedupe range\n");

ltp/fsx.c +1 -0

+
 	if (length == 0) {
 		if (!quiet && testcalls > simulatedopcount)
 			prt("skipping zero length copy range\n");

Quoted 5 lines · Dave Chinner

On Wed, Nov 07, 2018 at 05:38:43PM -0800, Darrick J. Wong wrote:

ltp/fsx.c +0 -0

@@ -1416,6 +1420,11 @@ do_copy_range(unsigned offset, unsigned length, unsigned dest)
 	loff_t o1, o2;
 	ssize_t nr;
 

ltp/fsx.c do_copy_range(unsigned offset, unsigned length, unsigned dest) · line 1426

Quoted 4 lines

ltp/fsx.c +4 -0

+	offset -= offset % readbdy;
+	dest -= dest % writebdy;
+	if (o_direct)
+		length -= length % readbdy;

Don't we want byte-granularity copies if we're doing buffered copies?