[RFC v2 PATCH 0/7] Introduce nfs-readahead-udev

[RFC v2 PATCH 0/7] Introduce nfs-readahead-udev

[Thiago Becker]

Recent changes in the linux kernel caused NFS readahead to default to
128 from the previous default of 15 * rsize. This causes performance
penalties to some read-heavy workloads, which can be fixed by
tuning the readahead for that given mount.

Specifically, the read troughput on a sec=krb5p mount drops by 50-75%
when comparing the default readahead with a readahead of 15360.

Previous discussions:
https://lore.kernel.org/linux-nfs/20210803130717.2890565-1-trbecker@gmail.com/
I attempted to add a non-kernel option to mount.nfs, and it was
rejected.

https://lore.kernel.org/linux-nfs/20210811171402.947156-1-trbecker@gmail.com/
Attempted to add a mount option to the kernel, rejected as well.

I had started a separate tool to set the readahead of BDIs, but the
scope is specifically for NFS, so I would like to get the community
feeling for having this in nfs-utils.

This patch series introduces nfs-readahead-udev, a utility to
automatically set NFS readahead when NFS is mounted. The utility is
triggered by udev when a new BDI is added, returns to udev the value of
the readahead that should be used.

The tool currently supports setting read ahead per mountpoint, nfs major
version, or by a global default value.

Thiago Becker (7):
  Create nfs-readahead-udev
  readahead: configure udev
  readahead: create logging facility
  readahead: only set readahead for nfs devices.
  readahead: create the configuration file
  readahead: add mountpoint and fstype options
  readahead: documentation

 .gitignore                                    |   6 +
 configure.ac                                  |   4 +
 tools/Makefile.am                             |   2 +-
 tools/nfs-readahead-udev/99-nfs_bdi.rules.in  |   1 +
 tools/nfs-readahead-udev/Makefile.am          |  26 +++
 tools/nfs-readahead-udev/config_parser.c      |  25 +++
 tools/nfs-readahead-udev/config_parser.h      |  14 ++
 tools/nfs-readahead-udev/list.h               |  48 ++++
 tools/nfs-readahead-udev/log.h                |  16 ++
 tools/nfs-readahead-udev/main.c               | 211 ++++++++++++++++++
 .../nfs-readahead-udev/nfs-readahead-udev.man |  47 ++++
 tools/nfs-readahead-udev/parser.y             |  85 +++++++
 tools/nfs-readahead-udev/readahead.conf       |  15 ++
 tools/nfs-readahead-udev/scanner.l            |  19 ++
 tools/nfs-readahead-udev/syslog.c             |  47 ++++
 15 files changed, 565 insertions(+), 1 deletion(-)
 create mode 100644 tools/nfs-readahead-udev/99-nfs_bdi.rules.in
 create mode 100644 tools/nfs-readahead-udev/Makefile.am
 create mode 100644 tools/nfs-readahead-udev/config_parser.c
 create mode 100644 tools/nfs-readahead-udev/config_parser.h
 create mode 100644 tools/nfs-readahead-udev/list.h
 create mode 100644 tools/nfs-readahead-udev/log.h
 create mode 100644 tools/nfs-readahead-udev/main.c
 create mode 100644 tools/nfs-readahead-udev/nfs-readahead-udev.man
 create mode 100644 tools/nfs-readahead-udev/parser.y
 create mode 100644 tools/nfs-readahead-udev/readahead.conf
 create mode 100644 tools/nfs-readahead-udev/scanner.l
 create mode 100644 tools/nfs-readahead-udev/syslog.c

-- 
2.35.1

[RFC v2 PATCH 1/7] Create nfs-readahead-udev

[Thiago Becker]

This tool is invoked by udev to find and set the readahead value to NFS
mounts.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1946283
Signed-off-by: Thiago Becker <tbecker@redhat.com>
---
 .gitignore                           | 1 +
 configure.ac                         | 1 +
 tools/Makefile.am                    | 2 +-
 tools/nfs-readahead-udev/Makefile.am | 3 +++
 tools/nfs-readahead-udev/main.c      | 7 +++++++
 5 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 tools/nfs-readahead-udev/Makefile.am
 create mode 100644 tools/nfs-readahead-udev/main.c

diff --git a/.gitignore b/.gitignore
index c89d1cd2..c99269a4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -61,6 +61,7 @@ utils/statd/statd
 tools/locktest/testlk
 tools/getiversion/getiversion
 tools/nfsconf/nfsconf
+tools/nfs-readahead-udev/nfs-readahead-udev
 support/export/mount.h
 support/export/mount_clnt.c
 support/export/mount_xdr.c
diff --git a/configure.ac b/configure.ac
index e0f5a930..7e5ba5d9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -737,6 +737,7 @@ AC_CONFIG_FILES([
 	tools/rpcgen/Makefile
 	tools/mountstats/Makefile
 	tools/nfs-iostat/Makefile
+	tools/nfs-readahead-udev/Makefile
 	tools/rpcctl/Makefile
 	tools/nfsdclnts/Makefile
 	tools/nfsconf/Makefile
diff --git a/tools/Makefile.am b/tools/Makefile.am
index c3feabbe..621cde03 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -12,6 +12,6 @@ if CONFIG_NFSDCLD
 OPTDIRS += nfsdclddb
 endif
 
-SUBDIRS = locktest rpcdebug nlmtest mountstats nfs-iostat rpcctl nfsdclnts $(OPTDIRS)
+SUBDIRS = locktest rpcdebug nlmtest mountstats nfs-iostat rpcctl nfsdclnts nfs-readahead-udev $(OPTDIRS)
 
 MAINTAINERCLEANFILES = Makefile.in
diff --git a/tools/nfs-readahead-udev/Makefile.am b/tools/nfs-readahead-udev/Makefile.am
new file mode 100644
index 00000000..5455e954
--- /dev/null
+++ b/tools/nfs-readahead-udev/Makefile.am
@@ -0,0 +1,3 @@
+libexec_PROGRAMS = nfs-readahead-udev
+nfs_readahead_udev_SOURCES = main.c
+
diff --git a/tools/nfs-readahead-udev/main.c b/tools/nfs-readahead-udev/main.c
new file mode 100644
index 00000000..e454108e
--- /dev/null
+++ b/tools/nfs-readahead-udev/main.c
@@ -0,0 +1,7 @@
+#include <stdio.h>
+
+int main(int argc, char **argv, char **envp)
+{
+	unsigned int readahead = 128;
+	printf("%d\n", readahead);
+}
-- 
2.35.1

[RFC v2 PATCH 2/7] readahead: configure udev

[Thiago Becker]

Set the udev rule to call the readahead utility.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1946283
Signed-off-by: Thiago Becker <tbecker@redhat.com>
---
 tools/nfs-readahead-udev/99-nfs_bdi.rules.in | 1 +
 tools/nfs-readahead-udev/Makefile.am         | 8 ++++++++
 2 files changed, 9 insertions(+)
 create mode 100644 tools/nfs-readahead-udev/99-nfs_bdi.rules.in

diff --git a/tools/nfs-readahead-udev/99-nfs_bdi.rules.in b/tools/nfs-readahead-udev/99-nfs_bdi.rules.in
new file mode 100644
index 00000000..15f8a995
--- /dev/null
+++ b/tools/nfs-readahead-udev/99-nfs_bdi.rules.in
@@ -0,0 +1 @@
+SUBSYSTEM=="bdi", ACTION=="add", PROGRAM="_libexecdir_/nfs-readahead-udev", ATTR{read_ahead_kb}="%c"
diff --git a/tools/nfs-readahead-udev/Makefile.am b/tools/nfs-readahead-udev/Makefile.am
index 5455e954..873cc175 100644
--- a/tools/nfs-readahead-udev/Makefile.am
+++ b/tools/nfs-readahead-udev/Makefile.am
@@ -1,3 +1,11 @@
 libexec_PROGRAMS = nfs-readahead-udev
 nfs_readahead_udev_SOURCES = main.c
 
+udev_rulesdir = /etc/udev/rules.d
+udev_rules_DATA = 99-nfs_bdi.rules
+
+99-nfs_bdi.rules: 99-nfs_bdi.rules.in $(builddefs)
+	$(SED) "s|_libexecdir_|@libexecdir@|g" 99-nfs_bdi.rules.in > $@
+
+clean-local:
+	$(RM) 99-nfs_bdi.rules
-- 
2.35.1

[RFC v2 PATCH 3/7] readahead: create logging facility

[Thiago Becker]

Create logs for nfs-readahead-udev, logging to syslog.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1946283
Signed-off-by: Thiago Becker <tbecker@redhat.com>
---
 tools/nfs-readahead-udev/Makefile.am |  2 +-
 tools/nfs-readahead-udev/log.h       | 16 ++++++++++
 tools/nfs-readahead-udev/main.c      |  8 +++++
 tools/nfs-readahead-udev/syslog.c    | 47 ++++++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 1 deletion(-)
 create mode 100644 tools/nfs-readahead-udev/log.h
 create mode 100644 tools/nfs-readahead-udev/syslog.c

diff --git a/tools/nfs-readahead-udev/Makefile.am b/tools/nfs-readahead-udev/Makefile.am
index 873cc175..5078db9a 100644
--- a/tools/nfs-readahead-udev/Makefile.am
+++ b/tools/nfs-readahead-udev/Makefile.am
@@ -1,5 +1,5 @@
 libexec_PROGRAMS = nfs-readahead-udev
-nfs_readahead_udev_SOURCES = main.c
+nfs_readahead_udev_SOURCES = main.c syslog.c
 
 udev_rulesdir = /etc/udev/rules.d
 udev_rules_DATA = 99-nfs_bdi.rules
diff --git a/tools/nfs-readahead-udev/log.h b/tools/nfs-readahead-udev/log.h
new file mode 100644
index 00000000..2a14e552
--- /dev/null
+++ b/tools/nfs-readahead-udev/log.h
@@ -0,0 +1,16 @@
+#ifndef __ra_log_h__
+#define __ra_log_h__
+#include <stdarg.h>
+
+extern void log_open(void);
+extern void log_close(void);
+
+extern void debug(const char *, ...);
+extern void info(const char *, ...);
+extern void notice(const char *, ...);
+extern void warn(const char *, ...);
+extern void err(const char *, ...);
+extern void crit(const char *, ...);
+extern void alert(const char *, ...);
+extern void emerg(const char *, ...);
+#endif
diff --git a/tools/nfs-readahead-udev/main.c b/tools/nfs-readahead-udev/main.c
index e454108e..dd2c9f8c 100644
--- a/tools/nfs-readahead-udev/main.c
+++ b/tools/nfs-readahead-udev/main.c
@@ -1,7 +1,15 @@
 #include <stdio.h>
 
+#include "log.h"
+
 int main(int argc, char **argv, char **envp)
 {
 	unsigned int readahead = 128;
+
+	log_open();
+
+	info("Setting the readahead to 128\n");
+
+	log_close();
 	printf("%d\n", readahead);
 }
diff --git a/tools/nfs-readahead-udev/syslog.c b/tools/nfs-readahead-udev/syslog.c
new file mode 100644
index 00000000..5eeb2579
--- /dev/null
+++ b/tools/nfs-readahead-udev/syslog.c
@@ -0,0 +1,47 @@
+#include <syslog.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "log.h"
+
+#define MSG_SIZE (1024 * sizeof(char))
+
+void log_open(void)
+{
+	openlog("nfs_readahead", LOG_CONS | LOG_PID | LOG_NDELAY, LOG_LOCAL1);
+	setlogmask(LOG_UPTO(LOG_DEBUG));
+}
+
+void log_close(void)
+{
+	closelog();
+}
+
+static void vlog(int level, const char *fmt, va_list *args)
+{
+	char *msg = malloc(MSG_SIZE);
+	if (!msg)
+		return;
+
+	vsnprintf(msg, MSG_SIZE, fmt, *args);
+	syslog(level, "%s", msg);
+
+	free(msg);
+}
+
+#define GENERATE_LOGGER(name, level)		\
+	void name(const char *fmt, ...) {	\
+	va_list args;				\
+	va_start(args, fmt);			\
+	vlog(LOG_##level, fmt, &args);		\
+	va_end(args);				\
+}
+
+GENERATE_LOGGER(debug, DEBUG);
+GENERATE_LOGGER(info, INFO);
+GENERATE_LOGGER(notice, NOTICE);
+GENERATE_LOGGER(warn, WARNING);
+GENERATE_LOGGER(err, ERR);
+GENERATE_LOGGER(crit, CRIT);
+GENERATE_LOGGER(alert, ALERT);
+GENERATE_LOGGER(emerg, EMERG);
-- 
2.35.1

[RFC v2 PATCH 4/7] readahead: only set readahead for nfs devices.

[Thiago Becker]

Limit setting the readahead for nfs devices.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1946283
Signed-off-by: Thiago Becker <tbecker@redhat.com>
---
 tools/nfs-readahead-udev/99-nfs_bdi.rules.in |   2 +-
 tools/nfs-readahead-udev/Makefile.am         |   1 +
 tools/nfs-readahead-udev/main.c              | 132 ++++++++++++++++++-
 3 files changed, 133 insertions(+), 2 deletions(-)

diff --git a/tools/nfs-readahead-udev/99-nfs_bdi.rules.in b/tools/nfs-readahead-udev/99-nfs_bdi.rules.in
index 15f8a995..744c59be 100644
--- a/tools/nfs-readahead-udev/99-nfs_bdi.rules.in
+++ b/tools/nfs-readahead-udev/99-nfs_bdi.rules.in
@@ -1 +1 @@
-SUBSYSTEM=="bdi", ACTION=="add", PROGRAM="_libexecdir_/nfs-readahead-udev", ATTR{read_ahead_kb}="%c"
+SUBSYSTEM=="bdi", ACTION=="add", PROGRAM="_libexecdir_/nfs-readahead-udev %k", ATTR{read_ahead_kb}="%c"
diff --git a/tools/nfs-readahead-udev/Makefile.am b/tools/nfs-readahead-udev/Makefile.am
index 5078db9a..551d22e9 100644
--- a/tools/nfs-readahead-udev/Makefile.am
+++ b/tools/nfs-readahead-udev/Makefile.am
@@ -1,5 +1,6 @@
 libexec_PROGRAMS = nfs-readahead-udev
 nfs_readahead_udev_SOURCES = main.c syslog.c
+nfs_readahead_udev_LDFLAGS= -lmount
 
 udev_rulesdir = /etc/udev/rules.d
 udev_rules_DATA = 99-nfs_bdi.rules
diff --git a/tools/nfs-readahead-udev/main.c b/tools/nfs-readahead-udev/main.c
index dd2c9f8c..bbb408c0 100644
--- a/tools/nfs-readahead-udev/main.c
+++ b/tools/nfs-readahead-udev/main.c
@@ -1,15 +1,145 @@
 #include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#include <libmount/libmount.h>
+#include <sys/sysmacros.h>
 
 #include "log.h"
 
+#ifndef MOUNTINFO_PATH
+#define MOUNTINFO_PATH "/proc/self/mountinfo"
+#endif
+
+/* Device information from the system */
+struct device_info {
+	char *device_number;
+	dev_t dev;
+	char *mountpoint;
+	char *fstype;
+};
+
+/* Convert a string in the format n:m to a device number */
+static dev_t dev_from_arg(const char *device_number)
+{
+	char *s = strdup(device_number), *p;
+	char *maj_s, *min_s;
+	unsigned int maj, min;
+	dev_t dev;
+
+	maj_s = p = s;
+	for ( ; *p != ':'; p++)
+		;
+
+	*p = '\0';
+	min_s = p + 1;
+
+	maj = strtol(maj_s, NULL, 10);
+	min = strtol(min_s, NULL, 10);
+
+	dev = makedev(maj, min);
+
+	free(s);
+	return dev;
+}
+
+#define sfree(ptr) if (ptr) free(ptr)
+
+/* device_info maintenance */
+static void init_device_info(struct device_info *di, const char *device_number)
+{
+	di->device_number = strdup(device_number);
+	di->dev = dev_from_arg(device_number);
+	di->mountpoint = NULL;
+	di->fstype = NULL;
+}
+
+
+static void free_device_info(struct device_info *di)
+{
+	sfree(di->mountpoint);
+	sfree(di->fstype);
+	sfree(di->device_number);
+}
+
+static int get_mountinfo(const char *device_number, struct device_info *device_info, const char *mountinfo_path)
+{
+	int ret = 0;
+	struct libmnt_table *mnttbl;
+	struct libmnt_fs *fs;
+	char *target;
+
+	init_device_info(device_info, device_number);
+
+	mnttbl = mnt_new_table();
+
+	if ((ret = mnt_table_parse_file(mnttbl, mountinfo_path)) < 0)
+		goto out_free_tbl;
+
+	if ((fs = mnt_table_find_devno(mnttbl, device_info->dev, MNT_ITER_FORWARD)) == NULL) {
+		ret = ENOENT;
+		goto out_free_tbl;
+	}
+
+	if ((target = (char *)mnt_fs_get_target(fs)) == NULL) {
+		ret = ENOENT;
+		goto out_free_fs;
+	}
+
+	device_info->mountpoint = strdup(target);
+	target = (char *)mnt_fs_get_fstype(fs);
+	if (target)
+		device_info->fstype = strdup(target);
+
+out_free_fs:
+	mnt_free_fs(fs);
+out_free_tbl:
+	mnt_free_table(mnttbl);
+	free(device_info->device_number);
+	device_info->device_number = NULL;
+	return ret;
+}
+
+static int get_device_info(const char *device_number, struct device_info *device_info)
+{
+	int ret = ENOENT;
+	for (int retry_count = 0; retry_count < 10 && ret != 0; retry_count++)
+		ret = get_mountinfo(device_number, device_info, MOUNTINFO_PATH);
+
+	return ret;
+}
+
 int main(int argc, char **argv, char **envp)
 {
+	int ret = 0;
+	struct device_info device;
 	unsigned int readahead = 128;
 
 	log_open();
 
-	info("Setting the readahead to 128\n");
+	if (argc != 2) {
+		err("Expected device number as argument, got none\n");
+		return -EINVAL;
+	}
+
+	if ((ret = get_device_info(argv[1], &device)) != 0) {
+		err("Failed to find device %s (%d)\n", argv[1], ret);
+		goto out;
+	}
+
+	if (strncmp("nfs", device.fstype, 3) != 0) {
+		ret = -EINVAL;
+		info("Not setting the readahead for fstype %s\n", device.fstype);
+		goto out;
+	}
+
+	info("Setting %s readahead to 128\n", device.mountpoint);
 
 	log_close();
 	printf("%d\n", readahead);
+
+out:
+	free_device_info(&device);
+	return ret;
 }
-- 
2.35.1

[RFC v2 PATCH 5/7] readahead: create the configuration file

[Thiago Becker]

At this stage, the configuration file only accepts the default value.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1946283
Signed-off-by: Thiago Becker <tbecker@redhat.com>
---
 .gitignore                               |  5 ++
 configure.ac                             |  3 +
 tools/nfs-readahead-udev/Makefile.am     | 16 ++++-
 tools/nfs-readahead-udev/config_parser.c | 25 ++++++++
 tools/nfs-readahead-udev/config_parser.h | 14 +++++
 tools/nfs-readahead-udev/list.h          | 48 +++++++++++++++
 tools/nfs-readahead-udev/main.c          | 68 ++++++++++++++++++++-
 tools/nfs-readahead-udev/parser.y        | 78 ++++++++++++++++++++++++
 tools/nfs-readahead-udev/readahead.conf  |  1 +
 tools/nfs-readahead-udev/scanner.l       | 14 +++++
 10 files changed, 269 insertions(+), 3 deletions(-)
 create mode 100644 tools/nfs-readahead-udev/config_parser.c
 create mode 100644 tools/nfs-readahead-udev/config_parser.h
 create mode 100644 tools/nfs-readahead-udev/list.h
 create mode 100644 tools/nfs-readahead-udev/parser.y
 create mode 100644 tools/nfs-readahead-udev/readahead.conf
 create mode 100644 tools/nfs-readahead-udev/scanner.l

diff --git a/.gitignore b/.gitignore
index c99269a4..340ee8fb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,6 +17,7 @@ aclocal/ltoptions.m4
 aclocal/ltsugar.m4
 aclocal/ltversion.m4
 aclocal/lt~obsolete.m4
+ylwrap
 # files generated by configure
 confdefs.h
 config.cache
@@ -61,7 +62,11 @@ utils/statd/statd
 tools/locktest/testlk
 tools/getiversion/getiversion
 tools/nfsconf/nfsconf
+tools/nfs-readahead-udev/99-nfs_bdi.rules
 tools/nfs-readahead-udev/nfs-readahead-udev
+tools/nfs-readahead-udev/parser.c
+tools/nfs-readahead-udev/parser.h
+tools/nfs-readahead-udev/scanner.c
 support/export/mount.h
 support/export/mount_clnt.c
 support/export/mount_xdr.c
diff --git a/configure.ac b/configure.ac
index 7e5ba5d9..03cdf8f6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -300,7 +300,10 @@ AC_PROG_INSTALL
 AC_PROG_LN_S
 AC_PROG_MAKE_SET
 AC_PROG_LIBTOOL
+AC_PROG_YACC
 AM_PROG_CC_C_O
+AM_PROG_LEX
+
 
 if test "x$cross_compiling" = "xno"; then
 	CC_FOR_BUILD=${CC_FOR_BUILD-${CC-gcc}}
diff --git a/tools/nfs-readahead-udev/Makefile.am b/tools/nfs-readahead-udev/Makefile.am
index 551d22e9..010350aa 100644
--- a/tools/nfs-readahead-udev/Makefile.am
+++ b/tools/nfs-readahead-udev/Makefile.am
@@ -1,12 +1,24 @@
 libexec_PROGRAMS = nfs-readahead-udev
-nfs_readahead_udev_SOURCES = main.c syslog.c
+BUILT_SOURCES = parser.c parser.h scanner.c
+nfs_readahead_udev_SOURCES = main.c syslog.c config_parser.c parser.y scanner.l
 nfs_readahead_udev_LDFLAGS= -lmount
+AM_YFLAGS = -d
 
-udev_rulesdir = /etc/udev/rules.d
+udev_rulesdir = $(sysconfdir)/udev/rules.d
 udev_rules_DATA = 99-nfs_bdi.rules
 
+ra_confdir = $(sysconfdir)
+ra_conf_DATA = readahead.conf
+
 99-nfs_bdi.rules: 99-nfs_bdi.rules.in $(builddefs)
 	$(SED) "s|_libexecdir_|@libexecdir@|g" 99-nfs_bdi.rules.in > $@
 
 clean-local:
 	$(RM) 99-nfs_bdi.rules
+	$(RM) parser.c parser.h scanner.c
+
+parser.$(OBJEXT): CFLAGS += -Wno-strict-prototypes
+
+scanner.$(OBJEXT): CFLAGS += -Wno-strict-prototypes
+
+main.$(OBJEXT): CFLAGS += -DSYSCONFDIR=\"@sysconfdir@\"
diff --git a/tools/nfs-readahead-udev/config_parser.c b/tools/nfs-readahead-udev/config_parser.c
new file mode 100644
index 00000000..24d58a6b
--- /dev/null
+++ b/tools/nfs-readahead-udev/config_parser.c
@@ -0,0 +1,25 @@
+#include <stdlib.h>
+#include <string.h>
+#include "config_parser.h"
+
+struct config_entry *config_entry_new(void);
+
+struct config_entry *config_entry_new(void) {
+	struct config_entry *new = malloc(sizeof(struct config_entry));
+	if (!new) {
+		return NULL; // Make this an err_ptr
+	}
+
+	memset(new, 0, sizeof(struct config_entry));
+	list_init(&new->list);
+	return new;
+}
+
+
+void config_entry_free(struct config_entry *ce)
+{
+#define sfree(ce) if (ce) free(ce)
+	sfree(ce->mountpoint);
+	sfree(ce->fstype);
+#undef sfree
+}
diff --git a/tools/nfs-readahead-udev/config_parser.h b/tools/nfs-readahead-udev/config_parser.h
new file mode 100644
index 00000000..f9f138bb
--- /dev/null
+++ b/tools/nfs-readahead-udev/config_parser.h
@@ -0,0 +1,14 @@
+#ifndef __ra_libparser_h__
+#define __ra_libparser_h__
+#include "list.h"
+
+struct config_entry {
+	struct list_head list;
+	char *mountpoint;
+	char *fstype;
+	int readahead;
+};
+
+extern int parse_config(const char *, struct list_head *config_list);
+extern void config_entry_free(struct config_entry *);
+#endif
diff --git a/tools/nfs-readahead-udev/list.h b/tools/nfs-readahead-udev/list.h
new file mode 100644
index 00000000..69239502
--- /dev/null
+++ b/tools/nfs-readahead-udev/list.h
@@ -0,0 +1,48 @@
+#ifndef __ra_list_h__
+#define __ra_list_h__
+struct list_head
+{
+	struct list_head *next, *prev;
+};
+
+static inline void list_init(struct list_head *lh)
+{
+	lh->next = lh;
+	lh->prev = lh;
+}
+
+#define LIST_DECLARE(name)	 \
+	struct list_head name;	 \
+	list_init(&name);
+
+static inline void list_add(struct list_head *l, struct list_head *a)
+{
+	a->prev = l;
+	a->next = l->next;
+	l->next->prev = a;
+	l->next = a;
+}
+
+static inline void list_del(struct list_head *a)
+{
+	a->next->prev = a->prev;
+	a->prev->next = a->next;
+	a->next = a;
+	a->prev = a;
+}
+
+#define list_for_each(pos, head)				\
+	for (pos = (head)->next; pos != (head); pos = pos->next)
+
+#define list_free(head, free_func) ({				\
+	for (struct list_head *__lh = (head)->next;		\
+			__lh != head; __lh = (head)->next) {	\
+		list_del(__lh);					\
+		free_func(__lh);				\
+	}})
+
+#define containerof(p, type, field) ({				\
+	const __typeof__(((type *)0)->field) *__ptr = (p);	\
+	(type *)((char *)__ptr - offsetof(type, field) ); })
+
+#endif
diff --git a/tools/nfs-readahead-udev/main.c b/tools/nfs-readahead-udev/main.c
index bbb408c0..2bd76138 100644
--- a/tools/nfs-readahead-udev/main.c
+++ b/tools/nfs-readahead-udev/main.c
@@ -2,16 +2,23 @@
 #include <string.h>
 #include <stdlib.h>
 #include <errno.h>
