RemixVSL / iomemory-vsl

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

Compiling from branch fio-3.2.16.1732 on CentOS8 #92

Closed vladsol closed 3 years ago

vladsol commented 3 years ago

Is it possible?

Tried different GCC (8,9,10) versions, but, no luck.

/root/rpmbuild/BUILD/iomemory-vsl-3.2.16.1732/driver_source/kblock.c: In function 'kfio_disk_stat_write_update':
./include/linux/compiler-gcc.h:16:19: error: expected expression before '__asm__'
 #define barrier() __asm__ __volatile__("": : :"memory")
                   ^~~~~~~
./include/linux/preempt.h:242:29: note: in expansion of macro 'barrier'
 #define preempt_disable()   barrier()
                             ^~~~~~~
./include/linux/genhd.h:321:26: note: in expansion of macro 'preempt_disable'
 #define part_stat_lock() preempt_disable()
                          ^~~~~~~~~~~~~~~
/root/rpmbuild/BUILD/iomemory-vsl-3.2.16.1732/driver_source/kblock.c:930:15: note: in expansion of macro 'part_stat_lock'
         cpu = part_stat_lock();
               ^~~~~~~~~~~~~~
/root/rpmbuild/BUILD/iomemory-vsl-3.2.16.1732/driver_source/kblock.c: In function 'kfio_disk_stat_read_update':
./include/linux/compiler-gcc.h:16:19: error: expected expression before '__asm__'
 #define barrier() __asm__ __volatile__("": : :"memory")
                   ^~~~~~~
./include/linux/preempt.h:242:29: note: in expansion of macro 'barrier'
 #define preempt_disable()   barrier()
                             ^~~~~~~
./include/linux/genhd.h:321:26: note: in expansion of macro 'preempt_disable'
 #define part_stat_lock() preempt_disable()
                          ^~~~~~~~~~~~~~~
/root/rpmbuild/BUILD/iomemory-vsl-3.2.16.1732/driver_source/kblock.c:989:15: note: in expansion of macro 'part_stat_lock'
         cpu = part_stat_lock();
               ^~~~~~~~~~~~~~

On CentOS7 there is no problems. I'm trying to use 3.2.16 because my hd is iodrive2

bplein commented 3 years ago

Have you installed the correct kernel headers for your kernel? I'm not a developer but here's a checklist of information that might help us get you going, please forward the output of:

uname -r

As well as the commands you ran prior to the build. In other words, did you git clone followed by a git checkout of a specific branch, and if so, what was that branch?

Unless I have a specific need for a specific branch, I usually use master (i.e. just git clone, don't check out a branch)

vladsol commented 3 years ago

Have you installed the correct kernel headers for your kernel? I'm not a developer but here's a checklist of information that might help us get you going, please forward the output of:

uname -r

As well as the commands you ran prior to the build. In other words, did you git clone followed by a git checkout of a specific branch, and if so, what was that branch?

Unless I have a specific need for a specific branch, I usually use master (i.e. just git clone, don't check out a branch)

Unfortunately, my fusionio supported only in v3 driver.

The problem is that the implementation of part_stat_lock() in centos 4.18 is different from the implementation of part_stat_lock() in vanilla 4.18 kernel:

vanilla kernel:

#define part_stat_lock()    ({ rcu_read_lock(); get_cpu(); })

centos 4.18:

#define part_stat_lock()        preempt_disable()
bplein commented 3 years ago

I reproduced this with a clean install of CentOS 8 in a VM. Will have to wait for @snuf to chime in.

vladsol commented 3 years ago

Unfortunately, in readme.md for 3.2.16 there is a notice that this version is an unsupported

snuf commented 3 years ago

@vladsol as @bplein asked, which tag/branch are you trying to use master, v4.20.2 or fio-3.2.16.1732?

snuf commented 3 years ago

Seems like this was backported from 5.8: https://elixir.bootlin.com/linux/v5.7.19/source/include/linux/part_stat.h#L20 https://elixir.bootlin.com/linux/v5.8.18/source/include/linux/part_stat.h#L24

vladsol commented 3 years ago

fio-3.2.16.1732

fio-3.2.16.1732

vladsol commented 3 years ago

I'll try to re-use code from vanilla kernel.. Or is it better not to do this? :)

snuf commented 3 years ago

I'd not go down that path haha, better to use a newer kernel always. The backporting they do is all fun and games, but in reality it's such a bad idea. There is a reason newer kernels come out, and bugs get fixed in them :). I'm looking at if the 4.20 branch is easy to adapt and make work with a check.

vladsol commented 3 years ago

Unfortunately, i have no choice. As i understand correctly, 4.20 is not support my iodrive2 ?

snuf commented 3 years ago

Ehm no it should if it's a vanilla kernel. The problem is that Redhat/CentOS back ports things that have never existed into older kernels, but don't backport everything. Kind of leaving you with a frankenkernel that is somewhere in limbo land between 5.X and 4.18. So generally if you can move to a newer kernel you will end up with a more "vanilla" kernel on CentOS/Redhat because there is less that ends up in the twilight zone of living between the two. We've stopped supporting 4 because it's pretty old by now, and focussed on supporting 5 and up. The driver also contained a LOT of stuff for really old kernels and odd exceptions that has been stripped to keep it maintainable.

That said, it's not a huge effort to spin up a CentOS8 machine and see if we can backport the fix we already have into an older version of the driver that might make it compile on CentOS8.

BTW I do understand why they do this, but I just generally disagree with the practice :)

vladsol commented 3 years ago
-        cpu = part_stat_lock();
+        rcu_read_lock();
+        cpu = get_cpu();
-        part_stat_unlock();
+        do {
+            put_cpu();
+            rcu_read_unlock();
+        } while (0);

With this little fix, I managed to compile and run. There are no errors YET. I will watch :-) Yes, I have backups :)

snuf commented 3 years ago

partstat* doesn't require cpu anymore, as it is on 5.x onwards. You can just remove the cpu = which is easier, and the int cpu; definition above it.

snuf commented 3 years ago

actually the test should move upwards in that code block and have an else statement by the looks of it, that would prevent this from happening. The v4.20.2 compiles with that fix, so I might push that and cut it to v4.20.3

snuf commented 3 years ago

@vladsol the correct fix is in branch v4.20.3-fix_centos8 which also has a fix for compiling. I'll verify the branch, and then tag it with v4.20.3.

vladsol commented 3 years ago

partstat* doesn't require cpu anymore, as it is on 5.x onwards. You can just remove the cpu = which is easier, and the int cpu; definition above it.

Yes, you are right. KFIOC_X_PART_STAT_REQUIRES_CPU=0 in my case.

vladsol commented 3 years ago

Well,

@vladsol the correct fix is in branch v4.20.3-fix_centos8 which also has a fix for compiling. I'll verify the branch, and then tag it with v4.20.3.

error: File not found: /root/rpmbuild/BUILDROOT/iomemory-vsl-3.2.16.1732-1.0.el8.x86_64/usr/src/iomemory-vsl-3.2.16/license.c

vladsol commented 3 years ago

Well... in CentOS 8 is it not possible to build any version of driver. v5.x, v4.x, from master branch - no luck.

/root/rpmbuild/BUILD/iomemory-vsl-3.2.16.1732/driver_source/include/kblock_meta.h:27:29: error: too many arguments to function 'blk_alloc_queue'
     #define BLK_ALLOC_QUEUE blk_alloc_queue(kfio_make_request, node);
                             ^~~~~~~~~~~~~~~
/root/rpmbuild/BUILD/iomemory-vsl-3.2.16.1732/driver_source/kblock.c: In function 'kfio_alloc_queue':
/root/rpmbuild/BUILD/iomemory-vsl-3.2.16.1732/driver_source/include/kblock_meta.h:27:45: error: passing argument 1 of 'blk_alloc_queue' makes integer from pointer without a cast [-Werror=int-conversion]
     #define BLK_ALLOC_QUEUE blk_alloc_queue(kfio_make_request, node);
                                             ^~~~~~~~~~~~~~~~~
snuf commented 3 years ago

@vladsol v4.20.3-fix_centos8 has been tested on a vanilla CentOS 8 machine, it compiles and builds an RPM. Can you do a clean checkout of the repo and check out that specific branch? Can you also share what you're doing instead of just errors, it's hard to guess what you're doing.

I've attached the log of the test system building the module and the RPM on a vanilla system: typescript-2021-08-09T09:09:30.gz

edit: This is the log with the module loaded, in the log above the testing system stops at the upload to S3, this one goes all the way until it would do read/write testing on the device: typescript=2021-08-09T10:12:31.gz

vladsol commented 3 years ago

@vladsol v4.20.3-fix_centos8 has been tested on a vanilla CentOS 8 machine, it compiles and builds an RPM. Can you do a clean checkout of the repo and check out that specific branch? Can you also share what you're doing instead of just errors, it's hard to guess what you're doing.

I've attached the log of the test system building the module and the RPM on a vanilla system: typescript-2021-08-09T09:09:30.gz

edit: This is the log with the module loaded, in the log above the testing system stops at the upload to S3, this one goes all the way until it would do read/write testing on the device: typescript=2021-08-09T10:12:31.gz

OK. Step-by-step: https://pastebin.com/QdSSVtFQ

snuf commented 3 years ago

@vladsol thanks, that helps! please use make rpm as outlined in https://github.com/snuf/iomemory-vsl#rpm-centos--rhel instead of rpmbuild.