openzfs / zfs

OpenZFS on Linux and FreeBSD
https://openzfs.github.io/openzfs-docs
Other
10.32k stars 1.72k forks source link

Incompatible pointer type when building 2.1.2 on Hardkernel 4.9.295, 2.0.7 builds fine. #12952

Open akschu opened 2 years ago

akschu commented 2 years ago

System information

slackware64-current on odroid arm (Amlogic S905X3) based board (slarm64) with this kernel: https://github.com/hardkernel/linux (branch odroidg12-4.9.y) with this config: https://gitlab.com/sndwvs/images_build_kit/-/blob/arm/config/kernel/linux-meson-sm1-legacy.config

openzfs-2.1.2 release aarch64 kernel 4.9.295 gcc 11.2 glibc 2.33

While compiling the kernel modules I get:

CC [M] /tmp/zfs-2.1.2/module/zfs/../os/linux/zfs/zpl_super.o /tmp/zfs-2.1.2/module/zfs/../os/linux/zfs/zpl_file.c: In function ‘zpl_readpages’: /tmp/zfs-2.1.2/module/zfs/../os/linux/zfs/zpl_file.c:654:50: error: passing argument 3 of ‘read_cache_pages’ from incompatible pointer type [-Werror=incompatible-pointer-types] 654 return (read_cache_pages(mapping, pages, zpl_readpage_filler, NULL)); ^~~~~~~
int ()(void , struct page *)

In file included from ./include/linux/blkdev.h:14, from /tmp/zfs-2.1.2/include/os/linux/spl/sys/uio.h:30, from /tmp/zfs-2.1.2/include/os/linux/spl/sys/sunddi.h:28, from /tmp/zfs-2.1.2/include/sys/zfs_context.h:69, from /tmp/zfs-2.1.2/include/sys/spa.h:39, from /tmp/zfs-2.1.2/include/sys/dmu_objset.h:33, from /tmp/zfs-2.1.2/module/zfs/../os/linux/zfs/zpl_file.c:31: ./include/linux/pagemap.h:367:52: note: expected ‘int ()(struct file , struct page )’ but argument is of type ‘int ()(void , struct page )’ 367 | struct list_head pages, filler_t filler, void *data); | ~~^~ cc1: some warnings being treated as errors

Looks like one var is of type "int ()(struct file , struct page )" and the other "int ()(void , struct page )"

I tried compiling with CXXFLAGS="-Wno-incompatible-pointer-types" but that doesn't make it compile, if it's even safe to do so.

2.0.7 Builds totally fine.

akschu commented 2 years ago

I changed the title because I wanted to double check that 2.0.7 builds fine, and it does.

rincebrain commented 2 years ago

It seems like https://github.com/openzfs/zfs/commit/4affa09f3e6c4c17de8ac187d82071bb39ca57b1 sunk your battleship, and since 2.0.x didn't have that commit, it didn't have the issue.

@solbjorn

akschu commented 2 years ago

It seems like 4affa09 sunk your battleship, and since 2.0.x didn't have that commit, it didn't have the issue.

@solbjorn

Confirmed, patch -R on 4affa09f3e6c4c17de8ac187d82071bb39ca57b1 causes it to build. Any issues with running it like that?

rincebrain commented 2 years ago

AFAIK that patch solely cleaned up some casting that offended ClangCFI, so unless you're planning on building with that, I wouldn't think so.

solbjorn commented 2 years ago

@akschu, this is caused by the commit hardkernel/linux@4fd840d of the kernel you use.

Upstream kernel 4.9 has filler_t of the same type as ZFS callback does ([0]), but the "hardkernel" does not: [1]. Since it's effectively impossible to try to support all the kernels from around the world, this ZFS repo aims only upstream version. From this point of view, it's the users of custom kernels who should fix such by themselves. OTOH, this issue can relatively easily be fixed by adding a configure-time check for filler_t type. Since Sami changed its type in the "hardkernel" exactly for the purpose of ClangCFI, and the ZFS part would look even simpler on it (no need for splitting zpl_readpage() into two callbacks), I could do this.

@rincebrain, @behlendorf, what do you guys think? Is it fine to add a bit of code to ZFS to support third-party kernels?

If so, I need a couple days to dissect some free time slice for this. And I think it's better to change the title to mention the "hardkernel".

[0] https://elixir.bootlin.com/linux/v4.9.296/source/include/linux/pagemap.h#L228 [1] https://github.com/hardkernel/linux/blob/odroidg12-4.9.y/include/linux/pagemap.h#L228

rincebrain commented 2 years ago

Doesn't seem any stranger than grsec compat fixes or distro-specific fixes, to me, but I'm not the one anyone needs to convince. :)

(Also, the commit message on https://github.com/hardkernel/linux/commit/4fd840d1743308b2ef470534523009dd99b3ce2b seems to be originally from a very different patch, to the AES code, so...that seems wrong.)

Tony763 commented 2 years ago
error: passing argument 3 of ‘read_cache_pages’ from incompatible pointer type [-Werror=incompatible-pointer-types]
  return (read_cache_pages(mapping, pages, zpl_readpage_filler, NULL));

For me, happen on Jetson Nano 4.4 with 2.1.4

rincebrain commented 2 years ago

Mayhap https://github.com/openzfs/zfs/commit/23c13c7e807ec8abb368e00699a34ffe0bd50885 could make your life nicer?

e: hm, no, that's in 2.1.4.

Curious. Do you have a pointer to the kernel tree the Jetson Nano is running on?

e2: oh lovely, they changed the function signature. (Assuming NVIDIA did the same thing, at least - the L4T sources I found for 5.10 don't have such a change.)

One could imagine a configure-time check that swaps zpl_readpage_filler with zpl_readpage if it detects this case.

Tony763 commented 2 years ago

Jetpack 4.4 run 4.9.140-tegra.

rincebrain commented 2 years ago

Yup, they sure do have that patch.

Tony763 commented 2 years ago

l4t-r32.6 is jetpack 4.6 which is sadly not available for my carrier board. I will stick with version 2.0.7

stale[bot] commented 1 year ago

This issue has been automatically marked as "stale" because it has not had any activity for a while. It will be closed in 90 days if no further activity occurs. Thank you for your contributions.