* Re: [PATCH] netdev: fold name hash properly (v3)
From: Eric Dumazet @ 2009-11-10 17:37 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20091110092034.3d4ee9b0@nehalam>
Stephen Hemminger a écrit :
> The full_name_hash function does not produce well distributed values in
> the lower bits, so most code uses hash_32() to fold it. This is really
> a bug introduced when name hashing was added, back in 2.5 when I added
> name hashing.
>
> hash_32 is all that is needed since full_name_hash returns unsigned int
> which is only 32 bits on 64 bit platforms.
>
> Also, there is no point in using hash_32 on ifindex, because the is naturally
> sequential and usually well distributed.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Thanks Stephen
^ permalink raw reply
* sunrpc port allocation and IANA reserved list
From: Chris Friesen @ 2009-11-10 17:43 UTC (permalink / raw)
To: netdev, Linux kernel
By default sunrpc ports are allocated at random in the range 665-1023U.
However, there are many ports within this range which have been
reserved by the IANA (and others like port 921 which are not formally
reserved but are "well-known").
Given that a userspace application can be stopped and restarted at any
time, and a sunrpc registration can happen at any time, what is the
expected mechanism to prevent the kernel from allocating a port for use
by sunrpc that reserved or well-known?
Apparently Redhat and Debian have distro-specific ways of dealing with
this, but is there a standard solution? Should there be?
The current setup seems suboptimal.
Chris
^ permalink raw reply
* Re: sunrpc port allocation and IANA reserved list
From: Ben Hutchings @ 2009-11-10 17:53 UTC (permalink / raw)
To: Chris Friesen; +Cc: netdev, Linux kernel
In-Reply-To: <4AF9A63B.6010101@nortel.com>
On Tue, 2009-11-10 at 11:43 -0600, Chris Friesen wrote:
> By default sunrpc ports are allocated at random in the range 665-1023U.
> However, there are many ports within this range which have been
> reserved by the IANA (and others like port 921 which are not formally
> reserved but are "well-known").
>
> Given that a userspace application can be stopped and restarted at any
> time, and a sunrpc registration can happen at any time, what is the
> expected mechanism to prevent the kernel from allocating a port for use
> by sunrpc that reserved or well-known?
>
> Apparently Redhat and Debian have distro-specific ways of dealing with
> this, but is there a standard solution? Should there be?
>
> The current setup seems suboptimal.
I believe both RH and Debian are using the same implementation:
<http://cyberelk.net/tim/software/portreserve/>.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH 00/10] netdev: get rid of read_lock(&dev_base_lock) usages
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev
The goal is to eliminate dev_base_lock completely, and just use RCU
and rtnl_mutex for network devices. This series gets rid of the many
of the users of dev_base_lock just for reading.
--
^ permalink raw reply
* [PATCH 01/10] netdev: add netdev_continue_rcu
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Paul E. McKenney; +Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: continue-rcu.patch --]
[-- Type: text/plain, Size: 1771 bytes --]
This adds an RCU macro for continuing search, useful for some
network devices like vlan.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/include/linux/netdevice.h 2009-11-09 22:19:08.511480873 -0800
+++ b/include/linux/netdevice.h 2009-11-10 09:27:17.973376267 -0800
@@ -1079,6 +1079,8 @@ extern rwlock_t dev_base_lock; /* De
list_for_each_entry_safe(d, n, &(net)->dev_base_head, dev_list)
#define for_each_netdev_continue(net, d) \
list_for_each_entry_continue(d, &(net)->dev_base_head, dev_list)
+#define for_each_netdev_continue_rcu(net, d) \
+ list_for_each_entry_continue_rcu(d, &(net)->dev_base_head, dev_list)
#define net_device_entry(lh) list_entry(lh, struct net_device, dev_list)
static inline struct net_device *next_net_device(struct net_device *dev)
--- a/include/linux/rculist.h 2009-11-09 22:19:08.529480859 -0800
+++ b/include/linux/rculist.h 2009-11-10 09:27:17.974376609 -0800
@@ -262,6 +262,20 @@ static inline void list_splice_init_rcu(
(pos) = rcu_dereference((pos)->next))
/**
+ * list_for_each_entry_continue_rcu - continue iteration over list of given type
+ * @pos: the type * to use as a loop cursor.
+ * @head: the head for your list.
+ * @member: the name of the list_struct within the struct.
+ *
+ * Continue to iterate over list of given type, continuing after
+ * the current position.
+ */
+#define list_for_each_entry_continue_rcu(pos, head, member) \
+ for (pos = list_entry_rcu(pos->member.next, typeof(*pos), member); \
+ prefetch(pos->member.next), &pos->member != (head); \
+ pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
+
+/**
* hlist_del_rcu - deletes entry from hash list without re-initialization
* @n: the element to delete from the hash list.
*
--
^ permalink raw reply
* [PATCH 02/10] vlan: eliminate use of dev_base_lock
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Patrick McHardy; +Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: vlan-rdlock.patch --]
[-- Type: text/plain, Size: 1311 bytes --]
Do not need to use read_lock(&dev_base_lock), use RCU instead.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/8021q/vlanproc.c 2009-11-09 22:19:08.712481032 -0800
+++ b/net/8021q/vlanproc.c 2009-11-10 09:27:28.165378176 -0800
@@ -201,18 +201,17 @@ int vlan_proc_rem_dev(struct net_device
/* start read of /proc/net/vlan/config */
static void *vlan_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(rcu)
{
struct net_device *dev;
struct net *net = seq_file_net(seq);
loff_t i = 1;
- read_lock(&dev_base_lock);
-
+ rcu_read_lock();
if (*pos == 0)
return SEQ_START_TOKEN;
- for_each_netdev(net, dev) {
+ for_each_netdev_rcu(net, dev) {
if (!is_vlan_dev(dev))
continue;
@@ -234,7 +233,7 @@ static void *vlan_seq_next(struct seq_fi
if (v == SEQ_START_TOKEN)
dev = net_device_entry(&net->dev_base_head);
- for_each_netdev_continue(net, dev) {
+ for_each_netdev_continue_rcu(net, dev) {
if (!is_vlan_dev(dev))
continue;
@@ -245,9 +244,9 @@ static void *vlan_seq_next(struct seq_fi
}
static void vlan_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(rcu)
{
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int vlan_seq_show(struct seq_file *seq, void *v)
--
^ permalink raw reply
* [PATCH 03/10] net: use rcu for network scheduler API
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: sched-rcu.patch --]
[-- Type: text/plain, Size: 752 bytes --]
Use RCU to walk list of network devices in qdisc dump.
This could be optimized for large number of devices.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/sched/sch_api.c 2009-11-09 22:19:09.002480816 -0800
+++ b/net/sched/sch_api.c 2009-11-10 09:28:38.166439067 -0800
@@ -1279,9 +1279,10 @@ static int tc_dump_qdisc(struct sk_buff
s_idx = cb->args[0];
s_q_idx = q_idx = cb->args[1];
- read_lock(&dev_base_lock);
+
+ rcu_read_lock();
idx = 0;
- for_each_netdev(&init_net, dev) {
+ for_each_netdev_rcu(&init_net, dev) {
struct netdev_queue *dev_queue;
if (idx < s_idx)
@@ -1302,7 +1303,7 @@ cont:
}
done:
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
cb->args[0] = idx;
cb->args[1] = q_idx;
--
^ permalink raw reply
* [PATCH 04/10] AOE: use rcu to find network device
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Ed L. Cashin, Harvey Harrison,
Bartlomiej Zolnierkiewicz
Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: aoe-rcu.patch --]
[-- Type: text/plain, Size: 1240 bytes --]
This gets rid of another use of read_lock(&dev_base_lock) by using
RCU. Also, it only increments the reference count of the device actually
used rather than holding and releasing every device
Compile tested only.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/drivers/block/aoe/aoecmd.c 2009-11-09 22:19:06.082480836 -0800
+++ b/drivers/block/aoe/aoecmd.c 2009-11-10 09:28:38.222438732 -0800
@@ -296,17 +296,18 @@ aoecmd_cfg_pkts(ushort aoemajor, unsigne
struct sk_buff *skb;
struct net_device *ifp;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, ifp) {
- dev_hold(ifp);
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, ifp) {
if (!is_aoe_netif(ifp))
- goto cont;
+ continue;
skb = new_skb(sizeof *h + sizeof *ch);
if (skb == NULL) {
printk(KERN_INFO "aoe: skb alloc failure\n");
- goto cont;
+ continue;
}
+
+ dev_hold(ifp);
skb_put(skb, sizeof *h + sizeof *ch);
skb->dev = ifp;
__skb_queue_tail(queue, skb);
@@ -320,11 +321,8 @@ aoecmd_cfg_pkts(ushort aoemajor, unsigne
h->major = cpu_to_be16(aoemajor);
h->minor = aoeminor;
h->cmd = AOECMD_CFG;
-
-cont:
- dev_put(ifp);
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static void
--
^ permalink raw reply
* [PATCH 06/10] s390: use RCU to walk list of network devices
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Martin Schwidefsky, Heiko Carstens; +Cc: netdev, linux390
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: s390-readlock.patch --]
[-- Type: text/plain, Size: 1004 bytes --]
This is similar to other cases where for_each_netdev_rcu
can be used when gathering information.
By inspection, don't have platform or cross-build environment
to validate.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/arch/s390/appldata/appldata_net_sum.c 2009-11-09 22:19:05.593480476 -0800
+++ b/arch/s390/appldata/appldata_net_sum.c 2009-11-10 09:28:38.335438652 -0800
@@ -83,8 +83,9 @@ static void appldata_get_net_sum_data(vo
rx_dropped = 0;
tx_dropped = 0;
collisions = 0;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
const struct net_device_stats *stats = dev_get_stats(dev);
rx_packets += stats->rx_packets;
@@ -98,7 +99,8 @@ static void appldata_get_net_sum_data(vo
collisions += stats->collisions;
i++;
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
+
net_data->nr_interfaces = i;
net_data->rx_packets = rx_packets;
net_data->tx_packets = tx_packets;
--
^ permalink raw reply
* [PATCH 08/10] ipv6: use RCU to walk list of network devices
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: ipv6-rdlock.patch --]
[-- Type: text/plain, Size: 4200 bytes --]
No longer need read_lock(&dev_base_lock), use RCU instead.
This also needs to be optimized for large number of devices.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/ipv6/addrconf.c 2009-11-09 22:19:08.925480813 -0800
+++ b/net/ipv6/addrconf.c 2009-11-10 09:28:38.577438386 -0800
@@ -3828,9 +3828,9 @@ static int inet6_dump_ifinfo(struct sk_b
struct net_device *dev;
struct inet6_dev *idev;
- read_lock(&dev_base_lock);
+ rcu_read_lock();
idx = 0;
- for_each_netdev(net, dev) {
+ for_each_netdev_rcu(net, dev) {
if (idx < s_idx)
goto cont;
if ((idev = in6_dev_get(dev)) == NULL)
@@ -3843,7 +3843,7 @@ static int inet6_dump_ifinfo(struct sk_b
cont:
idx++;
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
cb->args[0] = idx;
return skb->len;
--- a/net/ipv6/anycast.c 2009-11-09 22:19:08.926480759 -0800
+++ b/net/ipv6/anycast.c 2009-11-10 09:28:38.577438386 -0800
@@ -431,7 +431,7 @@ static inline struct ifacaddr6 *ac6_get_
struct net *net = seq_file_net(seq);
state->idev = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct inet6_dev *idev;
idev = in6_dev_get(state->dev);
if (!idev)
@@ -482,9 +482,9 @@ static struct ifacaddr6 *ac6_get_idx(str
}
static void *ac6_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(rcu)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return ac6_get_idx(seq, *pos);
}
@@ -497,14 +497,14 @@ static void *ac6_seq_next(struct seq_fil
}
static void ac6_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(rcu)
{
struct ac6_iter_state *state = ac6_seq_private(seq);
if (likely(state->idev != NULL)) {
read_unlock_bh(&state->idev->lock);
in6_dev_put(state->idev);
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int ac6_seq_show(struct seq_file *seq, void *v)
--- a/net/ipv6/mcast.c 2009-11-09 22:19:08.929480873 -0800
+++ b/net/ipv6/mcast.c 2009-11-10 09:28:38.578438329 -0800
@@ -2375,7 +2375,7 @@ static inline struct ifmcaddr6 *igmp6_mc
struct net *net = seq_file_net(seq);
state->idev = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct inet6_dev *idev;
idev = in6_dev_get(state->dev);
if (!idev)
@@ -2426,9 +2426,9 @@ static struct ifmcaddr6 *igmp6_mc_get_id
}
static void *igmp6_mc_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(rcu)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return igmp6_mc_get_idx(seq, *pos);
}
@@ -2441,7 +2441,7 @@ static void *igmp6_mc_seq_next(struct se
}
static void igmp6_mc_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(rcu)
{
struct igmp6_mc_iter_state *state = igmp6_mc_seq_private(seq);
if (likely(state->idev != NULL)) {
@@ -2450,7 +2450,7 @@ static void igmp6_mc_seq_stop(struct seq
state->idev = NULL;
}
state->dev = NULL;
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int igmp6_mc_seq_show(struct seq_file *seq, void *v)
@@ -2507,7 +2507,7 @@ static inline struct ip6_sf_list *igmp6_
state->idev = NULL;
state->im = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct inet6_dev *idev;
idev = in6_dev_get(state->dev);
if (unlikely(idev == NULL))
@@ -2573,9 +2573,9 @@ static struct ip6_sf_list *igmp6_mcf_get
}
static void *igmp6_mcf_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(rcu)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return *pos ? igmp6_mcf_get_idx(seq, *pos - 1) : SEQ_START_TOKEN;
}
@@ -2591,7 +2591,7 @@ static void *igmp6_mcf_seq_next(struct s
}
static void igmp6_mcf_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(rcu)
{
struct igmp6_mcf_iter_state *state = igmp6_mcf_seq_private(seq);
if (likely(state->im != NULL)) {
@@ -2604,7 +2604,7 @@ static void igmp6_mcf_seq_stop(struct se
state->idev = NULL;
}
state->dev = NULL;
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int igmp6_mcf_seq_show(struct seq_file *seq, void *v)
--
^ permalink raw reply
* [PATCH 09/10] IPV4: use rcu to walk list of devices in IGMP
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: ipv4-igmp.patch --]
[-- Type: text/plain, Size: 2400 bytes --]
This also needs to be optimized for large number of devices.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/ipv4/igmp.c 2009-11-09 22:19:08.899543825 -0800
+++ b/net/ipv4/igmp.c 2009-11-10 09:28:38.636438291 -0800
@@ -2311,7 +2311,7 @@ static inline struct ip_mc_list *igmp_mc
struct igmp_mc_iter_state *state = igmp_mc_seq_private(seq);
state->in_dev = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct in_device *in_dev;
in_dev = in_dev_get(state->dev);
if (!in_dev)
@@ -2361,9 +2361,9 @@ static struct ip_mc_list *igmp_mc_get_id
}
static void *igmp_mc_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(rcu)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return *pos ? igmp_mc_get_idx(seq, *pos - 1) : SEQ_START_TOKEN;
}
@@ -2379,7 +2379,7 @@ static void *igmp_mc_seq_next(struct seq
}
static void igmp_mc_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(rcu)
{
struct igmp_mc_iter_state *state = igmp_mc_seq_private(seq);
if (likely(state->in_dev != NULL)) {
@@ -2388,7 +2388,7 @@ static void igmp_mc_seq_stop(struct seq_
state->in_dev = NULL;
}
state->dev = NULL;
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int igmp_mc_seq_show(struct seq_file *seq, void *v)
@@ -2462,7 +2462,7 @@ static inline struct ip_sf_list *igmp_mc
state->idev = NULL;
state->im = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct in_device *idev;
idev = in_dev_get(state->dev);
if (unlikely(idev == NULL))
@@ -2528,8 +2528,9 @@ static struct ip_sf_list *igmp_mcf_get_i
}
static void *igmp_mcf_seq_start(struct seq_file *seq, loff_t *pos)
+ __acquires(rcu)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return *pos ? igmp_mcf_get_idx(seq, *pos - 1) : SEQ_START_TOKEN;
}
@@ -2545,6 +2546,7 @@ static void *igmp_mcf_seq_next(struct se
}
static void igmp_mcf_seq_stop(struct seq_file *seq, void *v)
+ __releases(rcu)
{
struct igmp_mcf_iter_state *state = igmp_mcf_seq_private(seq);
if (likely(state->im != NULL)) {
@@ -2557,7 +2559,7 @@ static void igmp_mcf_seq_stop(struct seq
state->idev = NULL;
}
state->dev = NULL;
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int igmp_mcf_seq_show(struct seq_file *seq, void *v)
--
^ permalink raw reply
* [PATCH 07/10] decnet: use RCU to find network devices
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Christine Caulfield, David S. Miller, Hannes Eder,
Alexey
Cc: netdev, linux-decnet-users
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: decnet-readlock.patch --]
[-- Type: text/plain, Size: 1576 bytes --]
When showing device statistics use RCU rather than read_lock(&dev_base_lock)
Compile tested only.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/decnet/dn_dev.c 2009-11-10 09:30:55.557376454 -0800
+++ b/net/decnet/dn_dev.c 2009-11-10 09:40:03.847005394 -0800
@@ -856,9 +856,7 @@ int dn_dev_bind_default(__le16 *addr)
dev = dn_dev_get_default();
last_chance:
if (dev) {
- read_lock(&dev_base_lock);
rv = dn_dev_get_first(dev, addr);
- read_unlock(&dev_base_lock);
dev_put(dev);
if (rv == 0 || dev == init_net.loopback_dev)
return rv;
@@ -1323,18 +1321,18 @@ static inline int is_dn_dev(struct net_d
}
static void *dn_dev_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(&dev_base_lock)
+ __acquires(rcu)
{
int i;
struct net_device *dev;
- read_lock(&dev_base_lock);
+ rcu_read_lock();
if (*pos == 0)
return SEQ_START_TOKEN;
i = 1;
- for_each_netdev(&init_net, dev) {
+ for_each_netdev_rcu(&init_net, dev) {
if (!is_dn_dev(dev))
continue;
@@ -1355,7 +1353,7 @@ static void *dn_dev_seq_next(struct seq_
if (v == SEQ_START_TOKEN)
dev = net_device_entry(&init_net.dev_base_head);
- for_each_netdev_continue(&init_net, dev) {
+ for_each_netdev_continue_rcu(&init_net, dev) {
if (!is_dn_dev(dev))
continue;
@@ -1366,9 +1364,9 @@ static void *dn_dev_seq_next(struct seq_
}
static void dn_dev_seq_stop(struct seq_file *seq, void *v)
- __releases(&dev_base_lock)
+ __releases(rcu)
{
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static char *dn_type2asc(char type)
--
^ permalink raw reply
* [PATCH 10/10] CAN: use dev_get_by_index_rcu
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Oliver Hartkopp, Alexey Dobriyan, Lothar Wassmann; +Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: bcm-dev-rcu.patch --]
[-- Type: text/plain, Size: 614 bytes --]
Use new function to avoid doing read_lock().
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/can/bcm.c 2009-11-10 09:45:16.301376272 -0800
+++ b/net/can/bcm.c 2009-11-10 09:46:30.125005956 -0800
@@ -139,13 +139,13 @@ static char *bcm_proc_getifname(char *re
if (!ifindex)
return "any";
- read_lock(&dev_base_lock);
- dev = __dev_get_by_index(&init_net, ifindex);
+ rcu_read_lock();
+ dev = dev_get_by_index_rcu(&init_net, ifindex);
if (dev)
strcpy(result, dev->name);
else
strcpy(result, "???");
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return result;
}
--
^ permalink raw reply
* Re: RFC: net: allow to propagate errors through ->ndo_hard_start_xmit()
From: Jarek Poplawski @ 2009-11-10 17:57 UTC (permalink / raw)
To: Patrick McHardy
Cc: Linux Netdev List, Herbert Xu, David S. Miller, Stephen Hemminger
In-Reply-To: <4AF9A36F.7070807@trash.net>
On Tue, Nov 10, 2009 at 06:31:27PM +0100, Patrick McHardy wrote:
> Jarek Poplawski wrote:
> > On Mon, Nov 09, 2009 at 08:41:36PM +0100, Patrick McHardy wrote:
> >> I've updated my patch to propagate error values (errno and NET_XMIT
> >> codes) through ndo_hard_start_xmit() and incorporated the suggestions
> >> made last time, namely:
> >>
> >> - move slightly complicated return value check to inline function and
> >> add a few comments
> >>
> >> - fix error handling while in the middle of transmitting GSO skbs
> >>
> >> I've also audited the tree once again for invalid return values and
> >> found a single remaining instance in a Wimax driver, I'll take care
> >> of that later.
> >>
> >> Two questions remain:
> >>
> >> - I'm not sure the error handling in dev_hard_start_xmit() for GSO
> >> skbs is optimal. When the driver returns an error, it is assumed
> >> the current segment has been freed. The patch then frees the
> >> entire GSO skb, including all remaining segments. Alternatively
> >> it could try to transmit the remaining segments later.
> >
> > Anyway, it seems this freeing should be described in the changelog,
> > if not moved to a separate patch, since it fixes another problem,
> > unless I forgot something.
>
> What other problem are you refering to? I'm not aware of any
> problems in the existing function.
This patch is about propagating errors, so it's not clear why there
are some additional kfrees mixed with this. (But I see it's explained
below.)
>
> >> diff --git a/net/core/dev.c b/net/core/dev.c
> >> index bf629ac..1f5752d 100644
> >> --- a/net/core/dev.c
> >> +++ b/net/core/dev.c
> >> @@ -1756,7 +1756,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
> >> struct netdev_queue *txq)
> >> {
> >> const struct net_device_ops *ops = dev->netdev_ops;
> >> - int rc;
> >> + int rc = NETDEV_TX_OK;
> >
> > Isn't it enough to add this in one place only: before this
> > "goto out_kfree_skb"?
>
> Its only exists once in the version I sent out earlier.
>
> >> if (likely(!skb->next)) {
> >> if (!list_empty(&ptype_all))
> >> @@ -1804,6 +1804,8 @@ gso:
> >> nskb->next = NULL;
> >> rc = ops->ndo_start_xmit(nskb, dev);
> >> if (unlikely(rc != NETDEV_TX_OK)) {
> >> + if (rc & ~NETDEV_TX_MASK)
> >> + goto out_kfree_gso_skb;
> >
> > If e.g. (rc == NETDEV_TX_OK | NET_XMIT_CN), why exactly is this freeing
> > necessary now?
> >
> > Is e.g. (rc == NETDEV_TX_BUSY | NET_XMIT_CN) legal? If so, there would
> > be use after kfree, I guess. Otherwise, it should be documented above
> > (and maybe checked somewhere as well).
>
> NET_XMIT_CN is a valid return value, yes. But its not freeing the
> transmitted segment but the remaining ones. Its not strictly
> necessary, but its the easiest way to treat all errors similar.
> Otherwise you get complicated cases, f.i. when the driver returns
> NET_XMIT_CN for the first segment and NETDEV_TX_OK for the
> remaining ones.
It should be in the changelog and maybe a comment too. Even if it's
right it's a change of functionality/behavior here.
I still don't know if/why (rc == NETDEV_TX_BUSY | NET_XMIT_CN) is
OK. IMHO skb will be requeued after kfree here.
>
> >> nskb->next = skb->next;
> >> skb->next = nskb;
> >> return rc;
> >> @@ -1813,11 +1815,14 @@ gso:
> >> return NETDEV_TX_BUSY;
> >> } while (skb->next);
> >>
> >> - skb->destructor = DEV_GSO_CB(skb)->destructor;
> >> + rc = NETDEV_TX_OK;
> >
> > When is (rc != NETDEV_TX_OK) possible in this place?
>
> Its gone in the current version.
Why don't you send the current version?
Jarek P.
^ permalink raw reply
* [PATCH 05/10] parisc: use RCU to find network device
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Kyle McMartin, Helge Deller, Alexander Beregalov
Cc: netdev, linux-parisc
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: parisc-rdlock.patch --]
[-- Type: text/plain, Size: 943 bytes --]
Another place where RCU can be used instead of read_lock(&dev_base_lock)
This is by inspection, don't have platform or cross-build environment
to validate.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/drivers/parisc/led.c 2009-11-09 22:19:07.223480872 -0800
+++ b/drivers/parisc/led.c 2009-11-10 09:28:38.279438787 -0800
@@ -354,9 +354,8 @@ static __inline__ int led_get_net_activi
/* we are running as a workqueue task, so locking dev_base
* for reading should be OK */
- read_lock(&dev_base_lock);
rcu_read_lock();
- for_each_netdev(&init_net, dev) {
+ for_each_netdev_rcu(&init_net, dev) {
const struct net_device_stats *stats;
struct in_device *in_dev = __in_dev_get_rcu(dev);
if (!in_dev || !in_dev->ifa_list)
@@ -368,7 +367,6 @@ static __inline__ int led_get_net_activi
tx_total += stats->tx_packets;
}
rcu_read_unlock();
- read_unlock(&dev_base_lock);
retval = 0;
--
^ permalink raw reply
* Re: [PATCH 00/10] netdev: get rid of read_lock(&dev_base_lock) usages
From: Eric Dumazet @ 2009-11-10 18:18 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
Stephen Hemminger a écrit :
> The goal is to eliminate dev_base_lock completely, and just use RCU
> and rtnl_mutex for network devices. This series gets rid of the many
> of the users of dev_base_lock just for reading.
>
Nice, but I was doing all this work Stephen... maybe I am too slow ?
I believe you missed one of my patch (but David is traveling)
[PATCH net-next-2.6] ipv6: speedup inet6_dump_ifinfo()
This conflicts with your :
[PATCH 08/10] ipv6: use RCU to walk list of network devices
^ permalink raw reply
* Re: [PATCH 01/10] netdev: add netdev_continue_rcu
From: Eric Dumazet @ 2009-11-10 18:19 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, Paul E. McKenney, netdev
In-Reply-To: <20091110175647.200655064@vyatta.com>
Stephen Hemminger a écrit :
> This adds an RCU macro for continuing search, useful for some
> network devices like vlan.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: RFC: net: allow to propagate errors through ->ndo_hard_start_xmit()
From: Patrick McHardy @ 2009-11-10 18:20 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Linux Netdev List, Herbert Xu, David S. Miller, Stephen Hemminger
In-Reply-To: <20091110175736.GB4195@ami.dom.local>
Jarek Poplawski wrote:
> On Tue, Nov 10, 2009 at 06:31:27PM +0100, Patrick McHardy wrote:
>>
>>>> - I'm not sure the error handling in dev_hard_start_xmit() for GSO
>>>> skbs is optimal. When the driver returns an error, it is assumed
>>>> the current segment has been freed. The patch then frees the
>>>> entire GSO skb, including all remaining segments. Alternatively
>>>> it could try to transmit the remaining segments later.
>>> Anyway, it seems this freeing should be described in the changelog,
>>> if not moved to a separate patch, since it fixes another problem,
>>> unless I forgot something.
>> What other problem are you refering to? I'm not aware of any
>> problems in the existing function.
>
> This patch is about propagating errors, so it's not clear why there
> are some additional kfrees mixed with this. (But I see it's explained
> below.)
Well, to handle now propagated errors :) But sure, I'll fix up
the changelog when I return from dinner.
>>>> if (likely(!skb->next)) {
>>>> if (!list_empty(&ptype_all))
>>>> @@ -1804,6 +1804,8 @@ gso:
>>>> nskb->next = NULL;
>>>> rc = ops->ndo_start_xmit(nskb, dev);
>>>> if (unlikely(rc != NETDEV_TX_OK)) {
>>>> + if (rc & ~NETDEV_TX_MASK)
>>>> + goto out_kfree_gso_skb;
>>> If e.g. (rc == NETDEV_TX_OK | NET_XMIT_CN), why exactly is this freeing
>>> necessary now?
>>>
>>> Is e.g. (rc == NETDEV_TX_BUSY | NET_XMIT_CN) legal? If so, there would
>>> be use after kfree, I guess. Otherwise, it should be documented above
>>> (and maybe checked somewhere as well).
>> NET_XMIT_CN is a valid return value, yes. But its not freeing the
>> transmitted segment but the remaining ones. Its not strictly
>> necessary, but its the easiest way to treat all errors similar.
>> Otherwise you get complicated cases, f.i. when the driver returns
>> NET_XMIT_CN for the first segment and NETDEV_TX_OK for the
>> remaining ones.
>
> It should be in the changelog and maybe a comment too. Even if it's
> right it's a change of functionality/behavior here.
>
> I still don't know if/why (rc == NETDEV_TX_BUSY | NET_XMIT_CN) is
> OK. IMHO skb will be requeued after kfree here.
Ah I misread. NETDEV_TX_BUSY | NET_XMIT_CN is not valid. The
return value can be either a NETDEV_TX code, a NET_XMIT code
or an errno code. NETDEV_TX_OK, NET_XMIT_SUCCESS and no error
(errno) all have the value zero.
>>>> nskb->next = skb->next;
>>>> skb->next = nskb;
>>>> return rc;
>>>> @@ -1813,11 +1815,14 @@ gso:
>>>> return NETDEV_TX_BUSY;
>>>> } while (skb->next);
>>>>
>>>> - skb->destructor = DEV_GSO_CB(skb)->destructor;
>>>> + rc = NETDEV_TX_OK;
>>> When is (rc != NETDEV_TX_OK) possible in this place?
>> Its gone in the current version.
>
> Why don't you send the current version?
I did 2 hours ago :)
^ permalink raw reply
* Re: [PATCH 02/10] vlan: eliminate use of dev_base_lock
From: Eric Dumazet @ 2009-11-10 18:20 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, Patrick McHardy, netdev
In-Reply-To: <20091110175647.268344307@vyatta.com>
Stephen Hemminger a écrit :
> Do not need to use read_lock(&dev_base_lock), use RCU instead.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH 03/10] net: use rcu for network scheduler API
From: Eric Dumazet @ 2009-11-10 18:20 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20091110175647.339425269@vyatta.com>
Stephen Hemminger a écrit :
> Use RCU to walk list of network devices in qdisc dump.
> This could be optimized for large number of devices.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH 00/10] netdev: get rid of read_lock(&dev_base_lock) usages
From: Stephen Hemminger @ 2009-11-10 18:22 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <4AF9AE66.3010303@gmail.com>
On Tue, 10 Nov 2009 19:18:14 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Stephen Hemminger a écrit :
> > The goal is to eliminate dev_base_lock completely, and just use RCU
> > and rtnl_mutex for network devices. This series gets rid of the many
> > of the users of dev_base_lock just for reading.
> >
>
> Nice, but I was doing all this work Stephen... maybe I am too slow ?
>
> I believe you missed one of my patch (but David is traveling)
>
> [PATCH net-next-2.6] ipv6: speedup inet6_dump_ifinfo()
>
> This conflicts with your :
>
> [PATCH 08/10] ipv6: use RCU to walk list of network devices
>
Inflight conflict.
--
^ permalink raw reply
* Re: [PATCH 04/10] AOE: use rcu to find network device
From: Eric Dumazet @ 2009-11-10 18:23 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David Miller, Ed L. Cashin, Harvey Harrison,
Bartlomiej Zolnierkiewicz, netdev
In-Reply-To: <20091110175647.409162953@vyatta.com>
Stephen Hemminger a écrit :
> This gets rid of another use of read_lock(&dev_base_lock) by using
> RCU. Also, it only increments the reference count of the device actually
> used rather than holding and releasing every device
>
> Compile tested only.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* [PATCH] Documentation: rw_lock lessons learned
From: William Allen Simpson @ 2009-11-10 18:23 UTC (permalink / raw)
To: Linux Kernel Developers, Linux Kernel Network Developers
Cc: Eric Dumazet, Paul E. McKenney
[-- Attachment #1: Type: text/plain, Size: 263 bytes --]
In recent weeks, two different network projects erroneously
strayed down the rw_lock path. Update the Documentation
based upon comments in those threads.
---
Documentation/spinlocks.txt | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
[-- Attachment #2: spinlocks.txt.patch --]
[-- Type: text/plain, Size: 638 bytes --]
diff --git a/Documentation/spinlocks.txt b/Documentation/spinlocks.txt
index 619699d..c112052 100644
--- a/Documentation/spinlocks.txt
+++ b/Documentation/spinlocks.txt
@@ -233,4 +233,18 @@ indeed), while write-locks need to protect themselves against interrupts.
Linus
+----
+
+The implications of spin_locks on memory are further described in:
+
+ Documentation/memory-barriers.txt
+ (5) LOCK operations.
+ (6) UNLOCK operations.
+
+----
+
+We are working hard to remove reader-writer spinlocks (rw_lock) from the
+network stack, so please don't add a new one. Instead, see:
+
+ Documentation/RCU/rcu.txt
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH 00/10] netdev: get rid of read_lock(&dev_base_lock) usages
From: Stephen Hemminger @ 2009-11-10 18:24 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <4AF9AE66.3010303@gmail.com>
I was just trying to pick up the stragglers you left behind :-)
The nasty cases left are bonding (whose existing locking model is a pile
of crap), and sysfs (slightly less stinky). The bonding code just needs to
be rewritten to have a sane/simple model.
^ permalink raw reply
* Re: [PATCH 05/10] parisc: use RCU to find network device
From: Eric Dumazet @ 2009-11-10 18:26 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David Miller, Kyle McMartin, Helge Deller, Alexander Beregalov,
netdev, linux-parisc
In-Reply-To: <20091110175647.480041042@vyatta.com>
Stephen Hemminger a écrit :
> Another place where RCU can be used instead of read_lock(&dev_base_lock)
> This is by inspection, don't have platform or cross-build environment
> to validate.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
Duplicate of previously posted patch...
http://article.gmane.org/gmane.linux.network/143072
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox