Created attachment 636587 [details] Userspace port of linux/gfs2_ondisk.h gfs2-utils currently includes kernel headers (see below for a breakdown by util) which has several drawbacks including, building gfs2-utils spews a lot of warnings and builds can fail while running a custom kernel for which no headers are installed. I've attached a libgfs2_types.h which I wrote a while ago, so that it isn't lost. It's a userspace port of linux/gfs2_ondisk.h with added int types marked up for sparse analysis. I'm not convinced the non-bitwise typedefs are required, we can just use uint*_t instead I think. There are other kernel headers which aren't replaced by my libgfs2_types.h. The gfs_controld includes will disappear with gfs_controld soon. Out of the others, linux/kdev_t.h isn't required, linux/fs.h is only used for 3 block dev geometry ioctl macros, and linux/limits.h is only used for PATH_MAX. The drawback of fixing this issue is that a *lot* of code will need to be changed just to replace type names so it can get very messy unless it's done piece-by-piece over several patches. libgfs2: linux/types.h linux/limits.h linux/gfs2_ondisk.h linux/kdev_t.h linux/fs.h mkfs.gfs2: linux/types.h linux/gfs2_ondisk.h linux/falloc.h tunegfs2: linux/gfs2_ondisk.h gfs2_convert: linux/types.h linux/gfs2_ondisk.h gfs2_edit: linux/types.h linux/gfs2_ondisk.h gfs_controld: linux/dlmconstants.h linux/netlink.h
I've wondered about this before... some of the above are or should be obsolete though, for example gfs_controld will be going away and fallocate should be appearing in glibc shortly (if it hasn't already). kdev_t certainly seems wrong... what are we using that for? Likewise fs.h too. There is a glibc limits.h so again using a kernel version of this seems wrong. That really just leaves gfs2_ondisk.h and its dependency types.h. I'd certainly like to be rid of those, but it is also a tricky thing to do. We may land up needing to create our own set of types for the library - it is another big job though as you say and I'm not convinced that the effort is worth it just yet. Might be worth looking into alternatives though, I don't really see that there is a big issue in sharing headers with userspace, provided they only contain types which can be easily shared.
(In reply to comment #1) > I've wondered about this before... some of the above are or should be > obsolete though, for example gfs_controld will be going away and fallocate > should be appearing in glibc shortly (if it hasn't already). > > kdev_t certainly seems wrong... what are we using that for? Likewise fs.h > too. kdev_t.h isn't being used for anything, it turns out. I suspect there are other places where we're including headers unnecessarily but discovering them isn't an easy task. fs.h is used for ioctl macros: BLKRAGET, BLKBSZGET, and BLKROGET in device_geometry.c. I'm not aware of any userspace headers which expose these macros but we could probably just copy them in since we already do that with other, similar macros in device_geometry.c > There is a glibc limits.h so again using a kernel version of this seems > wrong. Agreed. Although glibc limits.h uses linux/limits.h indirectly to define PATH_MAX, using limits.h probably makes more sense. > That really just leaves gfs2_ondisk.h and its dependency types.h. I'd > certainly like to be rid of those, but it is also a tricky thing to do. We > may land up needing to create our own set of types for the library - it is > another big job though as you say and I'm not convinced that the effort is > worth it just yet. Might be worth looking into alternatives though, I don't > really see that there is a big issue in sharing headers with userspace, > provided they only contain types which can be easily shared. I agree it's not a high priority. I probably won't put effort into this until after gfs_controld is gone at least.
We had this topic going and back and forward a half million times before :) Having a local copy (beside how it is mangled to fit userland) in gfs2-utils.git is a very bad idea. Specially if for any reason the on-kernel version does change (or might change). We had this experience before, let´s not repeat the mistake. Either keep status-quo to detect mismatches or consider export gfs2_ondisk.h from kernel to userland as many other kernel headers do. /usr/include/linux, as you know, is more or less cat kernel-headers | lots_of_magic > kernel-headers-usable-by-userland. I don´t see a reason why gfs2_ondisk can´t land in there to be honest. IIRC the magic will/should take care of the various types handling.
Well there's another issue here because, for some reason I don't fully understand, in gfs2-utils we include the headers from the kernel-devel package (i.e. /lib/modules/`uname -r`/source/include/linux/*) which take precedence over the userspace-ported headers from the kernel-headers package (/usr/include/linux/*) and that's why we see all the warnings. So I've just tested building gfs2-utils without the -I which pulls in the kernel-devel headers and it compiles just fine with the kernel-headers headers without any warnings. So the question is, do we really need to use the kernel-devel headers instead of the sanitized-for-glibc kernel-headers headers?
I suspect not. Lets use the properly sanitized headers.
(In reply to comment #4) > Well there's another issue here because, for some reason I don't fully > understand, in gfs2-utils we include the headers from the kernel-devel > package (i.e. /lib/modules/`uname -r`/source/include/linux/*) which take > precedence over the userspace-ported headers from the kernel-headers package > (/usr/include/linux/*) and that's why we see all the warnings. It´s probably a left over from the old days when there was no sanitized headers :) +1 to use the userland ones.
Closing this one now that we've got to the bottom of it.