Hi all,
I simplified some xxxof() macros,
by adding a new macro memberof(),
which implements a common operation in many of them.
I also splitted many of those macros into tiny headers,
since I noticed that touching those headers implied
recompiling almost the whole kernel.
Hopefully after this patch there will be less
things to recompile after touching one of those.
Having simpler headers means that now one can
include one of those without pulling too much stuff
that might break other stuff.
I removed some unnecessary casts too.
Every few commits in this series
and of course after the last commit
I rebuilt the kernel and run for a while with it without any problems.
Please note that I have written very few kernel code
and for example some files wouldn't let me include some of these files,
so I didn't change those.
What I mean is that,
even though this is super obvious and shouldn't break stuff,
and I'm not new to C,
I'm quite new to the kernel,
and ask that reviewers take deep look, please.
In the first and second commits
I changed a lot of stuff in many parts,
and that's why I CCd so many people (also in this cover letter).
However, to avoid spamming,
and since it would be a nightmare to
find all the relevant people affected in so many different areas,
I only CCd in 01, 02 and in the cover letter.
If anyone is interested in reading the full patch set,
I sent it to the LKML.
Thanks,
Alex
Alejandro Colomar (17):
linux/container_of.h: Add memberof(T, m)
Use memberof(T, m) instead of explicit NULL dereference
Replace some uses of memberof() by its wrappers
linux/memberof.h: Move memberof() to separate header
linux/typeof_member.h: Move typeof_member() to a separate header
Simplify sizeof(typeof_member()) to sizeof_field()
linux/NULL.h: Move NULL to a separate header
linux/offsetof.h: Move offsetof(T, m) to a separate header
linux/offsetof.h: Implement offsetof() in terms of memberof()
linux/container_of.h: Implement container_of_safe() in terms of
container_of()
linux/container_of.h: Cosmetic
linux/container_of.h: Remove unnecessary cast to (void *)
linux/sizeof_field.h: Move sizeof_field(T, m) to a separate header
include/linux/: Include a smaller header if just for NULL
linux/offsetofend.h: Move offsetofend(T, m) to a separate header
linux/array_size.h: Move ARRAY_SIZE(arr) to a separate header
include/: Include <linux/array_size.h> for ARRAY_SIZE()
arch/x86/include/asm/bootparam_utils.h | 3 +-
arch/x86/kernel/signal_compat.c | 5 ++--
drivers/gpu/drm/i915/i915_sw_fence.c | 1 +
drivers/gpu/drm/i915/i915_utils.h | 5 ++--
drivers/gpu/drm/i915/intel_runtime_pm.h | 3 +-
drivers/net/ethernet/emulex/benet/be.h | 10 +++----
drivers/net/ethernet/i825xx/ether1.c | 7 +++--
drivers/platform/x86/wmi.c | 3 +-
drivers/scsi/be2iscsi/be.h | 12 ++++----
drivers/scsi/be2iscsi/be_cmds.h | 5 +++-
fs/btrfs/ctree.h | 5 ++--
fs/proc/inode.c | 1 +
include/acpi/actypes.h | 4 ++-
include/crypto/internal/blake2b.h | 1 +
include/crypto/internal/blake2s.h | 1 +
include/crypto/internal/chacha.h | 1 +
include/drm/drm_mipi_dbi.h | 1 +
include/drm/drm_mode_object.h | 1 +
include/kunit/test.h | 1 +
include/linux/NULL.h | 10 +++++++
include/linux/arm_ffa.h | 1 +
include/linux/array_size.h | 15 ++++++++++
include/linux/blk_types.h | 1 +
include/linux/can/core.h | 1 +
include/linux/clk-provider.h | 1 +
include/linux/container_of.h | 28 ++++++++++-------
include/linux/counter.h | 1 +
include/linux/crash_core.h | 1 +
include/linux/efi.h | 1 +
include/linux/extable.h | 2 +-
include/linux/f2fs_fs.h | 1 +
include/linux/filter.h | 3 ++
include/linux/fs.h | 1 +
include/linux/genl_magic_func.h | 1 +
include/linux/hashtable.h | 1 +
include/linux/ieee80211.h | 1 +
include/linux/kbuild.h | 3 ++
include/linux/kernel.h | 7 +----
include/linux/kfifo.h | 1 +
include/linux/kvm_host.h | 3 ++
include/linux/libata.h | 1 +
include/linux/llist.h | 1 +
include/linux/memberof.h | 11 +++++++
include/linux/mlx5/device.h | 1 +
include/linux/mlx5/driver.h | 1 +
include/linux/mm_types.h | 1 +
include/linux/moduleparam.h | 3 ++
include/linux/mtd/rawnand.h | 1 +
include/linux/netdevice.h | 1 +
include/linux/netfilter.h | 1 +
include/linux/nvme-fc.h | 2 ++
include/linux/offsetof.h | 17 +++++++++++
include/linux/offsetofend.h | 19 ++++++++++++
include/linux/pagemap.h | 1 +
include/linux/phy.h | 1 +
include/linux/phy_led_triggers.h | 1 +
include/linux/pinctrl/machine.h | 1 +
include/linux/property.h | 1 +
include/linux/rcupdate.h | 1 +
include/linux/rcupdate_wait.h | 1 +
include/linux/regmap.h | 1 +
include/linux/sched/task.h | 1 +
include/linux/sizeof_field.h | 14 +++++++++
include/linux/skb_array.h | 1 +
include/linux/skbuff.h | 1 +
include/linux/skmsg.h | 3 ++
include/linux/slab.h | 2 ++
include/linux/spinlock_types.h | 1 +
include/linux/stddef.h | 30 +++----------------
include/linux/string.h | 5 +++-
include/linux/surface_aggregator/controller.h | 1 +
include/linux/surface_aggregator/serial_hub.h | 1 +
include/linux/swap.h | 1 +
include/linux/ti-emif-sram.h | 1 +
include/linux/typeof_member.h | 11 +++++++
include/linux/ucs2_string.h | 2 +-
include/linux/vdpa.h | 1 +
include/linux/virtio_config.h | 17 ++++++-----
include/linux/wireless.h | 2 ++
include/net/bond_3ad.h | 1 +
include/net/dsa.h | 1 +
include/net/ip_vs.h | 1 +
include/net/netfilter/nf_conntrack_tuple.h | 1 +
include/net/netfilter/nf_tables.h | 1 +
include/net/netlink.h | 1 +
include/rdma/uverbs_ioctl.h | 1 +
include/rdma/uverbs_named_ioctl.h | 1 +
include/scsi/scsi_host.h | 1 +
include/sound/soc-dapm.h | 1 +
include/sound/soc.h | 1 +
include/trace/events/wbt.h | 1 +
include/uapi/linux/netfilter/xt_sctp.h | 1 +
include/xen/hvm.h | 1 +
kernel/kallsyms.c | 3 +-
94 files changed, 255 insertions(+), 79 deletions(-)
create mode 100644 include/linux/NULL.h
create mode 100644 include/linux/array_size.h
create mode 100644 include/linux/memberof.h
create mode 100644 include/linux/offsetof.h
create mode 100644 include/linux/offsetofend.h
create mode 100644 include/linux/sizeof_field.h
create mode 100644 include/linux/typeof_member.h
--
2.33.1
Many xxxof_member() macros make use of the same construction
to refer to a member of a struct
without needing a variable of the struct type.
memberof(T, m)
simplifies all of those,
avoids possible mistakes in repetition,
adds a meaningful name to the construction,
and improves readability by avoiding too many parentheses together.
Signed-off-by: Alejandro Colomar <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Miguel Ojeda <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
Cc: Jonathan Cameron <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Corey Minyard <[email protected]>
Cc: Christian Brauner <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Kees Cook <[email protected]>
---
include/linux/container_of.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 2f4944b791b8..199c78a3bf29 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -5,6 +5,9 @@
#include <linux/build_bug.h>
#include <linux/err.h>
+
+#define memberof(T, m) (((T *) NULL)->m)
+
#define typeof_member(T, m) typeof(((T*)0)->m)
/**
--
2.33.1
Touching files so used for the kernel,
forces 'make' to recompile most of the kernel.
Having those definitions in more granular files
helps avoid recompiling so much of the kernel.
Signed-off-by: Alejandro Colomar <[email protected]>
---
drivers/gpu/drm/i915/i915_sw_fence.c | 1 +
drivers/platform/x86/wmi.c | 1 +
fs/proc/inode.c | 1 +
include/linux/container_of.h | 2 --
include/linux/typeof_member.h | 11 +++++++++++
include/linux/virtio_config.h | 1 +
kernel/kallsyms.c | 1 +
7 files changed, 16 insertions(+), 2 deletions(-)
create mode 100644 include/linux/typeof_member.h
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index c589a681da77..911ad08978b2 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -8,6 +8,7 @@
#include <linux/dma-fence.h>
#include <linux/irq_work.h>
#include <linux/dma-resv.h>
+#include <linux/typeof_member.h>
#include "i915_sw_fence.h"
#include "i915_selftest.h"
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index c34341f4da76..5daadcafd808 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -33,6 +33,7 @@
#include <linux/uuid.h>
#include <linux/wmi.h>
#include <linux/fs.h>
+#include <linux/typeof_member.h>
#include <uapi/linux/wmi.h>
MODULE_AUTHOR("Carlos Corbacho");
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 599eb724ff2d..e73e55e8dad4 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -25,6 +25,7 @@
#include <linux/slab.h>
#include <linux/mount.h>
#include <linux/bug.h>
+#include <linux/typeof_member.h>
#include <linux/uaccess.h>
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 2f1eb5ddd20a..082188b5cd29 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -7,8 +7,6 @@
#include <linux/memberof.h>
-#define typeof_member(T, m) typeof(memberof(T, m))
-
/**
* container_of - cast a member of a structure out to the containing structure
* @ptr: the pointer to the member.
diff --git a/include/linux/typeof_member.h b/include/linux/typeof_member.h
new file mode 100644
index 000000000000..38aa030d86d2
--- /dev/null
+++ b/include/linux/typeof_member.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_TYPEOF_MEMBER_H
+#define _LINUX_TYPEOF_MEMBER_H
+
+#include <linux/memberof.h>
+
+
+#define typeof_member(T, m) typeof(memberof(T, m))
+
+
+#endif /* _LINUX_TYPEOF_MEMBER_H */
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index ddbd51208ce1..ff1752441e5d 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -8,6 +8,7 @@
#include <linux/virtio.h>
#include <linux/virtio_byteorder.h>
#include <linux/compiler_types.h>
+#include <linux/typeof_member.h>
#include <uapi/linux/virtio_config.h>
struct irq_affinity;
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 3011bc33a5ba..3bf04d89d72c 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -29,6 +29,7 @@
#include <linux/compiler.h>
#include <linux/module.h>
#include <linux/kernel.h>
+#include <linux/typeof_member.h>
/*
* These will be re-linked against their real values
--
2.33.1
Signed-off-by: Alejandro Colomar <[email protected]>
Cc: Ajit Khaparde <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Corey Minyard <[email protected]>
Cc: Chris Mason <[email protected]>
Cc: Christian Brauner <[email protected]>
Cc: David Sterba <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Jason Wang <[email protected]>
Cc: Jitendra Bhivare <[email protected]>
Cc: John Hubbard <[email protected]>
Cc: John S. Gruber <[email protected]>
Cc: Jonathan Cameron <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: Josef Bacik <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Ketan Mukadam <[email protected]>
Cc: Len Brown <[email protected]>
Cc: "Michael S. Tsirkin" <[email protected]>
Cc: Miguel Ojeda <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
Cc: Rodrigo Vivi <[email protected]>
Cc: Russell King <[email protected]>
Cc: Somnath Kotur <[email protected]>
Cc: Sriharsha Basavapatna <[email protected]>
Cc: Subbu Seetharaman <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
---
arch/x86/include/asm/bootparam_utils.h | 3 ++-
arch/x86/kernel/signal_compat.c | 5 +++--
drivers/gpu/drm/i915/i915_utils.h | 5 ++---
drivers/gpu/drm/i915/intel_runtime_pm.h | 2 +-
drivers/net/ethernet/emulex/benet/be.h | 7 ++++---
drivers/net/ethernet/i825xx/ether1.c | 7 +++++--
drivers/scsi/be2iscsi/be.h | 7 ++++---
drivers/scsi/be2iscsi/be_cmds.h | 5 ++++-
fs/btrfs/ctree.h | 5 +++--
include/acpi/actypes.h | 4 +++-
include/linux/container_of.h | 6 +++---
include/linux/virtio_config.h | 14 +++++++-------
12 files changed, 41 insertions(+), 29 deletions(-)
diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
index 981fe923a59f..71b28b5fb088 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -3,6 +3,7 @@
#define _ASM_X86_BOOTPARAM_UTILS_H
#include <asm/bootparam.h>
+#include <linux/container_of.h>
/*
* This file is included from multiple environments. Do not
@@ -19,7 +20,7 @@
* private magic, so it is better to leave it unchanged.
*/
-#define sizeof_mbr(type, member) ({ sizeof(((type *)0)->member); })
+#define sizeof_mbr(type, member) ({ sizeof(memberof(type, member)); })
#define BOOT_PARAM_PRESERVE(struct_member) \
{ \
diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c
index b52407c56000..28420a7df056 100644
--- a/arch/x86/kernel/signal_compat.c
+++ b/arch/x86/kernel/signal_compat.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/compat.h>
+#include <linux/container_of.h>
#include <linux/uaccess.h>
#include <linux/ptrace.h>
@@ -69,8 +70,8 @@ static inline void signal_compat_build_tests(void)
* structure stays within the padding size (checked
* above).
*/
-#define CHECK_CSI_SIZE(name, size) BUILD_BUG_ON(size != sizeof(((compat_siginfo_t *)0)->_sifields.name))
-#define CHECK_SI_SIZE(name, size) BUILD_BUG_ON(size != sizeof(((siginfo_t *)0)->_sifields.name))
+#define CHECK_CSI_SIZE(name, size) BUILD_BUG_ON(size != sizeof(memberof(compat_siginfo_t ,_sifields.name)))
+#define CHECK_SI_SIZE(name, size) BUILD_BUG_ON(size != sizeof(memberof(siginfo_t, _sifields.name)))
CHECK_CSI_OFFSET(_kill);
CHECK_CSI_SIZE (_kill, 2*sizeof(int));
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index 7a5925072466..2e3ec28f4f1b 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -25,6 +25,7 @@
#ifndef __I915_UTILS_H
#define __I915_UTILS_H
+#include <linux/container_of.h>
#include <linux/list.h>
#include <linux/overflow.h>
#include <linux/sched.h>
@@ -192,8 +193,6 @@ __check_struct_size(size_t base, size_t arr, size_t count, size_t *size)
#define page_pack_bits(ptr, bits) ptr_pack_bits(ptr, bits, PAGE_SHIFT)
#define page_unpack_bits(ptr, bits) ptr_unpack_bits(ptr, bits, PAGE_SHIFT)
-#define struct_member(T, member) (((T *)0)->member)
-
#define ptr_offset(ptr, member) offsetof(typeof(*(ptr)), member)
#define fetch_and_zero(ptr) ({ \
@@ -215,7 +214,7 @@ static __always_inline ptrdiff_t ptrdiff(const void *a, const void *b)
*/
#define container_of_user(ptr, type, member) ({ \
void __user *__mptr = (void __user *)(ptr); \
- BUILD_BUG_ON_MSG(!__same_type(*(ptr), struct_member(type, member)) && \
+ BUILD_BUG_ON_MSG(!__same_type(*(ptr), memberof(type, member)) && \
!__same_type(*(ptr), void), \
"pointer type mismatch in container_of()"); \
((type __user *)(__mptr - offsetof(type, member))); })
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.h b/drivers/gpu/drm/i915/intel_runtime_pm.h
index 47a85fab4130..6cb04bef62ad 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.h
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.h
@@ -73,7 +73,7 @@ struct intel_runtime_pm {
};
#define BITS_PER_WAKEREF \
- BITS_PER_TYPE(struct_member(struct intel_runtime_pm, wakeref_count))
+ BITS_PER_TYPE(memberof(struct intel_runtime_pm, wakeref_count))
#define INTEL_RPM_WAKELOCK_SHIFT (BITS_PER_WAKEREF / 2)
#define INTEL_RPM_WAKELOCK_BIAS (1 << INTEL_RPM_WAKELOCK_SHIFT)
#define INTEL_RPM_RAW_WAKEREF_MASK (INTEL_RPM_WAKELOCK_BIAS - 1)
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index 8689d4a51fe5..dc790bae2451 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -14,6 +14,7 @@
#ifndef BE_H
#define BE_H
+#include <linux/container_of.h>
#include <linux/pci.h>
#include <linux/etherdevice.h>
#include <linux/delay.h>
@@ -824,7 +825,7 @@ extern const struct ethtool_ops be_ethtool_ops;
/* Returns bit offset within a DWORD of a bitfield */
#define AMAP_BIT_OFFSET(_struct, field) \
- (((size_t)&(((_struct *)0)->field))%32)
+ (((size_t)&(memberof(_struct, field)))%32)
/* Returns the bit mask of the field that is NOT shifted into location. */
static inline u32 amap_mask(u32 bitsize)
@@ -843,7 +844,7 @@ amap_set(void *ptr, u32 dw_offset, u32 mask, u32 offset, u32 value)
#define AMAP_SET_BITS(_struct, field, ptr, val) \
amap_set(ptr, \
offsetof(_struct, field)/32, \
- amap_mask(sizeof(((_struct *)0)->field)), \
+ amap_mask(sizeof(memberof(_struct, field))), \
AMAP_BIT_OFFSET(_struct, field), \
val)
@@ -856,7 +857,7 @@ static inline u32 amap_get(void *ptr, u32 dw_offset, u32 mask, u32 offset)
#define AMAP_GET_BITS(_struct, field, ptr) \
amap_get(ptr, \
offsetof(_struct, field)/32, \
- amap_mask(sizeof(((_struct *)0)->field)), \
+ amap_mask(sizeof(memberof(_struct, field))), \
AMAP_BIT_OFFSET(_struct, field))
#define GET_RX_COMPL_V0_BITS(field, ptr) \
diff --git a/drivers/net/ethernet/i825xx/ether1.c b/drivers/net/ethernet/i825xx/ether1.c
index c612ef526d16..4dece3476d31 100644
--- a/drivers/net/ethernet/i825xx/ether1.c
+++ b/drivers/net/ethernet/i825xx/ether1.c
@@ -28,6 +28,7 @@
* 1.07 RMK 13/05/2000 Updated for 2.3.99-pre8
*/
+#include <linux/containerof.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/types.h>
@@ -80,8 +81,10 @@ static char version[] = "ether1 ethernet driver (c) 2000 Russell King v1.07\n";
#define DISABLEIRQS 1
#define NORMALIRQS 0
-#define ether1_readw(dev, addr, type, offset, svflgs) ether1_inw_p (dev, addr + (int)(&((type *)0)->offset), svflgs)
-#define ether1_writew(dev, val, addr, type, offset, svflgs) ether1_outw_p (dev, val, addr + (int)(&((type *)0)->offset), svflgs)
+#define ether1_readw(dev, addr, type, offset, svflgs) \
+ ether1_inw_p(dev, addr + (int)(&(memberof(type, offset))), svflgs)
+#define ether1_writew(dev, val, addr, type, offset, svflgs) \
+ ether1_outw_p(dev, val, addr + (int)(&(memberof(type, offset))), svflgs)
static inline unsigned short
ether1_inw_p (struct net_device *dev, int addr, int svflgs)
diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h
index 4c58a02590c7..6830e07fd37a 100644
--- a/drivers/scsi/be2iscsi/be.h
+++ b/drivers/scsi/be2iscsi/be.h
@@ -10,6 +10,7 @@
#ifndef BEISCSI_H
#define BEISCSI_H
+#include <linux/container_of.h>
#include <linux/pci.h>
#include <linux/if_vlan.h>
#include <linux/irq_poll.h>
@@ -153,7 +154,7 @@ struct be_ctrl_info {
/* Returns bit offset within a DWORD of a bitfield */
#define AMAP_BIT_OFFSET(_struct, field) \
- (((size_t)&(((_struct *)0)->field))%32)
+ (((size_t)&(memberof(_struct, field)))%32)
/* Returns the bit mask of the field that is NOT shifted into location. */
static inline u32 amap_mask(u32 bitsize)
@@ -172,7 +173,7 @@ static inline void amap_set(void *ptr, u32 dw_offset, u32 mask,
#define AMAP_SET_BITS(_struct, field, ptr, val) \
amap_set(ptr, \
offsetof(_struct, field)/32, \
- amap_mask(sizeof(((_struct *)0)->field)), \
+ amap_mask(sizeof(memberof(_struct, field))), \
AMAP_BIT_OFFSET(_struct, field), \
val)
@@ -185,7 +186,7 @@ static inline u32 amap_get(void *ptr, u32 dw_offset, u32 mask, u32 offset)
#define AMAP_GET_BITS(_struct, field, ptr) \
amap_get(ptr, \
offsetof(_struct, field)/32, \
- amap_mask(sizeof(((_struct *)0)->field)), \
+ amap_mask(sizeof(memberof(_struct, field))), \
AMAP_BIT_OFFSET(_struct, field))
#define be_dws_cpu_to_le(wrb, len) swap_dws(wrb, len)
diff --git a/drivers/scsi/be2iscsi/be_cmds.h b/drivers/scsi/be2iscsi/be_cmds.h
index 5f9f0b18ddf3..bb6ee43769d5 100644
--- a/drivers/scsi/be2iscsi/be_cmds.h
+++ b/drivers/scsi/be2iscsi/be_cmds.h
@@ -10,6 +10,9 @@
#ifndef BEISCSI_CMDS_H
#define BEISCSI_CMDS_H
+
+#include <linux/container_of.h>
+
/**
* The driver sends configuration and managements command requests to the
* firmware in the BE. These requests are communicated to the processor
@@ -1300,7 +1303,7 @@ struct be_cmd_get_port_name {
/* Returns the number of items in the field array. */
#define BE_NUMBER_OF_FIELD(_type_, _field_) \
- (sizeof_field(_type_, _field_)/sizeof((((_type_ *)0)->_field_[0])))\
+ (sizeof_field(_type_, _field_) / sizeof(memberof(_type_, _field_[0])))
/**
* Different types of iSCSI completions to host driver for both initiator
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 7553e9dc5f93..442b6f937b22 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -6,6 +6,7 @@
#ifndef BTRFS_CTREE_H
#define BTRFS_CTREE_H
+#include <linux/container_of.h>
#include <linux/mm.h>
#include <linux/sched/signal.h>
#include <linux/highmem.h>
@@ -1575,13 +1576,13 @@ static inline void put_unaligned_le8(u8 val, void *p)
read_extent_buffer(eb, (char *)(result), \
((unsigned long)(ptr)) + \
offsetof(type, member), \
- sizeof(((type *)0)->member)))
+ sizeof(memberof(type, member))))
#define write_eb_member(eb, ptr, type, member, result) (\
write_extent_buffer(eb, (char *)(result), \
((unsigned long)(ptr)) + \
offsetof(type, member), \
- sizeof(((type *)0)->member)))
+ sizeof(memberof(type, member))))
#define DECLARE_BTRFS_SETGET_BITS(bits) \
u##bits btrfs_get_token_##bits(struct btrfs_map_token *token, \
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index ff8b3c913f21..c3b0eccb3377 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -10,6 +10,8 @@
#ifndef __ACTYPES_H__
#define __ACTYPES_H__
+#include <linux/container_of.h>
+
/* acpisrc:struct_defs -- for acpisrc conversion */
/*
@@ -508,7 +510,7 @@ typedef u64 acpi_integer;
#define ACPI_TO_POINTER(i) ACPI_CAST_PTR (void, (acpi_size) (i))
#define ACPI_TO_INTEGER(p) ACPI_PTR_DIFF (p, (void *) 0)
-#define ACPI_OFFSET(d, f) ACPI_PTR_DIFF (&(((d *) 0)->f), (void *) 0)
+#define ACPI_OFFSET(d, f) ACPI_PTR_DIFF (&(memberof(d, f)), (void *) 0)
#define ACPI_PHYSADDR_TO_PTR(i) ACPI_TO_POINTER(i)
#define ACPI_PTR_TO_PHYSADDR(i) ACPI_TO_INTEGER(i)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 199c78a3bf29..227418cb8c99 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -8,7 +8,7 @@
#define memberof(T, m) (((T *) NULL)->m)
-#define typeof_member(T, m) typeof(((T*)0)->m)
+#define typeof_member(T, m) typeof(memberof(T, m))
/**
* container_of - cast a member of a structure out to the containing structure
@@ -19,7 +19,7 @@
*/
#define container_of(ptr, type, member) ({ \
void *__mptr = (void *)(ptr); \
- static_assert(__same_type(*(ptr), ((type *)0)->member) || \
+ static_assert(__same_type(*(ptr), memberof(type, member)) || \
__same_type(*(ptr), void), \
"pointer type mismatch in container_of()"); \
((type *)(__mptr - offsetof(type, member))); })
@@ -34,7 +34,7 @@
*/
#define container_of_safe(ptr, type, member) ({ \
void *__mptr = (void *)(ptr); \
- static_assert(__same_type(*(ptr), ((type *)0)->member) || \
+ static_assert(__same_type(*(ptr), memberof(type, member)) || \
__same_type(*(ptr), void), \
"pointer type mismatch in container_of_safe()"); \
IS_ERR_OR_NULL(__mptr) ? ERR_CAST(__mptr) : \
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 4d107ad31149..a1e656c0d94a 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -328,12 +328,12 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
)
#define __virtio_native_type(structname, member) \
- typeof(virtio_to_cpu(NULL, ((structname*)0)->member))
+ typeof(virtio_to_cpu(NULL, memberof(structname, member)))
/* Config space accessors. */
#define virtio_cread(vdev, structname, member, ptr) \
do { \
- typeof(((structname*)0)->member) virtio_cread_v; \
+ typeof(memberof(structname, member)) virtio_cread_v; \
\
might_sleep(); \
/* Sanity check: must match the member's type */ \
@@ -362,8 +362,8 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
/* Config space accessors. */
#define virtio_cwrite(vdev, structname, member, ptr) \
do { \
- typeof(((structname*)0)->member) virtio_cwrite_v = \
- cpu_to_virtio(vdev, *(ptr), ((structname*)0)->member); \
+ typeof(memberof(structname, member)) virtio_cwrite_v = \
+ cpu_to_virtio(vdev, *(ptr), memberof(structname, member)); \
\
might_sleep(); \
/* Sanity check: must match the member's type */ \
@@ -397,7 +397,7 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
/* LE (e.g. modern) Config space accessors. */
#define virtio_cread_le(vdev, structname, member, ptr) \
do { \
- typeof(((structname*)0)->member) virtio_cread_v; \
+ typeof(memberof(structname, member)) virtio_cread_v; \
\
might_sleep(); \
/* Sanity check: must match the member's type */ \
@@ -425,8 +425,8 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
#define virtio_cwrite_le(vdev, structname, member, ptr) \
do { \
- typeof(((structname*)0)->member) virtio_cwrite_v = \
- virtio_cpu_to_le(*(ptr), ((structname*)0)->member); \
+ typeof(memberof(structname, member)) virtio_cwrite_v = \
+ virtio_cpu_to_le(*(ptr), memberof(structname, member)); \
\
might_sleep(); \
/* Sanity check: must match the member's type */ \
--
2.33.1
offsetof(), sizeof_field(), and typeof_member()
are common wrappers over memberof()
that simplify and make more consistent the code
around memberof().
Signed-off-by: Alejandro Colomar <[email protected]>
---
arch/x86/include/asm/bootparam_utils.h | 4 ++--
arch/x86/kernel/signal_compat.c | 6 +++---
drivers/net/ethernet/emulex/benet/be.h | 11 +++++------
drivers/net/ethernet/i825xx/ether1.c | 6 +++---
drivers/scsi/be2iscsi/be.h | 13 ++++++-------
drivers/scsi/be2iscsi/be_cmds.h | 4 ++--
fs/btrfs/ctree.h | 6 +++---
include/acpi/actypes.h | 4 ++--
include/linux/virtio_config.h | 8 ++++----
9 files changed, 30 insertions(+), 32 deletions(-)
diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
index 71b28b5fb088..728c9245fe33 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -3,7 +3,7 @@
#define _ASM_X86_BOOTPARAM_UTILS_H
#include <asm/bootparam.h>
-#include <linux/container_of.h>
+#include <linux/stddef.h>
/*
* This file is included from multiple environments. Do not
@@ -20,7 +20,7 @@
* private magic, so it is better to leave it unchanged.
*/
-#define sizeof_mbr(type, member) ({ sizeof(memberof(type, member)); })
+#define sizeof_mbr(type, member) ({ sizeof_field(type, member); })
#define BOOT_PARAM_PRESERVE(struct_member) \
{ \
diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c
index 28420a7df056..f8b37577d983 100644
--- a/arch/x86/kernel/signal_compat.c
+++ b/arch/x86/kernel/signal_compat.c
@@ -1,8 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/compat.h>
-#include <linux/container_of.h>
#include <linux/uaccess.h>
#include <linux/ptrace.h>
+#include <linux/stddef.h>
/*
* The compat_siginfo_t structure and handing code is very easy
@@ -70,8 +70,8 @@ static inline void signal_compat_build_tests(void)
* structure stays within the padding size (checked
* above).
*/
-#define CHECK_CSI_SIZE(name, size) BUILD_BUG_ON(size != sizeof(memberof(compat_siginfo_t ,_sifields.name)))
-#define CHECK_SI_SIZE(name, size) BUILD_BUG_ON(size != sizeof(memberof(siginfo_t, _sifields.name)))
+#define CHECK_CSI_SIZE(name, size) BUILD_BUG_ON(size != sizeof_field(compat_siginfo_t ,_sifields.name))
+#define CHECK_SI_SIZE(name, size) BUILD_BUG_ON(size != sizeof_field(siginfo_t, _sifields.name))
CHECK_CSI_OFFSET(_kill);
CHECK_CSI_SIZE (_kill, 2*sizeof(int));
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index dc790bae2451..1c80d19cf59f 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -14,10 +14,10 @@
#ifndef BE_H
#define BE_H
-#include <linux/container_of.h>
#include <linux/pci.h>
#include <linux/etherdevice.h>
#include <linux/delay.h>
+#include <linux/stddef.h>
#include <net/tcp.h>
#include <net/ip.h>
#include <net/ipv6.h>
@@ -824,8 +824,7 @@ extern const struct ethtool_ops be_ethtool_ops;
(size) + (PAGE_SIZE_4K - 1)) >> PAGE_SHIFT_4K))
/* Returns bit offset within a DWORD of a bitfield */
-#define AMAP_BIT_OFFSET(_struct, field) \
- (((size_t)&(memberof(_struct, field)))%32)
+#define AMAP_BIT_OFFSET(_struct, field) (offsetof(_struct, field) % 32)
/* Returns the bit mask of the field that is NOT shifted into location. */
static inline u32 amap_mask(u32 bitsize)
@@ -843,8 +842,8 @@ amap_set(void *ptr, u32 dw_offset, u32 mask, u32 offset, u32 value)
#define AMAP_SET_BITS(_struct, field, ptr, val) \
amap_set(ptr, \
- offsetof(_struct, field)/32, \
- amap_mask(sizeof(memberof(_struct, field))), \
+ offsetof(_struct, field) / 32, \
+ amap_mask(sizeof_field(_struct, field)), \
AMAP_BIT_OFFSET(_struct, field), \
val)
@@ -857,7 +856,7 @@ static inline u32 amap_get(void *ptr, u32 dw_offset, u32 mask, u32 offset)
#define AMAP_GET_BITS(_struct, field, ptr) \
amap_get(ptr, \
offsetof(_struct, field)/32, \
- amap_mask(sizeof(memberof(_struct, field))), \
+ amap_mask(sizeof_field(_struct, field)), \
AMAP_BIT_OFFSET(_struct, field))
#define GET_RX_COMPL_V0_BITS(field, ptr) \
diff --git a/drivers/net/ethernet/i825xx/ether1.c b/drivers/net/ethernet/i825xx/ether1.c
index 4dece3476d31..938b5948883c 100644
--- a/drivers/net/ethernet/i825xx/ether1.c
+++ b/drivers/net/ethernet/i825xx/ether1.c
@@ -28,7 +28,6 @@
* 1.07 RMK 13/05/2000 Updated for 2.3.99-pre8
*/
-#include <linux/containerof.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/types.h>
@@ -37,6 +36,7 @@
#include <linux/ioport.h>
#include <linux/in.h>
#include <linux/slab.h>
+#include <linux/stddef.h>
#include <linux/string.h>
#include <linux/errno.h>
#include <linux/device.h>
@@ -82,9 +82,9 @@ static char version[] = "ether1 ethernet driver (c) 2000 Russell King v1.07\n";
#define NORMALIRQS 0
#define ether1_readw(dev, addr, type, offset, svflgs) \
- ether1_inw_p(dev, addr + (int)(&(memberof(type, offset))), svflgs)
+ ether1_inw_p(dev, addr + offsetof(type, offset), svflgs)
#define ether1_writew(dev, val, addr, type, offset, svflgs) \
- ether1_outw_p(dev, val, addr + (int)(&(memberof(type, offset))), svflgs)
+ ether1_outw_p(dev, val, addr + offsetof(type, offset), svflgs)
static inline unsigned short
ether1_inw_p (struct net_device *dev, int addr, int svflgs)
diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h
index 6830e07fd37a..ec44b0d86ae4 100644
--- a/drivers/scsi/be2iscsi/be.h
+++ b/drivers/scsi/be2iscsi/be.h
@@ -10,10 +10,10 @@
#ifndef BEISCSI_H
#define BEISCSI_H
-#include <linux/container_of.h>
#include <linux/pci.h>
#include <linux/if_vlan.h>
#include <linux/irq_poll.h>
+#include <linux/stddef.h>
#define FW_VER_LEN 32
#define MCC_Q_LEN 128
#define MCC_CQ_LEN 256
@@ -153,8 +153,7 @@ struct be_ctrl_info {
(size) + (PAGE_SIZE_4K - 1)) >> PAGE_SHIFT_4K))
/* Returns bit offset within a DWORD of a bitfield */
-#define AMAP_BIT_OFFSET(_struct, field) \
- (((size_t)&(memberof(_struct, field)))%32)
+#define AMAP_BIT_OFFSET(_struct, field) (offsetof(_struct, field) % 32)
/* Returns the bit mask of the field that is NOT shifted into location. */
static inline u32 amap_mask(u32 bitsize)
@@ -172,8 +171,8 @@ static inline void amap_set(void *ptr, u32 dw_offset, u32 mask,
#define AMAP_SET_BITS(_struct, field, ptr, val) \
amap_set(ptr, \
- offsetof(_struct, field)/32, \
- amap_mask(sizeof(memberof(_struct, field))), \
+ offsetof(_struct, field) / 32, \
+ amap_mask(sizeof_field(_struct, field)), \
AMAP_BIT_OFFSET(_struct, field), \
val)
@@ -185,8 +184,8 @@ static inline u32 amap_get(void *ptr, u32 dw_offset, u32 mask, u32 offset)
#define AMAP_GET_BITS(_struct, field, ptr) \
amap_get(ptr, \
- offsetof(_struct, field)/32, \
- amap_mask(sizeof(memberof(_struct, field))), \
+ offsetof(_struct, field) / 32, \
+ amap_mask(sizeof_field(_struct, field)), \
AMAP_BIT_OFFSET(_struct, field))
#define be_dws_cpu_to_le(wrb, len) swap_dws(wrb, len)
diff --git a/drivers/scsi/be2iscsi/be_cmds.h b/drivers/scsi/be2iscsi/be_cmds.h
index bb6ee43769d5..370ca9131090 100644
--- a/drivers/scsi/be2iscsi/be_cmds.h
+++ b/drivers/scsi/be2iscsi/be_cmds.h
@@ -11,7 +11,7 @@
#define BEISCSI_CMDS_H
-#include <linux/container_of.h>
+#include <linux/stddef.h>
/**
* The driver sends configuration and managements command requests to the
@@ -1303,7 +1303,7 @@ struct be_cmd_get_port_name {
/* Returns the number of items in the field array. */
#define BE_NUMBER_OF_FIELD(_type_, _field_) \
- (sizeof_field(_type_, _field_) / sizeof(memberof(_type_, _field_[0])))
+ (sizeof_field(_type_, _field_) / sizeof_field(_type_, _field_[0]))
/**
* Different types of iSCSI completions to host driver for both initiator
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 442b6f937b22..cbad9202f213 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -6,7 +6,6 @@
#ifndef BTRFS_CTREE_H
#define BTRFS_CTREE_H
-#include <linux/container_of.h>
#include <linux/mm.h>
#include <linux/sched/signal.h>
#include <linux/highmem.h>
@@ -29,6 +28,7 @@
#include <linux/refcount.h>
#include <linux/crc32c.h>
#include <linux/iomap.h>
+#include <linux/stddef.h>
#include "extent-io-tree.h"
#include "extent_io.h"
#include "extent_map.h"
@@ -1576,13 +1576,13 @@ static inline void put_unaligned_le8(u8 val, void *p)
read_extent_buffer(eb, (char *)(result), \
((unsigned long)(ptr)) + \
offsetof(type, member), \
- sizeof(memberof(type, member))))
+ sizeof_field(type, member)))
#define write_eb_member(eb, ptr, type, member, result) (\
write_extent_buffer(eb, (char *)(result), \
((unsigned long)(ptr)) + \
offsetof(type, member), \
- sizeof(memberof(type, member))))
+ sizeof_field(type, member)))
#define DECLARE_BTRFS_SETGET_BITS(bits) \
u##bits btrfs_get_token_##bits(struct btrfs_map_token *token, \
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index c3b0eccb3377..e0bcfd37a005 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -10,7 +10,7 @@
#ifndef __ACTYPES_H__
#define __ACTYPES_H__
-#include <linux/container_of.h>
+#include <linux/stddef.h>
/* acpisrc:struct_defs -- for acpisrc conversion */
@@ -510,7 +510,7 @@ typedef u64 acpi_integer;
#define ACPI_TO_POINTER(i) ACPI_CAST_PTR (void, (acpi_size) (i))
#define ACPI_TO_INTEGER(p) ACPI_PTR_DIFF (p, (void *) 0)
-#define ACPI_OFFSET(d, f) ACPI_PTR_DIFF (&(memberof(d, f)), (void *) 0)
+#define ACPI_OFFSET(d, f) offsetof(d, f)
#define ACPI_PHYSADDR_TO_PTR(i) ACPI_TO_POINTER(i)
#define ACPI_PTR_TO_PHYSADDR(i) ACPI_TO_INTEGER(i)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index a1e656c0d94a..4fd659e26450 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -333,7 +333,7 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
/* Config space accessors. */
#define virtio_cread(vdev, structname, member, ptr) \
do { \
- typeof(memberof(structname, member)) virtio_cread_v; \
+ typeof_member(structname, member) virtio_cread_v; \
\
might_sleep(); \
/* Sanity check: must match the member's type */ \
@@ -362,7 +362,7 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
/* Config space accessors. */
#define virtio_cwrite(vdev, structname, member, ptr) \
do { \
- typeof(memberof(structname, member)) virtio_cwrite_v = \
+ typeof_member(structname, member) virtio_cwrite_v = \
cpu_to_virtio(vdev, *(ptr), memberof(structname, member)); \
\
might_sleep(); \
@@ -397,7 +397,7 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
/* LE (e.g. modern) Config space accessors. */
#define virtio_cread_le(vdev, structname, member, ptr) \
do { \
- typeof(memberof(structname, member)) virtio_cread_v; \
+ typeof_member(structname, member) virtio_cread_v; \
\
might_sleep(); \
/* Sanity check: must match the member's type */ \
@@ -425,7 +425,7 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
#define virtio_cwrite_le(vdev, structname, member, ptr) \
do { \
- typeof(memberof(structname, member)) virtio_cwrite_v = \
+ typeof_member(structname, member) virtio_cwrite_v = \
virtio_cpu_to_le(*(ptr), memberof(structname, member)); \
\
might_sleep(); \
--
2.33.1
Touching files so used for the kernel,
such as <linux/stddef.h> or <linux/container_of.h>
forces 'make' to recompile most of the kernel.
Having those definitions in more granular files
helps avoid recompiling so much of the kernel.
Signed-off-by: Alejandro Colomar <[email protected]>
---
drivers/gpu/drm/i915/i915_utils.h | 2 +-
drivers/gpu/drm/i915/intel_runtime_pm.h | 1 +
include/linux/container_of.h | 3 +--
include/linux/memberof.h | 9 +++++++++
include/linux/virtio_config.h | 1 +
5 files changed, 13 insertions(+), 3 deletions(-)
create mode 100644 include/linux/memberof.h
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index 2e3ec28f4f1b..bb91e6763d34 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -25,8 +25,8 @@
#ifndef __I915_UTILS_H
#define __I915_UTILS_H
-#include <linux/container_of.h>
#include <linux/list.h>
+#include <linux/memberof.h>
#include <linux/overflow.h>
#include <linux/sched.h>
#include <linux/types.h>
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.h b/drivers/gpu/drm/i915/intel_runtime_pm.h
index 6cb04bef62ad..72f5396eea70 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.h
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.h
@@ -6,6 +6,7 @@
#ifndef __INTEL_RUNTIME_PM_H__
#define __INTEL_RUNTIME_PM_H__
+#include <linux/memberof.h>
#include <linux/types.h>
#include "intel_wakeref.h"
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 227418cb8c99..2f1eb5ddd20a 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -4,10 +4,9 @@
#include <linux/build_bug.h>
#include <linux/err.h>
+#include <linux/memberof.h>
-#define memberof(T, m) (((T *) NULL)->m)
-
#define typeof_member(T, m) typeof(memberof(T, m))
/**
diff --git a/include/linux/memberof.h b/include/linux/memberof.h
new file mode 100644
index 000000000000..3853c91d1fb2
--- /dev/null
+++ b/include/linux/memberof.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_MEMBEROF_H
+#define _LINUX_MEMBEROF_H
+
+
+#define memberof(T, m) (((T *) NULL)->m)
+
+
+#endif /* _LINUX_MEMBEROF_H */
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 4fd659e26450..ddbd51208ce1 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -4,6 +4,7 @@
#include <linux/err.h>
#include <linux/bug.h>
+#include <linux/memberof.h>
#include <linux/virtio.h>
#include <linux/virtio_byteorder.h>
#include <linux/compiler_types.h>
--
2.33.1
There's no need to use typeof() on the input to sizeof().
Signed-off-by: Alejandro Colomar <[email protected]>
---
drivers/platform/x86/wmi.c | 4 ++--
kernel/kallsyms.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 5daadcafd808..0b1c49aa6d5f 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -33,7 +33,7 @@
#include <linux/uuid.h>
#include <linux/wmi.h>
#include <linux/fs.h>
-#include <linux/typeof_member.h>
+#include <linux/stddef.h>
#include <uapi/linux/wmi.h>
MODULE_AUTHOR("Carlos Corbacho");
@@ -54,7 +54,7 @@ struct guid_block {
u8 instance_count;
u8 flags;
} __packed;
-static_assert(sizeof(typeof_member(struct guid_block, guid)) == 16);
+static_assert(sizeof_field(struct guid_block, guid) == 16);
static_assert(sizeof(struct guid_block) == 20);
static_assert(__alignof__(struct guid_block) == 1);
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 3bf04d89d72c..9a1e986fac0d 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -29,7 +29,7 @@
#include <linux/compiler.h>
#include <linux/module.h>
#include <linux/kernel.h>
-#include <linux/typeof_member.h>
+#include <linux/stddef.h>
/*
* These will be re-linked against their real values
@@ -467,7 +467,7 @@ static int __sprint_symbol(char *buffer, unsigned long address,
if (add_buildid && buildid) {
/* build ID should match length of sprintf */
#if IS_ENABLED(CONFIG_MODULES)
- static_assert(sizeof(typeof_member(struct module, build_id)) == 20);
+ static_assert(sizeof_field(struct module, build_id) == 20);
#endif
len += sprintf(buffer + len, " %20phN", buildid);
}
--
2.33.1
Touching files so used for the kernel,
forces 'make' to recompile most of the kernel.
Having those definitions in more granular files
helps avoid recompiling so much of the kernel.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/NULL.h | 10 ++++++++++
include/linux/memberof.h | 2 ++
include/linux/stddef.h | 3 +--
3 files changed, 13 insertions(+), 2 deletions(-)
create mode 100644 include/linux/NULL.h
diff --git a/include/linux/NULL.h b/include/linux/NULL.h
new file mode 100644
index 000000000000..628eacaf598a
--- /dev/null
+++ b/include/linux/NULL.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_NULL_H
+#define _LINUX_NULL_H
+
+
+#undef NULL
+#define NULL ((void *)0)
+
+
+#endif /* _LINUX_NULL_H */
diff --git a/include/linux/memberof.h b/include/linux/memberof.h
index 3853c91d1fb2..6c5eb70bc60f 100644
--- a/include/linux/memberof.h
+++ b/include/linux/memberof.h
@@ -2,6 +2,8 @@
#ifndef _LINUX_MEMBEROF_H
#define _LINUX_MEMBEROF_H
+#include <linux/NULL.h>
+
#define memberof(T, m) (((T *) NULL)->m)
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index ca507bd5f808..0fbbaa83a9d8 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -3,9 +3,8 @@
#define _LINUX_STDDEF_H
#include <uapi/linux/stddef.h>
+#include <linux/NULL.h>
-#undef NULL
-#define NULL ((void *)0)
enum {
false = 0,
--
2.33.1
Use memberof() instead of explicitly dereferencing a null pointer.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/offsetof.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/offsetof.h b/include/linux/offsetof.h
index d0e2f1c34aae..809aed37ad29 100644
--- a/include/linux/offsetof.h
+++ b/include/linux/offsetof.h
@@ -3,13 +3,14 @@
#define _LINUX_OFFSETOF_H
#include <linux/compiler_types.h>
+#include <linux/memberof.h>
#undef offsetof
#ifdef __compiler_offsetof
#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
#else
-#define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER)
+#define offsetof(TYPE, MEMBER) ((size_t)&memberof(TYPE, MEMBER))
#endif
--
2.33.1
Touching files so used for the kernel,
forces 'make' to recompile most of the kernel.
Having those definitions in more granular files
helps avoid recompiling so much of the kernel.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/arm_ffa.h | 1 +
include/linux/blk_types.h | 1 +
include/linux/can/core.h | 1 +
include/linux/container_of.h | 1 +
include/linux/crash_core.h | 1 +
include/linux/f2fs_fs.h | 1 +
include/linux/filter.h | 1 +
include/linux/fs.h | 1 +
include/linux/ieee80211.h | 1 +
include/linux/kbuild.h | 3 +++
include/linux/kvm_host.h | 1 +
include/linux/libata.h | 1 +
include/linux/llist.h | 1 +
include/linux/mlx5/device.h | 1 +
include/linux/mlx5/driver.h | 1 +
include/linux/mm_types.h | 1 +
include/linux/netdevice.h | 1 +
include/linux/nvme-fc.h | 2 ++
include/linux/offsetof.h | 16 ++++++++++++++++
include/linux/rcupdate.h | 1 +
include/linux/sched/task.h | 1 +
include/linux/skb_array.h | 1 +
include/linux/skbuff.h | 1 +
include/linux/slab.h | 1 +
include/linux/spinlock_types.h | 1 +
include/linux/stddef.h | 8 +-------
include/linux/string.h | 1 +
include/linux/surface_aggregator/serial_hub.h | 1 +
include/linux/swap.h | 1 +
include/linux/ti-emif-sram.h | 1 +
include/linux/vdpa.h | 1 +
include/linux/virtio_config.h | 1 +
include/linux/wireless.h | 2 ++
33 files changed, 52 insertions(+), 7 deletions(-)
create mode 100644 include/linux/offsetof.h
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
index 85651e41ded8..e6e4c96018d2 100644
--- a/include/linux/arm_ffa.h
+++ b/include/linux/arm_ffa.h
@@ -8,6 +8,7 @@
#include <linux/device.h>
#include <linux/module.h>
+#include <linux/offsetof.h>
#include <linux/types.h>
#include <linux/uuid.h>
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index fe065c394fff..2515a329811c 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -10,6 +10,7 @@
#include <linux/bvec.h>
#include <linux/device.h>
#include <linux/ktime.h>
+#include <linux/offsetof.h>
struct bio_set;
struct bio;
diff --git a/include/linux/can/core.h b/include/linux/can/core.h
index 5fb8d0e3f9c1..945853463289 100644
--- a/include/linux/can/core.h
+++ b/include/linux/can/core.h
@@ -15,6 +15,7 @@
#define _CAN_CORE_H
#include <linux/can.h>
+#include <linux/offsetof.h>
#include <linux/skbuff.h>
#include <linux/netdevice.h>
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 082188b5cd29..45aa73f5e392 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -5,6 +5,7 @@
#include <linux/build_bug.h>
#include <linux/err.h>
#include <linux/memberof.h>
+#include <linux/offsetof.h>
/**
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index de62a722431e..3802410ed61d 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -5,6 +5,7 @@
#include <linux/linkage.h>
#include <linux/elfcore.h>
#include <linux/elf.h>
+#include <linux/offsetof.h>
#define CRASH_CORE_NOTE_NAME "CORE"
#define CRASH_CORE_NOTE_HEAD_BYTES ALIGN(sizeof(struct elf_note), 4)
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index d445150c5350..411213cb771c 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -8,6 +8,7 @@
#ifndef _LINUX_F2FS_FS_H
#define _LINUX_F2FS_FS_H
+#include <linux/offsetof.h>
#include <linux/pagemap.h>
#include <linux/types.h>
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 24b7ed2677af..3b6bd782e1d6 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -17,6 +17,7 @@
#include <linux/set_memory.h>
#include <linux/kallsyms.h>
#include <linux/if_vlan.h>
+#include <linux/offsetof.h>
#include <linux/vmalloc.h>
#include <linux/sockptr.h>
#include <crypto/sha1.h>
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1cb616fc1105..90077f100579 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -41,6 +41,7 @@
#include <linux/stddef.h>
#include <linux/mount.h>
#include <linux/cred.h>
+#include <linux/offsetof.h>
#include <asm/byteorder.h>
#include <uapi/linux/fs.h>
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 11d7af260f20..ee9c68833c8d 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -18,6 +18,7 @@
#include <linux/types.h>
#include <linux/if_ether.h>
#include <linux/etherdevice.h>
+#include <linux/offsetof.h>
#include <asm/byteorder.h>
#include <asm/unaligned.h>
diff --git a/include/linux/kbuild.h b/include/linux/kbuild.h
index e7be517aaaf6..cfe4c02f6167 100644
--- a/include/linux/kbuild.h
+++ b/include/linux/kbuild.h
@@ -2,6 +2,9 @@
#ifndef __LINUX_KBUILD_H
#define __LINUX_KBUILD_H
+#include <linux/offsetof.h>
+
+
#define DEFINE(sym, val) \
asm volatile("\n.ascii \"->" #sym " %0 " #val "\"" : : "i" (val))
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 9e0667e3723e..289778b9e6fa 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -29,6 +29,7 @@
#include <linux/refcount.h>
#include <linux/nospec.h>
#include <linux/notifier.h>
+#include <linux/offsetof.h>
#include <asm/signal.h>
#include <linux/kvm.h>
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 2a8404b26083..ae6b043011eb 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -23,6 +23,7 @@
#include <linux/cdrom.h>
#include <linux/sched.h>
#include <linux/async.h>
+#include <linux/offsetof.h>
/*
* Define if arch has non-standard setup. This is a _PCI_ standard
diff --git a/include/linux/llist.h b/include/linux/llist.h
index 85bda2d02d65..aa04f80cf37b 100644
--- a/include/linux/llist.h
+++ b/include/linux/llist.h
@@ -50,6 +50,7 @@
#include <linux/atomic.h>
#include <linux/container_of.h>
+#include <linux/offsetof.h>
#include <linux/stddef.h>
#include <linux/types.h>
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index 9c25edfd59a6..e4e5dbbbb45e 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -33,6 +33,7 @@
#ifndef MLX5_DEVICE_H
#define MLX5_DEVICE_H
+#include <linux/offsetof.h>
#include <linux/types.h>
#include <rdma/ib_verbs.h>
#include <linux/mlx5/mlx5_ifc.h>
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index a623ec635947..da3cb7f10fe3 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -49,6 +49,7 @@
#include <linux/notifier.h>
#include <linux/refcount.h>
#include <linux/auxiliary_bus.h>
+#include <linux/offsetof.h>
#include <linux/mlx5/device.h>
#include <linux/mlx5/doorbell.h>
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index bb8c6f5f19bc..d93f3606cfcc 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -11,6 +11,7 @@
#include <linux/rwsem.h>
#include <linux/completion.h>
#include <linux/cpumask.h>
+#include <linux/offsetof.h>
#include <linux/uprobes.h>
#include <linux/rcupdate.h>
#include <linux/page-flags-layout.h>
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3ec42495a43a..ca45dda19890 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -25,6 +25,7 @@
#include <linux/bug.h>
#include <linux/delay.h>
#include <linux/atomic.h>
+#include <linux/offsetof.h>
#include <linux/prefetch.h>
#include <asm/cache.h>
#include <asm/byteorder.h>
diff --git a/include/linux/nvme-fc.h b/include/linux/nvme-fc.h
index 51fe44e0328b..02ac9c80fc1a 100644
--- a/include/linux/nvme-fc.h
+++ b/include/linux/nvme-fc.h
@@ -11,6 +11,8 @@
#ifndef _NVME_FC_H
#define _NVME_FC_H 1
+#include <linux/offsetof.h>
+
#include <uapi/scsi/fc/fc_fs.h>
#define NVME_CMD_FORMAT_ID 0xFD
diff --git a/include/linux/offsetof.h b/include/linux/offsetof.h
new file mode 100644
index 000000000000..d0e2f1c34aae
--- /dev/null
+++ b/include/linux/offsetof.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_OFFSETOF_H
+#define _LINUX_OFFSETOF_H
+
+#include <linux/compiler_types.h>
+
+
+#undef offsetof
+#ifdef __compiler_offsetof
+#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
+#else
+#define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER)
+#endif
+
+
+#endif /* _LINUX_OFFSETOF_H */
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 5e0beb5c5659..310537479e54 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -29,6 +29,7 @@
#include <linux/lockdep.h>
#include <asm/processor.h>
#include <linux/cpumask.h>
+#include <linux/offsetof.h>
#define ULONG_CMP_GE(a, b) (ULONG_MAX / 2 >= (a) - (b))
#define ULONG_CMP_LT(a, b) (ULONG_MAX / 2 < (a) - (b))
diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index ba88a6987400..19fecc23d6fd 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -7,6 +7,7 @@
* functionality:
*/
+#include <linux/offsetof.h>
#include <linux/sched.h>
#include <linux/uaccess.h>
diff --git a/include/linux/skb_array.h b/include/linux/skb_array.h
index e2d45b7cb619..2b04a33e5cf5 100644
--- a/include/linux/skb_array.h
+++ b/include/linux/skb_array.h
@@ -20,6 +20,7 @@
#include <linux/ptr_ring.h>
#include <linux/skbuff.h>
#include <linux/if_vlan.h>
+#include <linux/offsetof.h>
#endif
struct skb_array {
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 686a666d073d..9abcc13bede2 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -16,6 +16,7 @@
#include <linux/bug.h>
#include <linux/bvec.h>
#include <linux/cache.h>
+#include <linux/offsetof.h>
#include <linux/rbtree.h>
#include <linux/socket.h>
#include <linux/refcount.h>
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 181045148b06..61cb3191566f 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -13,6 +13,7 @@
#define _LINUX_SLAB_H
#include <linux/gfp.h>
+#include <linux/offsetof.h>
#include <linux/overflow.h>
#include <linux/types.h>
#include <linux/workqueue.h>
diff --git a/include/linux/spinlock_types.h b/include/linux/spinlock_types.h
index 2dfa35ffec76..bbef450b7abf 100644
--- a/include/linux/spinlock_types.h
+++ b/include/linux/spinlock_types.h
@@ -10,6 +10,7 @@
*/
#include <linux/spinlock_types_raw.h>
+#include <linux/offsetof.h>
#ifndef CONFIG_PREEMPT_RT
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index 0fbbaa83a9d8..6ab0f57eccde 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -4,6 +4,7 @@
#include <uapi/linux/stddef.h>
#include <linux/NULL.h>
+#include <linux/offsetof.h>
enum {
@@ -11,13 +12,6 @@ enum {
true = 1
};
-#undef offsetof
-#ifdef __compiler_offsetof
-#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
-#else
-#define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER)
-#endif
-
/**
* sizeof_field() - Report the size of a struct field in bytes
*
diff --git a/include/linux/string.h b/include/linux/string.h
index b6572aeca2f5..2ddcd560659b 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -6,6 +6,7 @@
#include <linux/types.h> /* for size_t */
#include <linux/stddef.h> /* for NULL */
#include <linux/errno.h> /* for E2BIG */
+#include <linux/offsetof.h>
#include <linux/stdarg.h>
#include <uapi/linux/string.h>
diff --git a/include/linux/surface_aggregator/serial_hub.h b/include/linux/surface_aggregator/serial_hub.h
index c3de43edcffa..95e8f54677ea 100644
--- a/include/linux/surface_aggregator/serial_hub.h
+++ b/include/linux/surface_aggregator/serial_hub.h
@@ -16,6 +16,7 @@
#include <linux/kref.h>
#include <linux/ktime.h>
#include <linux/list.h>
+#include <linux/offsetof.h>
#include <linux/types.h>
diff --git a/include/linux/swap.h b/include/linux/swap.h
index d1ea44b31f19..76b732c6744e 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -12,6 +12,7 @@
#include <linux/fs.h>
#include <linux/pagemap.h>
#include <linux/atomic.h>
+#include <linux/offsetof.h>
#include <linux/page-flags.h>
#include <uapi/linux/mempolicy.h>
#include <asm/page.h>
diff --git a/include/linux/ti-emif-sram.h b/include/linux/ti-emif-sram.h
index 2fc854155c27..611b7d7f4651 100644
--- a/include/linux/ti-emif-sram.h
+++ b/include/linux/ti-emif-sram.h
@@ -17,6 +17,7 @@
#define __LINUX_TI_EMIF_H
#include <linux/kbuild.h>
+#include <linux/offsetof.h>
#include <linux/types.h>
#ifndef __ASSEMBLY__
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index c3011ccda430..1bf977122197 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -5,6 +5,7 @@
#include <linux/kernel.h>
#include <linux/device.h>
#include <linux/interrupt.h>
+#include <linux/offsetof.h>
#include <linux/vhost_iotlb.h>
#include <linux/virtio_net.h>
#include <linux/if_ether.h>
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index ff1752441e5d..87423a50cd13 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -8,6 +8,7 @@
#include <linux/virtio.h>
#include <linux/virtio_byteorder.h>
#include <linux/compiler_types.h>
+#include <linux/offsetof.h>
#include <linux/typeof_member.h>
#include <uapi/linux/virtio_config.h>
diff --git a/include/linux/wireless.h b/include/linux/wireless.h
index 2d1b54556eff..99e9cc56ba01 100644
--- a/include/linux/wireless.h
+++ b/include/linux/wireless.h
@@ -10,6 +10,8 @@
#ifndef _LINUX_WIRELESS_H
#define _LINUX_WIRELESS_H
+#include <linux/offsetof.h>
+
#include <uapi/linux/wireless.h>
#ifdef CONFIG_COMPAT
--
2.33.1
Avoid duplicate code. There's only one different statement. Let it be so.
Note: I'm not sure if we really need a void pointer for IS_ERR_OR_NULL(),
or we could remove that line altogether.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/container_of.h | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 45aa73f5e392..addd3993fa60 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -30,12 +30,13 @@
*
* If IS_ERR_OR_NULL(ptr), ptr is returned unchanged.
*/
-#define container_of_safe(ptr, type, member) ({ \
+#define container_of_safe(ptr, type, member) ( \
+{ \
void *__mptr = (void *)(ptr); \
- static_assert(__same_type(*(ptr), memberof(type, member)) || \
- __same_type(*(ptr), void), \
- "pointer type mismatch in container_of_safe()"); \
+ \
IS_ERR_OR_NULL(__mptr) ? ERR_CAST(__mptr) : \
- ((type *)(__mptr - offsetof(type, member))); })
+ container_of(ptr, type, member); \
+} \
+)
#endif /* _LINUX_CONTAINER_OF_H */
--
2.33.1
Place braces more similar to where a function would have them.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/container_of.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index addd3993fa60..2100adb9d109 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -15,12 +15,16 @@
* @member: the name of the member within the struct.
*
*/
-#define container_of(ptr, type, member) ({ \
+#define container_of(ptr, type, member) ( \
+{ \
void *__mptr = (void *)(ptr); \
+ \
static_assert(__same_type(*(ptr), memberof(type, member)) || \
__same_type(*(ptr), void), \
"pointer type mismatch in container_of()"); \
- ((type *)(__mptr - offsetof(type, member))); })
+ ((type *)(__mptr - offsetof(type, member))); \
+} \
+)
/**
* container_of_safe - cast a member of a structure out to the containing structure
--
2.33.1
Casts are dangerous.
Every pointer converts implicitly to (void *).
Remove the unnecessary cast.
Since this macro is used with some pointers-to-const,
removing the cast triggers warnings about that
(implicit conversion from poitner-to-const to pointer-to-void).
To solve it, since we don't use the pointer to modify its contents,
we can simply use a (const void *).
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/container_of.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 2100adb9d109..62df2ba21c20 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -17,7 +17,7 @@
*/
#define container_of(ptr, type, member) ( \
{ \
- void *__mptr = (void *)(ptr); \
+ const void *__mptr = (ptr); \
\
static_assert(__same_type(*(ptr), memberof(type, member)) || \
__same_type(*(ptr), void), \
@@ -36,7 +36,7 @@
*/
#define container_of_safe(ptr, type, member) ( \
{ \
- void *__mptr = (void *)(ptr); \
+ const void *__mptr = (ptr); \
\
IS_ERR_OR_NULL(__mptr) ? ERR_CAST(__mptr) : \
container_of(ptr, type, member); \
--
2.33.1
Touching files so used for the kernel,
forces 'make' to recompile most of the kernel.
Having those definitions in more granular files
helps avoid recompiling so much of the kernel.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/efi.h | 1 +
include/linux/filter.h | 1 +
include/linux/kvm_host.h | 1 +
include/linux/phy_led_triggers.h | 1 +
include/linux/sizeof_field.h | 14 ++++++++++++++
include/linux/slab.h | 1 +
include/linux/stddef.h | 9 +--------
7 files changed, 20 insertions(+), 8 deletions(-)
create mode 100644 include/linux/sizeof_field.h
diff --git a/include/linux/efi.h b/include/linux/efi.h
index dbd39b20e034..47aecde48326 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -25,6 +25,7 @@
#include <linux/reboot.h>
#include <linux/uuid.h>
#include <linux/screen_info.h>
+#include <linux/sizeof_field.h>
#include <asm/page.h>
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 3b6bd782e1d6..ebb0ae480533 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -18,6 +18,7 @@
#include <linux/kallsyms.h>
#include <linux/if_vlan.h>
#include <linux/offsetof.h>
+#include <linux/sizeof_field.h>
#include <linux/vmalloc.h>
#include <linux/sockptr.h>
#include <crypto/sha1.h>
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 289778b9e6fa..f89a516f3a39 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -30,6 +30,7 @@
#include <linux/nospec.h>
#include <linux/notifier.h>
#include <linux/offsetof.h>
+#include <linux/sizeof_field.h>
#include <asm/signal.h>
#include <linux/kvm.h>
diff --git a/include/linux/phy_led_triggers.h b/include/linux/phy_led_triggers.h
index 5c4d7a755101..86581b985161 100644
--- a/include/linux/phy_led_triggers.h
+++ b/include/linux/phy_led_triggers.h
@@ -10,6 +10,7 @@ struct phy_device;
#include <linux/leds.h>
#include <linux/phy.h>
+#include <linux/sizeof_field.h>
#define PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE 11
diff --git a/include/linux/sizeof_field.h b/include/linux/sizeof_field.h
new file mode 100644
index 000000000000..7ff541cb21af
--- /dev/null
+++ b/include/linux/sizeof_field.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_SIZEOF_FIELD_H
+#define _LINUX_SIZEOF_FIELD_H
+
+/**
+ * sizeof_field() - Report the size of a struct field in bytes
+ *
+ * @TYPE: The structure containing the field of interest
+ * @MEMBER: The field to return the size of
+ */
+#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
+
+
+#endif /* _LINUX_SIZEOF_FIELD_H */
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 61cb3191566f..c2bd24c60803 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -18,6 +18,7 @@
#include <linux/types.h>
#include <linux/workqueue.h>
#include <linux/percpu-refcount.h>
+#include <linux/sizeof_field.h>
/*
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index 6ab0f57eccde..5c9aedb5e6ad 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -5,6 +5,7 @@
#include <uapi/linux/stddef.h>
#include <linux/NULL.h>
#include <linux/offsetof.h>
+#include <linux/sizeof_field.h>
enum {
@@ -12,14 +13,6 @@ enum {
true = 1
};
-/**
- * sizeof_field() - Report the size of a struct field in bytes
- *
- * @TYPE: The structure containing the field of interest
- * @MEMBER: The field to return the size of
- */
-#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
-
/**
* offsetofend() - Report the offset of a struct field within the struct
*
--
2.33.1
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/extable.h | 2 +-
include/linux/string.h | 2 +-
include/linux/ucs2_string.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/extable.h b/include/linux/extable.h
index 4ab9e78f313b..7650a7adce76 100644
--- a/include/linux/extable.h
+++ b/include/linux/extable.h
@@ -2,7 +2,7 @@
#ifndef _LINUX_EXTABLE_H
#define _LINUX_EXTABLE_H
-#include <linux/stddef.h> /* for NULL */
+#include <linux/NULL.h>
#include <linux/types.h>
struct module;
diff --git a/include/linux/string.h b/include/linux/string.h
index 2ddcd560659b..5a22a93c53f5 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -3,8 +3,8 @@
#define _LINUX_STRING_H_
#include <linux/compiler.h> /* for inline */
+#include <linux/NULL.h>
#include <linux/types.h> /* for size_t */
-#include <linux/stddef.h> /* for NULL */
#include <linux/errno.h> /* for E2BIG */
#include <linux/offsetof.h>
#include <linux/stdarg.h>
diff --git a/include/linux/ucs2_string.h b/include/linux/ucs2_string.h
index cf3ada3e820e..c05242de728e 100644
--- a/include/linux/ucs2_string.h
+++ b/include/linux/ucs2_string.h
@@ -2,8 +2,8 @@
#ifndef _LINUX_UCS2_STRING_H_
#define _LINUX_UCS2_STRING_H_
+#include <linux/NULL.h>
#include <linux/types.h> /* for size_t */
-#include <linux/stddef.h> /* for NULL */
typedef u16 ucs2_char_t;
--
2.33.1
Touching files so used for the kernel,
forces 'make' to recompile most of the kernel.
Having those definitions in more granular files
helps avoid recompiling so much of the kernel.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/filter.h | 1 +
include/linux/offsetofend.h | 19 +++++++++++++++++++
include/linux/skmsg.h | 1 +
include/linux/stddef.h | 10 +---------
include/linux/string.h | 1 +
5 files changed, 23 insertions(+), 9 deletions(-)
create mode 100644 include/linux/offsetofend.h
diff --git a/include/linux/filter.h b/include/linux/filter.h
index ebb0ae480533..5593d0365694 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -18,6 +18,7 @@
#include <linux/kallsyms.h>
#include <linux/if_vlan.h>
#include <linux/offsetof.h>
+#include <linux/offsetofend.h>
#include <linux/sizeof_field.h>
#include <linux/vmalloc.h>
#include <linux/sockptr.h>
diff --git a/include/linux/offsetofend.h b/include/linux/offsetofend.h
new file mode 100644
index 000000000000..4f4d0e1b667a
--- /dev/null
+++ b/include/linux/offsetofend.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_OFFSETOFEND_H
+#define _LINUX_OFFSETOFEND_H
+
+#include <linux/offsetof.h>
+#include <linux/sizeof_field.h>
+
+
+/**
+ * offsetofend() - Report the offset of a struct field within the struct
+ *
+ * @TYPE: The type of the structure
+ * @MEMBER: The member within the structure to get the end offset of
+ */
+#define offsetofend(TYPE, MEMBER) \
+ (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER))
+
+
+#endif /* _LINUX_OFFSETOFEND_H */
diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index 584d94be9c8b..46e76f4ff0de 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -6,6 +6,7 @@
#include <linux/bpf.h>
#include <linux/filter.h>
+#include <linux/offsetofend.h>
#include <linux/scatterlist.h>
#include <linux/skbuff.h>
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index 5c9aedb5e6ad..da50edf4b28d 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -5,6 +5,7 @@
#include <uapi/linux/stddef.h>
#include <linux/NULL.h>
#include <linux/offsetof.h>
+#include <linux/offsetofend.h>
#include <linux/sizeof_field.h>
@@ -13,15 +14,6 @@ enum {
true = 1
};
-/**
- * offsetofend() - Report the offset of a struct field within the struct
- *
- * @TYPE: The type of the structure
- * @MEMBER: The member within the structure to get the end offset of
- */
-#define offsetofend(TYPE, MEMBER) \
- (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER))
-
/**
* struct_group() - Wrap a set of declarations in a mirrored struct
*
diff --git a/include/linux/string.h b/include/linux/string.h
index 5a22a93c53f5..555b6f00c73d 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -7,6 +7,7 @@
#include <linux/types.h> /* for size_t */
#include <linux/errno.h> /* for E2BIG */
#include <linux/offsetof.h>
+#include <linux/offsetofend.h>
#include <linux/stdarg.h>
#include <uapi/linux/string.h>
--
2.33.1
Touching files so used for the kernel,
forces 'make' to recompile most of the kernel.
Having those definitions in more granular files
helps avoid recompiling so much of the kernel.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/array_size.h | 15 +++++++++++++++
include/linux/clk-provider.h | 1 +
include/linux/counter.h | 1 +
include/linux/genl_magic_func.h | 1 +
include/linux/hashtable.h | 1 +
include/linux/kernel.h | 7 +------
include/linux/kfifo.h | 1 +
include/linux/kvm_host.h | 1 +
include/linux/moduleparam.h | 3 +++
include/linux/mtd/rawnand.h | 1 +
include/linux/netfilter.h | 1 +
include/linux/pagemap.h | 1 +
include/linux/phy.h | 1 +
include/linux/pinctrl/machine.h | 1 +
include/linux/property.h | 1 +
include/linux/rcupdate_wait.h | 1 +
include/linux/regmap.h | 1 +
include/linux/skmsg.h | 2 ++
include/linux/string.h | 1 +
include/linux/surface_aggregator/controller.h | 1 +
20 files changed, 37 insertions(+), 6 deletions(-)
create mode 100644 include/linux/array_size.h
diff --git a/include/linux/array_size.h b/include/linux/array_size.h
new file mode 100644
index 000000000000..4f62840f808a
--- /dev/null
+++ b/include/linux/array_size.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_ARRAY_SIZE_H
+#define _LINUX_ARRAY_SIZE_H
+
+#include <linux/compiler.h>
+
+
+/**
+ * ARRAY_SIZE - get the number of elements in array @arr
+ * @arr: array to be sized
+ */
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
+
+
+#endif /* _LINUX_ARRAY_SIZE_H */
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index f59c875271a0..f6860d22d1ab 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -6,6 +6,7 @@
#ifndef __LINUX_CLK_PROVIDER_H
#define __LINUX_CLK_PROVIDER_H
+#include <linux/array_size.h>
#include <linux/of.h>
#include <linux/of_clk.h>
diff --git a/include/linux/counter.h b/include/linux/counter.h
index b7d0a00a61cf..f7f6f2e50390 100644
--- a/include/linux/counter.h
+++ b/include/linux/counter.h
@@ -6,6 +6,7 @@
#ifndef _COUNTER_H_
#define _COUNTER_H_
+#include <linux/array_size.h>
#include <linux/cdev.h>
#include <linux/device.h>
#include <linux/kernel.h>
diff --git a/include/linux/genl_magic_func.h b/include/linux/genl_magic_func.h
index 939b1a8f571b..e3b5bd816bcd 100644
--- a/include/linux/genl_magic_func.h
+++ b/include/linux/genl_magic_func.h
@@ -2,6 +2,7 @@
#ifndef GENL_MAGIC_FUNC_H
#define GENL_MAGIC_FUNC_H
+#include <linux/array_size.h>
#include <linux/build_bug.h>
#include <linux/genl_magic_struct.h>
diff --git a/include/linux/hashtable.h b/include/linux/hashtable.h
index f6c666730b8c..09c5f1522b06 100644
--- a/include/linux/hashtable.h
+++ b/include/linux/hashtable.h
@@ -7,6 +7,7 @@
#ifndef _LINUX_HASHTABLE_H
#define _LINUX_HASHTABLE_H
+#include <linux/array_size.h>
#include <linux/list.h>
#include <linux/types.h>
#include <linux/kernel.h>
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 77755ac3e189..1437bfaadec5 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -2,6 +2,7 @@
#ifndef _LINUX_KERNEL_H
#define _LINUX_KERNEL_H
+#include <linux/array_size.h>
#include <linux/stdarg.h>
#include <linux/align.h>
#include <linux/limits.h>
@@ -39,12 +40,6 @@
#define READ 0
#define WRITE 1
-/**
- * ARRAY_SIZE - get the number of elements in array @arr
- * @arr: array to be sized
- */
-#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
-
#define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
#define u64_to_user_ptr(x) ( \
diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h
index 86249476b57f..ef0e4b979ba0 100644
--- a/include/linux/kfifo.h
+++ b/include/linux/kfifo.h
@@ -36,6 +36,7 @@
* to lock the reader.
*/
+#include <linux/array_size.h>
#include <linux/kernel.h>
#include <linux/spinlock.h>
#include <linux/stddef.h>
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index f89a516f3a39..67dfcf9dd166 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -3,6 +3,7 @@
#define __KVM_HOST_H
+#include <linux/array_size.h>
#include <linux/types.h>
#include <linux/hardirq.h>
#include <linux/list.h>
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 962cd41a2cb5..4a6eb8ce7c94 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -2,10 +2,13 @@
#ifndef _LINUX_MODULE_PARAMS_H
#define _LINUX_MODULE_PARAMS_H
/* (C) Copyright 2001, 2002 Rusty Russell IBM Corporation */
+
+#include <linux/array_size.h>
#include <linux/init.h>
#include <linux/stringify.h>
#include <linux/kernel.h>
+
/* You can override this manually, but generally this should match the
module name. */
#ifdef MODULE
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index b2f9dd3cbd69..cdb8e92db7ec 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -13,6 +13,7 @@
#ifndef __LINUX_MTD_RAWNAND_H
#define __LINUX_MTD_RAWNAND_H
+#include <linux/array_size.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/nand.h>
#include <linux/mtd/flashchip.h>
diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index 3fda1a508733..11a01c1fcc3c 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -2,6 +2,7 @@
#ifndef __LINUX_NETFILTER_H
#define __LINUX_NETFILTER_H
+#include <linux/array_size.h>
#include <linux/init.h>
#include <linux/skbuff.h>
#include <linux/net.h>
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 1a0c646eb6ff..529282a85cb3 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -5,6 +5,7 @@
/*
* Copyright 1995 Linus Torvalds
*/
+#include <linux/array_size.h>
#include <linux/mm.h>
#include <linux/fs.h>
#include <linux/list.h>
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 96e43fbb2dd8..ca86f7990751 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -11,6 +11,7 @@
#ifndef __PHY_H
#define __PHY_H
+#include <linux/array_size.h>
#include <linux/compiler.h>
#include <linux/spinlock.h>
#include <linux/ethtool.h>
diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h
index e987dc9fd2af..6c264dd0e163 100644
--- a/include/linux/pinctrl/machine.h
+++ b/include/linux/pinctrl/machine.h
@@ -11,6 +11,7 @@
#ifndef __LINUX_PINCTRL_MACHINE_H
#define __LINUX_PINCTRL_MACHINE_H
+#include <linux/array_size.h>
#include <linux/bug.h>
#include <linux/pinctrl/pinctrl-state.h>
diff --git a/include/linux/property.h b/include/linux/property.h
index 88fa726a76df..add29cf6c0c4 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -10,6 +10,7 @@
#ifndef _LINUX_PROPERTY_H_
#define _LINUX_PROPERTY_H_
+#include <linux/array_size.h>
#include <linux/bits.h>
#include <linux/fwnode.h>
#include <linux/types.h>
diff --git a/include/linux/rcupdate_wait.h b/include/linux/rcupdate_wait.h
index 699b938358bf..a321404eeec0 100644
--- a/include/linux/rcupdate_wait.h
+++ b/include/linux/rcupdate_wait.h
@@ -6,6 +6,7 @@
* RCU synchronization types and methods:
*/
+#include <linux/array_size.h>
#include <linux/rcupdate.h>
#include <linux/completion.h>
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index e3c9a25a853a..e039cd815fc1 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -10,6 +10,7 @@
* Author: Mark Brown <[email protected]>
*/
+#include <linux/array_size.h>
#include <linux/list.h>
#include <linux/rbtree.h>
#include <linux/ktime.h>
diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index 46e76f4ff0de..2f1bdc81fafb 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -4,6 +4,7 @@
#ifndef _LINUX_SKMSG_H
#define _LINUX_SKMSG_H
+#include <linux/array_size.h>
#include <linux/bpf.h>
#include <linux/filter.h>
#include <linux/offsetofend.h>
@@ -14,6 +15,7 @@
#include <net/tcp.h>
#include <net/strparser.h>
+
#define MAX_MSG_FRAGS MAX_SKB_FRAGS
#define NR_MSG_FRAG_IDS (MAX_MSG_FRAGS + 1)
diff --git a/include/linux/string.h b/include/linux/string.h
index 555b6f00c73d..88324a05c34a 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -2,6 +2,7 @@
#ifndef _LINUX_STRING_H_
#define _LINUX_STRING_H_
+#include <linux/array_size.h>
#include <linux/compiler.h> /* for inline */
#include <linux/NULL.h>
#include <linux/types.h> /* for size_t */
diff --git a/include/linux/surface_aggregator/controller.h b/include/linux/surface_aggregator/controller.h
index 74bfdffaf7b0..9a355c38132c 100644
--- a/include/linux/surface_aggregator/controller.h
+++ b/include/linux/surface_aggregator/controller.h
@@ -12,6 +12,7 @@
#ifndef _LINUX_SURFACE_AGGREGATOR_CONTROLLER_H
#define _LINUX_SURFACE_AGGREGATOR_CONTROLLER_H
+#include <linux/array_size.h>
#include <linux/completion.h>
#include <linux/device.h>
#include <linux/types.h>
--
2.33.1
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/crypto/internal/blake2b.h | 1 +
include/crypto/internal/blake2s.h | 1 +
include/crypto/internal/chacha.h | 1 +
include/drm/drm_mipi_dbi.h | 1 +
include/drm/drm_mode_object.h | 1 +
include/kunit/test.h | 1 +
include/net/bond_3ad.h | 1 +
include/net/dsa.h | 1 +
include/net/ip_vs.h | 1 +
include/net/netfilter/nf_conntrack_tuple.h | 1 +
include/net/netfilter/nf_tables.h | 1 +
include/net/netlink.h | 1 +
include/rdma/uverbs_ioctl.h | 1 +
include/rdma/uverbs_named_ioctl.h | 1 +
include/scsi/scsi_host.h | 1 +
include/sound/soc-dapm.h | 1 +
include/sound/soc.h | 1 +
include/trace/events/wbt.h | 1 +
include/uapi/linux/netfilter/xt_sctp.h | 1 +
include/xen/hvm.h | 1 +
20 files changed, 20 insertions(+)
diff --git a/include/crypto/internal/blake2b.h b/include/crypto/internal/blake2b.h
index 982fe5e8471c..870561f85a51 100644
--- a/include/crypto/internal/blake2b.h
+++ b/include/crypto/internal/blake2b.h
@@ -9,6 +9,7 @@
#include <crypto/blake2b.h>
#include <crypto/internal/hash.h>
+#include <linux/array_size.h>
#include <linux/string.h>
void blake2b_compress_generic(struct blake2b_state *state,
diff --git a/include/crypto/internal/blake2s.h b/include/crypto/internal/blake2s.h
index 8e50d487500f..6d7649759bc9 100644
--- a/include/crypto/internal/blake2s.h
+++ b/include/crypto/internal/blake2s.h
@@ -9,6 +9,7 @@
#include <crypto/blake2s.h>
#include <crypto/internal/hash.h>
+#include <linux/array_size.h>
#include <linux/string.h>
void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
diff --git a/include/crypto/internal/chacha.h b/include/crypto/internal/chacha.h
index b085dc1ac151..0d27df9ecbfa 100644
--- a/include/crypto/internal/chacha.h
+++ b/include/crypto/internal/chacha.h
@@ -5,6 +5,7 @@
#include <crypto/chacha.h>
#include <crypto/internal/skcipher.h>
+#include <linux/array_size.h>
#include <linux/crypto.h>
struct chacha_ctx {
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 05e194958265..9ca24caa3f91 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -8,6 +8,7 @@
#ifndef __LINUX_MIPI_DBI_H
#define __LINUX_MIPI_DBI_H
+#include <linux/array_size.h>
#include <linux/mutex.h>
#include <drm/drm_device.h>
#include <drm/drm_simple_kms_helper.h>
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
index c34a3e8030e1..83c1f4eef982 100644
--- a/include/drm/drm_mode_object.h
+++ b/include/drm/drm_mode_object.h
@@ -23,6 +23,7 @@
#ifndef __DRM_MODESET_H__
#define __DRM_MODESET_H__
+#include <linux/array_size.h>
#include <linux/kref.h>
#include <drm/drm_lease.h>
struct drm_object_properties;
diff --git a/include/kunit/test.h b/include/kunit/test.h
index b26400731c02..6d316249be95 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -12,6 +12,7 @@
#include <kunit/assert.h>
#include <kunit/try-catch.h>
+#include <linux/array_size.h>
#include <linux/container_of.h>
#include <linux/err.h>
#include <linux/init.h>
diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
index 38785d48baff..a1fceafcb630 100644
--- a/include/net/bond_3ad.h
+++ b/include/net/bond_3ad.h
@@ -7,6 +7,7 @@
#define _NET_BOND_3AD_H
#include <asm/byteorder.h>
+#include <linux/array_size.h>
#include <linux/skbuff.h>
#include <linux/netdevice.h>
#include <linux/if_ether.h>
diff --git a/include/net/dsa.h b/include/net/dsa.h
index eff5c44ba377..3d6effe14cca 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -7,6 +7,7 @@
#ifndef __LINUX_NET_DSA_H
#define __LINUX_NET_DSA_H
+#include <linux/array_size.h>
#include <linux/if.h>
#include <linux/if_ether.h>
#include <linux/list.h>
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index ff1804a0c469..0abb6706145c 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -10,6 +10,7 @@
#include <asm/types.h> /* for __uXX types */
+#include <linux/array_size.h>
#include <linux/list.h> /* for struct list_head */
#include <linux/spinlock.h> /* for struct rwlock_t */
#include <linux/atomic.h> /* for struct atomic_t */
diff --git a/include/net/netfilter/nf_conntrack_tuple.h b/include/net/netfilter/nf_conntrack_tuple.h
index 9334371c94e2..473ba3943b59 100644
--- a/include/net/netfilter/nf_conntrack_tuple.h
+++ b/include/net/netfilter/nf_conntrack_tuple.h
@@ -11,6 +11,7 @@
#ifndef _NF_CONNTRACK_TUPLE_H
#define _NF_CONNTRACK_TUPLE_H
+#include <linux/array_size.h>
#include <linux/netfilter/x_tables.h>
#include <linux/netfilter/nf_conntrack_tuple_common.h>
#include <linux/list_nulls.h>
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index a0d9e0b47ab8..324f23b7a2c4 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -3,6 +3,7 @@
#define _NET_NF_TABLES_H
#include <asm/unaligned.h>
+#include <linux/array_size.h>
#include <linux/list.h>
#include <linux/netfilter.h>
#include <linux/netfilter/nfnetlink.h>
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 7a2a9d3144ba..bc5cdb6d2f99 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -2,6 +2,7 @@
#ifndef __NET_NETLINK_H
#define __NET_NETLINK_H
+#include <linux/array_size.h>
#include <linux/types.h>
#include <linux/netlink.h>
#include <linux/jiffies.h>
diff --git a/include/rdma/uverbs_ioctl.h b/include/rdma/uverbs_ioctl.h
index 23bb404aba12..8d08414ca386 100644
--- a/include/rdma/uverbs_ioctl.h
+++ b/include/rdma/uverbs_ioctl.h
@@ -6,6 +6,7 @@
#ifndef _UVERBS_IOCTL_
#define _UVERBS_IOCTL_
+#include <linux/array_size.h>
#include <rdma/uverbs_types.h>
#include <linux/uaccess.h>
#include <rdma/rdma_user_ioctl.h>
diff --git a/include/rdma/uverbs_named_ioctl.h b/include/rdma/uverbs_named_ioctl.h
index ee7873f872c3..1882ce8cb0c2 100644
--- a/include/rdma/uverbs_named_ioctl.h
+++ b/include/rdma/uverbs_named_ioctl.h
@@ -6,6 +6,7 @@
#ifndef _UVERBS_NAMED_IOCTL_
#define _UVERBS_NAMED_IOCTL_
+#include <linux/array_size.h>
#include <rdma/uverbs_ioctl.h>
#ifndef UVERBS_MODULE_NAME
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index ebe059badba0..34c453591953 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -2,6 +2,7 @@
#ifndef _SCSI_SCSI_HOST_H
#define _SCSI_SCSI_HOST_H
+#include <linux/array_size.h>
#include <linux/device.h>
#include <linux/list.h>
#include <linux/types.h>
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index c3039e97929a..6585e53a6f9b 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -10,6 +10,7 @@
#ifndef __LINUX_SND_SOC_DAPM_H
#define __LINUX_SND_SOC_DAPM_H
+#include <linux/array_size.h>
#include <linux/types.h>
#include <sound/control.h>
#include <sound/soc-topology.h>
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 8e6dd8a257c5..4de5c7dbdcd2 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -10,6 +10,7 @@
#ifndef __LINUX_SND_SOC_H
#define __LINUX_SND_SOC_H
+#include <linux/array_size.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/types.h>
diff --git a/include/trace/events/wbt.h b/include/trace/events/wbt.h
index 9c66e59d859c..7a8a83d061ed 100644
--- a/include/trace/events/wbt.h
+++ b/include/trace/events/wbt.h
@@ -5,6 +5,7 @@
#if !defined(_TRACE_WBT_H) || defined(TRACE_HEADER_MULTI_READ)
#define _TRACE_WBT_H
+#include <linux/array_size.h>
#include <linux/tracepoint.h>
#include "../../../block/blk-wbt.h"
diff --git a/include/uapi/linux/netfilter/xt_sctp.h b/include/uapi/linux/netfilter/xt_sctp.h
index b4d804a9fccb..748fedc04228 100644
--- a/include/uapi/linux/netfilter/xt_sctp.h
+++ b/include/uapi/linux/netfilter/xt_sctp.h
@@ -2,6 +2,7 @@
#ifndef _XT_SCTP_H_
#define _XT_SCTP_H_
+#include <linux/array_size.h>
#include <linux/types.h>
#define XT_SCTP_SRC_PORTS 0x01
diff --git a/include/xen/hvm.h b/include/xen/hvm.h
index b7fd7fc9ad41..adac232da84d 100644
--- a/include/xen/hvm.h
+++ b/include/xen/hvm.h
@@ -3,6 +3,7 @@
#ifndef XEN_HVM_H__
#define XEN_HVM_H__
+#include <linux/array_size.h>
#include <xen/interface/hvm/params.h>
#include <asm/xen/hypercall.h>
--
2.33.1
On Fri, 19 Nov 2021, Alejandro Colomar <[email protected]> wrote:
> Hi all,
>
> I simplified some xxxof() macros,
> by adding a new macro memberof(),
> which implements a common operation in many of them.
>
> I also splitted many of those macros into tiny headers,
> since I noticed that touching those headers implied
> recompiling almost the whole kernel.
>
> Hopefully after this patch there will be less
> things to recompile after touching one of those.
>
> Having simpler headers means that now one can
> include one of those without pulling too much stuff
> that might break other stuff.
>
> I removed some unnecessary casts too.
>
> Every few commits in this series
> and of course after the last commit
> I rebuilt the kernel and run for a while with it without any problems.
>
> Please note that I have written very few kernel code
> and for example some files wouldn't let me include some of these files,
> so I didn't change those.
>
> What I mean is that,
> even though this is super obvious and shouldn't break stuff,
> and I'm not new to C,
> I'm quite new to the kernel,
> and ask that reviewers take deep look, please.
>
>
> In the first and second commits
> I changed a lot of stuff in many parts,
> and that's why I CCd so many people (also in this cover letter).
> However, to avoid spamming,
> and since it would be a nightmare to
> find all the relevant people affected in so many different areas,
> I only CCd in 01, 02 and in the cover letter.
> If anyone is interested in reading the full patch set,
> I sent it to the LKML.
I think with the patch split you have this would be a nightmare to get
merged. Please consider refactoring the headers first, and once those
are reviewed and merged, you can proceed with using them elsewhere. For
example, we'd want the drm/i915 changes in patches separate from changes
to other drivers or the core headers.
BR,
Jani.
>
>
> Thanks,
> Alex
>
>
> Alejandro Colomar (17):
> linux/container_of.h: Add memberof(T, m)
> Use memberof(T, m) instead of explicit NULL dereference
> Replace some uses of memberof() by its wrappers
> linux/memberof.h: Move memberof() to separate header
> linux/typeof_member.h: Move typeof_member() to a separate header
> Simplify sizeof(typeof_member()) to sizeof_field()
> linux/NULL.h: Move NULL to a separate header
> linux/offsetof.h: Move offsetof(T, m) to a separate header
> linux/offsetof.h: Implement offsetof() in terms of memberof()
> linux/container_of.h: Implement container_of_safe() in terms of
> container_of()
> linux/container_of.h: Cosmetic
> linux/container_of.h: Remove unnecessary cast to (void *)
> linux/sizeof_field.h: Move sizeof_field(T, m) to a separate header
> include/linux/: Include a smaller header if just for NULL
> linux/offsetofend.h: Move offsetofend(T, m) to a separate header
> linux/array_size.h: Move ARRAY_SIZE(arr) to a separate header
> include/: Include <linux/array_size.h> for ARRAY_SIZE()
>
> arch/x86/include/asm/bootparam_utils.h | 3 +-
> arch/x86/kernel/signal_compat.c | 5 ++--
> drivers/gpu/drm/i915/i915_sw_fence.c | 1 +
> drivers/gpu/drm/i915/i915_utils.h | 5 ++--
> drivers/gpu/drm/i915/intel_runtime_pm.h | 3 +-
> drivers/net/ethernet/emulex/benet/be.h | 10 +++----
> drivers/net/ethernet/i825xx/ether1.c | 7 +++--
> drivers/platform/x86/wmi.c | 3 +-
> drivers/scsi/be2iscsi/be.h | 12 ++++----
> drivers/scsi/be2iscsi/be_cmds.h | 5 +++-
> fs/btrfs/ctree.h | 5 ++--
> fs/proc/inode.c | 1 +
> include/acpi/actypes.h | 4 ++-
> include/crypto/internal/blake2b.h | 1 +
> include/crypto/internal/blake2s.h | 1 +
> include/crypto/internal/chacha.h | 1 +
> include/drm/drm_mipi_dbi.h | 1 +
> include/drm/drm_mode_object.h | 1 +
> include/kunit/test.h | 1 +
> include/linux/NULL.h | 10 +++++++
> include/linux/arm_ffa.h | 1 +
> include/linux/array_size.h | 15 ++++++++++
> include/linux/blk_types.h | 1 +
> include/linux/can/core.h | 1 +
> include/linux/clk-provider.h | 1 +
> include/linux/container_of.h | 28 ++++++++++-------
> include/linux/counter.h | 1 +
> include/linux/crash_core.h | 1 +
> include/linux/efi.h | 1 +
> include/linux/extable.h | 2 +-
> include/linux/f2fs_fs.h | 1 +
> include/linux/filter.h | 3 ++
> include/linux/fs.h | 1 +
> include/linux/genl_magic_func.h | 1 +
> include/linux/hashtable.h | 1 +
> include/linux/ieee80211.h | 1 +
> include/linux/kbuild.h | 3 ++
> include/linux/kernel.h | 7 +----
> include/linux/kfifo.h | 1 +
> include/linux/kvm_host.h | 3 ++
> include/linux/libata.h | 1 +
> include/linux/llist.h | 1 +
> include/linux/memberof.h | 11 +++++++
> include/linux/mlx5/device.h | 1 +
> include/linux/mlx5/driver.h | 1 +
> include/linux/mm_types.h | 1 +
> include/linux/moduleparam.h | 3 ++
> include/linux/mtd/rawnand.h | 1 +
> include/linux/netdevice.h | 1 +
> include/linux/netfilter.h | 1 +
> include/linux/nvme-fc.h | 2 ++
> include/linux/offsetof.h | 17 +++++++++++
> include/linux/offsetofend.h | 19 ++++++++++++
> include/linux/pagemap.h | 1 +
> include/linux/phy.h | 1 +
> include/linux/phy_led_triggers.h | 1 +
> include/linux/pinctrl/machine.h | 1 +
> include/linux/property.h | 1 +
> include/linux/rcupdate.h | 1 +
> include/linux/rcupdate_wait.h | 1 +
> include/linux/regmap.h | 1 +
> include/linux/sched/task.h | 1 +
> include/linux/sizeof_field.h | 14 +++++++++
> include/linux/skb_array.h | 1 +
> include/linux/skbuff.h | 1 +
> include/linux/skmsg.h | 3 ++
> include/linux/slab.h | 2 ++
> include/linux/spinlock_types.h | 1 +
> include/linux/stddef.h | 30 +++----------------
> include/linux/string.h | 5 +++-
> include/linux/surface_aggregator/controller.h | 1 +
> include/linux/surface_aggregator/serial_hub.h | 1 +
> include/linux/swap.h | 1 +
> include/linux/ti-emif-sram.h | 1 +
> include/linux/typeof_member.h | 11 +++++++
> include/linux/ucs2_string.h | 2 +-
> include/linux/vdpa.h | 1 +
> include/linux/virtio_config.h | 17 ++++++-----
> include/linux/wireless.h | 2 ++
> include/net/bond_3ad.h | 1 +
> include/net/dsa.h | 1 +
> include/net/ip_vs.h | 1 +
> include/net/netfilter/nf_conntrack_tuple.h | 1 +
> include/net/netfilter/nf_tables.h | 1 +
> include/net/netlink.h | 1 +
> include/rdma/uverbs_ioctl.h | 1 +
> include/rdma/uverbs_named_ioctl.h | 1 +
> include/scsi/scsi_host.h | 1 +
> include/sound/soc-dapm.h | 1 +
> include/sound/soc.h | 1 +
> include/trace/events/wbt.h | 1 +
> include/uapi/linux/netfilter/xt_sctp.h | 1 +
> include/xen/hvm.h | 1 +
> kernel/kallsyms.c | 3 +-
> 94 files changed, 255 insertions(+), 79 deletions(-)
> create mode 100644 include/linux/NULL.h
> create mode 100644 include/linux/array_size.h
> create mode 100644 include/linux/memberof.h
> create mode 100644 include/linux/offsetof.h
> create mode 100644 include/linux/offsetofend.h
> create mode 100644 include/linux/sizeof_field.h
> create mode 100644 include/linux/typeof_member.h
--
Jani Nikula, Intel Open Source Graphics Center
Hi Jani,
On 11/19/21 13:47, Jani Nikula wrote:
> On Fri, 19 Nov 2021, Alejandro Colomar <[email protected]> wrote:
>> In the first and second commits
>> I changed a lot of stuff in many parts,
>> and that's why I CCd so many people (also in this cover letter).
>> However, to avoid spamming,
>> and since it would be a nightmare to
>> find all the relevant people affected in so many different areas,
>> I only CCd in 01, 02 and in the cover letter.
>> If anyone is interested in reading the full patch set,
>> I sent it to the LKML.
>
> I think with the patch split you have this would be a nightmare to get
> merged. Please consider refactoring the headers first, and once those
> are reviewed and merged, you can proceed with using them elsewhere. For
> example, we'd want the drm/i915 changes in patches separate from changes
> to other drivers or the core headers.
So,
would it be preferable something like this?:
Patch set 1:
- Add <linux/memberof.h> with memberof()
- Split offsetof() to <linux/offsetof.h>
- Split offsetofend() to <linux/offsetofend.h>
- Split typeof_member() to <linux/typeof_member.h>
- Split sizeof_field() to <linux/sizeof_field.h>
- Split NULL to <linux/NULL.h>
- Split ARRAY_SIZE() to <linux/array_size.h>
- Implement offsetof() in terms of memberof()
- Implement typeof_member() in terms of memberof()
- Implement sizeof_field() in terms of memberof()
- Implement container_of_safe() in terms of container_of()
- Remove unnecessary cast from container_of[_safe]()
- Cosmetic changes
Patch set 2:
- And in a different patch set, fix all other files
that make use of these macros.
Patch 1 without editing any other files except for the basic ones,
and adding includes where the definition had been previously,
to not break stuff.
And then,
start patching individual subsystems and
send tiny patch sets to each of them?
For the first part,
I agree it is better.
I'll change it to do that.
I'll send an v2 with less changes and more organized.
For the second part,
I'll see what I can do after the first one has been reviewed.
I'll do tiny patches with a few changes to one or few files,
so that I can reorganize them easily with a rebase -i afterwards,
and then decide.
Thanks,
Alex
>
>>
>>
>> Alejandro Colomar (17):
>> linux/container_of.h: Add memberof(T, m)
>> Use memberof(T, m) instead of explicit NULL dereference
>> Replace some uses of memberof() by its wrappers
>> linux/memberof.h: Move memberof() to separate header
>> linux/typeof_member.h: Move typeof_member() to a separate header
>> Simplify sizeof(typeof_member()) to sizeof_field()
>> linux/NULL.h: Move NULL to a separate header
>> linux/offsetof.h: Move offsetof(T, m) to a separate header
>> linux/offsetof.h: Implement offsetof() in terms of memberof()
>> linux/container_of.h: Implement container_of_safe() in terms of
>> container_of()
>> linux/container_of.h: Cosmetic
>> linux/container_of.h: Remove unnecessary cast to (void *)
>> linux/sizeof_field.h: Move sizeof_field(T, m) to a separate header
>> include/linux/: Include a smaller header if just for NULL
>> linux/offsetofend.h: Move offsetofend(T, m) to a separate header
>> linux/array_size.h: Move ARRAY_SIZE(arr) to a separate header
>> include/: Include <linux/array_size.h> for ARRAY_SIZE()
>>
>> arch/x86/include/asm/bootparam_utils.h | 3 +-
>> arch/x86/kernel/signal_compat.c | 5 ++--
>> drivers/gpu/drm/i915/i915_sw_fence.c | 1 +
>> drivers/gpu/drm/i915/i915_utils.h | 5 ++--
>> drivers/gpu/drm/i915/intel_runtime_pm.h | 3 +-
>> drivers/net/ethernet/emulex/benet/be.h | 10 +++----
>> drivers/net/ethernet/i825xx/ether1.c | 7 +++--
>> drivers/platform/x86/wmi.c | 3 +-
>> drivers/scsi/be2iscsi/be.h | 12 ++++----
>> drivers/scsi/be2iscsi/be_cmds.h | 5 +++-
>> fs/btrfs/ctree.h | 5 ++--
>> fs/proc/inode.c | 1 +
>> include/acpi/actypes.h | 4 ++-
>> include/crypto/internal/blake2b.h | 1 +
>> include/crypto/internal/blake2s.h | 1 +
>> include/crypto/internal/chacha.h | 1 +
>> include/drm/drm_mipi_dbi.h | 1 +
>> include/drm/drm_mode_object.h | 1 +
>> include/kunit/test.h | 1 +
>> include/linux/NULL.h | 10 +++++++
>> include/linux/arm_ffa.h | 1 +
>> include/linux/array_size.h | 15 ++++++++++
>> include/linux/blk_types.h | 1 +
>> include/linux/can/core.h | 1 +
>> include/linux/clk-provider.h | 1 +
>> include/linux/container_of.h | 28 ++++++++++-------
>> include/linux/counter.h | 1 +
>> include/linux/crash_core.h | 1 +
>> include/linux/efi.h | 1 +
>> include/linux/extable.h | 2 +-
>> include/linux/f2fs_fs.h | 1 +
>> include/linux/filter.h | 3 ++
>> include/linux/fs.h | 1 +
>> include/linux/genl_magic_func.h | 1 +
>> include/linux/hashtable.h | 1 +
>> include/linux/ieee80211.h | 1 +
>> include/linux/kbuild.h | 3 ++
>> include/linux/kernel.h | 7 +----
>> include/linux/kfifo.h | 1 +
>> include/linux/kvm_host.h | 3 ++
>> include/linux/libata.h | 1 +
>> include/linux/llist.h | 1 +
>> include/linux/memberof.h | 11 +++++++
>> include/linux/mlx5/device.h | 1 +
>> include/linux/mlx5/driver.h | 1 +
>> include/linux/mm_types.h | 1 +
>> include/linux/moduleparam.h | 3 ++
>> include/linux/mtd/rawnand.h | 1 +
>> include/linux/netdevice.h | 1 +
>> include/linux/netfilter.h | 1 +
>> include/linux/nvme-fc.h | 2 ++
>> include/linux/offsetof.h | 17 +++++++++++
>> include/linux/offsetofend.h | 19 ++++++++++++
>> include/linux/pagemap.h | 1 +
>> include/linux/phy.h | 1 +
>> include/linux/phy_led_triggers.h | 1 +
>> include/linux/pinctrl/machine.h | 1 +
>> include/linux/property.h | 1 +
>> include/linux/rcupdate.h | 1 +
>> include/linux/rcupdate_wait.h | 1 +
>> include/linux/regmap.h | 1 +
>> include/linux/sched/task.h | 1 +
>> include/linux/sizeof_field.h | 14 +++++++++
>> include/linux/skb_array.h | 1 +
>> include/linux/skbuff.h | 1 +
>> include/linux/skmsg.h | 3 ++
>> include/linux/slab.h | 2 ++
>> include/linux/spinlock_types.h | 1 +
>> include/linux/stddef.h | 30 +++----------------
>> include/linux/string.h | 5 +++-
>> include/linux/surface_aggregator/controller.h | 1 +
>> include/linux/surface_aggregator/serial_hub.h | 1 +
>> include/linux/swap.h | 1 +
>> include/linux/ti-emif-sram.h | 1 +
>> include/linux/typeof_member.h | 11 +++++++
>> include/linux/ucs2_string.h | 2 +-
>> include/linux/vdpa.h | 1 +
>> include/linux/virtio_config.h | 17 ++++++-----
>> include/linux/wireless.h | 2 ++
>> include/net/bond_3ad.h | 1 +
>> include/net/dsa.h | 1 +
>> include/net/ip_vs.h | 1 +
>> include/net/netfilter/nf_conntrack_tuple.h | 1 +
>> include/net/netfilter/nf_tables.h | 1 +
>> include/net/netlink.h | 1 +
>> include/rdma/uverbs_ioctl.h | 1 +
>> include/rdma/uverbs_named_ioctl.h | 1 +
>> include/scsi/scsi_host.h | 1 +
>> include/sound/soc-dapm.h | 1 +
>> include/sound/soc.h | 1 +
>> include/trace/events/wbt.h | 1 +
>> include/uapi/linux/netfilter/xt_sctp.h | 1 +
>> include/xen/hvm.h | 1 +
>> kernel/kallsyms.c | 3 +-
>> 94 files changed, 255 insertions(+), 79 deletions(-)
>> create mode 100644 include/linux/NULL.h
>> create mode 100644 include/linux/array_size.h
>> create mode 100644 include/linux/memberof.h
>> create mode 100644 include/linux/offsetof.h
>> create mode 100644 include/linux/offsetofend.h
>> create mode 100644 include/linux/sizeof_field.h
>> create mode 100644 include/linux/typeof_member.h
>
--
Alejandro Colomar
Linux man-pages comaintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
On Fri, 19 Nov 2021, "Alejandro Colomar (man-pages)" <[email protected]> wrote:
> Hi Jani,
>
> On 11/19/21 13:47, Jani Nikula wrote:
>> On Fri, 19 Nov 2021, Alejandro Colomar <[email protected]> wrote:
>>> In the first and second commits
>>> I changed a lot of stuff in many parts,
>>> and that's why I CCd so many people (also in this cover letter).
>>> However, to avoid spamming,
>>> and since it would be a nightmare to
>>> find all the relevant people affected in so many different areas,
>>> I only CCd in 01, 02 and in the cover letter.
>>> If anyone is interested in reading the full patch set,
>>> I sent it to the LKML.
>>
>> I think with the patch split you have this would be a nightmare to get
>> merged. Please consider refactoring the headers first, and once those
>> are reviewed and merged, you can proceed with using them elsewhere. For
>> example, we'd want the drm/i915 changes in patches separate from changes
>> to other drivers or the core headers.
>
> So,
> would it be preferable something like this?:
>
> Patch set 1:
> - Add <linux/memberof.h> with memberof()
> - Split offsetof() to <linux/offsetof.h>
> - Split offsetofend() to <linux/offsetofend.h>
> - Split typeof_member() to <linux/typeof_member.h>
> - Split sizeof_field() to <linux/sizeof_field.h>
> - Split NULL to <linux/NULL.h>
> - Split ARRAY_SIZE() to <linux/array_size.h>
> - Implement offsetof() in terms of memberof()
> - Implement typeof_member() in terms of memberof()
> - Implement sizeof_field() in terms of memberof()
> - Implement container_of_safe() in terms of container_of()
> - Remove unnecessary cast from container_of[_safe]()
> - Cosmetic changes
>
> Patch set 2:
> - And in a different patch set, fix all other files
> that make use of these macros.
>
>
> Patch 1 without editing any other files except for the basic ones,
> and adding includes where the definition had been previously,
> to not break stuff.
>
> And then,
> start patching individual subsystems and
> send tiny patch sets to each of them?
>
>
> For the first part,
> I agree it is better.
> I'll change it to do that.
> I'll send an v2 with less changes and more organized.
>
>
> For the second part,
> I'll see what I can do after the first one has been reviewed.
> I'll do tiny patches with a few changes to one or few files,
> so that I can reorganize them easily with a rebase -i afterwards,
> and then decide.
Sounds about right. I presume just the first series is going to generate
quite a bit of discussion, in particular <linux/NULL.h> looks like
everyone's going to have an opinion. And for that, you really don't need
or want all the users (patch series 2) Cc'd.
BR,
Jani.
>
>
> Thanks,
> Alex
>
>>
>>>
>>>
>>> Alejandro Colomar (17):
>>> linux/container_of.h: Add memberof(T, m)
>>> Use memberof(T, m) instead of explicit NULL dereference
>>> Replace some uses of memberof() by its wrappers
>>> linux/memberof.h: Move memberof() to separate header
>>> linux/typeof_member.h: Move typeof_member() to a separate header
>>> Simplify sizeof(typeof_member()) to sizeof_field()
>>> linux/NULL.h: Move NULL to a separate header
>>> linux/offsetof.h: Move offsetof(T, m) to a separate header
>>> linux/offsetof.h: Implement offsetof() in terms of memberof()
>>> linux/container_of.h: Implement container_of_safe() in terms of
>>> container_of()
>>> linux/container_of.h: Cosmetic
>>> linux/container_of.h: Remove unnecessary cast to (void *)
>>> linux/sizeof_field.h: Move sizeof_field(T, m) to a separate header
>>> include/linux/: Include a smaller header if just for NULL
>>> linux/offsetofend.h: Move offsetofend(T, m) to a separate header
>>> linux/array_size.h: Move ARRAY_SIZE(arr) to a separate header
>>> include/: Include <linux/array_size.h> for ARRAY_SIZE()
>>>
>>> arch/x86/include/asm/bootparam_utils.h | 3 +-
>>> arch/x86/kernel/signal_compat.c | 5 ++--
>>> drivers/gpu/drm/i915/i915_sw_fence.c | 1 +
>>> drivers/gpu/drm/i915/i915_utils.h | 5 ++--
>>> drivers/gpu/drm/i915/intel_runtime_pm.h | 3 +-
>>> drivers/net/ethernet/emulex/benet/be.h | 10 +++----
>>> drivers/net/ethernet/i825xx/ether1.c | 7 +++--
>>> drivers/platform/x86/wmi.c | 3 +-
>>> drivers/scsi/be2iscsi/be.h | 12 ++++----
>>> drivers/scsi/be2iscsi/be_cmds.h | 5 +++-
>>> fs/btrfs/ctree.h | 5 ++--
>>> fs/proc/inode.c | 1 +
>>> include/acpi/actypes.h | 4 ++-
>>> include/crypto/internal/blake2b.h | 1 +
>>> include/crypto/internal/blake2s.h | 1 +
>>> include/crypto/internal/chacha.h | 1 +
>>> include/drm/drm_mipi_dbi.h | 1 +
>>> include/drm/drm_mode_object.h | 1 +
>>> include/kunit/test.h | 1 +
>>> include/linux/NULL.h | 10 +++++++
>>> include/linux/arm_ffa.h | 1 +
>>> include/linux/array_size.h | 15 ++++++++++
>>> include/linux/blk_types.h | 1 +
>>> include/linux/can/core.h | 1 +
>>> include/linux/clk-provider.h | 1 +
>>> include/linux/container_of.h | 28 ++++++++++-------
>>> include/linux/counter.h | 1 +
>>> include/linux/crash_core.h | 1 +
>>> include/linux/efi.h | 1 +
>>> include/linux/extable.h | 2 +-
>>> include/linux/f2fs_fs.h | 1 +
>>> include/linux/filter.h | 3 ++
>>> include/linux/fs.h | 1 +
>>> include/linux/genl_magic_func.h | 1 +
>>> include/linux/hashtable.h | 1 +
>>> include/linux/ieee80211.h | 1 +
>>> include/linux/kbuild.h | 3 ++
>>> include/linux/kernel.h | 7 +----
>>> include/linux/kfifo.h | 1 +
>>> include/linux/kvm_host.h | 3 ++
>>> include/linux/libata.h | 1 +
>>> include/linux/llist.h | 1 +
>>> include/linux/memberof.h | 11 +++++++
>>> include/linux/mlx5/device.h | 1 +
>>> include/linux/mlx5/driver.h | 1 +
>>> include/linux/mm_types.h | 1 +
>>> include/linux/moduleparam.h | 3 ++
>>> include/linux/mtd/rawnand.h | 1 +
>>> include/linux/netdevice.h | 1 +
>>> include/linux/netfilter.h | 1 +
>>> include/linux/nvme-fc.h | 2 ++
>>> include/linux/offsetof.h | 17 +++++++++++
>>> include/linux/offsetofend.h | 19 ++++++++++++
>>> include/linux/pagemap.h | 1 +
>>> include/linux/phy.h | 1 +
>>> include/linux/phy_led_triggers.h | 1 +
>>> include/linux/pinctrl/machine.h | 1 +
>>> include/linux/property.h | 1 +
>>> include/linux/rcupdate.h | 1 +
>>> include/linux/rcupdate_wait.h | 1 +
>>> include/linux/regmap.h | 1 +
>>> include/linux/sched/task.h | 1 +
>>> include/linux/sizeof_field.h | 14 +++++++++
>>> include/linux/skb_array.h | 1 +
>>> include/linux/skbuff.h | 1 +
>>> include/linux/skmsg.h | 3 ++
>>> include/linux/slab.h | 2 ++
>>> include/linux/spinlock_types.h | 1 +
>>> include/linux/stddef.h | 30 +++----------------
>>> include/linux/string.h | 5 +++-
>>> include/linux/surface_aggregator/controller.h | 1 +
>>> include/linux/surface_aggregator/serial_hub.h | 1 +
>>> include/linux/swap.h | 1 +
>>> include/linux/ti-emif-sram.h | 1 +
>>> include/linux/typeof_member.h | 11 +++++++
>>> include/linux/ucs2_string.h | 2 +-
>>> include/linux/vdpa.h | 1 +
>>> include/linux/virtio_config.h | 17 ++++++-----
>>> include/linux/wireless.h | 2 ++
>>> include/net/bond_3ad.h | 1 +
>>> include/net/dsa.h | 1 +
>>> include/net/ip_vs.h | 1 +
>>> include/net/netfilter/nf_conntrack_tuple.h | 1 +
>>> include/net/netfilter/nf_tables.h | 1 +
>>> include/net/netlink.h | 1 +
>>> include/rdma/uverbs_ioctl.h | 1 +
>>> include/rdma/uverbs_named_ioctl.h | 1 +
>>> include/scsi/scsi_host.h | 1 +
>>> include/sound/soc-dapm.h | 1 +
>>> include/sound/soc.h | 1 +
>>> include/trace/events/wbt.h | 1 +
>>> include/uapi/linux/netfilter/xt_sctp.h | 1 +
>>> include/xen/hvm.h | 1 +
>>> kernel/kallsyms.c | 3 +-
>>> 94 files changed, 255 insertions(+), 79 deletions(-)
>>> create mode 100644 include/linux/NULL.h
>>> create mode 100644 include/linux/array_size.h
>>> create mode 100644 include/linux/memberof.h
>>> create mode 100644 include/linux/offsetof.h
>>> create mode 100644 include/linux/offsetofend.h
>>> create mode 100644 include/linux/sizeof_field.h
>>> create mode 100644 include/linux/typeof_member.h
>>
--
Jani Nikula, Intel Open Source Graphics Center
On Fri, Nov 19, 2021 at 12:36 PM Alejandro Colomar
<[email protected]> wrote:
>
> Alejandro Colomar (17):
> linux/container_of.h: Add memberof(T, m)
> Use memberof(T, m) instead of explicit NULL dereference
> Replace some uses of memberof() by its wrappers
> linux/memberof.h: Move memberof() to separate header
> linux/typeof_member.h: Move typeof_member() to a separate header
> Simplify sizeof(typeof_member()) to sizeof_field()
> linux/NULL.h: Move NULL to a separate header
> linux/offsetof.h: Move offsetof(T, m) to a separate header
> linux/offsetof.h: Implement offsetof() in terms of memberof()
> linux/container_of.h: Implement container_of_safe() in terms of
> container_of()
> linux/container_of.h: Cosmetic
> linux/container_of.h: Remove unnecessary cast to (void *)
My feeling is that this takes the separation too far: by having this many header
files that end up being included from practically every single .c file
in the kernel,
I think you end up making compile speed worse overall.
If your goal is to avoid having to recompile as much of the kernel
after touching
a header, I think a better approach is to help untangle the dependencies, e.g.
by splitting out type definitions from headers with inline functions (most
indirect header dependencies are on type definitions) and by focusing on
linux/fs.h, linux/sched.h, linux/mm.h and how they interact with the rest of the
headers. At the moment, these are included in most .c files and they in turn
include a ton of other headers.
Arnd
On Fri, Nov 19, 2021 at 02:16:03PM +0100, Alejandro Colomar (man-pages) wrote:
> On 11/19/21 13:47, Jani Nikula wrote:
> > On Fri, 19 Nov 2021, Alejandro Colomar <[email protected]> wrote:
...
> Patch set 1:
> - Add <linux/memberof.h> with memberof()
> - Split offsetof() to <linux/offsetof.h>
> - Split offsetofend() to <linux/offsetofend.h>
> - Split typeof_member() to <linux/typeof_member.h>
> - Split sizeof_field() to <linux/sizeof_field.h>
> - Split NULL to <linux/NULL.h>
> - Split ARRAY_SIZE() to <linux/array_size.h>
Isn't it way too small granularity? I agree on having the separate header
for ARRAY_SIZE() and it was discussed, but the rest...
--
With Best Regards,
Andy Shevchenko
Hi Arnd,
On 11/19/21 15:47, Arnd Bergmann wrote:
> On Fri, Nov 19, 2021 at 12:36 PM Alejandro Colomar
> <[email protected]> wrote:
>>
>> Alejandro Colomar (17):
>> linux/container_of.h: Add memberof(T, m)
>> Use memberof(T, m) instead of explicit NULL dereference
>> Replace some uses of memberof() by its wrappers
>> linux/memberof.h: Move memberof() to separate header
>> linux/typeof_member.h: Move typeof_member() to a separate header
>> Simplify sizeof(typeof_member()) to sizeof_field()
>> linux/NULL.h: Move NULL to a separate header
>> linux/offsetof.h: Move offsetof(T, m) to a separate header
>> linux/offsetof.h: Implement offsetof() in terms of memberof()
>> linux/container_of.h: Implement container_of_safe() in terms of
>> container_of()
>> linux/container_of.h: Cosmetic
>> linux/container_of.h: Remove unnecessary cast to (void *)
>
> My feeling is that this takes the separation too far: by having this many header
> files that end up being included from practically every single .c file
> in the kernel,
> I think you end up making compile speed worse overall.
>
> If your goal is to avoid having to recompile as much of the kernel
> after touching
> a header, I think a better approach is to help untangle the dependencies, e.g.
> by splitting out type definitions from headers with inline functions (most
> indirect header dependencies are on type definitions) and by focusing on
> linux/fs.h, linux/sched.h, linux/mm.h and how they interact with the rest of the
> headers. At the moment, these are included in most .c files and they in turn
> include a ton of other headers.
Yes, I would like to untangle the dependencies.
The main reason I started doing this splitting
is because I wouldn't be able to include
<linux/stddef.h> in some headers,
because it pulled too much stuff that broke unrelated things.
So that's why I started from there.
I for example would like to get NULL in memberof()
without puling anything else,
so <linux/NULL.h> makes sense for that.
It's clear that every .c wants NULL,
but it's not so clear that every .c wants
everything that <linux/stddef.h> pulls indirectly.
But I'll note that linux/fs.h, linux/sched.h, linux/mm.h are
interesting headers for further splitting.
BTW, I also have a longstanding doubt about
how header files are organized in the kernel,
and which headers can and cannot be included
from which other files.
For example I see that files in samples or scripts or tools,
that redefine many things such as offsetof() or ARRAY_SIZE(),
and I don't know if there's a good reason for that,
or if I should simply remove all that stuff and
include <linux/offsetof.h> everywhere I see offsetof() being used.
Thanks,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
> +#define memberof(T, m) (((T *) NULL)->m)
> +
> #define typeof_member(T, m) typeof(((T*)0)->m)
No.
Your macro exists already under different name.
sizeof_member and typeof_member exist, you don't anything more.
Hi Alexey,
On 11/19/21 16:07, Alexey Dobriyan wrote:
>> +#define memberof(T, m) (((T *) NULL)->m)
>> +
>> #define typeof_member(T, m) typeof(((T*)0)->m)
>
> No.
>
> Your macro exists already under different name.
>
> sizeof_member and typeof_member exist, you don't anything more.
>
The macro memberof() exists in one file (or 2)
under the name struct_member(), IIRC.
sizeof_member(), which is actually sizeof_field, IIRC,
and typeof_member(),
make use of this construction,
but they unnecessarily repeat it,
and there are other cases where code wants exactly memberof(),
not any of its wrappers.
I think that macro makes a lot of sense.
If you prefer the name struct_member() instead of memberof(),
I'm fine with it.
I just found that name much later than writing memberof(),
and memberof() was more consistent
with the typical naming of xxxof() for similar macros.
Thanks,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
On Fri, Nov 19, 2021 at 04:06:27PM +0100, Alejandro Colomar (man-pages) wrote:
> Hi Arnd,
>
> On 11/19/21 15:47, Arnd Bergmann wrote:
> > On Fri, Nov 19, 2021 at 12:36 PM Alejandro Colomar
> > <[email protected]> wrote:
> >>
> >> Alejandro Colomar (17):
> >> linux/container_of.h: Add memberof(T, m)
> >> Use memberof(T, m) instead of explicit NULL dereference
> >> Replace some uses of memberof() by its wrappers
> >> linux/memberof.h: Move memberof() to separate header
> >> linux/typeof_member.h: Move typeof_member() to a separate header
> >> Simplify sizeof(typeof_member()) to sizeof_field()
> >> linux/NULL.h: Move NULL to a separate header
> >> linux/offsetof.h: Move offsetof(T, m) to a separate header
> >> linux/offsetof.h: Implement offsetof() in terms of memberof()
> >> linux/container_of.h: Implement container_of_safe() in terms of
> >> container_of()
> >> linux/container_of.h: Cosmetic
> >> linux/container_of.h: Remove unnecessary cast to (void *)
> >
> > My feeling is that this takes the separation too far: by having this many header
> > files that end up being included from practically every single .c file
> > in the kernel,
> > I think you end up making compile speed worse overall.
> >
> > If your goal is to avoid having to recompile as much of the kernel
> > after touching
> > a header, I think a better approach is to help untangle the dependencies, e.g.
> > by splitting out type definitions from headers with inline functions (most
> > indirect header dependencies are on type definitions) and by focusing on
> > linux/fs.h, linux/sched.h, linux/mm.h and how they interact with the rest of the
> > headers. At the moment, these are included in most .c files and they in turn
> > include a ton of other headers.
>
> Yes, I would like to untangle the dependencies.
>
> The main reason I started doing this splitting
> is because I wouldn't be able to include
> <linux/stddef.h> in some headers,
> because it pulled too much stuff that broke unrelated things.
>
> So that's why I started from there.
>
> I for example would like to get NULL in memberof()
> without puling anything else,
> so <linux/NULL.h> makes sense for that.
I don't believe that the code that uses NULL won't include types.h.
--
With Best Regards,
Andy Shevchenko
Hi Andy,
On 11/19/21 16:34, Andy Shevchenko wrote:
> On Fri, Nov 19, 2021 at 04:06:27PM +0100, Alejandro Colomar (man-pages) wrote:
>> Yes, I would like to untangle the dependencies.
>>
>> The main reason I started doing this splitting
>> is because I wouldn't be able to include
>> <linux/stddef.h> in some headers,
>> because it pulled too much stuff that broke unrelated things.
>>
>> So that's why I started from there.
>>
>> I for example would like to get NULL in memberof()
>> without puling anything else,
>> so <linux/NULL.h> makes sense for that.
>
> I don't believe that the code that uses NULL won't include types.h.
I'm not sure about the error I got (I didn't write it down),
but I got a compilation error.
That's why I split NULL.
If one could anwer my doubt,
I would be in better position to learn how to avoid them.
See below.
On 11/19/21 16:06, Alejandro Colomar (man-pages) wrote:
> BTW, I also have a longstanding doubt about
> how header files are organized in the kernel,
> and which headers can and cannot be included
> from which other files.
>
> For example I see that files in samples or scripts or tools,
> that redefine many things such as offsetof() or ARRAY_SIZE(),
> and I don't know if there's a good reason for that,
> or if I should simply remove all that stuff and
> include <linux/offsetof.h> everywhere I see offsetof() being used.
Thanks,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
[trimmed CC]
On 11/19/21 16:38, Alejandro Colomar (man-pages) wrote:
> On 11/19/21 16:34, Andy Shevchenko wrote:
>> On Fri, Nov 19, 2021 at 04:06:27PM +0100, Alejandro Colomar (man-pages) wrote:
>>> Yes, I would like to untangle the dependencies.
>>>
>>> The main reason I started doing this splitting
>>> is because I wouldn't be able to include
>>> <linux/stddef.h> in some headers,
>>> because it pulled too much stuff that broke unrelated things.
>>>
>>> So that's why I started from there.
>>>
>>> I for example would like to get NULL in memberof()
>>> without puling anything else,
>>> so <linux/NULL.h> makes sense for that.
>>
>> I don't believe that the code that uses NULL won't include types.h.
>
> I'm not sure about the error I got (I didn't write it down),
> but I got a compilation error.
> That's why I split NULL.
Now that I think about it twice,
since I'm rewriting these changes from scratch,
I think the error might have been
not due to pulling too much stuff,
but due to circular dependencies.
Having more granularity
helps precisely define the dependencies.
I think the problem was in
<linux/memberof.h> requiring NULL from <linux/stddef.h>
<linux/stddef.h> requiring memberof() from <linux/memberof.h>
or something like that.
Regards,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
On Fri, Nov 19, 2021 at 04:43:04PM +0100, Alejandro Colomar (man-pages) wrote:
> On 11/19/21 16:38, Alejandro Colomar (man-pages) wrote:
> > On 11/19/21 16:34, Andy Shevchenko wrote:
> >> On Fri, Nov 19, 2021 at 04:06:27PM +0100, Alejandro Colomar (man-pages) wrote:
> >>> Yes, I would like to untangle the dependencies.
> >>>
> >>> The main reason I started doing this splitting
> >>> is because I wouldn't be able to include
> >>> <linux/stddef.h> in some headers,
> >>> because it pulled too much stuff that broke unrelated things.
> >>>
> >>> So that's why I started from there.
> >>>
> >>> I for example would like to get NULL in memberof()
> >>> without puling anything else,
> >>> so <linux/NULL.h> makes sense for that.
> >>
> >> I don't believe that the code that uses NULL won't include types.h.
> >
> > I'm not sure about the error I got (I didn't write it down),
> > but I got a compilation error.
> > That's why I split NULL.
>
> Now that I think about it twice,
> since I'm rewriting these changes from scratch,
> I think the error might have been
> not due to pulling too much stuff,
> but due to circular dependencies.
>
> Having more granularity
> helps precisely define the dependencies.
>
> I think the problem was in
> <linux/memberof.h> requiring NULL from <linux/stddef.h>
> <linux/stddef.h> requiring memberof() from <linux/memberof.h>
> or something like that.
There is no memberof.h in the kernel.?Something is done wrongly on your series.
--
With Best Regards,
Andy Shevchenko
On 11/19/21 16:49, Andy Shevchenko wrote:
>>
>> I think the problem was in
>> <linux/memberof.h> requiring NULL from <linux/stddef.h>
>> <linux/stddef.h> requiring memberof() from <linux/memberof.h>
>> or something like that.
>
> There is no memberof.h in the kernel. Something is done wrongly on your series.
memberof.h was my first addition in this patch series.
Since I replaced (((T *)0)->m) by memberof(),
and that construction is used in <linux/stddef.h>
for example for sizeof_field(),
I included <linux/memberof.h> from <linux/stddef.h>.
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
On Fri, Nov 19, 2021 at 4:06 PM Alejandro Colomar (man-pages)
<[email protected]> wrote:
> On 11/19/21 15:47, Arnd Bergmann wrote:
> > On Fri, Nov 19, 2021 at 12:36 PM Alejandro Colomar
>
> Yes, I would like to untangle the dependencies.
>
> The main reason I started doing this splitting
> is because I wouldn't be able to include
> <linux/stddef.h> in some headers,
> because it pulled too much stuff that broke unrelated things.
>
> So that's why I started from there.
>
> I for example would like to get NULL in memberof()
> without puling anything else,
> so <linux/NULL.h> makes sense for that.
>
> It's clear that every .c wants NULL,
> but it's not so clear that every .c wants
> everything that <linux/stddef.h> pulls indirectly.
From what I can tell, linux/stddef.h is tiny, I don't think it's really
worth optimizing this part. I have spent some time last year
trying to untangle some of the more interesting headers, but ended
up not completing this as there are some really hard problems
once you start getting to the interesting bits.
The approach I tried was roughly:
- For each header in the kernel, create a preprocessed version
that includes all the indirect includes, from that start a set
of lookup tables that record which header is eventually included
by which ones, and the size of each preprocessed header in
bytes
- For a given kernel configuration (e.g. defconfig or allmodconfig)
that I'm most interested in, look at which files are built, and what
the direct includes are in the source files.
- Sort the headers by the product of the number of direct includes
and the preprocessed size: the largest ones are those that are
worth looking at first.
- use graphviz to visualize the directed graph showing the includes
between the top 100 headers in that list. You get something like
I had in [1], or the version afterwards at [2].
- split out unneeded indirect includes from the headers in the center
of that graph, typically by splitting out struct definitions.
- repeat.
The main problem with this approach is that as soon as you start
actually reducing the unneeded indirect includes, you end up with
countless .c files that no longer build because they are missing a
direct include for something that was always included somewhere
deep underneath, so I needed a second set of scripts to add
direct includes to every .c file.
On the plus side, I did see something on the order of a 30%
compile speed improvement with clang, which is insane
given that this only removed dead definitions.
> But I'll note that linux/fs.h, linux/sched.h, linux/mm.h are
> interesting headers for further splitting.
>
>
> BTW, I also have a longstanding doubt about
> how header files are organized in the kernel,
> and which headers can and cannot be included
> from which other files.
>
> For example I see that files in samples or scripts or tools,
> that redefine many things such as offsetof() or ARRAY_SIZE(),
> and I don't know if there's a good reason for that,
> or if I should simply remove all that stuff and
> include <linux/offsetof.h> everywhere I see offsetof() being used.
The main issue here is that user space code should not
include anything outside of include/uapi/ and arch/*/include/uapi/
offsetof() is defined in include/linux/stddef.h, so this is by
definition not accessible here. It appears that there is also
an include/uapi/linux/stddef.h that is really strange because
it includes linux/compiler_types.h, which in turn is outside
of uapi/. This should probably be fixed.
Arnd
[1] https://drive.google.com/file/d/14IKifYDadg2W5fMsefxr4373jizo9bLl/view?usp=sharing
[2] https://drive.google.com/file/d/1pWQcv3_ZXGqZB8ogV-JOfoV-WJN2UNnd/view?usp=sharing
On Fri, Nov 19, 2021 at 04:52:22PM +0100, Alejandro Colomar (man-pages) wrote:
> On 11/19/21 16:49, Andy Shevchenko wrote:
> >>
> >> I think the problem was in
> >> <linux/memberof.h> requiring NULL from <linux/stddef.h>
> >> <linux/stddef.h> requiring memberof() from <linux/memberof.h>
> >> or something like that.
> >
> > There is no memberof.h in the kernel.?Something is done wrongly on your series.
>
> memberof.h was my first addition in this patch series.
>
> Since I replaced (((T *)0)->m) by memberof(),
> and that construction is used in <linux/stddef.h>
> for example for sizeof_field(),
> I included <linux/memberof.h> from <linux/stddef.h>.
Then why you need that header to be separated? What circular dependency
it resolves? In case you are wondering about my activity in the area,
the problematic piece is that headers including headers which includes
the first headers again.
Try to inline bitmap_alloc() and you will get what I'm struggling with.
The biggest (as far as I see now) part of it is the mess called "kernel.h"
that is included by other headers. And my idea to get rid of that.
Btw, Arnd gave actually a good suggestion what to look at.
--
With Best Regards,
Andy Shevchenko
On Fri, Nov 19, 2021 at 04:57:46PM +0100, Arnd Bergmann wrote:
> On Fri, Nov 19, 2021 at 4:06 PM Alejandro Colomar (man-pages)
> <[email protected]> wrote:
> > On 11/19/21 15:47, Arnd Bergmann wrote:
> > > On Fri, Nov 19, 2021 at 12:36 PM Alejandro Colomar
> >
> > Yes, I would like to untangle the dependencies.
> >
> > The main reason I started doing this splitting
> > is because I wouldn't be able to include
> > <linux/stddef.h> in some headers,
> > because it pulled too much stuff that broke unrelated things.
> >
> > So that's why I started from there.
> >
> > I for example would like to get NULL in memberof()
> > without puling anything else,
> > so <linux/NULL.h> makes sense for that.
> >
> > It's clear that every .c wants NULL,
> > but it's not so clear that every .c wants
> > everything that <linux/stddef.h> pulls indirectly.
>
> From what I can tell, linux/stddef.h is tiny, I don't think it's really
> worth optimizing this part. I have spent some time last year
> trying to untangle some of the more interesting headers, but ended
> up not completing this as there are some really hard problems
> once you start getting to the interesting bits.
>
> The approach I tried was roughly:
>
> - For each header in the kernel, create a preprocessed version
> that includes all the indirect includes, from that start a set
> of lookup tables that record which header is eventually included
> by which ones, and the size of each preprocessed header in
> bytes
>
> - For a given kernel configuration (e.g. defconfig or allmodconfig)
> that I'm most interested in, look at which files are built, and what
> the direct includes are in the source files.
>
> - Sort the headers by the product of the number of direct includes
> and the preprocessed size: the largest ones are those that are
> worth looking at first.
>
> - use graphviz to visualize the directed graph showing the includes
> between the top 100 headers in that list. You get something like
> I had in [1], or the version afterwards at [2].
>
> - split out unneeded indirect includes from the headers in the center
> of that graph, typically by splitting out struct definitions.
>
> - repeat.
>
> The main problem with this approach is that as soon as you start
> actually reducing the unneeded indirect includes, you end up with
> countless .c files that no longer build because they are missing a
> direct include for something that was always included somewhere
> deep underneath, so I needed a second set of scripts to add
> direct includes to every .c file.
Can't it be done with cocci support?
> On the plus side, I did see something on the order of a 30%
> compile speed improvement with clang, which is insane
> given that this only removed dead definitions.
Thumb up!
> > But I'll note that linux/fs.h, linux/sched.h, linux/mm.h are
> > interesting headers for further splitting.
> >
> >
> > BTW, I also have a longstanding doubt about
> > how header files are organized in the kernel,
> > and which headers can and cannot be included
> > from which other files.
> >
> > For example I see that files in samples or scripts or tools,
> > that redefine many things such as offsetof() or ARRAY_SIZE(),
> > and I don't know if there's a good reason for that,
> > or if I should simply remove all that stuff and
> > include <linux/offsetof.h> everywhere I see offsetof() being used.
>
> The main issue here is that user space code should not
> include anything outside of include/uapi/ and arch/*/include/uapi/
>
> offsetof() is defined in include/linux/stddef.h, so this is by
> definition not accessible here. It appears that there is also
> an include/uapi/linux/stddef.h that is really strange because
> it includes linux/compiler_types.h, which in turn is outside
> of uapi/. This should probably be fixed.
>
> Arnd
>
> [1] https://drive.google.com/file/d/14IKifYDadg2W5fMsefxr4373jizo9bLl/view?usp=sharing
> [2] https://drive.google.com/file/d/1pWQcv3_ZXGqZB8ogV-JOfoV-WJN2UNnd/view?usp=sharing
--
With Best Regards,
Andy Shevchenko
Hi Arnd,
On 11/19/21 16:57, Arnd Bergmann wrote:
>
> From what I can tell, linux/stddef.h is tiny, I don't think it's really
> worth optimizing this part. I have spent some time last year
> trying to untangle some of the more interesting headers, but ended
> up not completing this as there are some really hard problems
> once you start getting to the interesting bits.
In this case it was not about being worth it or not,
but that the fact that adding memberof() would break,
unless I use 0 instead of NULL for the implementation of memberof(),
which I'm against,
or I split stddef.
If I don't do either of those,
I'm creating a circular dependency,
and it doesn't compile.
>
> The approach I tried was roughly:
>
> - For each header in the kernel, create a preprocessed version
> that includes all the indirect includes, from that start a set
> of lookup tables that record which header is eventually included
> by which ones, and the size of each preprocessed header in
> bytes
>
> - For a given kernel configuration (e.g. defconfig or allmodconfig)
> that I'm most interested in, look at which files are built, and what
> the direct includes are in the source files.
>
> - Sort the headers by the product of the number of direct includes
> and the preprocessed size: the largest ones are those that are
> worth looking at first.
>
> - use graphviz to visualize the directed graph showing the includes
> between the top 100 headers in that list. You get something like
> I had in [1], or the version afterwards at [2].
>
> - split out unneeded indirect includes from the headers in the center
> of that graph, typically by splitting out struct definitions.
>
> - repeat.
>
> The main problem with this approach is that as soon as you start
> actually reducing the unneeded indirect includes, you end up with
> countless .c files that no longer build because they are missing a
> direct include for something that was always included somewhere
> deep underneath, so I needed a second set of scripts to add
> direct includes to every .c file.
>
> On the plus side, I did see something on the order of a 30%
> compile speed improvement with clang, which is insane
> given that this only removed dead definitions.
Huh!
I'd like to see the kernel some day
not having _any_ hidden dependencies.
For the moment,
since my intent is familiarizing with kernel programming,
and not necessarily improving performance considerably
(at least not in the first rounds of changes),
I prefer starting where it more directly affects
what I initially intended to change in the kernel,
which in this case was adding memberof().
>
>> But I'll note that linux/fs.h, linux/sched.h, linux/mm.h are
>> interesting headers for further splitting.
>>
>>
>> BTW, I also have a longstanding doubt about
>> how header files are organized in the kernel,
>> and which headers can and cannot be included
>> from which other files.
>>
>> For example I see that files in samples or scripts or tools,
>> that redefine many things such as offsetof() or ARRAY_SIZE(),
>> and I don't know if there's a good reason for that,
>> or if I should simply remove all that stuff and
>> include <linux/offsetof.h> everywhere I see offsetof() being used.
>
> The main issue here is that user space code should not
> include anything outside of include/uapi/ and arch/*/include/uapi/
Okay. That's good to know.
So everything can use uapi code,
and uapi code can only use uapi code,
right?
Every duplicate definition of something outside of uapi
should/could be removed.
>
> offsetof() is defined in include/linux/stddef.h, so this is by
> definition not accessible here. It appears that there is also
> an include/uapi/linux/stddef.h that is really strange because
> it includes linux/compiler_types.h, which in turn is outside
> of uapi/. This should probably be fixed.
I see.
Then,
perhaps it would be better to define offsetof() _only_ inside uapi/,
and use that definition from everywhere else,
and therefore remove the non-uapi version,
right?
Thanks,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
On Fri, Nov 19, 2021 at 5:10 PM Andy Shevchenko
<[email protected]> wrote:
> On Fri, Nov 19, 2021 at 04:57:46PM +0100, Arnd Bergmann wrote:
> > The main problem with this approach is that as soon as you start
> > actually reducing the unneeded indirect includes, you end up with
> > countless .c files that no longer build because they are missing a
> > direct include for something that was always included somewhere
> > deep underneath, so I needed a second set of scripts to add
> > direct includes to every .c file.
>
> Can't it be done with cocci support?
There are many ways of doing it, but they all tend to suffer from the
problem of identifying which headers are actually needed based on
the contents of a file, and also figuring out where to put the extra
#include if there are complex #ifdefs.
For reference, see below for the naive pattern matching I tried.
This is obviously incomplete and partially wrong.
Arnd
---
#!/bin/bash
GITARGS="$@"
declare HEADER
declare SEARCH
declare FILES
declare OTHERHEADERS
# insert <foo/baz.h> alphabetically after the nearest <foo/bar.h>
insertafter() {
MYFILES=`git grep -l "#include.*<${HEADER%/*}/.*>" $FILES`
if [ -z "$MYFILES" ] ; then return ; fi
MYFILES=`grep -wL $HEADER $FILES`
if [ -z "$MYFILES" ] ; then return ; fi
echo after $HEADER $MYFILES
echo $OTHERHEADERS | tr '[:space:]' '\n' | sort -ru | grep
^${HEADER%/*} | grep -A10000 ^$HEADER | grep -v ^$HEADER | tr / . |
while read i ; do
if [ -z "$MYFILES" ] ; then return ; fi
echo AFTER $i
echo $MYFILES | xargs sed -i '/include.*<'$i'>/ a #include '"<$HEADER>"
MYFILES=`grep -wL $HEADER $MYFILES`
done
}
# insert <foo/bar.h> alphabetically after the nearest <foo/baz.h>
insertbefore() {
MYFILES=`git grep -l "#include.*<${HEADER%/*}/.*>" $FILES`
if [ -z "$MYFILES" ] ; then return ; fi
MYFILES=`grep -wL $HEADER $FILES`
if [ -z "$MYFILES" ] ; then return ; fi
echo before $HEADER $MYFILES
echo $OTHERHEADERS | tr '[:space:]' '\n' | sort -u | grep
^${HEADER%/*} | grep -A10000 ^$HEADER | grep -v ^$HEADER | tr / . |
while read i ; do
if [ -z "$MYFILES" ] ; then return ; fi
echo BEFORE $i
echo $MYFILES | xargs sed -i '/include.*<'$i'>/ i #include '"<$HEADER>"
MYFILES=`grep -wL $HEADER $MYFILES`
done
}
# insert <foo/bar.h> before first <qux/quux.h>
insertcategory() {
MYFILES=`git grep -l "#include.*<.*/.*>" $FILES`
if [ -z "$MYFILES" ] ; then return ; fi
MYFILES=`grep -wL $HEADER $FILES`
if [ -z "$MYFILES" ] ; then return ; fi
for f in $MYFILES ; do
sed -i -e "/^#include.*<.*\/.*>/ { i #include <$HEADER>" -e " ; :L; n;
bL; } " $f
done
}
insertafterlocal() {
MYFILES=`git grep -l "#include.*\".*\"" $FILES`
if [ -z "$MYFILES" ] ; then return ; fi
MYFILES=`grep -wL $HEADER $FILES`
if [ -z "$MYFILES" ] ; then return ; fi
for f in $MYFILES ; do
sed -i -e "/^#include.*\".*\/.*\"/ { a #include <$HEADER>" -e " ; :L;
n; bL; } " $f
done
}
x() {
HEADER="$1"
SEARCH="$2"
echo $HEADER
FILES=`git grep -wl "$SEARCH" | grep -v
"^\(Documentation\|include\|tools\|arch/.*/include\|arch/.*/boot/dts\|scripts\|samples\|arch/*/\(kernel/\|vdso\)\|lib/vdso\)\|classmap.h$"
| grep -v "\.S\>"`
if [ -z "$FILES" ] ; then return ; fi
FILES=`echo $FILES | xargs grep $HEADER -L `
if [ -z "$FILES" ] ; then return ; fi
OTHERHEADERS=`echo $FILES | xargs grep -h "include.*\<.*/.*\>" | cut
-f 2 -d\< | cut -f 1 -d \> | grep ^[-_a-zA-Z0-9]*/ ; echo $HEADER`
insertafter
insertbefore
insertcategory
# insertafterlocal
}
# error: implicit declaration of function 'skb_tunnel_rx'
[-Werror,-Wimplicit-function-declaration]
#x linux/debug_locks.h "\(__\|\)debug_locks_off"
#x linux/stat.h
"S_I\(R\|W\|X\)\(USR\|GRP\|OTH\|UGO\)\|S_IRWX\(UGO\|U\|G\|O\)\|S_IALLUGO"
#x linux/stat.h "[A-Z0-9_]*ATTR_\(RW\|RO\|WO\)"
#x linux/dev_printk.h "dev_\(debug\|info\|warn\|err\|notice\|alert\)"
x net/scheduler.h "dev_init_scheduler\|dev_activate"
x linux/sysfs.h
"sysfs_\(create\|remove\|update\|merge\|add_file_to\)_\(\(bin_\|\)file\|file_self\|group\|link\|mount_point\)\(\|s\|_ns\|\)\|sysfs_\(ops\|notify\|chmod_file\|get_dirent\|notify_dirent\|put\)"
x linux/kref.h "kref_get_unless_zero\|kref_get\|kref_init\|kref_read\|kref_put"
x linux/skb_alloc.h
"\(dev\|netdev\|__dev\|__netdev\)_alloc_skb\(\|_ip_align\)\|skb_frag_must_loop\|skb_fill_page_desc\|skb_queue_purge\|skb_rbtree_purge\|netdev_alloc_frag\|skb_frag_address_safe"
x linux/mutex.h
"DEFINE_MUTEX\|\(device\|mutex\|tty\)_\(lock\|unlock\|is_locked\|is_writelocked\)\|usb_\(un\|\)lock_device\|BLOCKING_INIT_NOTIFIER_HEAD\|mutex_init"
x linux/page.h "PAGE_ALIGN\|PAGE_ALIGNED\|dma_map_single\|offset_in_page\|page_address\|virt_to_head_page"
x linux/pgtable.h
"PAGE_\(KERNEL_EXEC\|KERNEL_RO\|KERNEL\|READONLY\)\|pgprot_\(writecombine\|device\|noncached\)"
x linux/if_vlan.h "is_vlan_dev\|vlan_dev_vlan_id\|vlan_get_protocol"
x linux/skb_alloc.h
"skb_frag_must_loop\|\(__\|\)skb_\(fill_page_desc\|queue_purge\|frag_\(address\|ref\|unref_\)\)\|\(__\|\)\(dev\|netdev\|napi\)_alloc_\(page\|pages\|skb\)\|napi_consume_skb\|skb_free_frag"
x linux/gfp.h "__page_frag_cache_drain\|page_frag_alloc"
x linux/skbuff.h "skb_\(copy\|get\|put\)\|skb_queue_[a-z]*"
x linux/iopoll.h
"\(phy_read_mmd\|phy_read\|regmap_field_read\|regmap_read\|read[bwl]\)_poll_timeout\(_atomic\|\)"
x linux/in.h "IPPROTO_\(IP\|TCP\|UDP\|SCTP\)\|struct\ sockaddr_in"
x net/sock.h "\(RCV\|SEND\)_SHUTDOWN\|sk_pacing_shift_update\|sk_capable\|sk_user_ns\|sock_queue_err_skb"
x net/sock.h "SOCKET_I\|\(bh_\|\)\(un\|\)lock_sock\(_nested\|\|_fast\)\|skb_set_owner_w\|skb_orphan_partial"
x linux/bitops.h
"\(__\|\)\(assign\|set\|clear\|test\)_bit\|sign_extend32\|sign_extend64\|for_each_\(set\|clear\)_bit\|get_count_order"
x linux/fs.h "\(un\|\)register_chrdev\(_region\|\)\|compat_ptr_ioctl\|file_clone_open\|filp_open\|filp_close"
x linux/mem_encrypt.h "mem_encrypt_active"
x linux/backlight.h "struct\
\(backlight_ops\|backlight_properties\|backlight_device\)\|backlight_\(enable\|disable\)"
x linux/sched.h "task_pid\(_nr\|\)"
x linux/fb.h "KHZ2PICOS\|PICOS2KHZ\|fb_get_options"
x linux/fcntl.h "O_CLOEXEC\|O_NONBLOCK"
x linux/dma-mapping.h
"\(dma\|dmam\)_\(\(alloc\|free\)_\(coherent\|attrs\)\|\(un\|\)map_\(single\|sg\|page\)\(\_attrs\|\)\)\|dma_set_mask\(_and_coherent\)\|DMA_\(BIT_MASK\|TO_DEVICE\|FROM_DEVICE\|BIDIRECTIONAL\)\|dma_set_max_seg_\(size\|boundary\)\|dma_debug_add_bus"
x linux/file_operations.h
"\(noop\|default\)_llseek\|\(static\|extern\|const\).*struct\
file_operations\|DEFINE_\(SHOW\|SEQ\|DEBUGFS\)_ATTRIBUTE\|iminor\|imajor\|DEFINE_\(SIMPLE\|DEBUGFS\)_ATTRIBUTE\|DEFINE_DRM_[A-Z_]*_FOPS"
x linux/file_operations.h
"\(file\|vm_file\|filp\)->\(private_data\|f_mapping\|f_cred\)\|file_inode"
x linux/bitmap.h
"\(__\|\)bitmap_\(fill\|copy\|zero\|alloc\|zalloc\|and\|or\|xor\|set\|weight\|clear\)"
x linux/mm_types.h "mm->mmap_lock"
x linux/wait.h "\(wait_event\|wake_up\)\(_interruptible\|_killable\|\)\|init_waitqueue_head"
x linux/workqueue.h "DEFINE_STATIC_SRCU\|DEFINE_SRCU\|INIT_WORK"
x linux/timex.h "get_cycles\|random_get_entropy\|struct\
__kernel_timex\|do_adjtimex\|shift_right"
x linux/lockdep.h
"lockdep_assert_irqs_disabled\|lockdep_init_map[_a-z]*\|INIT_DELAYED_WORK"
x linux/kobject_ns.h "kobj_ns_\(drop\|ops\|grab_current\)"
x linux/sched/signal.h "rlimit\(.*\)"
x linux/wait_bit.h
"wake_up_bit\|wait_on_bit\(_timeout\|_io\|_lock\|_lock_io\|_action\|\)\|wait_var_event[a-z_]*"
x linux/io.h "\(__raw_\|\)\(in\|out\|read\|write\)\(b\|w\|l\|q\)\(_p\|_relaxed\|\)\|\(ioread\|iowrite\)\(8\|16\|32\)\(\|_be\)\|wr32\|memset_io"
x linux/mm.h "\(get\|pin\)_user_pages\(_fast\|_locked\|_remote\|unlocked\|\)"
x linux/interrupt.h
"\(free_\|request_\|devm_request_\|enable_\|disable_\)\(threaded_\|\)irq"
x linux/completion.h
"\(re\|\)init_completion\|wait_for_completion\([a-z_]*\)\|struct\
completion\|complete(.*)\|DECLARE_COMPLETION_ONSTACK"
x linux/scatterlist.h
"sg_copy_to_buffer\|sg_nents_for_len\|sg_set_buf\|sg_chain\|sg_pcopy_to_buffer\|for_each_sg\|sg_next\|sg_init_table\|sg_virt\|sg_page\|sg_alloc_table\|sg_set_page\|sg_init_one\|sg_zero_buffer\|sg_dma_page_iter\|for_each_sg_dma_page\|sg_free_table"
x linux/hash.h "hash\(32_ptr\|ptr\|\(_32\|_64\)\(\|_generic\)\)"
x linux/bitmap.h "DECLARE_BITMAP\|bitmap_\(find\|allocate\|release\)_region"
x linux/rwsem.h
"\(up\|down\|downgrade\)_\(read\|write\)\(_killable\|\)\|BLOCKING_INIT_NOTIFIER_HEAD"
x linux/smp.h "get_cpu\|put_cpu\|smp_processor_id"
x linux/if_ether.h "sysfs_format_mac"
x linux/pgtable.h "swapper_pg_dir\|ZERO_PAGE"
x linux/jiffies.h
"jiffies\|get_jiffies_64\|SHIFT_HZ\|jiffies_to_.secs\|preset_lpj\|.secs_to_jiffies"
x linux/page.h "PAGE_ALIGN\|get_page\|put_page\|unpin_user_page\|page_maybe_dma_pinned\|page_to_nid\|page_zone\|page_zonenum\|unpin_user_pages_dirty_lock"
x linux/page.h "PageHighMem"
x linux/pid.h "pid_task\|get_pid\|find_vpid\|find_pid_ns\|put_pid"
x linux/sched.h
"\(wait_event\|wake_up\|wake_up_all\)\(_interruptible\|_killable\|_process\|_timeout\|\)\|current_cred\|kthread_run\|current_euid\|current_user_ns\|lockdep_assert_irqs_disabled"
x linux/of.h "of_parse_phandle[a-z_]*\|of_node->phandle"
x linux/sched/prio.h "MAX_RT_PRIO"
x linux/sched.h "SCHED_FIFO"
x linux/kdev_t.h "\(MINOR\|MAJOR\)(.*)"
x linux/idr.h "\(ida\|idr\)_\(alloc\|simple_get\|find\|for_each[a-z_]*\|destroy\|get_next\|get_cursor\)"
x linux/sched/signal.h "signal_pending"
x linux/math64.h
"\(div_\|div64_\)\(s64\|u64\)\(_rem\|\)\|div64_long\|div64_ul\|DIV64_U64_ROUND_\(UP\|CLOSEST\)\|DIV_ROUND_CLOSEST_ULL\|mul_u64_u32_div"
x linux/eventfd.h "eventfd_\(signal\|ctx_fdget\|ctx_put\)"
x linux/mm.h "find_vma\(_prev\|\)\|put_vaddr_frames\|frame_vector_pages\|frame_vector_to_pages\|frame_vector_destroy\|set_page_dirty\(_lock\|\)\|try_to_release_page\|write_one_page"
x linux/fs.h "AOP_TRUNCATED_PAGE\|AOP_WRITEPAGE_ACTIVATE\|AOP_FLAG_CONT_EXPAND"
x linux/vmalloc.h
"\(vm\|kv\|kvz\)\(free\|alloc\)\|vmalloc_to_page\|is_vmalloc_addr\|dma_map_single\(_attrs\|\)"
x linux/memory.h "high_memory\|virt_addr_valid"
x linux/percpu.h "DEFINE_PER_CPU[A-Z_]*\|per_cpu_ptr"
x linux/percpu-rwsem.h
"DEFINE_PER_CPU[A-Z_]*\|per_cpu_ptr\|percpu_\(down\|up\)_\(read\|write\)\(\|_trylock\)\|percpu_init_rwsem"
x linux/errno.h "EPROBE_DEFER\|EINVAL\|ENOMEM"
x linux/mmap_lock.h
"mmap_\(read\|write\|init\)_\(un\|try\|\)lock\(_non_owner\|_killable\|\)"
x net/net_namespace.h "get_net\|maybe_get_net\|put_net\|check_net\|net_eq"
x linux/nsproxy.h "current->nsproxy\|get_nsproxy\|put_nsproxy"
x linux/sched/task.h "\(put\|get\)_task_struct"
x linux/seq_file.h "seq_open\|single_open\|seq_printf\|seq->private"
x linux/page-flags.h "PG_\(buddy\|lru\|slab\|private\|swapcache\|swapmasked\)"
x linux/crash_core.h "VMCOREINFO_[A-Z]*"
x linux/nodemask.h "\(for_each\|first\)_\(memory_\|online_\|\)node"
x linux/interrupt.h "tasklet_\(init\|schedule\|disable\|enable\|unlock\|kill\)"
x linux/security.h "security_\(path_mknod\|bpf[a-z_]*\)"
x asm/byteorder.h
"\(be\|le\)\(16\|32\|64\)_to_cpu\|cpu_to_\(be\|le\)\(16\|32\|64\)\|\(ntoh\|hton\)\(s\|l\)"
x net/flow_dissector.h "struct flow_keys"
x linux/filter.h "sk_filter\|bpf_dump_raw_ok\|bpf_jit_enable"
x linux/mm.h "truncate_setsize\|truncate_pagecache"
x asm/unaligned.h "\(get\|put\)_unaligned\(\|_le\|_be\)\(16\|32\|64\|\)"
x linux/srcu.h "DEFINE_STATIC_SRCU\|DEFINE_SRCU"
x linux/seq_file_net.h "seq_file_\(single_\|\)net"
x linux/netdevice.h "struct\ napi_struct"
x asm/processor.h "cpu_relax"
x linux/fs.h "deactivate_locked_super\|file_dentry\|alloc_chrdev_region\|init_sync_kiocb\|get_file\|get_dma_buf\|rw_copy_check_uvector"
x linux/spinlock.h
"\(raw_\|arch_\|\)spin_\(try\|un\|\)lock\(_irq\|_irqsave\|_init\|\)\|dsb_sev\|ATOMIC_INIT_NOTIFIER_HEAD\|DEFINE_MUTEX"
x asm/tlbflush.h "flush_tlb_all"
x linux/workqueue.h "\(schedule\|queue\|cancel\)_\(delayed_\|\)work"
x linux/thread_info.h "current->[a-z_]*"
x linux/sched.h "current->[a-z_]*"
x linux/dcache.h "d_backing_dentry\|d_backing_inode\|d_path"
x linux/shrinker.h "register_shrinker\|prealloc_shrinker\|struct\
shrink_control"
x linux/fs_types.h "mapping->i_mmap\(\|rwsem\)"
x linux/fs.h "IOP_\(XATTR\|FASTPERM\|LOOKUP\|NOFOLLOW\|DEFAULT_READLINK\)"
x linux/mm.h "vmf_error\|filemap_fault\|clear_page_dirty_for_io\|vm_operations_struct"
x linux/pagemap.h "read_mapping_page\|read_cache_page\(_gfp\|\)"
x linux/slab.h "ZERO_SIZE_PTR"
x linux/capability.h "capable(.*)"
x linux/mm.h "truncate_inode_page[a-z_]*\|truncate_pageche"
x linux/fs.h "bdevname\|\(un\|\)register_blkdev\|revalidate_disk\|inode_\(un\|\)lock\|i_size_\(write\|read\)\|kill_fasync\|kernel_read_file_from_path"
x linux/fs.h "FMODE_\(WRITE\|READ\|EXEC\)"
x linux/fs_types.h "ATTR_MODE\|struct\ iattr"
x linux/fs_types.h
"\(bd_\|anon_\|\)inode->\(i_uid\|i_gid\|i_mmap\|i_generation\|i_sb\|i_opflags\|i_mmap_rwsem\|i_size\|i_data\|i_mapping\)"
x linux/path.h "path_get\|path_put"
x linux/file_operations.h
"simple_\(read_from_buffer\|write_to_buffer\|open\)\|nonseekable_open"
x linux/spinlock.h
"rwlock_init\|\(read\|write\)_\(lock\|unlock\)\(\|_irq\|\_bh\|_irqsave\)"
x linux/mod_devicetable.h
"\(platform\|of\|dmi\|mdio\|ispnp\|typec\|wmi\|i2c\|i3c\|spi\|slim\|sdio\|input\|pci\|pcmcia\|hda\|pnp\|acpi\|ccw\|ap\|hid\|usb\)_device_id"
x linux/filter.h "struct\ sk_filter\|bpf_compute_data_pointers\|struct bpf_prog"
x linux/if_ether.h "eth_hdr"
x linux/etherdevice.h "ether_addr_copy"
#inet_request_bound_dev_if
x linux/if_addr.h
"IFA_F_PERMANENT\|IFA_F_SECONDARY\|IFA_F_TENTATIVE\|IFA_F_OPTIMISTIC\|IFA_F_DEPRECATED\|IFA_MAX\|IFA_LOCAL\|struct\
ifaddrmsg"
x net/l3mdev.h "l3mdev_master_ifindex_rcu\|inet_sk_bound_l3mdev\|l3mdev_ip6_out"
x net/rtnetlink.h "struct\
rtnl_link_ops\|rtnl_link_register\|rtnl_lock\|MODULE_ALIAS_RTNL_LINK\|RTNL_FLAG_DOIT_UNLOCKED"
x linux/netdevice.h "enum\ tc_setup_type"
x linux/netlink.h "NL_SET_ERR_MSG_MOD"
x net/netlink.h
"nlmsg_parse_deprecated\|nlmsg_\(cancel\|data\|end\|msg_size\|attrlen\)\|nla_data\|nla_get_u32\|nla_put_in6_addr\|nla_total_size"
x linux/if_addrlabel.h "ifaddrlblmsg"
x linux/dcache.h "d_lookup_done\|file_dentry\|d_parent\|dget\|dput\|d_sb"
#inet_sk_bound_l3mdev
x net/netprio_cgroup.h
"sock_update_netprioidx\|task_netprioidx\|struct\ netprio_map"
x linux/device.h "\(get\|put\)_device"
x linux/kernel.h ARRAY_SIZE
x linux/bits.h "BIT(.*)\|GENMASK\|BITS_PER_BYTE"
x linux/of.h "of_get_property\|of_match_node\|of_get_child_by_name\|of_match_ptr\|of_property_read_[a-z0-9_]*"
x linux/pci-dma-compat.h
"PCI_DMA_FROMDEVICE\|PCI_DMA_TODEVICE\|PCI_DMA_NONE\|PCI_DMA_BIDIRECTIONAL\|pci_map_\(single\|sg\|page\)\|pci_\(zalloc\|alloc\|free\)_consistent\|pci_dma_mapping_error\|pci_set_\(dma_mask\|consistent_dma_mask\)"
x asm/compiler.h "__asmeq"
x asm/cpuidle.h "cpu_do_idle"
x crypto/hash.h
"SHASH_DESC_ON_STACK\|crypto_\(shash\|ahash\)_\(digest\|update\)\|ahash_request_set_crypt"
x linux/acpi.h "ACPI_PTR"
x linux/atomic.h
"INIT_WORK\|atomic\(\|64\|_long\)_\(get\|set\|add\|inc\|dec\|sub\|init\)"
x linux/bitops.h
"\(ror\|rol\|hweight\)\(8\|16\|32\|64\)\(fls\|ffs\)\(\|64\|_long\)\|__fls\|__ffs"
x linux/bpf-cgroup.h "bpf_cgroup_[a-z_]*\|cgroup_storage_type"
x linux/cgroup.h "cgroup_id\|task_dfl_cgroup\|cgroup_ancestor"
x linux/cpumask.h
"for_each_\(online\|possible\|present\)_cpu\|cpumask_\(of_node\|of_pci\)\|nr_cpu_ids"
x linux/cpumask.h "num_online_cpus"
x linux/debugobjects.h "debug_object_\(init\|activate\|free\|destroy\)"
x linux/device.h
"device_\(create\|remove\|update\)_\(\(bin_\|\)file\|group\|link\)\(\|s\|_ns\|\)"
x linux/device.h "devm_\(kfree\|kzalloc\|kmalloc\|kcalloc\)"
x linux/err.h "ERR_PTR\|PTR_ERR\|PTR_ERR_OR_ZERO\|IS_ERR\|MAX_ERRNO"
x linux/filter.h "bpf_stats_enabled_key"
x linux/gfp.h "\(__\|\)\(alloc\|free\|get_free\|get_zeroed\)_page\(s\|\)\(_exact\|_node\|\)"
x linux/gfp.h "pm_restore_gfp_mask\|gfpflags_allow_blocking"
x linux/gpio.h "\(devm_\|\)gpio_\(\(set_value\|get_value\)\(_cansleep\|\)\|request\(_one\|\)\|to_irq\|is_valid\|direction_[a-z]*\|free\)"
x linux/gpio/consumer.h
"GPIOD_\(ASIS\|IN\|OUT[A-Z_]*\)\|\(devm_\|\)gpiod_get\(\|_index\|_array\)\(_optional\)\|gpio_\(free\|request\direction_output\|direction_input\)"
x linux/hardirq.h "rcu_nmi_exit"
x linux/i2c.h "struct\ i2c_board_info"
x linux/if_link.h "ifla_vf_info"
x linux/ioctl.h "\(_IO\|_IOR\|_IOW\|_IOWR\)"
x linux/ioprio.h
"get_current_ioprio\|task_nice_io\(class\|prio\)\|init_sync_kiocb"
x linux/kobject.h "\(mm\|kernel\|hypervisor\|fs\)_kobj"
x linux/kobject.h "kernel_kobj"
x linux/kobject.h
"kobject_\(get\|put\|uevent\|get_path\|uevent_env\|set_name\|del\|create\|create_and_add\)\|add_uevent_var"
x linux/log2.h "is_power_of_2\|ilog2"
x linux/math64.h "do_div\|DIV_ROUND_UP_ULL\|DIV_ROUND_DOWN_ULL"
x linux/memcontrol.h "lock_page_memcg\|get_mem_cgroup_from_page\|mem_cgroup_put"
x linux/memory_hotplug.h
"get_online_mems\|put_online_mems\|movable_node_is_enabled\|pfn_to_online_page\|try_online_node"
x linux/mm.h "VMALLOC_START\|VMALLOC_END\|VMALLOC_TOTAL\|unmap_mapping_range\|si_meminfo\|totalram_pages"
x linux/mm.h "VM_READ\|VM_WRITE\|VM_EXEC\|VM_NONE\|VM_SHARED\|VM_DONTDUMP\|VM_DONTEXPAND\|VM_DONTCOPY\|\(io_\|\)remap_pfn_range"
x linux/mm.h "truncate_inode_pages_final\|truncate_inode_pages_range\|vma_pages\|set_page_dirty\|want_init_on_\(alloc\|free\)"
x linux/mmzone.h "MAX_ORDER\|KMALLOC_MAX_SIZE"
x linux/module.h
"MODULE_\(LICENSE\|AUTHOR\|DESCRIPTION\|PARM_DESC\|ALIAS.*\|DEVICE_TABLE\)"
x linux/module.h
"register_module_notifier\|MODULE_STATE_COMING\|MODULE_STATE_GOING\|try_module_get\|lookup_module_symbol_attrs\|lookup_module_symbol_attrs\|module_put\|__module_get"
x linux/notifier.h "struct\
notifier_block\|NOTIFY_DONE\|NOTIFY_OK\|\(atomic\|blocking\|raw\|srcu\)_notifier_chain_\(\(un\|\)register\|call_chain\)\|\(ATOMIC\|BLOCKING\|RAW\)_INIT_NOTIFIER_HEAD"
x linux/of_platform.h
"\(devm_\|\)of_platform_\(populate\|register_reconfig_notifier\|default_populate\|depopulate\)"
x linux/of_platform.h "of_find_device_by_node"
x linux/osq_lock.h "osq_\(lock_init\|lock\|unlock\|is_locked\)"
x linux/pagemap.h "page_endio\|mapping_\(set_\|\)gfp_mask"
x linux/poll.h "poll_wait"
x linux/proc_fs.h "struct\
proc_ops\|proc_mkdir\|proc_create\|remove_proc_\(entry\|file\)"
x linux/ptrace.h "instruction_pointer"
x linux/radix-tree.h "radix_tree_[a-z_]*\|RADIX_TREE_MAX_TAGS"
x linux/random.h "\(get_random_\|prandom\_\)\(u32\|u64\|long\|bytes\)"
x linux/ratelimit.h
"ratelimit_\(state_init\|set_flags\)\|WARN_ON_RATELIMIT\|WARN_RATELIMIT\|pr_[a-z]*_ratelimited"
x linux/rbtree.h
"rb_\(prev\|erase\|link_node\|first\|insert_color\|root\|node\)"
x linux/rbtree_latch.h "latch_tree_[a-z]*"
x linux/rculist.h
"\(list\|__hlist\|hlist\)_\(for_each_[a-z_]*\|next\|first\|tail\|add\|add_tail\|del\|del_init\|entry\)_rcu"
x linux/rcupdate.h "synchronize_rcu"
x linux/rcuwait.h "rcuwait_init\|rcuwait_wait_event"
x linux/regmap.h "regmap_read_poll_timeout\(_atomic\|\)"
x linux/sched.h "TASK_\(\(UN\|\)INTERRUPTIBLE\|NORMAL\|KILLABLE\)"
x linux/sched.h
"cond_resched\|set_current_state\|current->comm\|need_resched\|schedule_timeout\(_killable\|_interruptible\|_idle\|\)\|schedule()"
x linux/sched/pagefault.h "pagefault_\(en\|dis\)able"
x linux/semaphore.h "struct\
semaphore\|down_\(interruptible\|killable\|timeout\|trylock\)\|sema_init"
x linux/seqlock.h "\(__\|raw_\|\)\(read\|write\)_seqcount_begin"
x linux/skbuff.h
"skb_\(push\|pull\|tailroom\|trim\|shinfo\|share_check\|reserve\|queue_[a-z_]*\|headlen\|cloned\)"
x linux/slab.h "\(kfree\|kzalloc\|kmalloc\|kcalloc\)"
x linux/spinlock.h "vcpu_is_preempted"
x linux/string.h
"memcpy\|memcmp\|strcmp\|strncmp\|kmemdup\|memset\|strcat\|strncat\|strlcat\|strstr\|strlen"
x linux/swap.h "mark_page_accessed\|nr_free_buffer_pages"
x linux/swap.h "si_swapinfo\|shrink_all_memory"
x linux/sysctl.h
"proc_do\(string\|\(long\|int\|uint\)vec\)[a-z_]*\|proc_handler\|ctl_table\|register_sysctl[a-z_]*"
x linux/timekeeping.h
"ktime_get[_a-z0-9]*\|ktime_mono_to_real\|JFFS2_NOW\|getboottime64"
x linux/topology.h "cpu_to_node\|node_distance"
x linux/u64_stats_sync.h "u64_stats_\(init\|update_begin\)"
x linux/uaccess.h "\(__\|\)\(get\|put\|copy_from\|copy_to\)_user"
x linux/uprobes.h "uprobe_\(munmap\|mmap\)"
x linux/watch_queue.h
"watch_sizeof\|add_watch_to_object\|put_watch_queue\|remove_watch_list"
x linux/xarray.h "xa_\(alloc\|linux_32b\|erase\|load\|for_each\)\|XA_CHUNK_SIZE"
x net/gro.h "skb_gro_[a-z_0-z]*"
x net/gso.h "skb_gso_[a-z_0-z]*"
x sound/soc-component.h
"snd_soc_component\(_driver\|_set_sysclk\|_set_pll\|\)\|snd_soc_dapm_to_component"
On 11/19/21 17:18, Arnd Bergmann wrote:
> On Fri, Nov 19, 2021 at 5:10 PM Andy Shevchenko
> <[email protected]> wrote:
>> On Fri, Nov 19, 2021 at 04:57:46PM +0100, Arnd Bergmann wrote:
>
>>> The main problem with this approach is that as soon as you start
>>> actually reducing the unneeded indirect includes, you end up with
>>> countless .c files that no longer build because they are missing a
>>> direct include for something that was always included somewhere
>>> deep underneath, so I needed a second set of scripts to add
>>> direct includes to every .c file.
>>
>> Can't it be done with cocci support?
>
> There are many ways of doing it, but they all tend to suffer from the
> problem of identifying which headers are actually needed based on
> the contents of a file, and also figuring out where to put the extra
> #include if there are complex #ifdefs.
>
> For reference, see below for the naive pattern matching I tried.
> This is obviously incomplete and partially wrong.
FYI, if you may not know the tool,
theres include-what-you-use(1) (a.k.a. iwyu(1))[1],
although it is still not mature,
and I'm helping improve it a bit.
If I understood better the kernel Makefiles,
I'd try it.
You can try it yourselves.
I still can't use it for my own code,
since it has a lot of false positives.
Cheers,
Alex
[1]: <https://include-what-you-use.org/>
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
On Fri, Nov 19, 2021 at 5:12 PM Alejandro Colomar (man-pages)
<[email protected]> wrote:
>
> On 11/19/21 16:57, Arnd Bergmann wrote:
> >
> > From what I can tell, linux/stddef.h is tiny, I don't think it's really
> > worth optimizing this part. I have spent some time last year
> > trying to untangle some of the more interesting headers, but ended
> > up not completing this as there are some really hard problems
> > once you start getting to the interesting bits.
>
> In this case it was not about being worth it or not,
> but that the fact that adding memberof() would break,
> unless I use 0 instead of NULL for the implementation of memberof(),
> which I'm against, or I split stddef.
>
> If I don't do either of those,
> I'm creating a circular dependency,
> and it doesn't compile.
Sorry for missing the background here, but I don't see what that
dependency is. If memberof() is a macro, then including the definition
should not require having the NULL definition first, you just need to
have both at the time you use it.
> > The main issue here is that user space code should not
> > include anything outside of include/uapi/ and arch/*/include/uapi/
>
> Okay. That's good to know.
>
> So everything can use uapi code,
> and uapi code can only use uapi code,
> right?
Correct.
> > offsetof() is defined in include/linux/stddef.h, so this is by
> > definition not accessible here. It appears that there is also
> > an include/uapi/linux/stddef.h that is really strange because
> > it includes linux/compiler_types.h, which in turn is outside
> > of uapi/. This should probably be fixed.
>
> I see.
> Then,
> perhaps it would be better to define offsetof() _only_ inside uapi/,
> and use that definition from everywhere else,
> and therefore remove the non-uapi version,
> right?
No, because the user-space <stddef.h> provided by the compiler
also includes an offsetof() definition. In the uapi/ namespace, the
kernel must only provide definitions that do not clash with anything
in user space.
Arnd
On Fri, Nov 19, 2021 at 5:22 PM Alejandro Colomar (man-pages)
<[email protected]> wrote:
> On 11/19/21 17:18, Arnd Bergmann wrote:
> > On Fri, Nov 19, 2021 at 5:10 PM Andy Shevchenko
> > <[email protected]> wrote:
> >> On Fri, Nov 19, 2021 at 04:57:46PM +0100, Arnd Bergmann wrote:
> >
> >>> The main problem with this approach is that as soon as you start
> >>> actually reducing the unneeded indirect includes, you end up with
> >>> countless .c files that no longer build because they are missing a
> >>> direct include for something that was always included somewhere
> >>> deep underneath, so I needed a second set of scripts to add
> >>> direct includes to every .c file.
> >>
> >> Can't it be done with cocci support?
> >
> > There are many ways of doing it, but they all tend to suffer from the
> > problem of identifying which headers are actually needed based on
> > the contents of a file, and also figuring out where to put the extra
> > #include if there are complex #ifdefs.
> >
> > For reference, see below for the naive pattern matching I tried.
> > This is obviously incomplete and partially wrong.
>
> FYI, if you may not know the tool,
> theres include-what-you-use(1) (a.k.a. iwyu(1))[1],
> although it is still not mature,
> and I'm helping improve it a bit.
Yes, I know that one, I tried using it as well, but it did not really
scale to the size of the kernel as it requires having all files to use
the correct set of #include, and to know about all the definitions.
Arnd
On Fri, Nov 19, 2021 at 05:22:48PM +0100, Alejandro Colomar (man-pages) wrote:
>
>
> On 11/19/21 17:18, Arnd Bergmann wrote:
> > On Fri, Nov 19, 2021 at 5:10 PM Andy Shevchenko
> > <[email protected]> wrote:
> >> On Fri, Nov 19, 2021 at 04:57:46PM +0100, Arnd Bergmann wrote:
> >
> >>> The main problem with this approach is that as soon as you start
> >>> actually reducing the unneeded indirect includes, you end up with
> >>> countless .c files that no longer build because they are missing a
> >>> direct include for something that was always included somewhere
> >>> deep underneath, so I needed a second set of scripts to add
> >>> direct includes to every .c file.
> >>
> >> Can't it be done with cocci support?
> >
> > There are many ways of doing it, but they all tend to suffer from the
> > problem of identifying which headers are actually needed based on
> > the contents of a file, and also figuring out where to put the extra
> > #include if there are complex #ifdefs.
> >
> > For reference, see below for the naive pattern matching I tried.
> > This is obviously incomplete and partially wrong.
>
> FYI, if you may not know the tool,
> theres include-what-you-use(1) (a.k.a. iwyu(1))[1],
> although it is still not mature,
> and I'm helping improve it a bit.
Yes, I know the tool, but it produces insanity. Jonathan (maintainer
of IIO subsystem) actually found it useful after manual work applied.
Perhaps you can chat with him about usage of it in the Linux kernel.
> If I understood better the kernel Makefiles,
> I'd try it.
>
> You can try it yourselves.
> I still can't use it for my own code,
> since it has a lot of false positives.
> [1]: <https://include-what-you-use.org/>
--
With Best Regards,
Andy Shevchenko
On Fri, Nov 19, 2021 at 05:12:19PM +0100, Alejandro Colomar (man-pages) wrote:
> On 11/19/21 16:57, Arnd Bergmann wrote:
...
> > On the plus side, I did see something on the order of a 30%
> > compile speed improvement with clang, which is insane
> > given that this only removed dead definitions.
>
> Huh!
>
> I'd like to see the kernel some day
> not having _any_ hidden dependencies.
It's neither feasible nor practical. If we know the hard dependencies between
headers, why should we not use implicit inclusion?
We all know that bitmap.h includes bitops.h and this is good and a must, why
to avoid this?
--
With Best Regards,
Andy Shevchenko
On 11/19/21 17:37, Andy Shevchenko wrote:
> On Fri, Nov 19, 2021 at 05:12:19PM +0100, Alejandro Colomar (man-pages) wrote:
>> On 11/19/21 16:57, Arnd Bergmann wrote:
>
> ...
>
>>> On the plus side, I did see something on the order of a 30%
>>> compile speed improvement with clang, which is insane
>>> given that this only removed dead definitions.
>>
>> Huh!
>>
>> I'd like to see the kernel some day
>> not having _any_ hidden dependencies.
>
> It's neither feasible nor practical. If we know the hard dependencies between
> headers, why should we not use implicit inclusion?
>
> We all know that bitmap.h includes bitops.h and this is good and a must, why
> to avoid this?
>
Well, none at all may be too much.
But being close to it would simplify much changing and especially
understanding the code.
At least avoiding 10 levels of indirection of includes
would be a good start :)
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
Hi all,
I splitted some macros into separate headers,
to be able to use them
without pulling too many deps.
I also simplified some of themr
to be implemented in terms of the others
and to remove some unnecessary explicit casts.
And I added memberof(),
which gives name to a typical construction
to get the member of a struct
without needing a variable of that type.
The next step after this patch set
is another one removing all redefinitions
(at least all that are possible,
since these headers can't be included everywhere)
of these macros,
by including these new tiny headers.
Since these headers are so tiny and bring no dependencies,
they should break anything.
It was hard for me to get this working
because the order of includes _matters a lot_,
and which headers you include _matters_ even outside of uapi.
So I think this should help fix that,
by allowing headers to pull exactly what they want,
without all of the stuff that came with
<linux/compiler.h>
<linux/compiler_types.h>
<linux/stddef.h>.
I already have much of the next patch set ready,
and it removes hundreds of redefinitions of these macros,
which should be a good thing.
Then,
when there are (almost) no redefinitions of these macros,
I'll prepare a 3rd patch set that
explicitly includes these tiny headers
wherever these macros were already in use,
to allow for removal of other bigger headers
(although I won't remove anything,
to avoid silently breaking anything).
And then,
a 4th patch set will
attempt to find all uses of these macros
that were not even named
(i.e., hard-coded sizeof divisions).
Hope this is clear and
that you like these changes.
Cheers,
Alex
Alejandro Colomar (20):
linux/stddef.h, linux/offsetof.h: Split offsetof() into a separate
header
linux/stddef.h, linux/sizeof_field.h: Split sizeof_field() into a
separate header
linux/stddef.h, linux/offsetofend.h: Split offsetofend() into a
separate header
linux/stddef.h, linux/NULL.h: Split NULL into a separate header
linux/container_of.h, linux/typeof_member.h: Split typeof_member()
into a separate header
linux/kernel.h, linux/array_size.h: Split ARRAY_SIZE() into a separate
header
linux/memberof.h: Add memberof(T, m) macro
Implement container_of_safe() in terms of container_of()
Implement offsetof(), sizeof_member(), typeof_member(), and
container_of() in terms of memberof()
linux/container_of.h: Remove unnecessary cast
linux/container_of.h: Cosmetic
linux/must_be.h: Add must_be() to improve readability of
BUILD_BUG_ON_ZERO()
linux/build_bug.h, linux/must_be.h: Move BUILD_BUG_ON_ZERO to
<linux/must_be.h>
linux/compiler_types.h, linux/same_type.h: Split __same_type() to a
separate header
linux/compiler.h: Implement __must_be_array() in terms of __must_be()
linux/compiler.h, linux/array_size.h: Move __must_be_array() into
<linux/array_size.h>
linux/array_size.h: Add __is_array(a) to help readability
linux/power_of_2.h: Add __IS_POWER_OF_2(n) and __IS_POWER_OF_2_OR_0(n)
macros
linux/build_bug.h, linux/power_of_2.h: Move
[__]BUILD_BUG_ON_NOT_POWER_OF_2() to <linux/power_of_2.h>
linux/power_of_2.h: Implement [__]BUILD_BUG_ON_NOT_POWER_OF_2() in
terms of __IS_POWER_OF_2[_OR_0]()
drivers/gpu/drm/i915/i915_utils.h | 3 ++
drivers/net/can/usb/etas_es58x/es58x_core.h | 3 ++
include/linux/NULL.h | 10 +++++++
include/linux/array_size.h | 21 ++++++++++++++
include/linux/build_bug.h | 20 ++-----------
include/linux/compiler.h | 3 --
include/linux/compiler_types.h | 7 +++--
include/linux/container_of.h | 25 ++++++++--------
include/linux/kernel.h | 9 ++----
include/linux/memberof.h | 12 ++++++++
include/linux/must_be.h | 21 ++++++++++++++
include/linux/offsetof.h | 19 ++++++++++++
include/linux/offsetofend.h | 19 ++++++++++++
include/linux/overflow.h | 3 ++
include/linux/power_of_2.h | 17 +++++++++++
include/linux/same_type.h | 10 +++++++
include/linux/sizeof_field.h | 18 ++++++++++++
include/linux/stddef.h | 32 ++++-----------------
include/linux/typeof_member.h | 12 ++++++++
19 files changed, 197 insertions(+), 67 deletions(-)
create mode 100644 include/linux/NULL.h
create mode 100644 include/linux/array_size.h
create mode 100644 include/linux/memberof.h
create mode 100644 include/linux/must_be.h
create mode 100644 include/linux/offsetof.h
create mode 100644 include/linux/offsetofend.h
create mode 100644 include/linux/power_of_2.h
create mode 100644 include/linux/same_type.h
create mode 100644 include/linux/sizeof_field.h
create mode 100644 include/linux/typeof_member.h
--
2.33.1
Include <linux/offsetof.h> from <linux/stddef.h> for compatibility.
From <linux/offsetof.h>:
Include the same exact deps that <linux/stddef.h> had.
Changing that in any way broke my compilation.
Signed-off-by: Alejandro Colomar <[email protected]>
f
---
include/linux/offsetof.h | 17 +++++++++++++++++
include/linux/stddef.h | 11 ++++-------
2 files changed, 21 insertions(+), 7 deletions(-)
create mode 100644 include/linux/offsetof.h
diff --git a/include/linux/offsetof.h b/include/linux/offsetof.h
new file mode 100644
index 000000000000..3db431197687
--- /dev/null
+++ b/include/linux/offsetof.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_OFFSETOF_H
+#define _LINUX_OFFSETOF_H
+
+
+#include <uapi/linux/stddef.h>
+
+
+#undef offsetof
+#ifdef __compiler_offsetof
+#define offsetof(T, m) __compiler_offsetof(T, m)
+#else
+#define offsetof(T, m) ((size_t)&((T *)0)->m)
+#endif
+
+
+#endif /* _LINUX_OFFSETOF_H */
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index ca507bd5f808..41217710788e 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -2,8 +2,12 @@
#ifndef _LINUX_STDDEF_H
#define _LINUX_STDDEF_H
+
#include <uapi/linux/stddef.h>
+#include <linux/offsetof.h>
+
+
#undef NULL
#define NULL ((void *)0)
@@ -12,13 +16,6 @@ enum {
true = 1
};
-#undef offsetof
-#ifdef __compiler_offsetof
-#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
-#else
-#define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER)
-#endif
-
/**
* sizeof_field() - Report the size of a struct field in bytes
*
--
2.33.1
Include <linux/sizeof_field.h> from <linux/stddef.h> for compatibility.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/sizeof_field.h | 15 +++++++++++++++
include/linux/stddef.h | 9 +--------
2 files changed, 16 insertions(+), 8 deletions(-)
create mode 100644 include/linux/sizeof_field.h
diff --git a/include/linux/sizeof_field.h b/include/linux/sizeof_field.h
new file mode 100644
index 000000000000..15dc10fce8f3
--- /dev/null
+++ b/include/linux/sizeof_field.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_SIZEOF_FIELD_H
+#define _LINUX_SIZEOF_FIELD_H
+
+
+/**
+ * sizeof_field() - Report the size of a struct field in bytes
+ *
+ * @T: The structure containing the field of interest
+ * @m: The field (member) to return the size of
+ */
+#define sizeof_field(T, m) sizeof((((T *)0)->m))
+
+
+#endif /* _LINUX_SIZEOF_FIELD_H */
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index 41217710788e..1be2c71c1e85 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -6,6 +6,7 @@
#include <uapi/linux/stddef.h>
#include <linux/offsetof.h>
+#include <linux/sizeof_field.h>
#undef NULL
@@ -16,14 +17,6 @@ enum {
true = 1
};
-/**
- * sizeof_field() - Report the size of a struct field in bytes
- *
- * @TYPE: The structure containing the field of interest
- * @MEMBER: The field to return the size of
- */
-#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
-
/**
* offsetofend() - Report the offset of a struct field within the struct
*
--
2.33.1
Include <linux/offsetofend.h> from <linux/stddef.h> for compatibility.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/offsetofend.h | 19 +++++++++++++++++++
include/linux/stddef.h | 10 +---------
2 files changed, 20 insertions(+), 9 deletions(-)
create mode 100644 include/linux/offsetofend.h
diff --git a/include/linux/offsetofend.h b/include/linux/offsetofend.h
new file mode 100644
index 000000000000..20fe2905629d
--- /dev/null
+++ b/include/linux/offsetofend.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_OFFSETOFEND_H
+#define _LINUX_OFFSETOFEND_H
+
+
+#include <linux/offsetof.h>
+#include <linux/sizeof_field.h>
+
+
+/**
+ * offsetofend() - Report the offset of a struct field within the struct
+ *
+ * @T: The type of the structure
+ * @m: The member within the structure to get the end offset of
+ */
+#define offsetofend(T, m) (offsetof(T, m) + sizeof_field(T, m))
+
+
+#endif /* _LINUX_OFFSETOFEND_H */
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index 1be2c71c1e85..c2e7e5051ef3 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -6,6 +6,7 @@
#include <uapi/linux/stddef.h>
#include <linux/offsetof.h>
+#include <linux/offsetofend.h>
#include <linux/sizeof_field.h>
@@ -17,15 +18,6 @@ enum {
true = 1
};
-/**
- * offsetofend() - Report the offset of a struct field within the struct
- *
- * @TYPE: The type of the structure
- * @MEMBER: The member within the structure to get the end offset of
- */
-#define offsetofend(TYPE, MEMBER) \
- (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER))
-
/**
* struct_group() - Wrap a set of declarations in a mirrored struct
*
--
2.33.1
Include <linux/array_size.h> from <linux/kernel.h> for compatibility.
Include <linux/compiler.h> for __must_be_array().
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/array_size.h | 15 +++++++++++++++
include/linux/kernel.h | 9 +++------
2 files changed, 18 insertions(+), 6 deletions(-)
create mode 100644 include/linux/array_size.h
diff --git a/include/linux/array_size.h b/include/linux/array_size.h
new file mode 100644
index 000000000000..cba90f2248ef
--- /dev/null
+++ b/include/linux/array_size.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_ARRAY_SIZE_H
+#define _LINUX_ARRAY_SIZE_H
+
+#include <linux/compiler.h>
+
+
+/**
+ * ARRAY_SIZE - get the number of elements in array @a
+ * @a: array to be sized
+ */
+#define ARRAY_SIZE(a) (sizeof((a)) / sizeof((a)[0]) + __must_be_array(a))
+
+
+#endif /* _LINUX_ARRAY_SIZE_H */
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 77755ac3e189..db6021ce3e0e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -2,6 +2,8 @@
#ifndef _LINUX_KERNEL_H
#define _LINUX_KERNEL_H
+
+#include <linux/array_size.h>
#include <linux/stdarg.h>
#include <linux/align.h>
#include <linux/limits.h>
@@ -25,6 +27,7 @@
#include <uapi/linux/kernel.h>
+
#define STACK_MAGIC 0xdeadbeef
/**
@@ -39,12 +42,6 @@
#define READ 0
#define WRITE 1
-/**
- * ARRAY_SIZE - get the number of elements in array @arr
- * @arr: array to be sized
- */
-#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
-
#define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
#define u64_to_user_ptr(x) ( \
--
2.33.1
Include <linux/typeof_member.h> from <linux/container_of.h> for compatibility.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/container_of.h | 3 ++-
include/linux/typeof_member.h | 9 +++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
create mode 100644 include/linux/typeof_member.h
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 2f4944b791b8..220990db7b61 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -2,10 +2,11 @@
#ifndef _LINUX_CONTAINER_OF_H
#define _LINUX_CONTAINER_OF_H
+
#include <linux/build_bug.h>
#include <linux/err.h>
+#include <linux/typeof_member.h>
-#define typeof_member(T, m) typeof(((T*)0)->m)
/**
* container_of - cast a member of a structure out to the containing structure
diff --git a/include/linux/typeof_member.h b/include/linux/typeof_member.h
new file mode 100644
index 000000000000..1f3bfb3c17d2
--- /dev/null
+++ b/include/linux/typeof_member.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_TYPEOF_MEMBER_H
+#define _LINUX_TYPEOF_MEMBER_H
+
+
+#define typeof_member(T, m) typeof(((T *)0)->m)
+
+
+#endif /* _LINUX_TYPEOF_MEMBER_H */
--
2.33.1
Include <linux/NULL.h> from <linux/stddef.h> for compatibility.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/NULL.h | 10 ++++++++++
include/linux/stddef.h | 4 +---
2 files changed, 11 insertions(+), 3 deletions(-)
create mode 100644 include/linux/NULL.h
diff --git a/include/linux/NULL.h b/include/linux/NULL.h
new file mode 100644
index 000000000000..f09d8baecc80
--- /dev/null
+++ b/include/linux/NULL.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_NULL_H
+#define _LINUX_NULL_H
+
+
+#undef NULL
+#define NULL ((void *)0)
+
+
+#endif /* _LINUX_NULL_H */
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index c2e7e5051ef3..3f7ddcd5e0aa 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -5,14 +5,12 @@
#include <uapi/linux/stddef.h>
+#include <linux/NULL.h>
#include <linux/offsetof.h>
#include <linux/offsetofend.h>
#include <linux/sizeof_field.h>
-#undef NULL
-#define NULL ((void *)0)
-
enum {
false = 0,
true = 1
--
2.33.1
There's no more a need for the temporary variable __mptr,
since now it's only passed to functions that accept a 'const void *',
and everything can convert automatically to it,
reducing the need for the cast too.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/container_of.h | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 220990db7b61..03809348f333 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -30,12 +30,7 @@
*
* If IS_ERR_OR_NULL(ptr), ptr is returned unchanged.
*/
-#define container_of_safe(ptr, type, member) ({ \
- void *__mptr = (void *)(ptr); \
- static_assert(__same_type(*(ptr), ((type *)0)->member) || \
- __same_type(*(ptr), void), \
- "pointer type mismatch in container_of_safe()"); \
- IS_ERR_OR_NULL(__mptr) ? ERR_CAST(__mptr) : \
- ((type *)(__mptr - offsetof(type, member))); })
+#define container_of_safe(ptr, type, member) \
+ (IS_ERR_OR_NULL(ptr) ? ERR_CAST(ptr) : container_of(type, member))
#endif /* _LINUX_CONTAINER_OF_H */
--
2.33.1
(((T *)0)->m) if a typical construct used by some wrappers,
and also directly in many places.
Write a macro to encapsulate that construct,
and avoid repetition (and possible mistakes).
It also helps readability by reducing the complexity
of mentally parsing so many symbols together,
with a readable name.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/memberof.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
create mode 100644 include/linux/memberof.h
diff --git a/include/linux/memberof.h b/include/linux/memberof.h
new file mode 100644
index 000000000000..6d8cb3dabbf9
--- /dev/null
+++ b/include/linux/memberof.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_MEMBEROF_H
+#define _LINUX_MEMBEROF_H
+
+
+#include <linux/NULL.h>
+
+
+#define memberof(T, m) (((T *)NULL)->m)
+
+
+#endif /* _LINUX_MEMBEROF_H */
--
2.33.1
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/container_of.h | 3 ++-
include/linux/offsetof.h | 4 +++-
include/linux/sizeof_field.h | 5 ++++-
include/linux/typeof_member.h | 5 ++++-
4 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 03809348f333..d1a66e72d246 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -5,6 +5,7 @@
#include <linux/build_bug.h>
#include <linux/err.h>
+#include <linux/memberof.h>
#include <linux/typeof_member.h>
@@ -17,7 +18,7 @@
*/
#define container_of(ptr, type, member) ({ \
void *__mptr = (void *)(ptr); \
- static_assert(__same_type(*(ptr), ((type *)0)->member) || \
+ static_assert(__same_type(*(ptr), memberof(type, member)) || \
__same_type(*(ptr), void), \
"pointer type mismatch in container_of()"); \
((type *)(__mptr - offsetof(type, member))); })
diff --git a/include/linux/offsetof.h b/include/linux/offsetof.h
index 3db431197687..656c758479e8 100644
--- a/include/linux/offsetof.h
+++ b/include/linux/offsetof.h
@@ -5,12 +5,14 @@
#include <uapi/linux/stddef.h>
+#include <linux/memberof.h>
+
#undef offsetof
#ifdef __compiler_offsetof
#define offsetof(T, m) __compiler_offsetof(T, m)
#else
-#define offsetof(T, m) ((size_t)&((T *)0)->m)
+#define offsetof(T, m) ((size_t)&memberof(T, m))
#endif
diff --git a/include/linux/sizeof_field.h b/include/linux/sizeof_field.h
index 15dc10fce8f3..a64accc76055 100644
--- a/include/linux/sizeof_field.h
+++ b/include/linux/sizeof_field.h
@@ -3,13 +3,16 @@
#define _LINUX_SIZEOF_FIELD_H
+#include <linux/memberof.h>
+
+
/**
* sizeof_field() - Report the size of a struct field in bytes
*
* @T: The structure containing the field of interest
* @m: The field (member) to return the size of
*/
-#define sizeof_field(T, m) sizeof((((T *)0)->m))
+#define sizeof_field(T, m) sizeof(memberof(T, m))
#endif /* _LINUX_SIZEOF_FIELD_H */
diff --git a/include/linux/typeof_member.h b/include/linux/typeof_member.h
index 1f3bfb3c17d2..529535a55d40 100644
--- a/include/linux/typeof_member.h
+++ b/include/linux/typeof_member.h
@@ -3,7 +3,10 @@
#define _LINUX_TYPEOF_MEMBER_H
-#define typeof_member(T, m) typeof(((T *)0)->m)
+#include <linux/memberof.h>
+
+
+#define typeof_member(T, m) typeof(memberof(T, m))
#endif /* _LINUX_TYPEOF_MEMBER_H */
--
2.33.1
Casts are dangerous.
Moreover, (almost) everything converts to 'void *' implicitly,
with the exception of pointers to const.
Change the temporary to be const,
and therefore allow implicit conversion from everything.
Since we don't modify it,
this also adds safety.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/container_of.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index d1a66e72d246..682a2a606ee9 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -17,7 +17,7 @@
*
*/
#define container_of(ptr, type, member) ({ \
- void *__mptr = (void *)(ptr); \
+ const void *__mptr = (ptr); \
static_assert(__same_type(*(ptr), memberof(type, member)) || \
__same_type(*(ptr), void), \
"pointer type mismatch in container_of()"); \
--
2.33.1
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/build_bug.h | 13 ++-----------
include/linux/must_be.h | 12 ++++++++++++
2 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
index e3a0be2c90ad..539e3d5df027 100644
--- a/include/linux/build_bug.h
+++ b/include/linux/build_bug.h
@@ -2,19 +2,10 @@
#ifndef _LINUX_BUILD_BUG_H
#define _LINUX_BUILD_BUG_H
+
#include <linux/compiler.h>
+#include <linux/must_be.h>
-#ifdef __CHECKER__
-#define BUILD_BUG_ON_ZERO(e) (0)
-#else /* __CHECKER__ */
-/*
- * Force a compilation error if condition is true, but also produce a
- * result (of value 0 and type int), so the expression can be used
- * e.g. in a structure initializer (or where-ever else comma expressions
- * aren't permitted).
- */
-#define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
-#endif /* __CHECKER__ */
/* Force a compilation error if a constant expression is not a power of 2 */
#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) \
diff --git a/include/linux/must_be.h b/include/linux/must_be.h
index a2e0b0c890a6..d1ebeaca9cf8 100644
--- a/include/linux/must_be.h
+++ b/include/linux/must_be.h
@@ -3,6 +3,18 @@
#define _LINUX_MUST_BE_H
+#ifdef __CHECKER__
+#define BUILD_BUG_ON_ZERO(e) (0)
+#else /* __CHECKER__ */
+/*
+ * Force a compilation error if condition is true, but also produce a
+ * result (of value 0 and type int), so the expression can be used
+ * e.g. in a structure initializer (or where-ever else comma expressions
+ * aren't permitted).
+ */
+#define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
+#endif /* __CHECKER__ */
+
#define __must_be(e) BUILD_BUG_ON_ZERO(!(e))
--
2.33.1
Historically, BUILD_BUG_ON_ZERO() has been hard to read.
__must_be_array() is based on BUILD_BUG_ON_ZERO(),
and unlike BUILD_BUG_ON_*(),
it has a pretty readable name.
Let's generalize it with this kind of usage:
#define __must_be_array(a) must_be(__is_array(a))
So next step is to create macros of the kind:
__is_array(a)
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/must_be.h | 9 +++++++++
1 file changed, 9 insertions(+)
create mode 100644 include/linux/must_be.h
diff --git a/include/linux/must_be.h b/include/linux/must_be.h
new file mode 100644
index 000000000000..a2e0b0c890a6
--- /dev/null
+++ b/include/linux/must_be.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_MUST_BE_H
+#define _LINUX_MUST_BE_H
+
+
+#define __must_be(e) BUILD_BUG_ON_ZERO(!(e))
+
+
+#endif /* _LINUX_MUST_BE_H */
--
2.33.1
Place braces in a ({}) expression
similarly to how a function would have them.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/container_of.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 682a2a606ee9..3a31b6874944 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -16,12 +16,16 @@
* @member: the name of the member within the struct.
*
*/
-#define container_of(ptr, type, member) ({ \
+#define container_of(ptr, type, member) ( \
+{ \
const void *__mptr = (ptr); \
+ \
static_assert(__same_type(*(ptr), memberof(type, member)) || \
__same_type(*(ptr), void), \
"pointer type mismatch in container_of()"); \
- ((type *)(__mptr - offsetof(type, member))); })
+ ((type *)(__mptr - offsetof(type, member))); \
+} \
+)
/**
* container_of_safe - cast a member of a structure out to the containing structure
--
2.33.1
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/build_bug.h | 13 ++-----------
include/linux/must_be.h | 12 ++++++++++++
2 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
index e3a0be2c90ad..539e3d5df027 100644
--- a/include/linux/build_bug.h
+++ b/include/linux/build_bug.h
@@ -2,19 +2,10 @@
#ifndef _LINUX_BUILD_BUG_H
#define _LINUX_BUILD_BUG_H
+
#include <linux/compiler.h>
+#include <linux/must_be.h>
-#ifdef __CHECKER__
-#define BUILD_BUG_ON_ZERO(e) (0)
-#else /* __CHECKER__ */
-/*
- * Force a compilation error if condition is true, but also produce a
- * result (of value 0 and type int), so the expression can be used
- * e.g. in a structure initializer (or where-ever else comma expressions
- * aren't permitted).
- */
-#define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
-#endif /* __CHECKER__ */
/* Force a compilation error if a constant expression is not a power of 2 */
#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) \
diff --git a/include/linux/must_be.h b/include/linux/must_be.h
index a2e0b0c890a6..d1ebeaca9cf8 100644
--- a/include/linux/must_be.h
+++ b/include/linux/must_be.h
@@ -3,6 +3,18 @@
#define _LINUX_MUST_BE_H
+#ifdef __CHECKER__
+#define BUILD_BUG_ON_ZERO(e) (0)
+#else /* __CHECKER__ */
+/*
+ * Force a compilation error if condition is true, but also produce a
+ * result (of value 0 and type int), so the expression can be used
+ * e.g. in a structure initializer (or where-ever else comma expressions
+ * aren't permitted).
+ */
+#define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
+#endif /* __CHECKER__ */
+
#define __must_be(e) BUILD_BUG_ON_ZERO(!(e))
--
2.33.1
Include <linux/same_type.h> from <linux/compiler_types.h> for compatibility.
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/compiler_types.h | 7 ++++---
include/linux/same_type.h | 10 ++++++++++
2 files changed, 14 insertions(+), 3 deletions(-)
create mode 100644 include/linux/same_type.h
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 1d32f4c03c9e..5a1626e2d331 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -2,6 +2,10 @@
#ifndef __LINUX_COMPILER_TYPES_H
#define __LINUX_COMPILER_TYPES_H
+
+#include <linux/same_type.h>
+
+
#ifndef __ASSEMBLY__
#ifdef __CHECKER__
@@ -272,9 +276,6 @@ struct ftrace_likely_data {
#define asm_inline asm
#endif
-/* Are two types/vars the same type (ignoring qualifiers)? */
-#define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
-
/*
* __unqual_scalar_typeof(x) - Declare an unqualified scalar type, leaving
* non-scalar types unchanged.
diff --git a/include/linux/same_type.h b/include/linux/same_type.h
new file mode 100644
index 000000000000..2e45edda6d31
--- /dev/null
+++ b/include/linux/same_type.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_SAME_TYPE_H
+#define __LINUX_SAME_TYPE_H
+
+
+/* Are two types/vars the same type (ignoring qualifiers)? */
+#define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
+
+
+#endif /* __LINUX_SAME_TYPE_H */
--
2.33.1
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/compiler.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 3d5af56337bd..64d99335874d 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -255,7 +255,7 @@ static inline void *offset_to_ptr(const int *off)
#endif /* __ASSEMBLY__ */
/* &a[0] degrades to a pointer: a different type from an array */
-#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
+#define __must_be_array(a) __must_be(!__same_type((a), &(a)[0]))
/*
* This is needed in functions which generate the stack canary, see
--
2.33.1
Signed-off-by: Alejandro Colomar <[email protected]>
---
drivers/gpu/drm/i915/i915_utils.h | 3 +++
drivers/net/can/usb/etas_es58x/es58x_core.h | 3 +++
include/linux/array_size.h | 6 +++++-
include/linux/compiler.h | 3 ---
include/linux/overflow.h | 3 +++
5 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index 7a5925072466..f6fc1c72dfa7 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -25,6 +25,8 @@
#ifndef __I915_UTILS_H
#define __I915_UTILS_H
+
+#include <linux/array_size.h>
#include <linux/list.h>
#include <linux/overflow.h>
#include <linux/sched.h>
@@ -32,6 +34,7 @@
#include <linux/workqueue.h>
#include <linux/sched/clock.h>
+
struct drm_i915_private;
struct timer_list;
diff --git a/drivers/net/can/usb/etas_es58x/es58x_core.h b/drivers/net/can/usb/etas_es58x/es58x_core.h
index 826a15871573..a18f4215aeef 100644
--- a/drivers/net/can/usb/etas_es58x/es58x_core.h
+++ b/drivers/net/can/usb/etas_es58x/es58x_core.h
@@ -12,6 +12,8 @@
#ifndef __ES58X_COMMON_H__
#define __ES58X_COMMON_H__
+
+#include <linux/array_size.h>
#include <linux/types.h>
#include <linux/usb.h>
#include <linux/netdevice.h>
@@ -21,6 +23,7 @@
#include "es581_4.h"
#include "es58x_fd.h"
+
/* Driver constants */
#define ES58X_RX_URBS_MAX 5 /* Empirical value */
#define ES58X_TX_URBS_MAX 6 /* Empirical value */
diff --git a/include/linux/array_size.h b/include/linux/array_size.h
index cba90f2248ef..c2b4fddba564 100644
--- a/include/linux/array_size.h
+++ b/include/linux/array_size.h
@@ -2,9 +2,13 @@
#ifndef _LINUX_ARRAY_SIZE_H
#define _LINUX_ARRAY_SIZE_H
-#include <linux/compiler.h>
+#include <linux/must_be.h>
+#include <linux/same_type.h>
+/* &a[0] degrades to a pointer: a different type from an array */
+#define __must_be_array(a) __must_be(!__same_type((a), &(a)[0]))
+
/**
* ARRAY_SIZE - get the number of elements in array @a
* @a: array to be sized
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 64d99335874d..9286310d4a8b 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -254,9 +254,6 @@ static inline void *offset_to_ptr(const int *off)
#endif /* __ASSEMBLY__ */
-/* &a[0] degrades to a pointer: a different type from an array */
-#define __must_be_array(a) __must_be(!__same_type((a), &(a)[0]))
-
/*
* This is needed in functions which generate the stack canary, see
* arch/x86/kernel/smpboot.c::start_secondary() for an example.
diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index 4669632bd72b..12c6f736bcd0 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -2,9 +2,12 @@
#ifndef __LINUX_OVERFLOW_H
#define __LINUX_OVERFLOW_H
+
+#include <linux/array_size.h>
#include <linux/compiler.h>
#include <linux/limits.h>
+
/*
* We need to compute the minimum and maximum values representable in a given
* type. These macros may also be useful elsewhere. It would seem more obvious
--
2.33.1
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/array_size.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/array_size.h b/include/linux/array_size.h
index c2b4fddba564..4d364f8a1f50 100644
--- a/include/linux/array_size.h
+++ b/include/linux/array_size.h
@@ -7,7 +7,9 @@
/* &a[0] degrades to a pointer: a different type from an array */
-#define __must_be_array(a) __must_be(!__same_type((a), &(a)[0]))
+#define __is_array(a) (!__same_type((a), &(a)[0]))
+
+#define __must_be_array(a) __must_be(__is_array(a))
/**
* ARRAY_SIZE - get the number of elements in array @a
--
2.33.1
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/build_bug.h | 7 +------
include/linux/power_of_2.h | 9 +++++++++
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
index 539e3d5df027..65f64cd399be 100644
--- a/include/linux/build_bug.h
+++ b/include/linux/build_bug.h
@@ -5,14 +5,9 @@
#include <linux/compiler.h>
#include <linux/must_be.h>
+#include <linux/power_of_2.h>
-/* Force a compilation error if a constant expression is not a power of 2 */
-#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) \
- BUILD_BUG_ON(((n) & ((n) - 1)) != 0)
-#define BUILD_BUG_ON_NOT_POWER_OF_2(n) \
- BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))
-
/*
* BUILD_BUG_ON_INVALID() permits the compiler to check the validity of the
* expression but avoids the generation of any code, even if that expression
diff --git a/include/linux/power_of_2.h b/include/linux/power_of_2.h
index 812fe86eefcd..132592f0b718 100644
--- a/include/linux/power_of_2.h
+++ b/include/linux/power_of_2.h
@@ -3,8 +3,17 @@
#define _LINUX_POWER_OF_2_H
+#include <linux/build_bug.h>
+
+
#define __IS_POWER_OF_2_OR_0(n) (((n) & ((n) - 1)) == 0)
#define __IS_POWER_OF_2(n) (__IS_POWER_OF_2_OR_0(n) && ((n) != 0))
+/* Force a compilation error if a constant expression is not a power of 2 */
+#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) \
+ BUILD_BUG_ON(((n) & ((n) - 1)) != 0)
+#define BUILD_BUG_ON_NOT_POWER_OF_2(n) \
+ BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))
+
#endif /* _LINUX_POWER_OF_2_H */
--
2.33.1
Tested:
$ cat pow2.c
#include <stdio.h>
#define is_power_of_2_or_0(n) (((n) & ((n) - 1)) == 0)
#define is_power_of_2(n) (is_power_of_2_or_0(n) && ((n) != 0))
int main(void)
{
printf("%i, %i, %i\n", 8, is_power_of_2_or_0(8), is_power_of_2(8));
printf("%i, %i, %i\n", 7, is_power_of_2_or_0(7), is_power_of_2(7));
printf("%i, %i, %i\n", 6, is_power_of_2_or_0(6), is_power_of_2(6));
printf("%i, %i, %i\n", 5, is_power_of_2_or_0(5), is_power_of_2(5));
printf("%i, %i, %i\n", 4, is_power_of_2_or_0(4), is_power_of_2(4));
printf("%i, %i, %i\n", 3, is_power_of_2_or_0(3), is_power_of_2(3));
printf("%i, %i, %i\n", 2, is_power_of_2_or_0(2), is_power_of_2(2));
printf("%i, %i, %i\n", 1, is_power_of_2_or_0(1), is_power_of_2(1));
printf("%i, %i, %i\n", 0, is_power_of_2_or_0(0), is_power_of_2(0));
}
$ cc pow2.c
$ ./a.out
8, 1, 1
7, 0, 0
6, 0, 0
5, 0, 0
4, 1, 1
3, 0, 0
2, 1, 1
1, 1, 1
0, 1, 0
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/power_of_2.h | 10 ++++++++++
1 file changed, 10 insertions(+)
create mode 100644 include/linux/power_of_2.h
diff --git a/include/linux/power_of_2.h b/include/linux/power_of_2.h
new file mode 100644
index 000000000000..812fe86eefcd
--- /dev/null
+++ b/include/linux/power_of_2.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_POWER_OF_2_H
+#define _LINUX_POWER_OF_2_H
+
+
+#define __IS_POWER_OF_2_OR_0(n) (((n) & ((n) - 1)) == 0)
+#define __IS_POWER_OF_2(n) (__IS_POWER_OF_2_OR_0(n) && ((n) != 0))
+
+
+#endif /* _LINUX_POWER_OF_2_H */
--
2.33.1
Signed-off-by: Alejandro Colomar <[email protected]>
---
include/linux/power_of_2.h | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/linux/power_of_2.h b/include/linux/power_of_2.h
index 132592f0b718..80d11714b0ad 100644
--- a/include/linux/power_of_2.h
+++ b/include/linux/power_of_2.h
@@ -10,10 +10,8 @@
#define __IS_POWER_OF_2(n) (__IS_POWER_OF_2_OR_0(n) && ((n) != 0))
/* Force a compilation error if a constant expression is not a power of 2 */
-#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) \
- BUILD_BUG_ON(((n) & ((n) - 1)) != 0)
-#define BUILD_BUG_ON_NOT_POWER_OF_2(n) \
- BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))
+#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) BUILD_BUG_ON(!__IS_POWER_OF_2_OR_0(n))
+#define BUILD_BUG_ON_NOT_POWER_OF_2(n) BUILD_BUG_ON(!__IS_POWER_OF_2(n))
#endif /* _LINUX_POWER_OF_2_H */
--
2.33.1
On Sat, Nov 20, 2021 at 02:00:55PM +0100, Alejandro Colomar wrote:
> Historically, BUILD_BUG_ON_ZERO() has been hard to read.
> __must_be_array() is based on BUILD_BUG_ON_ZERO(),
> and unlike BUILD_BUG_ON_*(),
> it has a pretty readable name.
The best name is assert() which userspace uses and is standartised.
Hi Alexey,
On 11/20/21 16:05, Alexey Dobriyan wrote:
> On Sat, Nov 20, 2021 at 02:00:55PM +0100, Alejandro Colomar wrote:
>> Historically, BUILD_BUG_ON_ZERO() has been hard to read.
>> __must_be_array() is based on BUILD_BUG_ON_ZERO(),
>> and unlike BUILD_BUG_ON_*(),
>> it has a pretty readable name.
>
> The best name is assert() which userspace uses and is standartised.
Yes, assert() is almost the same thing.
In this case, it would be better named static_assert(),
since it's a compile-time assert().
However,
there's still one slight difference
between static_assert() and must_be():
static_assert() is limited;
it cannot be used in some places,
such as in the implementation of ARRAY_SIZE().
The following doesn't compile:
#define __arraycount(a) (sizeof((arr)) / sizeof((arr)[0])
#define ARRAY_SIZE(a) (__arraycount(a) + static_assert(is_array(a)))
And if you change it to be:
#define ARRAY_SIZE(a) ( \
{ \
static_assert(is_array(a)); \
__arraycount(a); \
} \
)
then the macro can't be used at file scope
(since ({}) can't be used at file scope).
The good thing about __must_be() is that it evaluates to 0,
which allows you to use it
everywhere a 0 can be used.
My own implementation of __must_be() is
more standards compliant,
and is:
#define must_be(e) ( \
0 * (int)sizeof( \
struct { \
static_assert(e); \
char ISO_C_forbids_a_struct_with_no_members__; \
} \
) \
)
I would like this to superseed
the kernel's BUILD_BUG_ON_ZERO(),
but it makes use of C2X static_assert().
I don't know how much that can be a problem.
But please consider this proposal.
Thanks,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
On Sat, Nov 20, 2021 at 3:03 PM Alejandro Colomar
<[email protected]> wrote:
>
> Place braces in a ({}) expression
> similarly to how a function would have them.
I don't think we need this change. ({ on one line is pretty much
idiomatic for macros.
--
With Best Regards,
Andy Shevchenko
On Sat, Nov 20, 2021 at 3:02 PM Alejandro Colomar
<[email protected]> wrote:
>
> Include <linux/offsetof.h> from <linux/stddef.h> for compatibility.
>
> From <linux/offsetof.h>:
> Include the same exact deps that <linux/stddef.h> had.
> Changing that in any way broke my compilation.
The commit message does not explain why you are doing this, what's the
problem it fixes.
--
With Best Regards,
Andy Shevchenko
On Sat, Nov 20, 2021 at 3:03 PM Alejandro Colomar
<[email protected]> wrote:
>
> Include <linux/sizeof_field.h> from <linux/stddef.h> for compatibility.
Why? What is this change supposed to solve?
--
With Best Regards,
Andy Shevchenko
Hi Andy,
On 11/20/21 17:14, Andy Shevchenko wrote:
> On Sat, Nov 20, 2021 at 3:02 PM Alejandro Colomar
> <[email protected]> wrote:
>>
>> Include <linux/offsetof.h> from <linux/stddef.h> for compatibility.
>>
>> From <linux/offsetof.h>:
>> Include the same exact deps that <linux/stddef.h> had.
>> Changing that in any way broke my compilation.
>
> The commit message does not explain why you are doing this, what's the
> problem it fixes.
>
Hmm, yes, after many attempts,
that one doesn't fix anything.
I think I should drop the split of offsetof[end](),
since I'm forced to include the same headers indirectly anyway.
For the rest,
such as sizeof_field(),
I'm allowed to include just a tiny header with the definition
in the following cases:
a91eb933173e tools/virtio/: Don't redefine container_of()
836786f021cc tools/usb/: Don't redefine container_of()
f51e3d656990 tools/testing/selftests/rcutorture/: Don't redefine
container_of()
99aa6a86fb21 samples/bpf/: Don't redefine container_of()
0463de8c8e86 tools/testing/selftests/vm/: Don't redefine ARRAY_SIZE()
9cda268b981d tools/testing/selftests/timens/: Don't redefine ARRAY_SIZE()
02efb34c1afa tools/testing/selftests/sparc/: Don't redefine ARRAY_SIZE()
4973f19407f0 tools/testing/selftests/seccomp/: Don't redefine ARRAY_SIZE()
ecaee90a1b3c tools/testing/selftests/rseq/: Don't redefine ARRAY_SIZE()
2392de0d71f2 tools/testing/selftests/openat2/: Don't redefine ARRAY_SIZE()
bbc657cb231d tools/testing/selftests/net/: Don't redefine ARRAY_SIZE()
f5eae2dee077 tools/testing/selftests/landlock/: Don't redefine ARRAY_SIZE()
c35ca31451ef tools/testing/selftests/kselftest_harness.h: Don't redefine
ARRAY_SIZE()
76fce3e8f3bd tools/testing/selftests/ir/: Don't redefine ARRAY_SIZE()
4f7f4632effd tools/testing/selftests/core/: Don't redefine ARRAY_SIZE()
71352d2db882 tools/testing/selftests/cgroup/: Don't redefine ARRAY_SIZE()
d2f0dc48706b tools/testing/selftests/bpf/: Don't redefine ARRAY_SIZE()
11760b380c35 tools/testing/selftests/arm64/: Don't redefine ARRAY_SIZE()
ea04d7799b42 tools/vm/: Don't redefine ARRAY_SIZE()
d5e54883d374 tools/virtio/: Don't redefine ARRAY_SIZE()
ab4f6b9fb71b tools/usb/: Don't redefine ARRAY_SIZE()
dfa264d3150d tools/spi/: Don't redefine ARRAY_SIZE()
0513d57ad00e tools/power/: Don't redefine ARRAY_SIZE()
418a8fa44788 tools/lib/traceevent/: Don't redefine ARRAY_SIZE()
bb9b11f95249 tools/iio/: Don't redefine ARRAY_SIZE()
3350c5b02479 tools/gpio/: Don't redefine ARRAY_SIZE()
dab0e175d727 tools/firewire/: Don't redefine ARRAY_SIZE()
a7457639b307 tools/bpf/: Don't redefine ARRAY_SIZE()
c651930196f1 tools/arch/: Don't redefine ARRAY_SIZE()
a2ba541db10c samples/seccomp/: Don't redefine ARRAY_SIZE()
ed5609ed8785 samples/bpf/: Don't redefine ARRAY_SIZE()
9b568cd21579 lib/zstd/: Don't redefine ARRAY_SIZE()
4f2d185b6ccc lib/mpi/: Don't redefine ARRAY_SIZE()
a14b402ce8dc include/acpi/: Don't redefine ARRAY_SIZE()
34dc6551c0a4 fs/unicode/: Don't redefine ARRAY_SIZE()
7ac3580d0d03 fs/orangefs/: Don't redefine ARRAY_SIZE()
9ff567cbf6d3 drivers/net/ethernet/: Don't redefine ARRAY_SIZE()
a8c503ade00a arch/um/: Don't redefine ARRAY_SIZE()
b5ffd6f335d2 arch/powerpc/: Don't redefine ARRAY_SIZE()
f5acf6fd58b1 arch/mips/: Don't redefine ARRAY_SIZE()
78347d718990 tools/testing/selftests/bpf/: Don't redefine NULL
d6ecbed02c1c include/acpi/: Don't redefine NULL
f6bd9b2ddfa2 drivers/video/: Don't redefine NULL
f9b15eccf7ba drivers/net/: Don't redefine NULL
eb07b8abb7b9 drivers/media/: Don't redefine NULL
f3e8e2322350 crypto/: Don't redefine NULL
888c3a451ba1 arch/parisc/: Don't redefine NULL
95a5dd02310d arch/mips/: Don't redefine NULL
0e1f3fb47007 tools/testing/selftests/bpf/: Don't redefine sizeof_field()
ddfa13d5dd76 tools/testing/selftests/bpf/: Don't redefine offsetofend()
bd393bf9fd41 tools/usb/: Don't redefine offsetof()
fd17a1d549f1 tools/testing/selftests/bpf/: Don't redefine offsetof()
f32aed18c34d samples/bpf/: Don't redefine offsetof()
b2755f698986 drivers/staging/: Don't redefine offsetof()
That is the draft that I have for patch set 2.
As an example, let me show you one of those commits:
$ git show 0e1f3fb47007
commit 0e1f3fb47007ca93df6f2b998868c892258e6ea9
Author: Alejandro Colomar <[email protected]>
Date: Fri Nov 19 18:28:30 2021 +0100
tools/testing/selftests/bpf/: Don't redefine sizeof_field()
Signed-off-by: Alejandro Colomar <[email protected]>
diff --git a/tools/testing/selftests/bpf/bpf_util.h
b/tools/testing/selftests/bpf/bpf_util.h
index 28c29292bacd..394377e4aa14 100644
--- a/tools/testing/selftests/bpf/bpf_util.h
+++ b/tools/testing/selftests/bpf/bpf_util.h
@@ -8,6 +8,7 @@
#include <errno.h>
#include <bpf/libbpf.h> /* libbpf_num_possible_cpus */
#include <linux/offsetofend.h>
+#include <linux/sizeof_field.h>
static inline unsigned int bpf_num_possible_cpus(void)
{
@@ -32,9 +33,5 @@ static inline unsigned int bpf_num_possible_cpus(void)
# define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
#endif
-#ifndef sizeof_field
-#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
-#endif
-
#endif /* __BPF_UTIL__ */
diff --git a/tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c
b/tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c
index 5f4e87ee949a..5fafb54910cd 100644
--- a/tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c
+++ b/tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c
@@ -6,6 +6,7 @@
#include <stdbool.h>
#include <sys/types.h>
#include <sys/socket.h>
+#include <linux/sizeof_field.h>
#include <linux/tcp.h>
#include <linux/socket.h>
#include <linux/bpf.h>
@@ -15,9 +16,6 @@
#define BPF_PROG_TEST_TCP_HDR_OPTIONS
#include "test_tcp_hdr_options.h"
-#ifndef sizeof_field
-#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
-#endif
__u8 test_kind = TCPOPT_EXP;
__u16 test_magic = 0xeB9F;
--
Alejandro Colomar
Linux man-pages comaintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
On Sat, 2021-11-20 at 18:12 +0200, Andy Shevchenko wrote:
> On Sat, Nov 20, 2021 at 3:03 PM Alejandro Colomar
> <[email protected]> wrote:
> >
> > Place braces in a ({}) expression
> > similarly to how a function would have them.
>
> I don't think we need this change. ({ on one line is pretty much
> idiomatic for macros.
true
$ grep-2.5.4 -rP --include=*.[ch] -oh '\(\s*\\?\s*\{' * | \
sort | uniq -c | sort -rn
2756 ({
13 {
13 ( {
4 ( \
2 ( \
2 ( \
2 ( \
1 ( \
1 ( \
1 ( \
1 ( \
1 {
Hi Joe and Andy,
On 11/20/21 17:33, Joe Perches wrote:
> On Sat, 2021-11-20 at 18:12 +0200, Andy Shevchenko wrote:
>> On Sat, Nov 20, 2021 at 3:03 PM Alejandro Colomar
>> <[email protected]> wrote:
>>>
>>> Place braces in a ({}) expression
>>> similarly to how a function would have them.
>>
>> I don't think we need this change. ({ on one line is pretty much
>> idiomatic for macros.
>
> true
>
> $ grep-2.5.4 -rP --include=*.[ch] -oh '\(\s*\\?\s*\{' * | \
> sort | uniq -c | sort -rn
> 2756 ({
> 13 {
> 13 ( {
> 4 ( \
> 2 ( \
> 2 ( \
> 2 ( \
> 1 ( \
> 1 ( \
> 1 ( \
> 1 ( \
> 1 {
>
>
Thanks,
I'll drop that one.
Cheers,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
Hi Alejandro,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linux/master linus/master v5.16-rc1 next-20211118]
[cannot apply to drm-intel/for-linux-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Alejandro-Colomar/linux-stddef-h-linux-offsetof-h-Split-offsetof-into-a-separate-header/20211120-220144
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: csky-randconfig-s031-20211118 (attached as .config)
compiler: csky-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/ed03be33a3de1708b5a06ea31cc6cd8573890649
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Alejandro-Colomar/linux-stddef-h-linux-offsetof-h-Split-offsetof-into-a-separate-header/20211120-220144
git checkout ed03be33a3de1708b5a06ea31cc6cd8573890649
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=csky
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
drivers/char/ipmi/ipmi_msghandler.c:200:17: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/char/ipmi/ipmi_msghandler.c:200:17: sparse: struct ipmi_user [noderef] __rcu *
drivers/char/ipmi/ipmi_msghandler.c:200:17: sparse: struct ipmi_user *
drivers/char/ipmi/ipmi_msghandler.c:1251:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/char/ipmi/ipmi_msghandler.c:1251:9: sparse: struct ipmi_user [noderef] __rcu *
drivers/char/ipmi/ipmi_msghandler.c:1251:9: sparse: struct ipmi_user *
drivers/char/ipmi/ipmi_msghandler.c:1320:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/char/ipmi/ipmi_msghandler.c:1320:9: sparse: struct ipmi_user [noderef] __rcu *
drivers/char/ipmi/ipmi_msghandler.c:1320:9: sparse: struct ipmi_user *
>> drivers/char/ipmi/ipmi_msghandler.c:3701:25: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void const *__mptr @@ got struct list_head [noderef] __rcu * @@
drivers/char/ipmi/ipmi_msghandler.c:203:9: sparse: sparse: context imbalance in 'acquire_ipmi_user' - different lock contexts for basic block
drivers/char/ipmi/ipmi_msghandler.c: note: in included file (through include/linux/notifier.h, include/linux/memory_hotplug.h, include/linux/mmzone.h, ...):
include/linux/srcu.h:188:9: sparse: sparse: context imbalance in 'release_ipmi_user' - unexpected unlock
drivers/char/ipmi/ipmi_msghandler.c:942:9: sparse: sparse: context imbalance in 'deliver_response' - different lock contexts for basic block
drivers/char/ipmi/ipmi_msghandler.c:1303:13: sparse: sparse: context imbalance in '_ipmi_destroy_user' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:1385:5: sparse: sparse: context imbalance in 'ipmi_get_version' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:1407:5: sparse: sparse: context imbalance in 'ipmi_set_my_address' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:1429:5: sparse: sparse: context imbalance in 'ipmi_get_my_address' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:1451:5: sparse: sparse: context imbalance in 'ipmi_set_my_LUN' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:1473:5: sparse: sparse: context imbalance in 'ipmi_get_my_LUN' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:1495:5: sparse: sparse: context imbalance in 'ipmi_get_maintenance_mode' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:1520:5: sparse: sparse: context imbalance in 'ipmi_set_maintenance_mode' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:1562:5: sparse: sparse: context imbalance in 'ipmi_set_gets_events' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:1659:5: sparse: sparse: context imbalance in 'ipmi_register_for_cmd' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:1704:5: sparse: sparse: context imbalance in 'ipmi_unregister_for_cmd' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:1869:39: sparse: sparse: context imbalance in 'smi_send' - unexpected unlock
drivers/char/ipmi/ipmi_msghandler.c:2372:5: sparse: sparse: context imbalance in 'ipmi_request_settime' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:2411:5: sparse: sparse: context imbalance in 'ipmi_request_supply_msgs' - wrong count at exit
drivers/char/ipmi/ipmi_msghandler.c:4687:39: sparse: sparse: context imbalance in 'handle_new_recv_msgs' - unexpected unlock
drivers/char/ipmi/ipmi_msghandler.c:4744:9: sparse: sparse: context imbalance in 'smi_recv_tasklet' - different lock contexts for basic block
drivers/char/ipmi/ipmi_msghandler.c:4779:39: sparse: sparse: context imbalance in 'ipmi_smi_msg_received' - unexpected unlock
drivers/char/ipmi/ipmi_msghandler.c:4875:39: sparse: sparse: context imbalance in 'check_msg_timeout' - unexpected unlock
vim +3701 drivers/char/ipmi/ipmi_msghandler.c
b2c03941b50944 Corey Minyard 2006-12-06 3674
6a0d23ed338ed7 Corey Minyard 2018-04-11 3675 void ipmi_unregister_smi(struct ipmi_smi *intf)
^1da177e4c3f41 Linus Torvalds 2005-04-16 3676 {
^1da177e4c3f41 Linus Torvalds 2005-04-16 3677 struct ipmi_smi_watcher *w;
e86ee2d44b4405 Corey Minyard 2018-04-05 3678 int intf_num = intf->intf_num, index;
^1da177e4c3f41 Linus Torvalds 2005-04-16 3679
bca0324d09e413 Corey Minyard 2006-12-06 3680 mutex_lock(&ipmi_interfaces_mutex);
b2c03941b50944 Corey Minyard 2006-12-06 3681 intf->intf_num = -1;
7ea0ed2b5be817 Corey Minyard 2014-11-06 3682 intf->in_shutdown = true;
bca0324d09e413 Corey Minyard 2006-12-06 3683 list_del_rcu(&intf->link);
bca0324d09e413 Corey Minyard 2006-12-06 3684 mutex_unlock(&ipmi_interfaces_mutex);
e86ee2d44b4405 Corey Minyard 2018-04-05 3685 synchronize_srcu(&ipmi_interfaces_srcu);
^1da177e4c3f41 Linus Torvalds 2005-04-16 3686
e86ee2d44b4405 Corey Minyard 2018-04-05 3687 /* At this point no users can be added to the interface. */
^1da177e4c3f41 Linus Torvalds 2005-04-16 3688
c70d749986f6f1 Corey Minyard 2008-04-29 3689 /*
c70d749986f6f1 Corey Minyard 2008-04-29 3690 * Call all the watcher interfaces to tell them that
e86ee2d44b4405 Corey Minyard 2018-04-05 3691 * an interface is going away.
c70d749986f6f1 Corey Minyard 2008-04-29 3692 */
e86ee2d44b4405 Corey Minyard 2018-04-05 3693 mutex_lock(&smi_watchers_mutex);
393d2cc354d150 Corey Minyard 2005-11-07 3694 list_for_each_entry(w, &smi_watchers, link)
b2c03941b50944 Corey Minyard 2006-12-06 3695 w->smi_gone(intf_num);
b2c03941b50944 Corey Minyard 2006-12-06 3696 mutex_unlock(&smi_watchers_mutex);
393d2cc354d150 Corey Minyard 2005-11-07 3697
e86ee2d44b4405 Corey Minyard 2018-04-05 3698 index = srcu_read_lock(&intf->users_srcu);
e86ee2d44b4405 Corey Minyard 2018-04-05 3699 while (!list_empty(&intf->users)) {
e86ee2d44b4405 Corey Minyard 2018-04-05 3700 struct ipmi_user *user =
e86ee2d44b4405 Corey Minyard 2018-04-05 @3701 container_of(list_next_rcu(&intf->users),
e86ee2d44b4405 Corey Minyard 2018-04-05 3702 struct ipmi_user, link);
e86ee2d44b4405 Corey Minyard 2018-04-05 3703
e86ee2d44b4405 Corey Minyard 2018-04-05 3704 _ipmi_destroy_user(user);
e86ee2d44b4405 Corey Minyard 2018-04-05 3705 }
e86ee2d44b4405 Corey Minyard 2018-04-05 3706 srcu_read_unlock(&intf->users_srcu, index);
e86ee2d44b4405 Corey Minyard 2018-04-05 3707
2512e40e48d21d Corey Minyard 2018-08-22 3708 if (intf->handlers->shutdown)
e86ee2d44b4405 Corey Minyard 2018-04-05 3709 intf->handlers->shutdown(intf->send_info);
e86ee2d44b4405 Corey Minyard 2018-04-05 3710
e86ee2d44b4405 Corey Minyard 2018-04-05 3711 cleanup_smi_msgs(intf);
e86ee2d44b4405 Corey Minyard 2018-04-05 3712
e86ee2d44b4405 Corey Minyard 2018-04-05 3713 ipmi_bmc_unregister(intf);
e86ee2d44b4405 Corey Minyard 2018-04-05 3714
e86ee2d44b4405 Corey Minyard 2018-04-05 3715 cleanup_srcu_struct(&intf->users_srcu);
393d2cc354d150 Corey Minyard 2005-11-07 3716 kref_put(&intf->refcount, intf_free);
^1da177e4c3f41 Linus Torvalds 2005-04-16 3717 }
c70d749986f6f1 Corey Minyard 2008-04-29 3718 EXPORT_SYMBOL(ipmi_unregister_smi);
^1da177e4c3f41 Linus Torvalds 2005-04-16 3719
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Sat, Nov 20, 2021 at 4:05 PM Alexey Dobriyan <[email protected]> wrote:
>
> On Sat, Nov 20, 2021 at 02:00:55PM +0100, Alejandro Colomar wrote:
> > Historically, BUILD_BUG_ON_ZERO() has been hard to read.
> > __must_be_array() is based on BUILD_BUG_ON_ZERO(),
> > and unlike BUILD_BUG_ON_*(),
> > it has a pretty readable name.
>
> The best name is assert() which userspace uses and is standartised.
I would argue that this macro is best left out: we have BUILD_BUG_ON()
as the interface that everyone knows, having another macro that has the
same results only makes things more confusing, and I would ask anyone
using it to use an open-coded BUILD_BUG_ON instead.
Arnd
On Sat, Nov 20, 2021 at 2:01 PM Alejandro Colomar
<[email protected]> wrote:
> +
> +#define __IS_POWER_OF_2_OR_0(n) (((n) & ((n) - 1)) == 0)
> +#define __IS_POWER_OF_2(n) (__IS_POWER_OF_2_OR_0(n) && ((n) != 0))
> +
There is already is_power_of_2() in include/linux/log2.h, which would
be preferred
in most cases. If you need a macro version, put it in the same file
and explain why it's
needed.
Arnd
On Sat, Nov 20, 2021 at 2:01 PM Alejandro Colomar
<[email protected]> wrote:
>
> /* &a[0] degrades to a pointer: a different type from an array */
> -#define __must_be_array(a) __must_be(!__same_type((a), &(a)[0]))
> +#define __is_array(a) (!__same_type((a), &(a)[0]))
> +
> +#define __must_be_array(a) __must_be(__is_array(a))
I'd go the other way here if you want to improve this, and open-code it
to avoid nested macros. That produces more readable error messages
with recent compilers that try to unwind each macro one at a time.
Arnd
On Sat, Nov 20, 2021 at 2:01 PM Alejandro Colomar
<[email protected]> wrote:
>
> Signed-off-by: Alejandro Colomar <[email protected]>
Each patch should describe why you move this, it's not clear what the
benefit is here.
> diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
> index 7a5925072466..f6fc1c72dfa7 100644
> --- a/drivers/gpu/drm/i915/i915_utils.h
> +++ b/drivers/gpu/drm/i915/i915_utils.h
> @@ -25,6 +25,8 @@
> #ifndef __I915_UTILS_H
> #define __I915_UTILS_H
>
> +
> +#include <linux/array_size.h>
Avoid adding the extra whitespace here.
Arnd
On Sat, Nov 20, 2021 at 2:00 PM Alejandro Colomar
<[email protected]> wrote:
> @@ -255,7 +255,7 @@ static inline void *offset_to_ptr(const int *off)
> #endif /* __ASSEMBLY__ */
>
> /* &a[0] degrades to a pointer: a different type from an array */
> -#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
> +#define __must_be_array(a) __must_be(!__same_type((a), &(a)[0]))
Please go the other way here and change all users of __must_be() to use
BUILD_BUG_ON_ZERO() or BUILD_BUG_ON() for consistency.
Arnd
On Sat, Nov 20, 2021 at 2:00 PM Alejandro Colomar
<[email protected]> wrote:
> --- /dev/null
> +++ b/include/linux/same_type.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __LINUX_SAME_TYPE_H
> +#define __LINUX_SAME_TYPE_H
> +
> +
> +/* Are two types/vars the same type (ignoring qualifiers)? */
> +#define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
> +
Having a separate header for this is really silly. Please find a
header with similar
definitions that you can add this to.
Arnd
On Sat, Nov 20, 2021 at 2:00 PM Alejandro Colomar
<[email protected]> wrote:
>
> There's no more a need for the temporary variable __mptr,
> since now it's only passed to functions that accept a 'const void *',
> and everything can convert automatically to it,
> reducing the need for the cast too.
The purpose of the temporary variable is to avoid evaluating the macro
multiple times, this is still required.
> * If IS_ERR_OR_NULL(ptr), ptr is returned unchanged.
> */
> -#define container_of_safe(ptr, type, member) ({ \
> - void *__mptr = (void *)(ptr); \
> - static_assert(__same_type(*(ptr), ((type *)0)->member) || \
> - __same_type(*(ptr), void), \
> - "pointer type mismatch in container_of_safe()"); \
> - IS_ERR_OR_NULL(__mptr) ? ERR_CAST(__mptr) : \
> - ((type *)(__mptr - offsetof(type, member))); })
> +#define container_of_safe(ptr, type, member) \
> + (IS_ERR_OR_NULL(ptr) ? ERR_CAST(ptr) : container_of(type, member))
It's possible that you may find a way to improve this macro using
__auto_type, as a way to reduce the macro expansion further.
Arnd
Hi Alejandro,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linux/master linus/master v5.16-rc2 next-20211118]
[cannot apply to drm-intel/for-linux-next mkl-can-next/testing]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Alejandro-Colomar/linux-stddef-h-linux-offsetof-h-Split-offsetof-into-a-separate-header/20211120-220144
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: ia64-randconfig-r016-20211122 (attached as .config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/f4bee28948d96faeac8bdf5ac6d2abdd718d1db3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Alejandro-Colomar/linux-stddef-h-linux-offsetof-h-Split-offsetof-into-a-separate-header/20211120-220144
git checkout f4bee28948d96faeac8bdf5ac6d2abdd718d1db3
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=ia64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:6,
from include/linux/cpumask.h:10,
from include/linux/mm_types_task.h:14,
from include/linux/mm_types.h:5,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from drivers/net/wireless/intel/iwlwifi/pcie/drv.c:9:
drivers/net/wireless/intel/iwlwifi/pcie/drv.c: In function 'iwl_pci_find_dev_info':
>> include/linux/array_size.h:12:24: warning: overflow in conversion from 'long unsigned int' to 'int' changes value from '18446744073709551615' to '-1' [-Woverflow]
12 | #define ARRAY_SIZE(a) (sizeof((a)) / sizeof((a)[0]) + __must_be_array(a))
| ^
drivers/net/wireless/intel/iwlwifi/pcie/drv.c:1344:18: note: in expansion of macro 'ARRAY_SIZE'
1344 | for (i = ARRAY_SIZE(iwl_dev_info_table) - 1; i >= 0; i--) {
| ^~~~~~~~~~
vim +12 include/linux/array_size.h
6
7
8 /**
9 * ARRAY_SIZE - get the number of elements in array @a
10 * @a: array to be sized
11 */
> 12 #define ARRAY_SIZE(a) (sizeof((a)) / sizeof((a)[0]) + __must_be_array(a))
13
14
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Sun, 21 Nov 2021, Arnd Bergmann <[email protected]> wrote:
> On Sat, Nov 20, 2021 at 2:01 PM Alejandro Colomar
> <[email protected]> wrote:
>>
>> Signed-off-by: Alejandro Colomar <[email protected]>
>
> Each patch should describe why you move this, it's not clear what the
> benefit is here.
>
>> diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
>> index 7a5925072466..f6fc1c72dfa7 100644
>> --- a/drivers/gpu/drm/i915/i915_utils.h
>> +++ b/drivers/gpu/drm/i915/i915_utils.h
>> @@ -25,6 +25,8 @@
>> #ifndef __I915_UTILS_H
>> #define __I915_UTILS_H
>>
>> +
>> +#include <linux/array_size.h>
>
> Avoid adding the extra whitespace here.
And throughout the series, really. In particular, please avoid adding
the double blank lines both in old and new files.
BR,
Jani.
>
> Arnd
--
Jani Nikula, Intel Open Source Graphics Center
On Sun, 21 Nov 2021, Arnd Bergmann <[email protected]> wrote:
> On Sat, Nov 20, 2021 at 2:01 PM Alejandro Colomar
> <[email protected]> wrote:
>> +
>> +#define __IS_POWER_OF_2_OR_0(n) (((n) & ((n) - 1)) == 0)
>> +#define __IS_POWER_OF_2(n) (__IS_POWER_OF_2_OR_0(n) && ((n) != 0))
>> +
>
> There is already is_power_of_2() in include/linux/log2.h, which would
> be preferred
> in most cases. If you need a macro version, put it in the same file
> and explain why it's
> needed.
Also, the macro argument n is evaluated 2-3 times. Please use
checkpatch.pl --strict argument on the patches, and it'll give you hints
about this stuff too.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
On Sat, Nov 20, 2021 at 02:00:43PM +0100, Alejandro Colomar wrote:
>
> Hi all,
>
> I splitted some macros into separate headers,
> to be able to use them
> without pulling too many deps.
>
> I also simplified some of themr
> to be implemented in terms of the others
> and to remove some unnecessary explicit casts.
>
> And I added memberof(),
> which gives name to a typical construction
> to get the member of a struct
> without needing a variable of that type.
>
>
> The next step after this patch set
> is another one removing all redefinitions
> (at least all that are possible,
> since these headers can't be included everywhere)
> of these macros,
> by including these new tiny headers.
> Since these headers are so tiny and bring no dependencies,
> they should break anything.
>
> It was hard for me to get this working
> because the order of includes _matters a lot_,
> and which headers you include _matters_ even outside of uapi.
> So I think this should help fix that,
> by allowing headers to pull exactly what they want,
> without all of the stuff that came with
> <linux/compiler.h>
> <linux/compiler_types.h>
> <linux/stddef.h>.
>
> I already have much of the next patch set ready,
> and it removes hundreds of redefinitions of these macros,
> which should be a good thing.
>
>
> Then,
> when there are (almost) no redefinitions of these macros,
> I'll prepare a 3rd patch set that
> explicitly includes these tiny headers
> wherever these macros were already in use,
> to allow for removal of other bigger headers
> (although I won't remove anything,
> to avoid silently breaking anything).
>
>
> And then,
> a 4th patch set will
> attempt to find all uses of these macros
> that were not even named
> (i.e., hard-coded sizeof divisions).
>
>
> Hope this is clear and
> that you like these changes.
What happens to the indentation in your emails?!
It looks like a bad poem :-)
On top of that, never start a new thread inside the previous one.
--
With Best Regards,
Andy Shevchenko
On 11/22/21 12:17, Andy Shevchenko wrote:
>
> What happens to the indentation in your emails?!
> It looks like a bad poem :-)
Ahh, hehe!
It's a habit of writing manual pages source code. It makes it easier to
edit the ideas while writing.
Recently, I learned that this goes back to Kernighan:
Brian W. Kernighan, 1974 [UNIX For Beginners]:
[
Hints for Preparing Documents
Most documents go through several versions
(always more than you expected)
before they are finally finished.
Accordingly,
you should do whatever possible
to make the job of changing them easy.
First,
when you do the purely mechanical operations of typing,
type so subsequent editing will be easy.
Start each sentence on a new line.
Make lines short,
and break lines at natural places,
such as after commas and semicolons,
rather than randomly.
Since most people change documents
by rewriting phrases and
adding, deleting and rearranging sentences,
these precautions simplify any editing you have to do later.
]
See <https://rhodesmill.org/brandon/2012/one-sentence-per-line/>.
>
> On top of that, never start a new thread inside the previous one.
>
ACK.
Thanks,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
On Fri, 19 Nov 2021 18:35:26 +0200
Andy Shevchenko <[email protected]> wrote:
> On Fri, Nov 19, 2021 at 05:22:48PM +0100, Alejandro Colomar (man-pages) wrote:
> >
> >
> > On 11/19/21 17:18, Arnd Bergmann wrote:
> > > On Fri, Nov 19, 2021 at 5:10 PM Andy Shevchenko
> > > <[email protected]> wrote:
> > >> On Fri, Nov 19, 2021 at 04:57:46PM +0100, Arnd Bergmann wrote:
> > >
> > >>> The main problem with this approach is that as soon as you start
> > >>> actually reducing the unneeded indirect includes, you end up with
> > >>> countless .c files that no longer build because they are missing a
> > >>> direct include for something that was always included somewhere
> > >>> deep underneath, so I needed a second set of scripts to add
> > >>> direct includes to every .c file.
> > >>
> > >> Can't it be done with cocci support?
> > >
> > > There are many ways of doing it, but they all tend to suffer from the
> > > problem of identifying which headers are actually needed based on
> > > the contents of a file, and also figuring out where to put the extra
> > > #include if there are complex #ifdefs.
> > >
> > > For reference, see below for the naive pattern matching I tried.
> > > This is obviously incomplete and partially wrong.
> >
> > FYI, if you may not know the tool,
> > theres include-what-you-use(1) (a.k.a. iwyu(1))[1],
> > although it is still not mature,
> > and I'm helping improve it a bit.
>
> Yes, I know the tool, but it produces insanity. Jonathan (maintainer
> of IIO subsystem) actually found it useful after manual work applied.
> Perhaps you can chat with him about usage of it in the Linux kernel.
IIO drivers use a fairly limited subset of headers, so it wasn't implausible
to produce a mapping file to get to fairly sane results.
https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=iio-iwyu-cleanups&id=8bc7ff8d5385e89a5199e792fda18dbf2ca8f2e5
If we did head towards a general mapping file that 'more or less made sense'
then maybe this tool can be fairly useful kernel wide.
Typical patch that results in clean checks with that mapping file is:
https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=iio-iwyu-cleanups&id=0eff2dd097add84c464710003c3bc9929f646427
There are always going to be questions of how many of the really low level
direct includes make sense though which was the stumbling block.
It is nice to get rid of the pointless includes though for things that due
to refactors etc are no longer used in a file, or were cut and paste from
another driver.
I've paused efforts on this front for now given series like this one can
have significant impact and it seems to be an active area at the moment.
Might revisit later this cycle.
Jonathan
>
> > If I understood better the kernel Makefiles,
> > I'd try it.
> >
> > You can try it yourselves.
> > I still can't use it for my own code,
> > since it has a lot of false positives.
>
> > [1]: <https://include-what-you-use.org/>
>
Hi Alejandro,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linux/master linus/master v5.16-rc2 next-20211118]
[cannot apply to drm-intel/for-linux-next mkl-can-next/testing]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Alejandro-Colomar/linux-stddef-h-linux-offsetof-h-Split-offsetof-into-a-separate-header/20211120-220144
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: mips-randconfig-s031-20211122 (attached as .config)
compiler: mips64-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/ed03be33a3de1708b5a06ea31cc6cd8573890649
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Alejandro-Colomar/linux-stddef-h-linux-offsetof-h-Split-offsetof-into-a-separate-header/20211120-220144
git checkout ed03be33a3de1708b5a06ea31cc6cd8573890649
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
command-line: note: in included file:
builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_ACQUIRE redefined
builtin:0:0: sparse: this was the original definition
builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_SEQ_CST redefined
builtin:0:0: sparse: this was the original definition
builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_ACQ_REL redefined
builtin:0:0: sparse: this was the original definition
builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_RELEASE redefined
builtin:0:0: sparse: this was the original definition
arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space '__user' of expression
arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space '__user' of expression
arch/mips/kernel/signal.c:280:13: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected unsigned int [noderef] __user *__pu_ptr @@ got unsigned int [usertype] * @@
arch/mips/kernel/signal.c:280:13: sparse: expected unsigned int [noderef] __user *__pu_ptr
arch/mips/kernel/signal.c:280:13: sparse: got unsigned int [usertype] *
arch/mips/kernel/signal.c:280:13: sparse: sparse: cast removes address space '__user' of expression
arch/mips/kernel/signal.c:293:23: sparse: sparse: cast removes address space '__user' of expression
arch/mips/kernel/signal.c:293:23: sparse: sparse: cast removes address space '__user' of expression
arch/mips/kernel/signal.c:293:23: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected unsigned int const [noderef] __user *__gu_ptr @@ got unsigned int * @@
arch/mips/kernel/signal.c:293:23: sparse: expected unsigned int const [noderef] __user *__gu_ptr
arch/mips/kernel/signal.c:293:23: sparse: got unsigned int *
arch/mips/kernel/signal.c:300:23: sparse: sparse: cast removes address space '__user' of expression
arch/mips/kernel/signal.c:300:23: sparse: sparse: cast removes address space '__user' of expression
arch/mips/kernel/signal.c:300:23: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected unsigned int const [noderef] __user *__gu_ptr @@ got unsigned int * @@
arch/mips/kernel/signal.c:300:23: sparse: expected unsigned int const [noderef] __user *__gu_ptr
arch/mips/kernel/signal.c:300:23: sparse: got unsigned int *
arch/mips/kernel/signal.c:667:17: sparse: sparse: symbol 'sys_rt_sigreturn' was not declared. Should it be static?
>> arch/mips/kernel/signal.c:157:14: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void const *__mptr @@ got void [noderef] __user *sc @@
arch/mips/kernel/signal.c:157:14: sparse: expected void const *__mptr
arch/mips/kernel/signal.c:157:14: sparse: got void [noderef] __user *sc
arch/mips/kernel/signal.c:157:12: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct ucontext [noderef] __user *uc @@ got struct ucontext * @@
arch/mips/kernel/signal.c:157:12: sparse: expected struct ucontext [noderef] __user *uc
arch/mips/kernel/signal.c:157:12: sparse: got struct ucontext *
>> arch/mips/kernel/signal.c:157:14: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void const *__mptr @@ got void [noderef] __user *sc @@
arch/mips/kernel/signal.c:157:14: sparse: expected void const *__mptr
arch/mips/kernel/signal.c:157:14: sparse: got void [noderef] __user *sc
arch/mips/kernel/signal.c:157:12: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct ucontext [noderef] __user *uc @@ got struct ucontext * @@
arch/mips/kernel/signal.c:157:12: sparse: expected struct ucontext [noderef] __user *uc
arch/mips/kernel/signal.c:157:12: sparse: got struct ucontext *
vim +157 arch/mips/kernel/signal.c
2db9ca0a355100 Paul Burton 2015-07-27 143
bf82cb30c7e58b Paul Burton 2015-07-27 144 /*
bf82cb30c7e58b Paul Burton 2015-07-27 145 * Extended context handling.
bf82cb30c7e58b Paul Burton 2015-07-27 146 */
bf82cb30c7e58b Paul Burton 2015-07-27 147
bf82cb30c7e58b Paul Burton 2015-07-27 148 static inline void __user *sc_to_extcontext(void __user *sc)
bf82cb30c7e58b Paul Burton 2015-07-27 149 {
bf82cb30c7e58b Paul Burton 2015-07-27 150 struct ucontext __user *uc;
bf82cb30c7e58b Paul Burton 2015-07-27 151
bf82cb30c7e58b Paul Burton 2015-07-27 152 /*
bf82cb30c7e58b Paul Burton 2015-07-27 153 * We can just pretend the sigcontext is always embedded in a struct
bf82cb30c7e58b Paul Burton 2015-07-27 154 * ucontext here, because the offset from sigcontext to extended
bf82cb30c7e58b Paul Burton 2015-07-27 155 * context is the same in the struct sigframe case.
bf82cb30c7e58b Paul Burton 2015-07-27 156 */
bf82cb30c7e58b Paul Burton 2015-07-27 @157 uc = container_of(sc, struct ucontext, uc_mcontext);
bf82cb30c7e58b Paul Burton 2015-07-27 158 return &uc->uc_extcontext;
bf82cb30c7e58b Paul Burton 2015-07-27 159 }
bf82cb30c7e58b Paul Burton 2015-07-27 160
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Fri, Nov 19, 2021 at 12:37 PM Alejandro Colomar
<[email protected]> wrote:
>
> Signed-off-by: Alejandro Colomar <[email protected]>
> Cc: Ajit Khaparde <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Andy Shevchenko <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Corey Minyard <[email protected]>
> Cc: Chris Mason <[email protected]>
> Cc: Christian Brauner <[email protected]>
> Cc: David Sterba <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Cc: Jason Wang <[email protected]>
> Cc: Jitendra Bhivare <[email protected]>
> Cc: John Hubbard <[email protected]>
> Cc: John S. Gruber <[email protected]>
> Cc: Jonathan Cameron <[email protected]>
> Cc: Joonas Lahtinen <[email protected]>
> Cc: Josef Bacik <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Ketan Mukadam <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: "Michael S. Tsirkin" <[email protected]>
> Cc: Miguel Ojeda <[email protected]>
> Cc: Mike Rapoport <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Rasmus Villemoes <[email protected]>
> Cc: Rodrigo Vivi <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Somnath Kotur <[email protected]>
> Cc: Sriharsha Basavapatna <[email protected]>
> Cc: Subbu Seetharaman <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> ---
> arch/x86/include/asm/bootparam_utils.h | 3 ++-
> arch/x86/kernel/signal_compat.c | 5 +++--
> drivers/gpu/drm/i915/i915_utils.h | 5 ++---
> drivers/gpu/drm/i915/intel_runtime_pm.h | 2 +-
> drivers/net/ethernet/emulex/benet/be.h | 7 ++++---
> drivers/net/ethernet/i825xx/ether1.c | 7 +++++--
> drivers/scsi/be2iscsi/be.h | 7 ++++---
> drivers/scsi/be2iscsi/be_cmds.h | 5 ++++-
> fs/btrfs/ctree.h | 5 +++--
> include/acpi/actypes.h | 4 +++-
The change in actypes.h would need to be submitted to the upstream
ACPICA project via https://github.com/acpica/acpica/
Thanks!
> include/linux/container_of.h | 6 +++---
> include/linux/virtio_config.h | 14 +++++++-------
> 12 files changed, 41 insertions(+), 29 deletions(-)