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. --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 >