On Wed, May 14, 2025 at 11:42:40PM +0000, Catherine Hoang wrote: > > On May 14, 2025, at 8:38 AM, Darrick J. Wong <djwong@xxxxxxxxxx> wrote: > > > > On Wed, May 14, 2025 at 01:47:20PM +0100, John Garry wrote: > >> On 14/05/2025 01:29, Catherine Hoang wrote: > >>> From: "Darrick J. Wong" <djwong@xxxxxxxxxx> > >>> > >>> Fix a few bugs in the single block atomic writes test, such as requiring > >>> directio, using page size for the ext4 max bsize, and making sure we check > >>> the max atomic write size. > >>> > >>> Cc: ritesh.list@xxxxxxxxx > >>> Signed-off-by: Catherine Hoang <catherine.hoang@xxxxxxxxxx> > >>> Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx> > >>> Signed-off-by: "Darrick J. Wong" <djwong@xxxxxxxxxx> > >>> --- > >>> common/rc | 2 +- > >>> tests/generic/765 | 4 ++-- > >>> 2 files changed, 3 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/common/rc b/common/rc > >>> index 657772e7..bc8dabc5 100644 > >>> --- a/common/rc > >>> +++ b/common/rc > >>> @@ -2989,7 +2989,7 @@ _require_xfs_io_command() > >>> fi > >>> if [ "$param" == "-A" ]; then > >>> opts+=" -d" > >>> - pwrite_opts+="-D -V 1 -b 4k" > >>> + pwrite_opts+="-d -V 1 -b 4k" > >> > >> according to the documentation for -b, 4096 is the default (so I don't think > >> that we need to set it explicitly). But is that flag even relevant to > >> pwritev2? > > > > The documentation is wrong -- on XFS the default is the fs blocksize. > > Everywhere else is 4k. > > > >> And setting -d in pwrite_opts means DIO for the input file, right? I am not > >> sure if that is required. > > > > It's not required, I mistook where that "-d" goes -- -d as an argument > > to xfs_io is necessary, but -d as an argument to the pwrite subcommand > > is not. It's also benign since we don't pass -i. > > > > Curiously the version of this patch in my tree doesn't have the extra > > -d... I wonder if I made that change and forgot to send it out. > > Hmm, it might have been from an old patch on my branch > that I forgot to update when I sent this out. Just to clarify, > this should just be > > pwrite_opts+="-V 1 -b 4k” > > right? Yep. --D > > --D > > > >>> fi > >>> testio=`$XFS_IO_PROG -f $opts -c \ > >>> "pwrite $pwrite_opts $param 0 4k" $testfile 2>&1` > >>> diff --git a/tests/generic/765 b/tests/generic/765 > >>> index 9bab3b8a..8695a306 100755 > >>> --- a/tests/generic/765 > >>> +++ b/tests/generic/765 > >>> @@ -28,7 +28,7 @@ get_supported_bsize() > >>> ;; > >>> "ext4") > >>> min_bsize=1024 > >>> - max_bsize=4096 > >>> + max_bsize=$(_get_page_size) > >> > >> looks ok > >> > >>> ;; > >>> *) > >>> _notrun "$FSTYP does not support atomic writes" > >>> @@ -73,7 +73,7 @@ test_atomic_writes() > >>> # Check that atomic min/max = FS block size > >>> test $file_min_write -eq $bsize || \ > >>> echo "atomic write min $file_min_write, should be fs block size $bsize" > >>> - test $file_min_write -eq $bsize || \ > >>> + test $file_max_write -eq $bsize || \ > >> > >> looks ok > >> > >>> echo "atomic write max $file_max_write, should be fs block size $bsize" > >>> test $file_max_segments -eq 1 || \ > >>> echo "atomic write max segments $file_max_segments, should be 1" > >> > >> > >> Thanks, > >> John > >
