diff mbox series

[4/5] xfs_copy: actually do directio writes to block devices

Message ID 170309218416.1607770.6525312328250244890.stgit@frogsfrogsfrogs (mailing list archive)
State Deferred, archived
Headers show
Series [1/5] libfrog: move 64-bit division wrappers to libfrog | expand

Commit Message

Darrick J. Wong Dec. 20, 2023, 5:12 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Not sure why block device targets don't get O_DIRECT in !buffered mode,
but it's misleading when the copy completes instantly only to stall
forever due to fsync-on-close.  Adjust the "write last sector" code to
allocate a properly aligned buffer.

In removing the onstack buffer for EOD writes, this also corrects the
buffer being larger than necessary -- the old code declared an array of
32768 pointers, whereas all we really need is an aligned 32768-byte
buffer.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 copy/xfs_copy.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Christoph Hellwig Dec. 21, 2023, 5:30 a.m. UTC | #1 
On Wed, Dec 20, 2023 at 09:12:28AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Not sure why block device targets don't get O_DIRECT in !buffered mode,
> but it's misleading when the copy completes instantly only to stall
> forever due to fsync-on-close.  Adjust the "write last sector" code to
> allocate a properly aligned buffer.
> 
> In removing the onstack buffer for EOD writes, this also corrects the
> buffer being larger than necessary -- the old code declared an array of
> 32768 pointers, whereas all we really need is an aligned 32768-byte
> buffer.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/copy/xfs_copy.c b/copy/xfs_copy.c
index d9a14a95..bcc807ed 100644
--- a/copy/xfs_copy.c
+++ b/copy/xfs_copy.c
@@ -832,13 +832,9 @@  main(int argc, char **argv)
 			do_out(_("Creating file %s\n"), target[i].name);
 
 			open_flags |= O_CREAT;
-			if (!buffered_output)
-				open_flags |= O_DIRECT;
 			write_last_block = 1;
 		} else if (S_ISREG(statbuf.st_mode))  {
 			open_flags |= O_TRUNC;
-			if (!buffered_output)
-				open_flags |= O_DIRECT;
 			write_last_block = 1;
 		} else  {
 			/*
@@ -855,6 +851,8 @@  main(int argc, char **argv)
 				exit(1);
 			}
 		}
+		if (!buffered_output)
+			open_flags |= O_DIRECT;
 
 		target[i].fd = open(target[i].name, open_flags, 0644);
 		if (target[i].fd < 0)  {
@@ -887,14 +885,15 @@  main(int argc, char **argv)
 				}
 			}
 		} else  {
-			char	*lb[XFS_MAX_SECTORSIZE] = { NULL };
+			char	*lb = memalign(wbuf_align, XFS_MAX_SECTORSIZE);
 			off64_t	off;
 			ssize_t	len;
 
 			/* ensure device files are sufficiently large */
+			memset(lb, 0, XFS_MAX_SECTORSIZE);
 
 			off = mp->m_sb.sb_dblocks * source_blocksize;
-			off -= sizeof(lb);
+			off -= XFS_MAX_SECTORSIZE;
 			len = pwrite(target[i].fd, lb, XFS_MAX_SECTORSIZE, off);
 			if (len < 0) {
 				do_log(_("%s:  failed to write last block\n"),
@@ -911,6 +910,7 @@  main(int argc, char **argv)
 					target[i].name);
 				exit(1);
 			}
+			free(lb);
 		}
 	}