readahead02: Fix on Btrfs

RFC PATCH New Cyril Hrubis 2 participants 5 messages Oct 6, 2016

lore

Share & tools

lore

Cyril Hrubis

author

2016-10-06 08:17 UTC #0 lore raw

RFC PATCH 1/1 readahead02: Fix on Btrfs

The Btrfs uses anonymous block devices for its subvolumes hence /sys/dev/block/$major:$minor/ is not created for these.

We have to use ioctl(BTRFS_IOC_FS_INFO, ...) to get fs UUID in order to map a path on Btrfs to a sysfs path that contains links to the devices.

TODO: What happens to readahead if there is more than one device backing
the Btrfs filesystem?

Also this is getting absurdly compliated, maybe we should rethink the test assertions so that we don't have to rely on reading the read_ahead_kb file, perhaps we can just try to guess the maximal size by calling the readahead in a loop with increasing size until it fails instead.

Signed-off-by Cyril Hrubis <[email protected]>

---
 testcases/kernel/syscalls/readahead/readahead02.c | 57 ++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

testcases/kernel/syscalls/readahead/readahead02.c +56 -1

index 2517a33..3c06596 100644
@@ -42,6 +42,7 @@
 #include <stdint.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <linux/btrfs.h>
 #include "config.h"
 #include "test.h"
 #include "safe_macros.h"

@@ -184,7 +185,53 @@ static void create_testfile(void)
 	free(tmp);
 }
 
-static long get_device_readahead(const char *fname)
+static long btrfs_get_device_readahead(const char *fname)
+{
+	int fd;
+	struct btrfs_ioctl_fs_info_args args;
+	unsigned char *id = args.fsid;
+	char buf[128];
+	char path[128];
+	DIR *dir;
+	struct dirent *dent;
+	int dflag = 0;
+	unsigned long ra_kb = 0;
+
+	fd = SAFE_OPEN(cleanup, fname, O_RDONLY);
+	SAFE_IOCTL(cleanup, fd, BTRFS_IOC_FS_INFO, &args);
+	SAFE_CLOSE(cleanup, fd);
+
+	snprintf(buf, sizeof(buf),
+	         "/sys/fs/btrfs/%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x/devices/",
+	         id[0], id[1], id[2], id[3], id[4], id[5], id[6], id[7],
+                 id[8], id[9], id[10], id[11], id[12], id[13], id[14], id[15]);
+
+	tst_resm(TINFO, "Looking for device in '%s'", buf);
+
+	dir = SAFE_OPENDIR(cleanup, buf);
+	while ((dent = SAFE_READDIR(cleanup, dir))) {
+		if (!strcmp(".", dent->d_name) || !strcmp("..", dent->d_name))
+			continue;
+
+		if (dflag) {
+			tst_brkm(TBROK, cleanup,
+		        "More than one device for Btrfs filesystem!");
+		}
+
+		snprintf(path, sizeof(path),
+		         "%s/%s/../queue/read_ahead_kb", buf, dent->d_name);
+
+		dflag = 1;
+	}
+	SAFE_CLOSEDIR(cleanup, dir);
+
+	tst_resm(TINFO, "Reading %s", path);
+	SAFE_FILE_SCANF(cleanup, path, "%ld", &ra_kb);
+
+	return ra_kb * 1024;
+}
+
+static long generic_get_device_readahead(const char *fname)
 {
 	struct stat st;
 	unsigned long ra_kb = 0;

@@ -205,6 +252,14 @@ static long get_device_readahead(const char *fname)
 	return ra_kb * 1024;
 }
 
