| Message ID | 20230911154534.4174265-2-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v1,1/2] net: core: Use the bitmap API to allocate bitmaps | expand |
On Mon, Sep 11, 2023 at 06:45:34PM +0300, Andy Shevchenko wrote: > It's rather a gigantic list of heards that is very hard to follow. > Sorting helps to see what's already included and what's not. > It improves a maintainability in a long term. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Hi Andy, At the risk of bike shedding, the sort function of Vim, when operating with the C locale, gives a slightly different order, as experssed by this incremental diff. I have no objections to your oder, but I'm slightly curious as to how it came about. diff --git a/net/core/dev.c b/net/core/dev.c index d795a6c5a591..770138babf7e 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -92,9 +92,9 @@ #include <linux/if_ether.h> #include <linux/if_macvlan.h> #include <linux/if_vlan.h> +#include <linux/in.h> #include <linux/indirect_call_wrapper.h> #include <linux/inetdevice.h> -#include <linux/in.h> #include <linux/init.h> #include <linux/interrupt.h> #include <linux/ip.h> @@ -105,9 +105,9 @@ #include <linux/mm.h> #include <linux/module.h> #include <linux/mutex.h> +#include <linux/net_namespace.h> #include <linux/netdevice.h> #include <linux/netfilter_netdev.h> -#include <linux/net_namespace.h> #include <linux/netpoll.h> #include <linux/once_lite.h> #include <linux/pm_runtime.h> @@ -142,8 +142,8 @@ #include <net/ip.h> #include <net/iw_handler.h> #include <net/mpls.h> -#include <net/netdev_rx_queue.h> #include <net/net_namespace.h> +#include <net/netdev_rx_queue.h> #include <net/pkt_cls.h> #include <net/pkt_sched.h> #include <net/sock.h>
On Tue, Sep 12, 2023 at 05:20:31PM +0200, Simon Horman wrote: > On Mon, Sep 11, 2023 at 06:45:34PM +0300, Andy Shevchenko wrote: > > It's rather a gigantic list of heards that is very hard to follow. > > Sorting helps to see what's already included and what's not. > > It improves a maintainability in a long term. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Hi Andy, > > At the risk of bike shedding, the sort function of Vim, when operating > with the C locale, gives a slightly different order, as experssed by > this incremental diff. > > I have no objections to your oder, but I'm slightly curious as > to how it came about. !sort which is external command. $ locale -k LC_COLLATE collate-nrules=4 collate-rulesets="" collate-symb-hash-sizemb=1303 collate-codeset="UTF-8"
On Tue, 2023-09-12 at 19:35 +0300, Andy Shevchenko wrote: > On Tue, Sep 12, 2023 at 05:20:31PM +0200, Simon Horman wrote: > > On Mon, Sep 11, 2023 at 06:45:34PM +0300, Andy Shevchenko wrote: > > > It's rather a gigantic list of heards that is very hard to follow. > > > Sorting helps to see what's already included and what's not. > > > It improves a maintainability in a long term. > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > Hi Andy, > > > > At the risk of bike shedding, the sort function of Vim, when operating > > with the C locale, gives a slightly different order, as experssed by > > this incremental diff. > > > > I have no objections to your oder, but I'm slightly curious as > > to how it came about. > > !sort which is external command. > > $ locale -k LC_COLLATE > collate-nrules=4 > collate-rulesets="" > collate-symb-hash-sizemb=1303 > collate-codeset="UTF-8" I'm unsure this change is worthy. It will make any later fix touching the header list more difficult to backport, and I don't see a great direct advantage. Please repost the first patch standalone. Thanks, Paolo
On Tue, Sep 12, 2023 at 06:53:23PM +0200, Paolo Abeni wrote: > On Tue, 2023-09-12 at 19:35 +0300, Andy Shevchenko wrote: > > On Tue, Sep 12, 2023 at 05:20:31PM +0200, Simon Horman wrote: > > > On Mon, Sep 11, 2023 at 06:45:34PM +0300, Andy Shevchenko wrote: > > > > It's rather a gigantic list of heards that is very hard to follow. > > > > Sorting helps to see what's already included and what's not. > > > > It improves a maintainability in a long term. > > > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > > > Hi Andy, > > > > > > At the risk of bike shedding, the sort function of Vim, when operating > > > with the C locale, gives a slightly different order, as experssed by > > > this incremental diff. > > > > > > I have no objections to your oder, but I'm slightly curious as > > > to how it came about. > > > > !sort which is external command. > > > > $ locale -k LC_COLLATE > > collate-nrules=4 > > collate-rulesets="" > > collate-symb-hash-sizemb=1303 > > collate-codeset="UTF-8" > > I'm unsure this change is worthy. It will make any later fix touching > the header list more difficult to backport, and I don't see a great > direct advantage. As Rasmus put it here https://lore.kernel.org/lkml/5eca0ab5-84be-2d8f-e0b3-c9fdfa961826@rasmusvillemoes.dk/ In short term you can argue that it's not beneficial, but in long term it's given less conflicts. > Please repost the first patch standalone. Why to repost, what did I miss? It's available via lore, just run b4 am -slt -P _ 20230911154534.4174265-1-andriy.shevchenko@linux.intel.com to get it :-)
On Tue, Sep 12, 2023 at 10:05 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Sep 12, 2023 at 06:53:23PM +0200, Paolo Abeni wrote: > > On Tue, 2023-09-12 at 19:35 +0300, Andy Shevchenko wrote: > > > On Tue, Sep 12, 2023 at 05:20:31PM +0200, Simon Horman wrote: > > > > On Mon, Sep 11, 2023 at 06:45:34PM +0300, Andy Shevchenko wrote: > > > > > It's rather a gigantic list of heards that is very hard to follow. > > > > > Sorting helps to see what's already included and what's not. > > > > > It improves a maintainability in a long term. > > > > > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > > > > > Hi Andy, > > > > > > > > At the risk of bike shedding, the sort function of Vim, when operating > > > > with the C locale, gives a slightly different order, as experssed by > > > > this incremental diff. > > > > > > > > I have no objections to your oder, but I'm slightly curious as > > > > to how it came about. > > > > > > !sort which is external command. > > > > > > $ locale -k LC_COLLATE > > > collate-nrules=4 > > > collate-rulesets="" > > > collate-symb-hash-sizemb=1303 > > > collate-codeset="UTF-8" > > > > I'm unsure this change is worthy. It will make any later fix touching > > the header list more difficult to backport, and I don't see a great > > direct advantage. > > As Rasmus put it here > https://lore.kernel.org/lkml/5eca0ab5-84be-2d8f-e0b3-c9fdfa961826@rasmusvillemoes.dk/ > In short term you can argue that it's not beneficial, but in long term it's given > less conflicts. I agree with Paolo. This is just code churn. The includes will become unsorted eventually. Headers might get renamed, split, etc. Keeping things sorted is a headache.
On Tue, Sep 12, 2023 at 10:07:35AM -0700, Alexei Starovoitov wrote: > On Tue, Sep 12, 2023 at 10:05 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Sep 12, 2023 at 06:53:23PM +0200, Paolo Abeni wrote: > > > On Tue, 2023-09-12 at 19:35 +0300, Andy Shevchenko wrote: > > > > On Tue, Sep 12, 2023 at 05:20:31PM +0200, Simon Horman wrote: > > > > > On Mon, Sep 11, 2023 at 06:45:34PM +0300, Andy Shevchenko wrote: ... > > > I'm unsure this change is worthy. It will make any later fix touching > > > the header list more difficult to backport, and I don't see a great > > > direct advantage. > > > > As Rasmus put it here > > https://lore.kernel.org/lkml/5eca0ab5-84be-2d8f-e0b3-c9fdfa961826@rasmusvillemoes.dk/ > > In short term you can argue that it's not beneficial, but in long term it's given > > less conflicts. > > I agree with Paolo. I see. > This is just code churn. > The includes will become unsorted eventually. > Headers might get renamed, split, etc. > Keeping things sorted is a headache. Keeping the mess is simpler, I agree. :-(
On Tue, 2023-09-12 at 20:04 +0300, Andy Shevchenko wrote: > On Tue, Sep 12, 2023 at 06:53:23PM +0200, Paolo Abeni wrote: > > On Tue, 2023-09-12 at 19:35 +0300, Andy Shevchenko wrote: > > > On Tue, Sep 12, 2023 at 05:20:31PM +0200, Simon Horman wrote: > > > > On Mon, Sep 11, 2023 at 06:45:34PM +0300, Andy Shevchenko wrote: > > > > > It's rather a gigantic list of heards that is very hard to follow. > > > > > Sorting helps to see what's already included and what's not. > > > > > It improves a maintainability in a long term. > > > > > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > > > > > Hi Andy, > > > > > > > > At the risk of bike shedding, the sort function of Vim, when operating > > > > with the C locale, gives a slightly different order, as experssed by > > > > this incremental diff. > > > > > > > > I have no objections to your oder, but I'm slightly curious as > > > > to how it came about. > > > > > > !sort which is external command. > > > > > > $ locale -k LC_COLLATE > > > collate-nrules=4 > > > collate-rulesets="" > > > collate-symb-hash-sizemb=1303 > > > collate-codeset="UTF-8" > > > > I'm unsure this change is worthy. It will make any later fix touching > > the header list more difficult to backport, and I don't see a great > > direct advantage. > > As Rasmus put it here > https://lore.kernel.org/lkml/5eca0ab5-84be-2d8f-e0b3-c9fdfa961826@rasmusvillemoes.dk/ > In short term you can argue that it's not beneficial, but in long term it's given > less conflicts. > > > Please repost the first patch standalone. > > Why to repost, what did I miss? It's available via lore, just run > > b4 am -slt -P _ 20230911154534.4174265-1-andriy.shevchenko@linux.intel.com > > to get it :-) It's fairly better if actions (changes) on patches are taken by the submitter: it scales way better, and if the other path take places we can be easily flooded with small (but likely increasingly less smaller) requests that will soon prevent any other activity from being taken. Please, repost the single patch, it would be easier to me. Thanks! Paolo
On Tue, Sep 12, 2023 at 07:25:48PM +0200, Paolo Abeni wrote: > On Tue, 2023-09-12 at 20:04 +0300, Andy Shevchenko wrote: > > On Tue, Sep 12, 2023 at 06:53:23PM +0200, Paolo Abeni wrote: > > > On Tue, 2023-09-12 at 19:35 +0300, Andy Shevchenko wrote: ... > > > Please repost the first patch standalone. > > > > Why to repost, what did I miss? It's available via lore, just run > > > > b4 am -slt -P _ 20230911154534.4174265-1-andriy.shevchenko@linux.intel.com > > > > to get it :-) > > It's fairly better if actions (changes) on patches are taken by the > submitter: it scales way better, and if the other path take places we > can be easily flooded with small (but likely increasingly less smaller) > requests that will soon prevent any other activity from being taken. > > Please, repost the single patch, it would be easier to me. Done.
diff --git a/net/core/dev.c b/net/core/dev.c index 85df22f05c38..d795a6c5a591 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -68,91 +68,94 @@ * - netif_rx() feedback */ -#include <linux/uaccess.h> +#include <linux/audit.h> #include <linux/bitmap.h> +#include <linux/bpf.h> +#include <linux/bpf_trace.h> #include <linux/capability.h> #include <linux/cpu.h> -#include <linux/types.h> -#include <linux/kernel.h> -#include <linux/hash.h> -#include <linux/slab.h> -#include <linux/sched.h> -#include <linux/sched/mm.h> -#include <linux/mutex.h> -#include <linux/rwsem.h> -#include <linux/string.h> -#include <linux/mm.h> -#include <linux/socket.h> -#include <linux/sockios.h> +#include <linux/cpu_rmap.h> +#include <linux/crash_dump.h> +#include <linux/ctype.h> +#include <linux/delay.h> +#include <linux/dmaengine.h> +#include <linux/err.h> #include <linux/errno.h> -#include <linux/interrupt.h> -#include <linux/if_ether.h> -#include <linux/netdevice.h> +#include <linux/errqueue.h> #include <linux/etherdevice.h> #include <linux/ethtool.h> -#include <linux/skbuff.h> +#include <linux/hash.h> +#include <linux/hashtable.h> +#include <linux/highmem.h> +#include <linux/hrtimer.h> +#include <linux/if_arp.h> +#include <linux/if_ether.h> +#include <linux/if_macvlan.h> +#include <linux/if_vlan.h> +#include <linux/indirect_call_wrapper.h> +#include <linux/inetdevice.h> +#include <linux/in.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/ip.h> +#include <linux/ipv6.h> +#include <linux/jhash.h> +#include <linux/kernel.h> #include <linux/kthread.h> -#include <linux/bpf.h> -#include <linux/bpf_trace.h> -#include <net/net_namespace.h> -#include <net/sock.h> -#include <net/busy_poll.h> +#include <linux/mm.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/netdevice.h> +#include <linux/netfilter_netdev.h> +#include <linux/net_namespace.h> +#include <linux/netpoll.h> +#include <linux/once_lite.h> +#include <linux/pm_runtime.h> +#include <linux/prandom.h> +#include <linux/random.h> +#include <linux/rcupdate.h> #include <linux/rtnetlink.h> +#include <linux/rwsem.h> +#include <linux/sched.h> +#include <linux/sched/mm.h> +#include <linux/sctp.h> +#include <linux/skbuff.h> +#include <linux/slab.h> +#include <linux/socket.h> +#include <linux/sockios.h> #include <linux/stat.h> +#include <linux/static_key.h> +#include <linux/string.h> +#include <linux/types.h> +#include <linux/uaccess.h> +#include <linux/vmalloc.h> + +#include <asm/current.h> + +#include <net/busy_poll.h> +#include <net/checksum.h> +#include <net/devlink.h> #include <net/dsa.h> #include <net/dst.h> #include <net/dst_metadata.h> #include <net/gro.h> -#include <net/pkt_sched.h> -#include <net/pkt_cls.h> -#include <net/checksum.h> -#include <net/xfrm.h> -#include <net/tcx.h> -#include <linux/highmem.h> -#include <linux/init.h> -#include <linux/module.h> -#include <linux/netpoll.h> -#include <linux/rcupdate.h> -#include <linux/delay.h> -#include <net/iw_handler.h> -#include <asm/current.h> -#include <linux/audit.h> -#include <linux/dmaengine.h> -#include <linux/err.h> -#include <linux/ctype.h> -#include <linux/if_arp.h> -#include <linux/if_vlan.h> -#include <linux/ip.h> #include <net/ip.h> +#include <net/iw_handler.h> #include <net/mpls.h> -#include <linux/ipv6.h> -#include <linux/in.h> -#include <linux/jhash.h> -#include <linux/random.h> +#include <net/netdev_rx_queue.h> +#include <net/net_namespace.h> +#include <net/pkt_cls.h> +#include <net/pkt_sched.h> +#include <net/sock.h> +#include <net/tcx.h> +#include <net/udp_tunnel.h> +#include <net/xfrm.h> + #include <trace/events/napi.h> #include <trace/events/net.h> -#include <trace/events/skb.h> #include <trace/events/qdisc.h> +#include <trace/events/skb.h> #include <trace/events/xdp.h> -#include <linux/inetdevice.h> -#include <linux/cpu_rmap.h> -#include <linux/static_key.h> -#include <linux/hashtable.h> -#include <linux/vmalloc.h> -#include <linux/if_macvlan.h> -#include <linux/errqueue.h> -#include <linux/hrtimer.h> -#include <linux/netfilter_netdev.h> -#include <linux/crash_dump.h> -#include <linux/sctp.h> -#include <net/udp_tunnel.h> -#include <linux/net_namespace.h> -#include <linux/indirect_call_wrapper.h> -#include <net/devlink.h> -#include <linux/pm_runtime.h> -#include <linux/prandom.h> -#include <linux/once_lite.h> -#include <net/netdev_rx_queue.h> #include "dev.h" #include "net-sysfs.h"
It's rather a gigantic list of heards that is very hard to follow. Sorting helps to see what's already included and what's not. It improves a maintainability in a long term. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- net/core/dev.c | 135 +++++++++++++++++++++++++------------------------ 1 file changed, 69 insertions(+), 66 deletions(-)