| Message ID | 1550739616-24054-10-git-send-email-sumit.garg@linaro.org |
|---|---|
| State | Accepted |
| Commit | 7c7ecaeb0179c4b69e6f6c1255656c4231669950 |
| Headers | show |
| Series | syscalls: add sync device test-cases | expand |
Hi! This tests fails for me for fuse based filesystems, the call returns 0 but does not write anything, the questions is if this is desired behavior or not. Also we should as well add a second test here that sync only a middle of a written region of a file and expects that the data written are somewhere between a few percents of the synced size.
On Mon, 25 Feb 2019 at 20:57, Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > This tests fails for me for fuse based filesystems, the call returns 0 > but does not write anything, the questions is if this is desired > behavior or not. > Interesting case. Do you see similar behaviour with other sync related syscalls? AFAIK, fuse file-system operations are implemented in user-space daemons exported to kernel as callbacks via libfuse. Maybe we need to check fsync() callback operation [1] implementation? [1] https://libfuse.github.io/doxygen/structfuse__operations.html > Also we should as well add a second test here that sync only a middle of > a written region of a file and expects that the data written are > somewhere between a few percents of the synced size. > Sure, will add a test for this. -Sumit > -- > Cyril Hrubis > chrubis@suse.cz
Hi! > > This tests fails for me for fuse based filesystems, the call returns 0 > > but does not write anything, the questions is if this is desired > > behavior or not. > > > > Interesting case. Do you see similar behaviour with other sync related syscalls? Just this one, I would have expected that the new syscall is not wired into fuse yet hence it does nothing silently instead of returning EOPNOTSUPP which could break applications... > AFAIK, fuse file-system operations are implemented in user-space > daemons exported to kernel as callbacks via libfuse. Maybe we need to > check fsync() callback operation [1] implementation? > > [1] https://libfuse.github.io/doxygen/structfuse__operations.html > > > Also we should as well add a second test here that sync only a middle of > > a written region of a file and expects that the data written are > > somewhere between a few percents of the synced size. > > > > Sure, will add a test for this. Ideally this should be put into the existing test in order to spare time needed to format the devices etc.
On Tue, Feb 26, 2019 at 11:57:11AM +0100, Cyril Hrubis wrote: > Hi! > > > This tests fails for me for fuse based filesystems, the call returns 0 > > > but does not write anything, the questions is if this is desired > > > behavior or not. > > > > > > > Interesting case. Do you see similar behaviour with other sync related syscalls? > > Just this one, I would have expected that the new syscall is not wired > into fuse yet hence it does nothing silently instead of returning > EOPNOTSUPP which could break applications... > > > AFAIK, fuse file-system operations are implemented in user-space > > daemons exported to kernel as callbacks via libfuse. Maybe we need to > > check fsync() callback operation [1] implementation? > > > > [1] https://libfuse.github.io/doxygen/structfuse__operations.html For file syncs I think (admitedly without having much VFS experience) that the writeback will end up being requested via the writepages VFS op; since we are writing back data from a single fd then the kernel doesn't need much help from the filesystem to find out which pages are affected so the "big" sync API in fuse is probably not needed here. Daniel. > > > > > Also we should as well add a second test here that sync only a middle of > > > a written region of a file and expects that the data written are > > > somewhere between a few percents of the synced size. > > > > > > > Sure, will add a test for this. > > Ideally this should be put into the existing test in order to spare time > needed to format the devices etc. > > -- > Cyril Hrubis > chrubis@suse.cz
On Tue, 26 Feb 2019 at 17:18, Daniel Thompson <daniel.thompson@linaro.org> wrote: > > On Tue, Feb 26, 2019 at 11:57:11AM +0100, Cyril Hrubis wrote: > > Hi! > > > > This tests fails for me for fuse based filesystems, the call returns 0 > > > > but does not write anything, the questions is if this is desired > > > > behavior or not. > > > > > > > > > > Interesting case. Do you see similar behaviour with other sync related syscalls? > > > > Just this one, I would have expected that the new syscall is not wired > > into fuse yet hence it does nothing silently instead of returning > > EOPNOTSUPP which could break applications... > > > > > AFAIK, fuse file-system operations are implemented in user-space > > > daemons exported to kernel as callbacks via libfuse. Maybe we need to > > > check fsync() callback operation [1] implementation? > > > > > > [1] https://libfuse.github.io/doxygen/structfuse__operations.html > > For file syncs I think (admitedly without having much VFS experience) > that the writeback will end up being requested via the writepages VFS op; > since we are writing back data from a single fd then the kernel doesn't > need much help from the filesystem to find out which pages are > affected so the "big" sync API in fuse is probably not needed here. > Hmm, I see in case of sync_file_range(), its handled via writepages VFS op. And fuse fsync() callback is used for fsync() and fdatasync() syscalls. > > Daniel. > > > > > > > > > Also we should as well add a second test here that sync only a middle of > > > > a written region of a file and expects that the data written are > > > > somewhere between a few percents of the synced size. > > > > > > > > > > Sure, will add a test for this. > > > > Ideally this should be put into the existing test in order to spare time > > needed to format the devices etc. > > Agree. -Sumit > > -- > > Cyril Hrubis > > chrubis@suse.cz
diff --git a/runtest/syscalls b/runtest/syscalls index aaad651..70d3561 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -1353,6 +1353,7 @@ syncfs01 syncfs01 #testcases for sync_file_range sync_file_range01 sync_file_range01 +sync_file_range02 sync_file_range02 syscall01 syscall01 diff --git a/testcases/kernel/syscalls/sync_file_range/.gitignore b/testcases/kernel/syscalls/sync_file_range/.gitignore index 3f6bd75..e6485f7 100644 --- a/testcases/kernel/syscalls/sync_file_range/.gitignore +++ b/testcases/kernel/syscalls/sync_file_range/.gitignore @@ -1 +1,2 @@ /sync_file_range01 +/sync_file_range02 diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c new file mode 100644 index 0000000..37ea68a --- /dev/null +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2019 Linaro Limited. All rights reserved. + * Author: Sumit Garg <sumit.garg@linaro.org> + */ + +/* + * sync_file_range02 + * + * It basically tests sync_file_range() to sync test file range having large + * dirty file pages to block device. Also, it tests all supported filesystems + * on a test block device. + */ + +#define _GNU_SOURCE +#include <errno.h> +#include <stdlib.h> +#include <stdio.h> +#include <sys/types.h> +#include "tst_test.h" +#include "lapi/sync_file_range.h" +#include "check_sync_file_range.h" + +#define MNTPOINT "mnt_point" +#define TST_FILE MNTPOINT"/test" +#define TST_FILE_SIZE_MB 32 +#define SIZE_MB (1024*1024) +#define MODE 0644 + +static void verify_sync_file_range(void) +{ + int fd; + unsigned long written; + + fd = SAFE_OPEN(TST_FILE, O_RDWR|O_CREAT, MODE); + + tst_dev_bytes_written(tst_device->dev); + + tst_fill_fd(fd, 0, SIZE_MB, TST_FILE_SIZE_MB); + + TEST(sync_file_range(fd, 0, TST_FILE_SIZE_MB * SIZE_MB, + SYNC_FILE_RANGE_WAIT_BEFORE | + SYNC_FILE_RANGE_WRITE | + SYNC_FILE_RANGE_WAIT_AFTER)); + + if (TST_RET) + tst_brk(TFAIL | TTERRNO, "sync_file_range() failed"); + + written = tst_dev_bytes_written(tst_device->dev); + + SAFE_CLOSE(fd); + + if (written >= SIZE_MB * TST_FILE_SIZE_MB) + tst_res(TPASS, "Test file range synced to device"); + else + tst_res(TFAIL, "Failed to sync test file range to device"); +} + +static void setup(void) +{ + if (!check_sync_file_range()) + tst_brk(TCONF, "sync_file_range() not supported"); +} + +static struct tst_test test = { + .needs_root = 1, + .mount_device = 1, + .all_filesystems = 1, + .mntpoint = MNTPOINT, + .setup = setup, + .test_all = verify_sync_file_range, +};
sync_file_range02 tests to sync file data range having large dirty file pages to block device. Also, it tests all supported filesystems on a test block device. Signed-off-by: Sumit Garg <sumit.garg@linaro.org> --- runtest/syscalls | 1 + .../kernel/syscalls/sync_file_range/.gitignore | 1 + .../syscalls/sync_file_range/sync_file_range02.c | 72 ++++++++++++++++++++++ 3 files changed, 74 insertions(+) create mode 100644 testcases/kernel/syscalls/sync_file_range/sync_file_range02.c