+#include <stddef.h>
 
 #include <libmount/libmount.h>
 #include <sys/sysmacros.h>
 
 #include "log.h"
+#include "list.h"
+#include "config_parser.h"
 
 #ifndef MOUNTINFO_PATH
 #define MOUNTINFO_PATH "/proc/self/mountinfo"
 #endif
 
+#ifndef READAHEAD_CONFIG_FILE
+#define READAHEAD_CONFIG_FILE SYSCONFDIR "/readahead.conf"
+#endif
+
 /* Device information from the system */
 struct device_info {
 	char *device_number;
@@ -110,6 +117,60 @@ static int get_device_info(const char *device_number, struct device_info *device
 	return ret;
 }
 
+static void config_entry_list_head_free(struct list_head *lh) {
+	struct config_entry *ce = containerof(lh, struct config_entry, list);
+	config_entry_free(ce);
+}
+
+static int match_config(struct device_info *di, struct config_entry *ce) {
+#define FIELD_CMP(field) \
+	(ce->field == NULL || (di->field != NULL && strcmp(di->field, ce->field) == 0))
+
+	if (!FIELD_CMP(mountpoint))
+		return 0;
+
+	if (!FIELD_CMP(fstype))
+		return 0;
+
+	debug("Device matched with config\n");
+	return 1;
+#undef FIELD_CMP
+}
+
+static int get_readahead(struct device_info *di, unsigned int *readahead)
+{
+	LIST_DECLARE(configs);
+	struct list_head *lh;
+	int ret = 0;
+	int default_ra = 0;
+
+	if ((ret = parse_config(READAHEAD_CONFIG_FILE, &configs)) != 0) {
+		err("Failed to read configuration (%d)\n", ret);
+		goto out;
+	}
+
+	list_for_each(lh, &configs) {
+		struct config_entry *ce = containerof(lh, struct config_entry, list);
+		if (ce->mountpoint == NULL && ce->fstype == NULL) {
+			default_ra = ce->readahead;
+			continue;
+		}
+
+		if (match_config(di, ce)) {
+			*readahead = ce->readahead;
+			goto out;
+		}
+	}
+
+	/* fallthrough */
+	debug("Setting readahead to default %d\n", default_ra);
+	*readahead = default_ra;
+
+out:
+	list_free(&configs, config_entry_list_head_free);
+	return ret;
+}
+
 int main(int argc, char **argv, char **envp)
 {
 	int ret = 0;
@@ -134,7 +195,12 @@ int main(int argc, char **argv, char **envp)
 		goto out;
 	}
 
-	info("Setting %s readahead to 128\n", device.mountpoint);
+	if ((ret = get_readahead(&device, &readahead)) != 0) {
+		err("Failed to find readahead (%d)\n", ret);
+		goto out;
+	}
+
+	info("Setting %s readahead to %u\n", device.mountpoint, readahead);
 
 	log_close();
 	printf("%d\n", readahead);
diff --git a/tools/nfs-readahead-udev/parser.y b/tools/nfs-readahead-udev/parser.y
new file mode 100644
index 00000000..f6db05c4
--- /dev/null
+++ b/tools/nfs-readahead-udev/parser.y
@@ -0,0 +1,78 @@
+%{
+#include <stdio.h>
+#include "parser.h"
+#include "config_parser.h"
+#include "log.h"
+
+extern int yylex();
+extern int yyparse();
+extern FILE *yyin;
+void yyerror(const char *s);
+
+// This should be visible only to this file
+extern struct config_entry *config_entry_new(void);
+
+struct config_entry *current;
+%}
+
+%union {
+	int ival;
+}
+
+%token <ival> INT
+%token EQ
+%token ENDL
+%token DEFAULT
+%token READAHEAD
+%token END_CONFIG 0
+
+%%
+config:
+	lines
+lines:
+	lines line | line | endls lines
+line:
+	tokens endls {
+		struct config_entry *new = config_entry_new();
+		list_add(&current->list, &new->list);
+		current = new;
+	}
+
+
+tokens:
+	tokens token | token
+
+token:
+	default | pair
+
+default:
+	DEFAULT
+
+pair:
+	READAHEAD EQ INT	{ current->readahead = $3; }
+
+endls:
+	endls ENDL | ENDL
+
+%%
+int parse_config(const char *filename, struct list_head *list)
+{
+	int ret;
+
+	yyin = fopen(filename, "r");
+	current = config_entry_new();
+	list_add(list, &current->list);
+
+	ret = yyparse();
+
+	/* The parser will create an empty entry that need to be eliminated */
+	list_del(&current->list);
+	free(current);
+	fclose(yyin);
+
+	return ret;
+}
+
+void yyerror(const char *s) {
+	err("Failed to parse configuration: %s", s);
+}
diff --git a/tools/nfs-readahead-udev/readahead.conf b/tools/nfs-readahead-udev/readahead.conf
new file mode 100644
index 00000000..988b30c7
--- /dev/null
+++ b/tools/nfs-readahead-udev/readahead.conf
@@ -0,0 +1 @@
+default				readahead=128
diff --git a/tools/nfs-readahead-udev/scanner.l b/tools/nfs-readahead-udev/scanner.l
new file mode 100644
index 00000000..d1ceb90b
--- /dev/null
+++ b/tools/nfs-readahead-udev/scanner.l
@@ -0,0 +1,14 @@
+%{
+#include "parser.h"
+#define yyterminate() return END_CONFIG
+%}
+%option noyywrap
+%%
+default		{ return DEFAULT; }
+readahead	{ return READAHEAD; }
+[ \t]		;
+#[^\n]*\n	{ return ENDL; }
+\n		{ return ENDL; }
+[0-9]+		{ yylval.ival = atoi(yytext); return INT; }
+=		{ return EQ; }
+%%
-- 
2.35.1

[RFC v2 PATCH 6/7] readahead: add mountpoint and fstype options

[Thiago Becker]

Add ways to configure the system by mountpoint or fstype.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1946283
Signed-off-by: Thiago Becker <tbecker@redhat.com>
---
 tools/nfs-readahead-udev/parser.y  | 15 +++++++++++----
 tools/nfs-readahead-udev/scanner.l |  5 +++++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/tools/nfs-readahead-udev/parser.y b/tools/nfs-readahead-udev/parser.y
index f6db05c4..5951c99d 100644
--- a/tools/nfs-readahead-udev/parser.y
+++ b/tools/nfs-readahead-udev/parser.y
@@ -10,20 +10,25 @@ extern FILE *yyin;
 void yyerror(const char *s);
 
 // This should be visible only to this file
-extern struct config_entry *config_entry_new(void);
+extern struct config_entry *config_entry_new();
 
 struct config_entry *current;
 %}
 
 %union {
+	char *sval;
 	int ival;
 }
 
+%token <sval> STRING
 %token <ival> INT
 %token EQ
 %token ENDL
 %token DEFAULT
+%token MOUNTPOINT
+%token FSTYPE
 %token READAHEAD
+%token <sval> FS
 %token END_CONFIG 0
 
 %%
@@ -35,7 +40,7 @@ line:
 	tokens endls {
 		struct config_entry *new = config_entry_new();
 		list_add(&current->list, &new->list);
-		current = new;
+		current = new;		
 	}
 
 
@@ -49,9 +54,11 @@ default:
 	DEFAULT
 
 pair:
-	READAHEAD EQ INT	{ current->readahead = $3; }
+	MOUNTPOINT EQ STRING	{ current->mountpoint = $3; }
+	| FSTYPE EQ FS		{ current->fstype = $3; }
+	| READAHEAD EQ INT	{ current->readahead = $3; }
 
-endls:
+endls: 
 	endls ENDL | ENDL
 
 %%
diff --git a/tools/nfs-readahead-udev/scanner.l b/tools/nfs-readahead-udev/scanner.l
index d1ceb90b..c6fb3f0c 100644
--- a/tools/nfs-readahead-udev/scanner.l
+++ b/tools/nfs-readahead-udev/scanner.l
@@ -5,10 +5,15 @@
 %option noyywrap
 %%
 default		{ return DEFAULT; }
