On Sun, Jul 28, 2024 at 3:28 PM Zorro Lang <zlang@xxxxxxxxxx> wrote:
>
> On Sat, Jul 27, 2024 at 12:28:16PM +0100, fdmanana@xxxxxxxxxx wrote:
> > From: Filipe Manana <fdmanana@xxxxxxxx>
> >
> > Test that doing a direct IO append write to a file when the input buffer
> > was not yet faulted in, does not result in an incorrect file size.
> >
> > This exercises a bug on btrfs reported by users and which is fixed by
> > the following kernel patch:
> >
> > "btrfs: fix corruption after buffer fault in during direct IO append write"
> >
> > Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
> > ---
> >
> > V2: Deal with partial writes by looping and writing any remaining data.
> > Don't exit when the first test fails, instead let the second test
> > run as well.
>
> With this change I got two error lines this time [1]. Last time (V1) I
> only got "Wrong file size after first write, got 8192 expected 4096".
Yes, it's expected.
As the changelog for v2 says, now the second test is run even if the
first one failed.
> Does this mean this v2 change help this case to be better?
I prefer it like that.
It's common in fstests to let all steps of a test run if possible
(i.e. we don't exit, call _fail, or anything equivalent, everywhere
unless the test can't proceed anymore).
>
> Thanks,
> Zorro
>
> [1]
> [root@dell-per750-41 xfstests]# ./check -s default generic/362
> SECTION -- default
> FSTYP -- btrfs
> PLATFORM -- Linux/x86_64 dell-xx-xxxxxx 6.10.0-0.rc7.20240712git43db1e03c086.62.fc41.x86_64 #1 SMP PREEMPT_DYNAMIC Fri Jul 12 22:31:14 UTC 2024
> MKFS_OPTIONS -- /dev/sda6
> MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda6 /mnt/scratch
>
> generic/362 0s ... - output mismatch (see /root/git/xfstests/results//default/generic/362.out.bad)
> --- tests/generic/362.out 2024-07-28 22:22:06.098982182 +0800
> +++ /root/git/xfstests/results//default/generic/362.out.bad 2024-07-28 22:23:16.622577397 +0800
> @@ -1,2 +1,4 @@
> QA output created by 362
> +Wrong file size after first write, got 8192 expected 4096
> +Wrong file size after second write, got 12288 expected 8192
> Silence is golden
>
>
> >
> > .gitignore | 1 +
> > src/Makefile | 2 +-
> > src/dio-append-buf-fault.c | 145 +++++++++++++++++++++++++++++++++++++
> > tests/generic/362 | 28 +++++++
> > tests/generic/362.out | 2 +
> > 5 files changed, 177 insertions(+), 1 deletion(-)
> > create mode 100644 src/dio-append-buf-fault.c
> > create mode 100755 tests/generic/362
> > create mode 100644 tests/generic/362.out
> >
> > diff --git a/.gitignore b/.gitignore
> > index b5f15162..97c7e001 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -72,6 +72,7 @@ tags
> > /src/deduperace
> > /src/detached_mounts_propagation
> > /src/devzero
> > +/src/dio-append-buf-fault
> > /src/dio-buf-fault
> > /src/dio-interleaved
> > /src/dio-invalidate-cache
> > diff --git a/src/Makefile b/src/Makefile
> > index 99796137..559209be 100644
> > --- a/src/Makefile
> > +++ b/src/Makefile
> > @@ -20,7 +20,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
> > t_get_file_time t_create_short_dirs t_create_long_dirs t_enospc \
> > t_mmap_writev_overlap checkpoint_journal mmap-rw-fault allocstale \
> > t_mmap_cow_memory_failure fake-dump-rootino dio-buf-fault rewinddir-test \
> > - readdir-while-renames
> > + readdir-while-renames dio-append-buf-fault
> >
> > LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
> > preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
> > diff --git a/src/dio-append-buf-fault.c b/src/dio-append-buf-fault.c
> > new file mode 100644
> > index 00000000..72c23265
> > --- /dev/null
> > +++ b/src/dio-append-buf-fault.c
> > @@ -0,0 +1,145 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2024 SUSE Linux Products GmbH. All Rights Reserved.
> > + */
> > +
> > +/*
> > + * Test a direct IO write in append mode with a buffer that was not faulted in
> > + * (or just partially) before the write.
> > + */
> > +
> > +/* Get the O_DIRECT definition. */
> > +#ifndef _GNU_SOURCE
> > +#define _GNU_SOURCE
> > +#endif
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <stdint.h>
> > +#include <fcntl.h>
> > +#include <errno.h>
> > +#include <string.h>
> > +#include <sys/mman.h>
> > +#include <sys/stat.h>
> > +
> > +static ssize_t do_write(int fd, const void *buf, size_t count)
> > +{
> > + while (count > 0) {
> > + ssize_t ret;
> > +
> > + ret = write(fd, buf, count);
> > + if (ret < 0) {
> > + if (errno == EINTR)
> > + continue;
> > + return ret;
> > + }
> > + count -= ret;
> > + buf += ret;
> > + }
> > + return 0;
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > + struct stat stbuf;
> > + int fd;
> > + long pagesize;
> > + void *buf;
> > + ssize_t ret;
> > +
> > + if (argc != 2) {
> > + fprintf(stderr, "Use: %s <file path>\n", argv[0]);
> > + return 1;
> > + }
> > +
> > + /*
> > + * First try an append write against an empty file of a buffer with a
> > + * size matching the page size. The buffer is not faulted in before
> > + * attempting the write.
> > + */
> > +
> > + fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC | O_DIRECT | O_APPEND, 0666);
> > + if (fd == -1) {
> > + perror("Failed to open/create file");
> > + return 2;
> > + }
> > +
> > + pagesize = sysconf(_SC_PAGE_SIZE);
> > + if (pagesize == -1) {
> > + perror("Failed to get page size");
> > + return 3;
> > + }
> > +
> > + buf = mmap(NULL, pagesize, PROT_READ | PROT_WRITE,
> > + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > + if (buf == MAP_FAILED) {
> > + perror("Failed to allocate first buffer");
> > + return 4;
> > + }
> > +
> > + ret = do_write(fd, buf, pagesize);
> > + if (ret < 0) {
> > + perror("First write failed");
> > + return 5;
> > + }
> > +
> > + ret = fstat(fd, &stbuf);
> > + if (ret < 0) {
> > + perror("First stat failed");
> > + return 6;
> > + }
> > +
> > + /* Don't exit on failure so that we run the second test below too. */
> > + if (stbuf.st_size != pagesize)
> > + fprintf(stderr,
> > + "Wrong file size after first write, got %jd expected %ld\n",
> > + (intmax_t)stbuf.st_size, pagesize);
> > +
> > + munmap(buf, pagesize);
> > + close(fd);
> > +
> > + /*
> > + * Now try an append write against an empty file of a buffer with a
> > + * size matching twice the page size. Only the first page of the buffer
> > + * is faulted in before attempting the write, so that the second page
> > + * should be faulted in during the write.
> > + */
> > + fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC | O_DIRECT | O_APPEND, 0666);
> > + if (fd == -1) {
> > + perror("Failed to open/create file");
> > + return 7;
> > + }
> > +
> > + buf = mmap(NULL, pagesize * 2, PROT_READ | PROT_WRITE,
> > + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > + if (buf == MAP_FAILED) {
> > + perror("Failed to allocate second buffer");
> > + return 8;
> > + }
> > +
> > + /* Fault in first page of the buffer before the write. */
> > + memset(buf, 0, 1);
> > +
> > + ret = do_write(fd, buf, pagesize * 2);
> > + if (ret < 0) {
> > + perror("Second write failed");
> > + return 9;
> > + }
> > +
> > + ret = fstat(fd, &stbuf);
> > + if (ret < 0) {
> > + perror("Second stat failed");
> > + return 10;
> > + }
> > +
> > + if (stbuf.st_size != pagesize * 2)
> > + fprintf(stderr,
> > + "Wrong file size after second write, got %jd expected %ld\n",
> > + (intmax_t)stbuf.st_size, pagesize * 2);
> > +
> > + munmap(buf, pagesize * 2);
> > + close(fd);
> > +
> > + return 0;
> > +}
> > diff --git a/tests/generic/362 b/tests/generic/362
> > new file mode 100755
> > index 00000000..2c127347
> > --- /dev/null
> > +++ b/tests/generic/362
> > @@ -0,0 +1,28 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (C) 2024 SUSE Linux Products GmbH. All Rights Reserved.
> > +#
> > +# FS QA Test 362
> > +#
> > +# Test that doing a direct IO append write to a file when the input buffer was
> > +# not yet faulted in, does not result in an incorrect file size.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick
> > +
> > +_require_test
> > +_require_odirect
> > +_require_test_program dio-append-buf-fault
> > +
> > +[ $FSTYP == "btrfs" ] && \
> > + _fixed_by_kernel_commit xxxxxxxxxxxx \
> > + "btrfs: fix corruption after buffer fault in during direct IO append write"
> > +
> > +# On error the test program writes messages to stderr, causing a golden output
> > +# mismatch and making the test fail.
> > +$here/src/dio-append-buf-fault $TEST_DIR/dio-append-buf-fault
> > +
> > +# success, all done
> > +echo "Silence is golden"
> > +status=0
> > +exit
> > diff --git a/tests/generic/362.out b/tests/generic/362.out
> > new file mode 100644
> > index 00000000..0ff40905
> > --- /dev/null
> > +++ b/tests/generic/362.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 362
> > +Silence is golden
> > --
> > 2.43.0
> >
> >
>
