[LTP] [PATCH] [RFC] readahead02: Fix on Btrfs

[LTP] [PATCH] [RFC] readahead02: Fix on Btrfs

[Cyril Hrubis]

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 <chrubis@suse.cz>
---
 testcases/kernel/syscalls/readahead/readahead02.c | 57 ++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 2517a33..3c06596 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -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.
-- 
2.7.3


-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] [RFC] readahead02: Fix on Btrfs

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: ltp@lists.linux.it
> Cc: "Jan Stancek" <jstancek@redhat.com>
> 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.
> 
> 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.

> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  testcases/kernel/syscalls/readahead/readahead02.c | 57
>  ++++++++++++++++++++++-
>  1 file changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c
> b/testcases/kernel/syscalls/readahead/readahead02.c
> index 2517a33..3c06596 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -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

[LTP] [PATCH] [RFC] readahead02: Fix on Btrfs

[Cyril Hrubis]

Hi!
> > 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...

> 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?

> > --- a/testcases/kernel/syscalls/readahead/readahead02.c
> > +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> > @@ -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.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] [RFC] readahead02: Fix on Btrfs

[Jan Stancek]

----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Thursday, 6 October, 2016 11:32:26 AM
> Subject: Re: [PATCH] [RFC] readahead02: Fix on Btrfs
> 
> Hi!
> > > 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...

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

> 
> > 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?

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

[LTP] [PATCH] [RFC] readahead02: Fix on Btrfs

[Cyril Hrubis]

Hi!
> > > 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?
> 
> 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.

-- 
Cyril Hrubis
chrubis@suse.cz