+static long get_device_readahead(const char *fname)
+{
+	if (tst_fs_type(cleanup, ".") == TST_BTRFS_MAGIC)
+		return btrfs_get_device_readahead(fname);
+
+	return generic_get_device_readahead(fname);
+}
+
 /* read_testfile - mmap testfile and read every page.
  * This functions measures how many I/O and time it takes to fully
  * read contents of test file.

Signature · 6 lines

2.7.3


-- 
Cyril Hrubis
[email protected]

Jan Stancek

2016-10-06 09:09 UTC #1 lore raw

RFC PATCH 1/1 readahead02: Fix on Btrfs

----- Original Message -----

Quoted 12 lines

From: "Cyril Hrubis" <[email protected]>
To: [email protected]
Cc: "Jan Stancek" <[email protected]>
Sent: Thursday, 6 October, 2016 10:17:52 AM
Subject: [PATCH] [RFC] readahead02: Fix on Btrfs
The Btrfs uses anonymous block devices for its subvolumes hence /sys/dev/block/$major:$minor/ is not created for these.
We have to use ioctl(BTRFS_IOC_FS_INFO, ...) to get fs UUID in order to map a path on Btrfs to a sysfs path that contains links to the devices.

Quoted 8 lines

TODO: What happens to readahead if there is more than one device backing
the Btrfs filesystem?
Also this is getting absurdly compliated, maybe we should rethink the test assertions so that we don't have to rely on reading the read_ahead_kb file, perhaps we can just try to guess the maximal size by calling the readahead in a loop with increasing size until it fails instead.

Syscall itself won't fail, it will silently make shorter read. If this patch goes through, then reading read_ahead_kb becomes useless:
https://lkml.org/lkml/2016/7/25/308

Perhaps, we should stop focusing on max size. We could change it to start with size of entire file, and for subsequent calls update file offset as max(MIN_SANE_READAHEAD, cache_increase_since_last_call), where MIN_SANE_READAHEAD would be some small arbitrary number. So there would be a guarantee it can eventually finish and any smaller readahead than that number would be considered a failure.

Quoted 13 lines

Signed-off-by Cyril Hrubis <[email protected]>
---
 testcases/kernel/syscalls/readahead/readahead02.c | 57
 ++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)
testcases/kernel/syscalls/readahead/readahead02.c +0 -0
index 2517a33..3c06596 100644

testcases/kernel/syscalls/readahead/readahead02.c line 45

Quoted 4 lines

testcases/kernel/syscalls/readahead/readahead02.c +1 -0

@@ -42,6 +42,7 @@
 #include <stdint.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <linux/btrfs.h>

This will be an issue on old distros/kernels.

Regards,
Jan

Cyril Hrubis

author

2016-10-06 09:32 UTC #2 lore raw

RFC PATCH 1/1 readahead02: Fix on Btrfs

Hi!

Quoted 7 lines

Quoted 5 lines
Also this is getting absurdly compliated, maybe we should rethink the test assertions so that we don't have to rely on reading the read_ahead_kb file, perhaps we can just try to guess the maximal size by calling the readahead in a loop with increasing size until it fails instead.
Syscall itself won't fail, it will silently make shorter read.

Ah, it would have been much easier if we got EINVAL instead...

Quoted 10 lines

If this patch goes through, then reading read_ahead_kb becomes useless:
https://lkml.org/lkml/2016/7/25/308
Perhaps, we should stop focusing on max size. We could change it to start with size of entire file, and for subsequent calls update file offset as max(MIN_SANE_READAHEAD, cache_increase_since_last_call), where MIN_SANE_READAHEAD would be some small arbitrary number. So there would be a guarantee it can eventually finish and any smaller readahead than that number would be considered a failure.

Sounds reasonable. Will you prepare a patch or should i work on it?

Quoted 9 lines

testcases/kernel/syscalls/readahead/readahead02.c line 45
Quoted 7 lines
testcases/kernel/syscalls/readahead/readahead02.c +1 -0
@@ -42,6 +42,7 @@
 #include <stdint.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <linux/btrfs.h>
This will be an issue on old distros/kernels.

Sure, this should be wrapped in ifdefs, but I hope that we figure out something simpler instead.

Signature · 2 lines

Cyril Hrubis
[email protected]

Jan Stancek

2016-10-06 09:56 UTC #3 lore raw

RFC PATCH 1/1 readahead02: Fix on Btrfs

----- Original Message -----

Quoted 13 lines

From: "Cyril Hrubis" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected]
Sent: Thursday, 6 October, 2016 11:32:26 AM
Subject: Re: [PATCH] [RFC] readahead02: Fix on Btrfs
Hi!
Quoted 6 lines
Quoted 5 lines
Also this is getting absurdly compliated, maybe we should rethink the test assertions so that we don't have to rely on reading the read_ahead_kb file, perhaps we can just try to guess the maximal size by calling the readahead in a loop with increasing size until it fails instead.

Quoted 3 lines

Quoted 1 line
Syscall itself won't fail, it will silently make shorter read.
Ah, it would have been much easier if we got EINVAL instead...

Or if readahead returned number of bytes read instead of 0.

Quoted 13 lines

Quoted 5 lines
If this patch goes through, then reading read_ahead_kb becomes useless:
https://lkml.org/lkml/2016/7/25/308
Quoted 7 lines
Perhaps, we should stop focusing on max size. We could change it to start with size of entire file, and for subsequent calls update file offset as max(MIN_SANE_READAHEAD, cache_increase_since_last_call), where MIN_SANE_READAHEAD
would be some small arbitrary number. So there would be a guarantee it can eventually finish and any smaller readahead than that number would be considered a failure.

Quoted 1 line

Sounds reasonable. Will you prepare a patch or should i work on it?

I want to finish writev patches first (very visible 4.8 syscall failure, if I don't count scenario in this thread). If you need this ASAP then go ahead, otherwise I can pick it up later.

Regards,
Jan

Cyril Hrubis

author

2016-10-06 11:16 UTC #4 lore raw

RFC PATCH 1/1 readahead02: Fix on Btrfs

Hi!

Quoted 12 lines

Quoted 12 lines
Quoted 11 lines
If this patch goes through, then reading read_ahead_kb becomes useless:
https://lkml.org/lkml/2016/7/25/308
Perhaps, we should stop focusing on max size. We could change it to start with size of entire file, and for subsequent calls update file offset as max(MIN_SANE_READAHEAD, cache_increase_since_last_call), where MIN_SANE_READAHEAD
would be some small arbitrary number. So there would be a guarantee it can eventually finish and any smaller readahead than that number would be considered a failure.

Quoted 5 lines

Quoted 1 line
Sounds reasonable. Will you prepare a patch or should i work on it?
I want to finish writev patches first (very visible 4.8 syscall failure, if I don't count scenario in this thread). If you need this ASAP then go ahead, otherwise I can pick it up later.

No rush, this test wasn't working on Btrfs for quite some time.

Signature · 2 lines

Cyril Hrubis
[email protected]