* [PATCH net-next] ip: initialize hash list
@ 2013-07-20 17:26 Stephen Hemminger
2013-07-20 17:46 ` Joe Perches
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Stephen Hemminger @ 2013-07-20 17:26 UTC (permalink / raw)
To: Pravin B Shelar, David Miller; +Cc: netdev
Rather than relying on the assumption that zero means empty on
hash list head, the code should use the initialization macro.
Same effect, but follows API and avoids future breakage if hlist
implementation changes.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
--- a/net/ipv4/ip_tunnel.c 2013-07-19 09:12:37.213529343 -0700
+++ b/net/ipv4/ip_tunnel.c 2013-07-19 09:37:00.960764421 -0700
@@ -838,10 +838,15 @@ int ip_tunnel_init_net(struct net *net,
{
struct ip_tunnel_net *itn = net_generic(net, ip_tnl_net_id);
struct ip_tunnel_parm parms;
+ unsigned i;
- itn->tunnels = kzalloc(IP_TNL_HASH_SIZE * sizeof(struct hlist_head), GFP_KERNEL);
+ itn->tunnels = kmalloc(IP_TNL_HASH_SIZE * sizeof(struct hlist_head),
+ GFP_KERNEL);
if (!itn->tunnels)
return -ENOMEM;
+
+ for (i = 0; i < IP_TNL_HASH_SIZE; i++)
+ INIT_HLIST_HEAD(&itn->tunnels[i]);
if (!ops) {
itn->fb_tunnel_dev = NULL;
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next] ip: initialize hash list
2013-07-20 17:26 [PATCH net-next] ip: initialize hash list Stephen Hemminger
@ 2013-07-20 17:46 ` Joe Perches
2013-07-22 14:52 ` Stephen Hemminger
2013-07-20 19:20 ` David Miller
2013-07-22 15:17 ` Eric Dumazet
2 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2013-07-20 17:46 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Pravin B Shelar, David Miller, netdev
On Sat, 2013-07-20 at 10:26 -0700, Stephen Hemminger wrote:
> Rather than relying on the assumption that zero means empty on
> hash list head, the code should use the initialization macro.
> Same effect, but follows API and avoids future breakage if hlist
> implementation changes.
[]
> --- a/net/ipv4/ip_tunnel.c 2013-07-19 09:12:37.213529343 -0700
[]
> - itn->tunnels = kzalloc(IP_TNL_HASH_SIZE * sizeof(struct hlist_head), GFP_KERNEL);
> + itn->tunnels = kmalloc(IP_TNL_HASH_SIZE * sizeof(struct hlist_head),
> + GFP_KERNEL);
> if (!itn->tunnels)
> return -ENOMEM;
> +
> + for (i = 0; i < IP_TNL_HASH_SIZE; i++)
> + INIT_HLIST_HEAD(&itn->tunnels[i]);
Hey Stephen.
Are you doing to do just this one or submit a series?
$ git grep "alloc.*sizeof.*hlist_head"
drivers/gpu/drm/i915/i915_gem_execbuffer.c: eb = kzalloc(count*sizeof(struct hlist_head) +
drivers/md/dm-bufio.c: c->cache_hash = vmalloc(sizeof(struct hlist_head) << DM_BUFIO_HASH_BITS);
fs/ecryptfs/messaging.c: ecryptfs_daemon_hash = kmalloc((sizeof(struct hlist_head)
fs/nfsd/nfscache.c: cache_hash = kcalloc(hashsize, sizeof(struct hlist_head), GFP_KERNEL);
kernel/trace/ftrace.c: stat->hash = kzalloc(sizeof(struct hlist_head) * size, GFP_KERNEL);
lib/lru_cache.c: slot = kcalloc(e_count, sizeof(struct hlist_head), GFP_KERNEL);
net/ipv4/ip_tunnel.c: itn->tunnels = kzalloc(IP_TNL_HASH_SIZE * sizeof(struct hlist_head), GFP_KERNEL);
net/mac80211/mesh_pathtbl.c: newtbl->hash_buckets = kzalloc(sizeof(struct hlist_head) *
net/mac80211/mesh_pathtbl.c: tbl_path->known_gates = kzalloc(sizeof(struct hlist_head), GFP_ATOMIC);
net/mac80211/mesh_pathtbl.c: tbl_mpp->known_gates = kzalloc(sizeof(struct hlist_head), GFP_ATOMIC);
net/openvswitch/datapath.c: dp->ports = kmalloc(DP_VPORT_HASH_BUCKETS * sizeof(struct hlist_head),
net/openvswitch/flow.c: buckets = flex_array_alloc(sizeof(struct hlist_head *),
net/openvswitch/vport.c: dev_table = kzalloc(VPORT_HASH_BUCKETS * sizeof(struct hlist_head),
net/tipc/name_table.c: table.types = kcalloc(TIPC_NAMETBL_SIZE, sizeof(struct hlist_head),
virt/kvm/irqchip.c: new = kzalloc(sizeof(*new) + (nr_rt_entries * sizeof(struct hlist_head))
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] ip: initialize hash list
2013-07-20 17:46 ` Joe Perches
@ 2013-07-22 14:52 ` Stephen Hemminger
2013-07-22 15:06 ` Joe Perches
0 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2013-07-22 14:52 UTC (permalink / raw)
To: Joe Perches; +Cc: Pravin B Shelar, David Miller, netdev
On Sat, 20 Jul 2013 10:46:20 -0700
Joe Perches <joe@perches.com> wrote:
> On Sat, 2013-07-20 at 10:26 -0700, Stephen Hemminger wrote:
> > Rather than relying on the assumption that zero means empty on
> > hash list head, the code should use the initialization macro.
> > Same effect, but follows API and avoids future breakage if hlist
> > implementation changes.
> []
> > --- a/net/ipv4/ip_tunnel.c 2013-07-19 09:12:37.213529343 -0700
> []
> > - itn->tunnels = kzalloc(IP_TNL_HASH_SIZE * sizeof(struct hlist_head), GFP_KERNEL);
> > + itn->tunnels = kmalloc(IP_TNL_HASH_SIZE * sizeof(struct hlist_head),
> > + GFP_KERNEL);
> > if (!itn->tunnels)
> > return -ENOMEM;
> > +
> > + for (i = 0; i < IP_TNL_HASH_SIZE; i++)
> > + INIT_HLIST_HEAD(&itn->tunnels[i]);
>
> Hey Stephen.
>
> Are you doing to do just this one or submit a series?
Hadn't planned on fixing more than one. And would not go outside code
that I actually use.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] ip: initialize hash list
2013-07-22 14:52 ` Stephen Hemminger
@ 2013-07-22 15:06 ` Joe Perches
2013-07-22 15:15 ` Eric Dumazet
0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2013-07-22 15:06 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Pravin B Shelar, David Miller, netdev
On Mon, 2013-07-22 at 07:52 -0700, Stephen Hemminger wrote:
> On Sat, 20 Jul 2013 10:46:20 -0700
> Joe Perches <joe@perches.com> wrote:
[]
> > Are you doing to do just this one or submit a series?
>
> Hadn't planned on fixing more than one. And would not go outside code
> that I actually use.
What a pity.
I think if you identify what you believe is a
trivial defect in your own code, it's both
polite and social to fix similar defects in
other code too.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] ip: initialize hash list
2013-07-22 15:06 ` Joe Perches
@ 2013-07-22 15:15 ` Eric Dumazet
2013-07-22 15:31 ` Joe Perches
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2013-07-22 15:15 UTC (permalink / raw)
To: Joe Perches; +Cc: Stephen Hemminger, Pravin B Shelar, David Miller, netdev
On Mon, 2013-07-22 at 08:06 -0700, Joe Perches wrote:
> On Mon, 2013-07-22 at 07:52 -0700, Stephen Hemminger wrote:
> > On Sat, 20 Jul 2013 10:46:20 -0700
> > Joe Perches <joe@perches.com> wrote:
> []
> > > Are you doing to do just this one or submit a series?
> >
> > Hadn't planned on fixing more than one. And would not go outside code
> > that I actually use.
>
> What a pity.
>
> I think if you identify what you believe is a
> trivial defect in your own code, it's both
> polite and social to fix similar defects in
> other code too.
Joe, I find this quite misplaced.
So far, there is no bug.
What about fixing real bugs instead ?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] ip: initialize hash list
2013-07-22 15:15 ` Eric Dumazet
@ 2013-07-22 15:31 ` Joe Perches
0 siblings, 0 replies; 9+ messages in thread
From: Joe Perches @ 2013-07-22 15:31 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Stephen Hemminger, Pravin B Shelar, David Miller, netdev
On Mon, 2013-07-22 at 08:15 -0700, Eric Dumazet wrote:
> On Mon, 2013-07-22 at 08:06 -0700, Joe Perches wrote:
> > On Mon, 2013-07-22 at 07:52 -0700, Stephen Hemminger wrote:
> > > On Sat, 20 Jul 2013 10:46:20 -0700
> > > Joe Perches <joe@perches.com> wrote:
> > []
> > > > Are you doing to do just this one or submit a series?
> > >
> > > Hadn't planned on fixing more than one. And would not go outside code
> > > that I actually use.
> >
> > What a pity.
> >
> > I think if you identify what you believe is a
> > trivial defect in your own code, it's both
> > polite and social to fix similar defects in
> > other code too.
>
> Joe, I find this quite misplaced.
Our opinions differ then.
What's "misplaced" about suggesting that a
person that identifies and submits a patch
for a code form viewed as sub-optimal could
fix the other instances of that code form?
> So far, there is no bug.
I did not even suggest there was a bug.
I wrote "trivial defect".
> What about fixing real bugs instead ?
That'd be good too, with the caveat that
those "real bugs" can take a rather more
intensive effort to identify, isolate and
correct without introducing other defects.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] ip: initialize hash list
2013-07-20 17:26 [PATCH net-next] ip: initialize hash list Stephen Hemminger
2013-07-20 17:46 ` Joe Perches
@ 2013-07-20 19:20 ` David Miller
2013-07-20 20:28 ` Joe Perches
2013-07-22 15:17 ` Eric Dumazet
2 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2013-07-20 19:20 UTC (permalink / raw)
To: stephen; +Cc: pshelar, netdev
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sat, 20 Jul 2013 10:26:57 -0700
> + unsigned i;
Please use an explicit "unsigned int", thank you.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] ip: initialize hash list
2013-07-20 19:20 ` David Miller
@ 2013-07-20 20:28 ` Joe Perches
0 siblings, 0 replies; 9+ messages in thread
From: Joe Perches @ 2013-07-20 20:28 UTC (permalink / raw)
To: David Miller; +Cc: stephen, pshelar, netdev
On Sat, 2013-07-20 at 12:20 -0700, David Miller wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Sat, 20 Jul 2013 10:26:57 -0700
>
> > + unsigned i;
>
> Please use an explicit "unsigned int", thank you.
Still a dozen+ of these in net
(I removed a few false positives)
$ grep -rP --include=*.[ch] '\bunsigned\s*+(?!(int|long|char|short|\*\/))' net
net/mac80211/ieee80211_i.h:typedef unsigned __bitwise__ ieee80211_tx_result;
net/mac80211/ieee80211_i.h:typedef unsigned __bitwise__ ieee80211_rx_result;
net/ceph/osdmap.c: unsigned len, num;
net/ceph/osdmap.c: (unsigned)pgid.pool;
net/core/netprio_cgroup.c:static int update_netprio(const void *v, struct file *file, unsigned n)
net/rose/rose_subr.c:int rose_parse_facilities(unsigned char *p, unsigned packet_len,
net/xfrm/xfrm_policy.c:#define XFRM_QUEUE_TMO_MIN ((unsigned)(HZ/10))
net/xfrm/xfrm_policy.c:#define XFRM_QUEUE_TMO_MAX ((unsigned)(60*HZ))
net/ipv4/netfilter/nf_nat_h323.c:static int set_h245_addr(struct sk_buff *skb, unsigned protoff,
net/ipv4/ip_vti.c: unsigned h0 = HASH(remote);
net/ipv4/ip_vti.c: unsigned h1 = HASH(local);
net/ipv4/ip_vti.c: unsigned h = 0;
net/sched/cls_rsvp.h:static unsigned int gen_handle(struct tcf_proto *tp, unsigned salt)
net/sched/cls_cgroup.c:static int update_classid(const void *v, struct file *file, unsigned n)
net/mac802154/wpan.c: unsigned len)
net/socket.c:static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg)
net/socket.c:long __sys_sendmsg(int fd, struct msghdr __user *msg, unsigned flags)
net/socket.c:long __sys_recvmsg(int fd, struct msghdr __user *msg, unsigned flags)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] ip: initialize hash list
2013-07-20 17:26 [PATCH net-next] ip: initialize hash list Stephen Hemminger
2013-07-20 17:46 ` Joe Perches
2013-07-20 19:20 ` David Miller
@ 2013-07-22 15:17 ` Eric Dumazet
2 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2013-07-22 15:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Pravin B Shelar, David Miller, netdev
On Sat, 2013-07-20 at 10:26 -0700, Stephen Hemminger wrote:
> Rather than relying on the assumption that zero means empty on
> hash list head, the code should use the initialization macro.
> Same effect, but follows API and avoids future breakage if hlist
> implementation changes.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>
>
> --- a/net/ipv4/ip_tunnel.c 2013-07-19 09:12:37.213529343 -0700
> +++ b/net/ipv4/ip_tunnel.c 2013-07-19 09:37:00.960764421 -0700
> @@ -838,10 +838,15 @@ int ip_tunnel_init_net(struct net *net,
> {
> struct ip_tunnel_net *itn = net_generic(net, ip_tnl_net_id);
> struct ip_tunnel_parm parms;
> + unsigned i;
>
> - itn->tunnels = kzalloc(IP_TNL_HASH_SIZE * sizeof(struct hlist_head), GFP_KERNEL);
> + itn->tunnels = kmalloc(IP_TNL_HASH_SIZE * sizeof(struct hlist_head),
> + GFP_KERNEL);
> if (!itn->tunnels)
> return -ENOMEM;
> +
> + for (i = 0; i < IP_TNL_HASH_SIZE; i++)
> + INIT_HLIST_HEAD(&itn->tunnels[i]);
>
What about adding a variant of INIT_HLIST_HEAD() that does nothing for
already cleared memory ?
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-07-22 15:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-20 17:26 [PATCH net-next] ip: initialize hash list Stephen Hemminger
2013-07-20 17:46 ` Joe Perches
2013-07-22 14:52 ` Stephen Hemminger
2013-07-22 15:06 ` Joe Perches
2013-07-22 15:15 ` Eric Dumazet
2013-07-22 15:31 ` Joe Perches
2013-07-20 19:20 ` David Miller
2013-07-20 20:28 ` Joe Perches
2013-07-22 15:17 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).