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".
Does this mean this v2 change help this case to be better?
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
>
>