+mountpoint	{ return MOUNTPOINT; }
+fstype		{ return FSTYPE; }
 readahead	{ return READAHEAD; }
+nfs4		{ yylval.sval = strdup(yytext); return FS; }
+nfs		{ yylval.sval = strdup(yytext); return FS; }
 [ \t]		;
 #[^\n]*\n	{ return ENDL; }
 \n		{ return ENDL; }
 [0-9]+		{ yylval.ival = atoi(yytext); return INT; }
+[a-zA-Z0-9/]+	{ yylval.sval = strdup(yytext); return STRING; }
 =		{ return EQ; }
 %%
-- 
2.35.1

[RFC v2 PATCH 7/7] readahead: documentation

[Thiago Becker]

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1946283
Signed-off-by: Thiago Becker <tbecker@redhat.com>
---
 tools/nfs-readahead-udev/Makefile.am          |  2 +
 .../nfs-readahead-udev/nfs-readahead-udev.man | 47 +++++++++++++++++++
 tools/nfs-readahead-udev/readahead.conf       | 14 ++++++
 3 files changed, 63 insertions(+)
 create mode 100644 tools/nfs-readahead-udev/nfs-readahead-udev.man

diff --git a/tools/nfs-readahead-udev/Makefile.am b/tools/nfs-readahead-udev/Makefile.am
index 010350aa..eaa9b90e 100644
--- a/tools/nfs-readahead-udev/Makefile.am
+++ b/tools/nfs-readahead-udev/Makefile.am
@@ -10,6 +10,8 @@ udev_rules_DATA = 99-nfs_bdi.rules
 ra_confdir = $(sysconfdir)
 ra_conf_DATA = readahead.conf
 
+man5_MANS = nfs-readahead-udev.man
+
 99-nfs_bdi.rules: 99-nfs_bdi.rules.in $(builddefs)
 	$(SED) "s|_libexecdir_|@libexecdir@|g" 99-nfs_bdi.rules.in > $@
 
diff --git a/tools/nfs-readahead-udev/nfs-readahead-udev.man b/tools/nfs-readahead-udev/nfs-readahead-udev.man
new file mode 100644
index 00000000..2477d5b3
--- /dev/null
+++ b/tools/nfs-readahead-udev/nfs-readahead-udev.man
@@ -0,0 +1,47 @@
+.\" Manpage for nfs-readahead-udev.
+.nh
+.ad l
+.TH man 5 "08 Mar 2022" "1.0" "nfs-readahead-udev man page"
+.SH NAME
+
+nfs-readahead-udev \- Find the readahead for a given NFS mount
+
+.SH SYNOPSIS
+
+nfs-readahead-udev <device>
+
+.SH DESCRIPTION
+
+\fInfs-readahead-udev\fR is a tool intended to be used with udev to set the \fIread_ahead_kb\fR parameter of NFS mounts, according to the configuration file (see \fICONFIGURATION\fR). \fIdevice\fR is the device number for the NFS backing device as provided by the kernel.
+
+.SH CONFIGURATION
+
+The configuration file (\fI/etc/readahead.conf\fR) contains the readahead configuration, and is formatted as follows.
+
+<LINES> ::= <LINES> <LINE> | <LINE>
+
+<LINE> ::= <TOKENS> <ENDL>
+
+<TOKENS> ::= <TOKENS> <TOKEN> | <TOKEN>
+
+<TOKEN> ::= default | <PAIR>
+
+<PAIR> ::= mountpoint = <mountpoint> | fstype = <nfs|nfs4> | readahead = <readahead>
+
+\fImountpoint\fR is the path in the system where the file system is mounted.
+
+\fIreadahead\fR is an integer to readahead.
+
+\fIfstype\fR is either \fInfs\fR or \fInfs4\fR. 
+
+.SH SEE ALSO
+
+mount.nfs(8), nfs(5), udev(7), bcc-readahead(8)
+
+.SH BUGS
+
+No known bugs.
+
+.SH AUTHOR
+
+Thiago Rafael Becker <trbecker@gmail.com>
diff --git a/tools/nfs-readahead-udev/readahead.conf b/tools/nfs-readahead-udev/readahead.conf
index 988b30c7..bce830f1 100644
--- a/tools/nfs-readahead-udev/readahead.conf
+++ b/tools/nfs-readahead-udev/readahead.conf
@@ -1 +1,15 @@
+# nfs-readahead-udev configuration file.
+#
+# This file configures the readahead for nfs mounts when those are anounced by the kernel.
+# The file is composed on lines that can contain either the default configuration (applied to
+# any nfs mount that does not match any of the other lines) or a combination of
+#   mountpoint=<mountpoint> where mountpoint is the mount point for the file system
+#   fstype=<nfs|nfs4> specifies that this configuration should only apply to a specific nfs
+#     version.
+# Every line must contain a readahead option, with the expected readahead value.
 default				readahead=128
+
+# mountpoint=/mnt		readahead=4194304
+# fstype=nfs			readahead=4194304
+# fstype=nfs4			readahead=4194304
+# mountpoint=/mnt	fstype=nfs4	readahead=4194304
-- 
2.35.1

Re: [RFC v2 PATCH 0/7] Introduce nfs-readahead-udev

[Steve Dickson]

On 3/11/22 2:06 PM, Thiago Becker wrote:
> Recent changes in the linux kernel caused NFS readahead to default to
> 128 from the previous default of 15 * rsize. This causes performance
> penalties to some read-heavy workloads, which can be fixed by
> tuning the readahead for that given mount.
> 
> Specifically, the read troughput on a sec=krb5p mount drops by 50-75%
> when comparing the default readahead with a readahead of 15360.
> 
> Previous discussions:
> https://lore.kernel.org/linux-nfs/20210803130717.2890565-1-trbecker@gmail.com/
> I attempted to add a non-kernel option to mount.nfs, and it was
> rejected.
> 
> https://lore.kernel.org/linux-nfs/20210811171402.947156-1-trbecker@gmail.com/
> Attempted to add a mount option to the kernel, rejected as well.
> 
> I had started a separate tool to set the readahead of BDIs, but the
> scope is specifically for NFS, so I would like to get the community
> feeling for having this in nfs-utils.
> 
> This patch series introduces nfs-readahead-udev, a utility to
> automatically set NFS readahead when NFS is mounted. The utility is
> triggered by udev when a new BDI is added, returns to udev the value of
> the readahead that should be used.
> 
> The tool currently supports setting read ahead per mountpoint, nfs major
> version, or by a global default value.
> 
> Thiago Becker (7):
>    Create nfs-readahead-udev
>    readahead: configure udev
>    readahead: create logging facility
>    readahead: only set readahead for nfs devices.
>    readahead: create the configuration file
>    readahead: add mountpoint and fstype options
>    readahead: documentation
> 
>   .gitignore                                    |   6 +
>   configure.ac                                  |   4 +
>   tools/Makefile.am                             |   2 +-
>   tools/nfs-readahead-udev/99-nfs_bdi.rules.in  |   1 +
>   tools/nfs-readahead-udev/Makefile.am          |  26 +++
>   tools/nfs-readahead-udev/config_parser.c      |  25 +++
>   tools/nfs-readahead-udev/config_parser.h      |  14 ++
>   tools/nfs-readahead-udev/list.h               |  48 ++++
>   tools/nfs-readahead-udev/log.h                |  16 ++
>   tools/nfs-readahead-udev/main.c               | 211 ++++++++++++++++++
>   .../nfs-readahead-udev/nfs-readahead-udev.man |  47 ++++
>   tools/nfs-readahead-udev/parser.y             |  85 +++++++
>   tools/nfs-readahead-udev/readahead.conf       |  15 ++
>   tools/nfs-readahead-udev/scanner.l            |  19 ++
>   tools/nfs-readahead-udev/syslog.c             |  47 ++++
>   15 files changed, 565 insertions(+), 1 deletion(-)
>   create mode 100644 tools/nfs-readahead-udev/99-nfs_bdi.rules.in
>   create mode 100644 tools/nfs-readahead-udev/Makefile.am
>   create mode 100644 tools/nfs-readahead-udev/config_parser.c
>   create mode 100644 tools/nfs-readahead-udev/config_parser.h
>   create mode 100644 tools/nfs-readahead-udev/list.h
>   create mode 100644 tools/nfs-readahead-udev/log.h
>   create mode 100644 tools/nfs-readahead-udev/main.c
>   create mode 100644 tools/nfs-readahead-udev/nfs-readahead-udev.man
>   create mode 100644 tools/nfs-readahead-udev/parser.y
>   create mode 100644 tools/nfs-readahead-udev/readahead.conf
>   create mode 100644 tools/nfs-readahead-udev/scanner.l
>   create mode 100644 tools/nfs-readahead-udev/syslog.c
> 
My apologies for waiting late on this... I was on PTO.

I really don't like the name of the is command. It
does not follow any of the naming conventions we used in the
past... Can you please rename the command to nfsrahead.

tia,

steved.

Re: [RFC v2 PATCH 7/7] readahead: documentation

[Steve Dickson]

On 3/11/22 2:06 PM, Thiago Becker wrote:
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1946283
> Signed-off-by: Thiago Becker <tbecker@redhat.com>
> ---
>   tools/nfs-readahead-udev/Makefile.am          |  2 +
>   .../nfs-readahead-udev/nfs-readahead-udev.man | 47 +++++++++++++++++++
>   tools/nfs-readahead-udev/readahead.conf       | 14 ++++++
>   3 files changed, 63 insertions(+)
>   create mode 100644 tools/nfs-readahead-udev/nfs-readahead-udev.man
> 
> diff --git a/tools/nfs-readahead-udev/Makefile.am b/tools/nfs-readahead-udev/Makefile.am
> index 010350aa..eaa9b90e 100644
> --- a/tools/nfs-readahead-udev/Makefile.am
> +++ b/tools/nfs-readahead-udev/Makefile.am
> @@ -10,6 +10,8 @@ udev_rules_DATA = 99-nfs_bdi.rules
>   ra_confdir = $(sysconfdir)
>   ra_conf_DATA = readahead.conf
>   
> +man5_MANS = nfs-readahead-udev.man
> +
>   99-nfs_bdi.rules: 99-nfs_bdi.rules.in $(builddefs)
>   	$(SED) "s|_libexecdir_|@libexecdir@|g" 99-nfs_bdi.rules.in > $@
>   
> diff --git a/tools/nfs-readahead-udev/nfs-readahead-udev.man b/tools/nfs-readahead-udev/nfs-readahead-udev.man
> new file mode 100644
> index 00000000..2477d5b3
> --- /dev/null
> +++ b/tools/nfs-readahead-udev/nfs-readahead-udev.man
> @@ -0,0 +1,47 @@
> +.\" Manpage for nfs-readahead-udev.
> +.nh
> +.ad l
> +.TH man 5 "08 Mar 2022" "1.0" "nfs-readahead-udev man page"
> +.SH NAME
> +
> +nfs-readahead-udev \- Find the readahead for a given NFS mount
> +
> +.SH SYNOPSIS
> +
> +nfs-readahead-udev <device>
> +
> +.SH DESCRIPTION
> +
> +\fInfs-readahead-udev\fR is a tool intended to be used with udev to set the \fIread_ahead_kb\fR parameter of NFS mounts, according to the configuration file (see \fICONFIGURATION\fR). \fIdevice\fR is the device number for the NFS backing device as provided by the kernel.
> +
> +.SH CONFIGURATION
> +
> +The configuration file (\fI/etc/readahead.conf\fR) contains the readahead configuration, and is formatted as follows.
> +
> +<LINES> ::= <LINES> <LINE> | <LINE>
> +
> +<LINE> ::= <TOKENS> <ENDL>
> +
> +<TOKENS> ::= <TOKENS> <TOKEN> | <TOKEN>
> +
> +<TOKEN> ::= default | <PAIR>
> +
> +<PAIR> ::= mountpoint = <mountpoint> | fstype = <nfs|nfs4> | readahead = <readahead>
> +
> +\fImountpoint\fR is the path in the system where the file system is mounted.
> +
> +\fIreadahead\fR is an integer to readahead.
> +
> +\fIfstype\fR is either \fInfs\fR or \fInfs4\fR.
> +
> +.SH SEE ALSO
> +
> +mount.nfs(8), nfs(5), udev(7), bcc-readahead(8)
> +
> +.SH BUGS
> +
> +No known bugs.
> +
> +.SH AUTHOR
I think it might make sense to added some examples
on how the command will be used.

> +
> +Thiago Rafael Becker <trbecker@gmail.com>
> diff --git a/tools/nfs-readahead-udev/readahead.conf b/tools/nfs-readahead-udev/readahead.conf
> index 988b30c7..bce830f1 100644
> --- a/tools/nfs-readahead-udev/readahead.conf
> +++ b/tools/nfs-readahead-udev/readahead.conf
> @@ -1 +1,15 @@
> +# nfs-readahead-udev configuration file.
> +#
> +# This file configures the readahead for nfs mounts when those are anounced by the kernel.
> +# The file is composed on lines that can contain either the default configuration (applied to
> +# any nfs mount that does not match any of the other lines) or a combination of
> +#   mountpoint=<mountpoint> where mountpoint is the mount point for the file system
> +#   fstype=<nfs|nfs4> specifies that this configuration should only apply to a specific nfs
> +#     version.
> +# Every line must contain a readahead option, with the expected readahead value.
>   default				readahead=128
> +
> +# mountpoint=/mnt		readahead=4194304
> +# fstype=nfs			readahead=4194304
> +# fstype=nfs4			readahead=4194304
> +# mountpoint=/mnt	fstype=nfs4	readahead=4194304
Would it make sense to try added these to nfs.conf?

I must admin I'm a bit impressed with your lex and
yacc routines in patch 5, I have not seen those
in a while.. but that does add more dependencies
to nfs-utils and as well as yet another config file
to manage.

steved.

Re: [RFC v2 PATCH 7/7] readahead: documentation

[Thiago Becker]

Thanks for the comments.


On Thu, Mar 17, 2022 at 12:37 PM Steve Dickson <steved@redhat.com> wrote:
> I think it might make sense to added some examples
> on how the command will be used.

Will do.

> Would it make sense to try added these to nfs.conf?
>
> I must admin I'm a bit impressed with your lex and
> yacc routines in patch 5, I have not seen those
> in a while.. but that does add more dependencies
> to nfs-utils and as well as yet another config file
> to manage.

I debated with a funko pop between writing the parser and using a parser
builder. Originally, the idea was to provide more possibilities, like
configuring the read ahead of other devices, but those BDIs don't
announce themselves when they are added like NFS does. I also don't know
how useful it would be. So this idea is scrapped for now.

Here are the debate conclusions:
  - writing the parser would add some complexity that lex/yacc handles
  - lex/yacc add build time dependencies and some bloat
  - error handling is painful with lex/yacc
  - lex/yacc are well known
  - using a parser combinator would be my preference, but I couldn't
    find any written in c that didn't add a new library to the systems

It seemed to me that using lex/yacc was the best choice taking into
account the needs of both limited storage systems and large servers.

I can still see some configurations that may be added in the future,
like setting the read ahead per server, network segment, but I believe
that the configurations we have cover most of the use cases.

I've added a new configuration because I planned to have a more fine
grained configuration for multiple file systems/designs, and that
would add a lot in nfs.conf (and tie the tool to nfs).

And, to be completely honest, I took what I've done in
https://github.com/trbecker/readahead-utils
and ported it to nfs-utils.

That's the history so far. I don't mind taking stuff out if it would
make the software more maintainable.


> steved.
>

thiago

Re: [RFC v2 PATCH 0/7] Introduce nfs-readahead-udev

[Thiago Becker]

On Tue, Mar 15, 2022 at 8:54 AM Steve Dickson <steved@redhat.com> wrote:
> My apologies for waiting late on this... I was on PTO.
>
> I really don't like the name of the is command. It
> does not follow any of the naming conventions we used in the
> past... Can you please rename the command to nfsrahead.

I have this on my git, I'm just waiting for more comments to release a
new version.

> tia,
>
> steved.

thiago