| Message ID | 20180202154104.1522809-1-arnd@arndb.de |
|---|---|
| State | Accepted |
| Commit | ca66e797120fb09b8138623fb4b563e952586ef5 |
| Headers | show |
| Series | [1/2] locking/qrwlock: include asm/byteorder.h as needed | expand |
Hi Arnd, 2018-02-03 0:40 GMT+09:00 Arnd Bergmann <arnd@arndb.de>: > Build testing with LTO found a couple of files that get compiled > differently depending on whether asm/byteorder.h gets included early > enough or not. In particular, include/asm-generic/qrwlock_types.h is > affected by this, but there are probably others as well. > > The symptom is a series of LTO link time warnings, including these: > > net/netlabel/netlabel_unlabeled.h:223: error: type of 'netlbl_unlhsh_add' does not match original declaration [-Werror=lto-type-mismatch] > int netlbl_unlhsh_add(struct net *net, > net/netlabel/netlabel_unlabeled.c:377: note: 'netlbl_unlhsh_add' was previously declared here > > include/net/ipv6.h:360: error: type of 'ipv6_renew_options_kern' does not match original declaration [-Werror=lto-type-mismatch] > ipv6_renew_options_kern(struct sock *sk, > net/ipv6/exthdrs.c:1162: note: 'ipv6_renew_options_kern' was previously declared here > > net/core/dev.c:761: note: 'dev_get_by_name_rcu' was previously declared here > struct net_device *dev_get_by_name_rcu(struct net *net, const char *name) > net/core/dev.c:761: note: code may be misoptimized unless -fno-strict-aliasing is used > > drivers/gpu/drm/i915/i915_drv.h:3377: error: type of 'i915_gem_object_set_to_wc_domain' does not match original declaration [-Werror=lto-type-mismatch] > i915_gem_object_set_to_wc_domain(struct drm_i915_gem_object *obj, bool write); > drivers/gpu/drm/i915/i915_gem.c:3639: note: 'i915_gem_object_set_to_wc_domain' was previously declared here > > include/linux/debugfs.h:92:9: error: type of 'debugfs_attr_read' does not match original declaration [-Werror=lto-type-mismatch] > ssize_t debugfs_attr_read(struct file *file, char __user *buf, > fs/debugfs/file.c:318: note: 'debugfs_attr_read' was previously declared here > > include/linux/rwlock_api_smp.h:30: error: type of '_raw_read_unlock' does not match original declaration [-Werror=lto-type-mismatch] > void __lockfunc _raw_read_unlock(rwlock_t *lock) __releases(lock); > kernel/locking/spinlock.c:246:26: note: '_raw_read_unlock' was previously declared here > > include/linux/fs.h:3308:5: error: type of 'simple_attr_open' does not match original declaration [-Werror=lto-type-mismatch] > int simple_attr_open(struct inode *inode, struct file *file, > fs/libfs.c:795: note: 'simple_attr_open' was previously declared here > > All of the above are caused by include/asm-generic/qrwlock_types.h failing > to include asm/byteorder.h after commit e0d02285f16e ("locking/qrwlock: > Use 'struct qrwlock' instead of 'struct __qrwlock'") in linux-4.15. > > Similar bugs may or may not exist in older kernels as well, but there > is no easy way to test those with link-time optimizations, and kernels > before 4.14 are harder to fix because they don't have Babu's patch series Is this patch only for stable kernel, not for the mainline? > We had similar issues with CONFIG_ symbols in the past and ended up > always including the configuration headers though linux/kconfig.h. > This works around the issue through that same file, defining either > __BIG_ENDIAN or __LITTLE_ENDIAN depending on CONFIG_CPU_BIG_ENDIAN, > which is now always set on all architectures since commit 4c97a0c8fee3 > ("arch: define CPU_BIG_ENDIAN for all fixed big endian archs"). > > Cc: stable@vger.kernel.org > Cc: Babu Moger <babu.moger@oracle.com> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > include/linux/kconfig.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h > index fec5076eda91..cc8fa109cfa3 100644 > --- a/include/linux/kconfig.h > +++ b/include/linux/kconfig.h > @@ -4,6 +4,12 @@ > > #include <generated/autoconf.h> > > +#ifdef CONFIG_CPU_BIG_ENDIAN > +#define __BIG_ENDIAN 4321 > +#else > +#define __LITTLE_ENDIAN 1234 > +#endif > + > #define __ARG_PLACEHOLDER_1 0, > #define __take_second_arg(__ignored, val, ...) val > > -- > 2.9.0 > If all architectures define CONFIG_CPU_BIG_ENDIAN or CONFIG_CPU_LITTLE_ENDIAN, is it possible to use this for endian test? i.e. Is it possible to replace like this? #ifdef __BIG_ENDIAN -> #ifdef CONFIG_CPU_BIG_ENDIAN #ifdef __LITTLE_ENDIAN -> #ifdef CONFIG_CPU_LITTLE_ENDIAN -- Best Regards Masahiro Yamada
diff --git a/include/asm-generic/qrwlock_types.h b/include/asm-generic/qrwlock_types.h index 137ecdd16daa..c36f1d5a2572 100644 --- a/include/asm-generic/qrwlock_types.h +++ b/include/asm-generic/qrwlock_types.h @@ -3,6 +3,7 @@ #define __ASM_GENERIC_QRWLOCK_TYPES_H #include <linux/types.h> +#include <asm/byteorder.h> #include <asm/spinlock_types.h> /*
Moving the qrwlock struct definition into a header file introduced a subtle bug on all little-endian machines, where some files in some configurations would see the fields in an incorrect order. This was found by building with an LTO enabled compiler that warns every time we try to link together files with incompatible data structures. A second patch changes linux/kconfig.h to always define the symbols, but this seems to be the root cause of most of the issues, so I'd suggest we do both. On a current linux-next kernel, I verified that this header is responsible for all type mismatches as a result from the endianess confusion. Cc: stable@vger.kernel.org Cc: Will Deacon <will.deacon@arm.com> Cc: Peter Zijlstra <peterz@infradead.org> Fixes: e0d02285f16e ("locking/qrwlock: Use 'struct qrwlock' instead of 'struct __qrwlock'") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- include/asm-generic/qrwlock_types.h | 1 + 1 file changed, 1 insertion(+) -- 2.9.0