[v4,9/9] syscalls/sync_file_range: add sync device test-case

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
Related show

Commit Message

Sumit Garg Feb. 21, 2019, 9 a.m.
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

Comments

Cyril Hrubis Feb. 25, 2019, 3:26 p.m. | #1
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.
Sumit Garg Feb. 26, 2019, 9:02 a.m. | #2
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
Cyril Hrubis Feb. 26, 2019, 10:57 a.m. | #3
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.
Daniel Thompson Feb. 26, 2019, 11:48 a.m. | #4
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
Sumit Garg Feb. 26, 2019, 1:34 p.m. | #5
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

Patch

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,
+};