We had a problem recently where btrfs would deadlock with O_DIRECT|O_DSYNC because of an unexpected dependency on ->fsync in iomap. This was only caught by chance with aiostress, because weirdly we don't actually test this particular configuration anywhere in xfstests. Fix this by adding a basic test that just does O_DIRECT|O_DSYNC writes. With this test the box deadlocks right away with Btrfs, which would have been helpful in finding this issue before the patches were merged. Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx> --- .gitignore | 1 + src/aio-dio-regress/dio-dsync.c | 61 +++++++++++++++++++++++++++++++++ tests/generic/609 | 44 ++++++++++++++++++++++++ tests/generic/group | 1 + 4 files changed, 107 insertions(+) create mode 100644 src/aio-dio-regress/dio-dsync.c create mode 100755 tests/generic/609 diff --git a/.gitignore b/.gitignore index 5f5c4a0f..07c8014b 100644 --- a/.gitignore +++ b/.gitignore @@ -175,6 +175,7 @@ /src/aio-dio-regress/aio-last-ref-held-by-io /src/aio-dio-regress/aiocp /src/aio-dio-regress/aiodio_sparse2 +/src/aio-dio-regress/dio-dsync /src/log-writes/replay-log /src/perf/*.pyc diff --git a/src/aio-dio-regress/dio-dsync.c b/src/aio-dio-regress/dio-dsync.c new file mode 100644 index 00000000..53cda9ac --- /dev/null +++ b/src/aio-dio-regress/dio-dsync.c @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: GPL-2.0-or-newer +/* + * Copyright (c) 2020 Facebook. + * All Rights Reserved. + * + * Do some O_DIRECT writes with O_DSYNC to exercise this path. + */ +#include <stdio.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <unistd.h> +#include <stdlib.h> +#include <errno.h> +#include <string.h> + +int main(int argc, char **argv) +{ + struct stat st; + char *buf; + ssize_t ret; + int fd, i; + int bufsize; + + if (argc != 2) { + printf("Usage: %s filename\n", argv[0]); + return 1; + } + + fd = open(argv[1], O_DIRECT | O_RDWR | O_TRUNC | O_CREAT | O_DSYNC, + 0644); + if (fd < 0) { + perror(argv[1]); + return 1; + } + + if (fstat(fd, &st)) { + perror(argv[1]); + return 1; + } + bufsize = st.st_blksize * 10; + + ret = posix_memalign((void **)&buf, st.st_blksize, bufsize); + if (ret) { + errno = ret; + perror("allocating buffer"); + return 1; + } + + memset(buf, 'a', bufsize); + for (i = 0; i < 10; i++) { + ret = write(fd, buf, bufsize); + if (ret < 0) { + perror("writing"); + return 1; + } + } + free(buf); + close(fd); + return 0; +} diff --git a/tests/generic/609 b/tests/generic/609 new file mode 100755 index 00000000..8a888eb9 --- /dev/null +++ b/tests/generic/609 @@ -0,0 +1,44 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2020 Josef Bacik. All Rights Reserved. +# +# FS QA Test 609 +# +# iomap can call generic_write_sync() if we're O_DSYNC, so write a basic test to +# exercise O_DSYNC so any unsuspecting file systems will get lockdep warnings if +# they're locking isn't compatible. +# +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 15 + +_cleanup() +{ + cd / + rm -f $tmp.* + rm -rf $TEST_DIR/file +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# Modify as appropriate. +_supported_fs generic +_supported_os Linux +_require_test +_require_aiodio dio-dsync + +$AIO_TEST $TEST_DIR/file + +echo "Silence is golden" +status=0 +exit diff --git a/tests/generic/group b/tests/generic/group index aa969bcb..ae2567a0 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -611,3 +611,4 @@ 606 auto attr quick dax 607 auto attr quick dax 608 auto attr quick dax +609 auto quick -- 2.26.2