On Fri, 19 Nov 2021, "Alejandro Colomar (man-pages)" <alx.manpages@xxxxxxxxx> wrote: > Hi Jani, > > On 11/19/21 13:47, Jani Nikula wrote: >> On Fri, 19 Nov 2021, Alejandro Colomar <alx.manpages@xxxxxxxxx> 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
