RemixVSL / iomemory-vsl

Updated Fusion-io iomemory VSL Linux (version 3.2.16) driver for recent kernels.
154 stars 28 forks source link

Doesn't compile with newer kernels #36

Closed UweMenges closed 5 years ago

UweMenges commented 5 years ago

Apparently disk_stats ticks were replaced: https://github.com/torvalds/linux/commit/b57e99b4b8b0ebdf9707424e7ddc0c392bdc5fe6

  CC [M]  /var/lib/dkms/iomemory-vsl/3.2.15/build/kcsr.o
In file included from ./include/linux/blkdev.h:11,
                 from /var/lib/dkms/iomemory-vsl/3.2.15/build/port-internal.h:63,
                 from /var/lib/dkms/iomemory-vsl/3.2.15/build/kblock.c:58:
/var/lib/dkms/iomemory-vsl/3.2.15/build/kblock.c: In function ‘kfio_disk_stat_write_update’:
./include/linux/genhd.h:299:38: error: ‘struct disk_stats’ has no member named ‘ticks’
  (per_cpu_ptr((part)->dkstats, (cpu))->field += (addnd))
                                      ^~
./include/linux/genhd.h:366:2: note: in expansion of macro ‘__part_stat_add’
  __part_stat_add((cpu), (part), field, addnd);   \
  ^~~~~~~~~~~~~~~
/var/lib/dkms/iomemory-vsl/3.2.15/build/kblock.c:904:9: note: in expansion of macro ‘part_stat_add’
         part_stat_add(cpu, &gd->part0, ticks[1],   kfio_div64_64(duration * HZ, 1000000));
         ^~~~~~~~~~~~~
./include/linux/genhd.h:299:38: error: ‘struct disk_stats’ has no member named ‘ticks’
  (per_cpu_ptr((part)->dkstats, (cpu))->field += (addnd))
                                      ^~
./include/linux/genhd.h:368:3: note: in expansion of macro ‘__part_stat_add’
   __part_stat_add((cpu), &part_to_disk((part))->part0, \
   ^~~~~~~~~~~~~~~
/var/lib/dkms/iomemory-vsl/3.2.15/build/kblock.c:904:9: note: in expansion of macro ‘part_stat_add’
         part_stat_add(cpu, &gd->part0, ticks[1],   kfio_div64_64(duration * HZ, 1000000));
         ^~~~~~~~~~~~~
/var/lib/dkms/iomemory-vsl/3.2.15/build/kblock.c: In function ‘kfio_disk_stat_read_update’:
./include/linux/genhd.h:299:38: error: ‘struct disk_stats’ has no member named ‘ticks’
  (per_cpu_ptr((part)->dkstats, (cpu))->field += (addnd))
                                      ^~
./include/linux/genhd.h:366:2: note: in expansion of macro ‘__part_stat_add’
  __part_stat_add((cpu), (part), field, addnd);   \
  ^~~~~~~~~~~~~~~
/var/lib/dkms/iomemory-vsl/3.2.15/build/kblock.c:944:9: note: in expansion of macro ‘part_stat_add’
         part_stat_add(cpu, &gd->part0, ticks[0],   kfio_div64_64(duration * HZ, 1000000));
         ^~~~~~~~~~~~~
./include/linux/genhd.h:299:38: error: ‘struct disk_stats’ has no member named ‘ticks’
  (per_cpu_ptr((part)->dkstats, (cpu))->field += (addnd))
                                      ^~
./include/linux/genhd.h:368:3: note: in expansion of macro ‘__part_stat_add’
   __part_stat_add((cpu), &part_to_disk((part))->part0, \
   ^~~~~~~~~~~~~~~
/var/lib/dkms/iomemory-vsl/3.2.15/build/kblock.c:944:9: note: in expansion of macro ‘part_stat_add’
         part_stat_add(cpu, &gd->part0, ticks[0],   kfio_div64_64(duration * HZ, 1000000));
         ^~~~~~~~~~~~~
/var/lib/dkms/iomemory-vsl/3.2.15/build/kblock.c: In function ‘kfio_elevator_change’:
/var/lib/dkms/iomemory-vsl/3.2.15/build/kblock.c:2236:5: error: implicit declaration of function ‘elevator_exit’; did you mean ‘elevator_alloc’? [-Werror=implicit-function-declaration]
     elevator_exit(q->elevator);
     ^~~~~~~~~~~~~
     elevator_alloc
/var/lib/dkms/iomemory-vsl/3.2.15/build/kblock.c:2241:9: error: implicit declaration of function ‘elevator_init’; did you mean ‘elevator_alloc’? [-Werror=implicit-function-declaration]
     if (elevator_init(q, name))
         ^~~~~~~~~~~~~
         elevator_alloc
cc1: some warnings being treated as errors
UweMenges commented 5 years ago

On elevator_init/exit: https://github.com/torvalds/linux/commit/a8a275c9c2fb6bc9b45ad3e4187469726e2af7d1

plappermaul commented 5 years ago

Thanks for the feedback and the patch. To make it generally compatible with older versions < 4.18 the tics issue should be surrounded by IF VERSION ... statements too.

Btw. do we get any problems if we skip the elevator setup? Maybe you can posta a /dev/fioa/queue/scheduler output.

UweMenges commented 5 years ago

I don't know the code or its surroundings, I just wanted to have it work on my Fedora 29 system. :) I'm not sure about 4.18 being the correct version to decide on things.

# grep . /sys/block/fio*/queue/scheduler
/sys/block/fioa/queue/scheduler:none
/sys/block/fiob/queue/scheduler:none

It seems it sets the desired scheduler anyhow. I wonder if the elevator setup could be skipped for earlier kernel versions?

For the current state here: I can use fio[ab], but not the fio-util binaries. Eg. on calling fio-status I get No devices were found in this system. Please check if the iomemory-vsl driver is loaded.. This seems due to EPERM while mmap()ing /dev/mem:

# tail -n 30 /tmp/strace.out 
22242 close(4)                          = 0
22242 openat(AT_FDCWD, "/sys/bus/pci/devices/0000:00:1f.4/config", O_RDONLY) = 4
22242 fstat(4, {st_mode=S_IFREG|0644, st_size=256, ...}) = 0
22242 read(4, "\206\200#\241\3\0\200\0021\0\5\f\0\0\0\0\4\240\262\337\0\0\0\0\0\0\0\0\0\0\0\0"..., 4096) = 256
22242 read(4, "", 4096)                 = 0
22242 close(4)                          = 0
22242 getdents64(3, /* 0 entries */, 32768) = 0
22242 close(3)                          = 0
22242 openat(AT_FDCWD, "/var/lock/fio-hwlock-6", O_WRONLY|O_CREAT, 0600) = 3
22242 fcntl(3, F_SETLK, {l_type=F_WRLCK, l_whence=SEEK_SET, l_start=0, l_len=0}) = 0
22242 openat(AT_FDCWD, "/sys/bus/pci/devices/0000:06:00.0/resource", O_RDONLY) = 4
22242 fstat(4, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
22242 read(4, "0x0000000000000000 0x00000000000"..., 4096) = 741
22242 close(4)                          = 0
22242 openat(AT_FDCWD, "/dev/mem", O_RDWR|O_SYNC) = 4
22242 mmap(NULL, 131072, PROT_READ|PROT_WRITE, MAP_SHARED, 4, 0xdf100000) = -1 EPERM (Operation not permitted)
22242 close(4)                          = 0
22242 openat(AT_FDCWD, "/var/lock/fio-hwlock-5", O_WRONLY|O_CREAT, 0600) = 4
22242 fcntl(4, F_SETLK, {l_type=F_WRLCK, l_whence=SEEK_SET, l_start=0, l_len=0}) = 0
22242 openat(AT_FDCWD, "/sys/bus/pci/devices/0000:05:00.0/resource", O_RDONLY) = 5
22242 fstat(5, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
22242 read(5, "0x0000000000000000 0x00000000000"..., 4096) = 741
22242 close(5)                          = 0
22242 openat(AT_FDCWD, "/dev/mem", O_RDWR|O_SYNC) = 5
22242 mmap(NULL, 131072, PROT_READ|PROT_WRITE, MAP_SHARED, 5, 0xdf300000) = -1 EPERM (Operation not permitted)
22242 close(5)                          = 0
22242 fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(0x88, 0xd), ...}) = 0
22242 write(1, "No devices were found in this sy"..., 89) = 89
22242 exit_group(0)                     = ?
22242 +++ exited with 0 +++

But I don't have an idea why it gets that.

UweMenges commented 5 years ago

More trouble ahead. Fedora 29 now has kernel 4.20.3-200.fc29.x86_64 and dkms fails to build the module:

  CC [M]  /var/lib/dkms/iomemory-vsl/3.2.15/build/kexports.o
/var/lib/dkms/iomemory-vsl/3.2.15/build/ktime.c: In function ‘fusion_getwallclocktime’:
/var/lib/dkms/iomemory-vsl/3.2.15/build/ktime.c:133:26: error: implicit declaration of function ‘current_kernel_time’; did you mean ‘current_kernel_time64’? [-Werror=implicit-function-declaration]                
     struct timespec ts = current_kernel_time();
                          ^~~~~~~~~~~~~~~~~~~
                          current_kernel_time64
/var/lib/dkms/iomemory-vsl/3.2.15/build/ktime.c:133:26: error: invalid initializer
cc1: some warnings being treated as errors
make[2]: *** [scripts/Makefile.build:291: /var/lib/dkms/iomemory-vsl/3.2.15/build/ktime.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [Makefile:1567: _module_/var/lib/dkms/iomemory-vsl/3.2.15/build] Error 2
make[1]: Leaving directory '/usr/src/kernels/4.20.3-200.fc29.x86_64'
make: *** [Makefile:101: modules] Error 2

Going the obvious route:

--- ./root/usr/src/iomemory-vsl-3.2.15/ktime.c.orig     2019-01-24 21:34:26.056270236 +0100
+++ ./root/usr/src/iomemory-vsl-3.2.15/ktime.c  2019-01-24 21:34:51.825468010 +0100
@@ -130,7 +130,7 @@
 /// @brief return current UTC wall clock time in seconds since the Unix epoch (Jan 1 1970).
 uint64_t noinline fusion_getwallclocktime(void)
 {
-    struct timespec ts = current_kernel_time();
+    struct timespec64 ts = current_kernel_time64();

     return (uint64_t)ts.tv_sec;
 }

leads to

  SHIPPED /var/lib/dkms/iomemory-vsl/3.2.15/build/kfio/x86_64_cc82_libkfio.o
  CC [M]  /var/lib/dkms/iomemory-vsl/3.2.15/build/module_param.o
  LD [M]  /var/lib/dkms/iomemory-vsl/3.2.15/build/iomemory-vsl.o
  Building modules, stage 2.
  MODPOST 1 modules
FATAL: modpost: GPL-incompatible module iomemory-vsl.ko uses GPL-only symbol 'ktime_get_real_seconds'
make[2]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
make[1]: *** [Makefile:1571: modules] Error 2
make[1]: Leaving directory '/usr/src/kernels/4.20.3-200.fc29.x86_64'
make: *** [Makefile:101: modules] Error 2

I think the issue is that https://github.com/torvalds/linux/commit/33e26418193f58d1895f2f968e1953b1caf8deb7#diff-03ab89da17a203808142f6a1f0272153L2183 removed get_seconds() and https://github.com/torvalds/linux/commit/dbe7aa622db96b5cd601f59d09c4f00b98b76079#diff-03ab89da17a203808142f6a1f0272153R707 added the replacement ktime_get_real_seconds() with EXPORT_SYMBOL_GPL.

plappermaul commented 5 years ago

I think ktime_get_coarse_real_ts64() is the way to go. See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.0-rc3&id=54e4f73cbe03dd0634548e40d12c442d377c36c4

UweMenges commented 5 years ago

I tried ktime_get_real_ts64() and ktime_get_coarse_real_ts64() as well, but this leads to the same error. I don't quite understand why, though.

plappermaul commented 5 years ago

Please retry compilation with newest next_generation branch.

UweMenges commented 5 years ago

Thanks, I could compile & build the rpm with latest. Using dkms, it apparently needs more "Proprietary" to "GPL" changes than https://github.com/snuf/iomemory-vsl/commit/55d587e9e961ce72da02e7342abd38b7d6d897df to succeed. This makes it build with dkms for me, but raises a license question:

diff --git a/root/usr/src/iomemory-vsl-3.2.15/Kbuild b/root/usr/src/iomemory-vsl-3.2.15/Kbuild
index f06ebcf..84f10e0 100644
--- a/root/usr/src/iomemory-vsl-3.2.15/Kbuild
+++ b/root/usr/src/iomemory-vsl-3.2.15/Kbuild
@@ -9,7 +9,7 @@ endif

 $(src)/license.c: $(src)/Kbuild
-       printf '#include "linux/module.h"\nMODULE_LICENSE("Proprietary");\n' >$@
+       printf '#include "linux/module.h"\nMODULE_LICENSE("GPL");\n' >$@

 obj-m := $(FIO_DRIVER_NAME).o
diff --git a/root/usr/src/iomemory-vsl-3.2.15/Makefile b/root/usr/src/iomemory-vsl-3.2.15/Makefile
index 99d45ac..856db9c 100644
--- a/root/usr/src/iomemory-vsl-3.2.15/Makefile
+++ b/root/usr/src/iomemory-vsl-3.2.15/Makefile
@@ -81,7 +81,7 @@ clean modules_clean:
                KFIO_LIB=$(KFIO_LIB) \
                clean
        rm -rf include/fio/port/linux/kfio_config.h kfio_config
-       sed -i 's/GPL/Proprietary/g' license.c
+#      sed -i 's/GPL/Proprietary/g' license.c

 include/fio/port/linux/kfio_config.h: kfio_config.sh
        ./$< -a $(FIOARCH) -o $@ -k $(KERNEL_SRC) -p -d $(CURDIR)/kfio_config -l $(FUSION_DEBUG)
plappermaul commented 5 years ago

As we are totally out of support with all this stuff here we must accept the tiny inconsistencies.

Bugs fixed. Closing the issue.