[PATCH net-next v1 1/2] net: core: Use the bitmap API to allocate bitmaps

[PATCH net-next v1 1/2] net: core: Use the bitmap API to allocate bitmaps

[Andy Shevchenko]

Use bitmap_zalloc() and bitmap_free() instead of hand-writing them.
It is less verbose and it improves the type checking and semantic.

While at it, add missing header inclusion (should be bitops.h,
but with the above change it becomes bitmap.h).

Suggested-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 net/core/dev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index ccff2b6ef958..85df22f05c38 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -69,7 +69,7 @@
  */
 
 #include <linux/uaccess.h>
-#include <linux/bitops.h>
+#include <linux/bitmap.h>
 #include <linux/capability.h>
 #include <linux/cpu.h>
 #include <linux/types.h>
@@ -1080,7 +1080,7 @@ static int __dev_alloc_name(struct net *net, const char *name, char *buf)
 			return -EINVAL;
 
 		/* Use one page as a bit array of possible slots */
-		inuse = (unsigned long *) get_zeroed_page(GFP_ATOMIC);
+		inuse = bitmap_zalloc(max_netdevices, GFP_ATOMIC);
 		if (!inuse)
 			return -ENOMEM;
 
@@ -1109,7 +1109,7 @@ static int __dev_alloc_name(struct net *net, const char *name, char *buf)
 		}
 
 		i = find_first_zero_bit(inuse, max_netdevices);
-		free_page((unsigned long) inuse);
+		bitmap_free(inuse);
 	}
 
 	snprintf(buf, IFNAMSIZ, name, i);
-- 
2.40.0.1.gaa8946217a0b

[PATCH net-next v1 2/2] net: core: Sort headers alphabetically

[Andy Shevchenko]

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

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"
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH net-next v1 2/2] net: core: Sort headers alphabetically

[Simon Horman]

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>

Re: [PATCH net-next v1 1/2] net: core: Use the bitmap API to allocate bitmaps

[Simon Horman]

On Mon, Sep 11, 2023 at 06:45:33PM +0300, Andy Shevchenko wrote:
> Use bitmap_zalloc() and bitmap_free() instead of hand-writing them.
> It is less verbose and it improves the type checking and semantic.
> 
> While at it, add missing header inclusion (should be bitops.h,
> but with the above change it becomes bitmap.h).
> 
> Suggested-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next v1 2/2] net: core: Sort headers alphabetically

[Andy Shevchenko]

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"

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next v1 2/2] net: core: Sort headers alphabetically

[Paolo Abeni]

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

Re: [PATCH net-next v1 2/2] net: core: Sort headers alphabetically

[Andy Shevchenko]

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

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next v1 2/2] net: core: Sort headers alphabetically

[Alexei Starovoitov]

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.

Re: [PATCH net-next v1 2/2] net: core: Sort headers alphabetically

[Andy Shevchenko]

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

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next v1 2/2] net: core: Sort headers alphabetically

[Paolo Abeni]

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

Re: [PATCH net-next v1 2/2] net: core: Sort headers alphabetically

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko