Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[Jani Nikula]

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

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[Jani Nikula]

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
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[Andy Shevchenko]

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...

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[Arnd Bergmann]

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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[Arnd Bergmann]

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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[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)
> <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

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[Arnd Bergmann]

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"
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[Arnd Bergmann]

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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[Andy Shevchenko]

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/>

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[Arnd Bergmann]

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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

[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


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 02/17] Use memberof(T, m) instead of explicit NULL dereference

[Rafael J. Wysocki]

On Fri, Nov 19, 2021 at 12:37 PM Alejandro Colomar
<alx.manpages@gmail.com> wrote:
>
> Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
> Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Corey Minyard <cminyard@mvista.com>
> Cc: Chris Mason <clm@fb.com>
> Cc: Christian Brauner <christian.brauner@ubuntu.com>
> Cc: David Sterba <dsterba@suse.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
> Cc: John Hubbard <jhubbard@nvidia.com>
> Cc: John S. Gruber <JohnSGruber@gmail.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Josef Bacik <josef@toxicpanda.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Ketan Mukadam <ketan.mukadam@broadcom.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Mike Rapoport <rppt@linux.ibm.com>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Somnath Kotur <somnath.kotur@broadcom.com>
> Cc: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
> Cc: Subbu Seetharaman <subbu.seetharaman@broadcom.com>
> Cc: <intel-gfx@lists.freedesktop.org>
> Cc: <linux-acpi@vger.kernel.org>
> Cc: <linux-arm-kernel@lists.infradead.org>
> Cc: <linux-btrfs@vger.kernel.org>
> Cc: <linux-scsi@vger.kernel.org>
> Cc: <netdev@vger.kernel.org>
> Cc: <virtualization@lists.linux-foundation.org>
> ---
>  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(-)
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization