| Message ID | 20211119113644.1600-1-alx.manpages@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add memberof(), split some headers, and slightly simplify code | expand |
On Fri, 19 Nov 2021, Alejandro Colomar <alx.manpages@gmail.com> 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
Hi Jani, On 11/19/21 13:47, Jani Nikula wrote: > On Fri, 19 Nov 2021, Alejandro Colomar <alx.manpages@gmail.com> 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 >
On Fri, 19 Nov 2021, "Alejandro Colomar (man-pages)" <alx.manpages@gmail.com> wrote: > Hi Jani, > > On 11/19/21 13:47, Jani Nikula wrote: >> On Fri, 19 Nov 2021, Alejandro Colomar <alx.manpages@gmail.com> 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 >>
On Fri, Nov 19, 2021 at 12:36 PM Alejandro Colomar <alx.manpages@gmail.com> 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 <alx.manpages@gmail.com> 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...
Hi Arnd, On 11/19/21 15:47, Arnd Bergmann wrote: > On Fri, Nov 19, 2021 at 12:36 PM Alejandro Colomar > <alx.manpages@gmail.com> 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
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 > > <alx.manpages@gmail.com> 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.
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
On Fri, Nov 19, 2021 at 4:06 PM Alejandro Colomar (man-pages) <alx.manpages@gmail.com> 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:57:46PM +0100, Arnd Bergmann wrote: > On Fri, Nov 19, 2021 at 4:06 PM Alejandro Colomar (man-pages) > <alx.manpages@gmail.com> 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
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
On Fri, Nov 19, 2021 at 5:10 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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 > <andriy.shevchenko@linux.intel.com> 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/>
On Fri, Nov 19, 2021 at 5:12 PM Alejandro Colomar (man-pages) <alx.manpages@gmail.com> 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) <alx.manpages@gmail.com> wrote: > On 11/19/21 17:18, Arnd Bergmann wrote: > > On Fri, Nov 19, 2021 at 5:10 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> 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 > > <andriy.shevchenko@linux.intel.com> 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/>
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?
On Fri, 19 Nov 2021 18:35:26 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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 > > > <andriy.shevchenko@linux.intel.com> 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 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