* [PATCH 09/11] netpoll: ethernet devices only
From: Stephen Hemminger @ 2007-11-03 18:43 UTC (permalink / raw)
To: David Miller, Satyam Sharma; +Cc: netdev
In-Reply-To: <20071103184314.216145305@linux-foundation.org>
[-- Attachment #1: netpoll-ether-only.patch --]
[-- Type: text/plain, Size: 687 bytes --]
Netpoll only works on Ethernet devices, so check during setup
rather than just failing silently later.
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/net/core/netpoll.c 2007-11-03 11:05:33.000000000 -0700
+++ b/net/core/netpoll.c 2007-11-03 11:08:23.000000000 -0700
@@ -653,6 +653,12 @@ int netpoll_setup(struct netpoll *np, st
unsigned long flags;
int err;
+ if (ndev->type != ARPHRD_ETHER) {
+ printk(KERN_ERR "netpoll: %s is not an ethernet device\n",
+ ndev->name);
+ return -EINVAL;
+ }
+
np->dev = ndev;
if (!ndev->npinfo) {
npinfo = kmalloc(sizeof(*npinfo), GFP_KERNEL);
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* [PATCH 06/11] netpoll: remove dev_name for npinfo
From: Stephen Hemminger @ 2007-11-03 18:43 UTC (permalink / raw)
To: David Miller, Satyam Sharma; +Cc: netdev
In-Reply-To: <20071103184314.216145305@linux-foundation.org>
[-- Attachment #1: netpoll-noname.patch --]
[-- Type: text/plain, Size: 9910 bytes --]
The device name was only in npinfo for netconsole target
configuration, so move it to netconsole. Netconsole only
needs the value during config, so no need to do all
the device name tracking etc..
Make functions for common code for instantiation and
start up.
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/drivers/net/netconsole.c 2007-11-03 09:26:27.000000000 -0700
+++ b/drivers/net/netconsole.c 2007-11-03 10:02:18.000000000 -0700
@@ -96,6 +96,7 @@ struct netconsole_target {
struct config_item item;
#endif
int enabled;
+ char dev_name[IFNAMSIZ];
struct netpoll np;
};
@@ -157,39 +158,67 @@ static void netconsole_target_put(struct
#endif /* CONFIG_NETCONSOLE_DYNAMIC */
+
+
+/*
+ * Allocate and initialize with defaults.
+ * Note that these targets get their config_item fields zeroed-out.
+ */
+static struct netconsole_target *new_target(void)
+{
+ struct netconsole_target *nt;
+
+ nt = kzalloc(sizeof(*nt), GFP_KERNEL);
+ if (nt) {
+ nt->np.name = "netconsole";
+ strlcpy(nt->dev_name, "eth0", IFNAMSIZ);
+ nt->np.local_port = 6665;
+ nt->np.remote_port = 6666;
+ memset(nt->np.remote_mac, 0xff, ETH_ALEN);
+ }
+
+ return nt;
+}
+
+static int start_target(struct netconsole_target *nt)
+{
+ struct net_device *dev;
+ int err;
+
+ dev = dev_get_by_name(&init_net, nt->dev_name);
+ if (!dev)
+ return -ENODEV;
+
+ err = netpoll_setup(&nt->np, dev);
+ if (err)
+ dev_put(dev);
+ else
+ nt->enabled = 1;
+ return err;
+}
+
+
/* Allocate new target (from boot/module param) and setup netpoll for it */
static struct netconsole_target *alloc_param_target(char *target_config)
{
int err = -ENOMEM;
struct netconsole_target *nt;
- /*
- * Allocate and initialize with defaults.
- * Note that these targets get their config_item fields zeroed-out.
- */
- nt = kzalloc(sizeof(*nt), GFP_KERNEL);
+ nt = new_target();
if (!nt) {
printk(KERN_ERR "netconsole: failed to allocate memory\n");
goto fail;
}
- nt->np.name = "netconsole";
- strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
- nt->np.local_port = 6665;
- nt->np.remote_port = 6666;
- memset(nt->np.remote_mac, 0xff, ETH_ALEN);
-
/* Parse parameters and setup netpoll */
- err = netpoll_parse_options(&nt->np, target_config);
+ err = netpoll_parse_options(&nt->np, target_config, nt->dev_name);
if (err)
goto fail;
- err = netpoll_setup(&nt->np);
+
+ err = start_target(nt);
if (err)
goto fail;
-
- nt->enabled = 1;
-
return nt;
fail:
@@ -279,7 +308,8 @@ static ssize_t show_enabled(struct netco
static ssize_t show_dev_name(struct netconsole_target *nt, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%s\n", nt->np.dev_name);
+ return snprintf(buf, PAGE_SIZE, "%s\n",
+ nt->enabled ? nt->np.dev->name : nt->dev_name);
}
static ssize_t show_local_port(struct netconsole_target *nt, char *buf)
@@ -339,14 +369,13 @@ static ssize_t store_enabled(struct netc
return enabled;
if (enabled) { /* 1 */
-
/*
* Skip netpoll_parse_options() -- all the attributes are
* already configured via configfs. Just print them out.
*/
netpoll_print_options(&nt->np);
- err = netpoll_setup(&nt->np);
+ err = start_target(nt);
if (err)
return err;
@@ -365,7 +394,7 @@ static ssize_t store_dev_name(struct net
const char *buf,
size_t count)
{
- size_t len;
+ char *cp;
if (nt->enabled) {
printk(KERN_ERR "netconsole: target (%s) is enabled, "
@@ -374,12 +403,12 @@ static ssize_t store_dev_name(struct net
return -EINVAL;
}
- strlcpy(nt->np.dev_name, buf, IFNAMSIZ);
+ strlcpy(nt->dev_name, buf, IFNAMSIZ);
/* Get rid of possible trailing newline from echo(1) */
- len = strnlen(nt->np.dev_name, IFNAMSIZ);
- if (nt->np.dev_name[len - 1] == '\n')
- nt->np.dev_name[len - 1] = '\0';
+ cp = strnchr(nt->dev_name, '\n', IFNAMSIZ);
+ if (cp)
+ *cp = '\0';
return strnlen(buf, count);
}
@@ -591,21 +620,7 @@ static struct config_item *make_netconso
unsigned long flags;
struct netconsole_target *nt;
- /*
- * Allocate and initialize with defaults.
- * Target is disabled at creation (enabled == 0).
- */
- nt = kzalloc(sizeof(*nt), GFP_KERNEL);
- if (!nt) {
- printk(KERN_ERR "netconsole: failed to allocate memory\n");
- return NULL;
- }
-
- nt->np.name = "netconsole";
- strlcpy(nt->np.dev_name, "eth0", IFNAMSIZ);
- nt->np.local_port = 6665;
- nt->np.remote_port = 6666;
- memset(nt->np.remote_mac, 0xff, ETH_ALEN);
+ nt = new_target();
/* Initialize the config_item member */
config_item_init_type_name(&nt->item, name, &netconsole_target_type);
@@ -660,40 +675,6 @@ static struct configfs_subsystem netcons
#endif /* CONFIG_NETCONSOLE_DYNAMIC */
-/* Handle network interface device notifications */
-static int netconsole_netdev_event(struct notifier_block *this,
- unsigned long event,
- void *ptr)
-{
- unsigned long flags;
- struct netconsole_target *nt;
- struct net_device *dev = ptr;
-
- if (!(event == NETDEV_CHANGENAME))
- goto done;
-
- spin_lock_irqsave(&target_list_lock, flags);
- list_for_each_entry(nt, &target_list, list) {
- netconsole_target_get(nt);
- if (nt->np.dev == dev) {
- switch (event) {
- case NETDEV_CHANGENAME:
- strlcpy(nt->np.dev_name, dev->name, IFNAMSIZ);
- break;
- }
- }
- netconsole_target_put(nt);
- }
- spin_unlock_irqrestore(&target_list_lock, flags);
-
-done:
- return NOTIFY_DONE;
-}
-
-static struct notifier_block netconsole_netdev_notifier = {
- .notifier_call = netconsole_netdev_event,
-};
-
static void write_msg(struct console *con, const char *msg, unsigned int len)
{
int frag, left;
@@ -755,22 +736,15 @@ static int __init init_netconsole(void)
}
}
- err = register_netdevice_notifier(&netconsole_netdev_notifier);
- if (err)
- goto fail;
-
err = dynamic_netconsole_init();
if (err)
- goto undonotifier;
+ goto fail;
register_console(&netconsole);
printk(KERN_INFO "netconsole: network logging started\n");
return err;
-undonotifier:
- unregister_netdevice_notifier(&netconsole_netdev_notifier);
-
fail:
printk(KERN_ERR "netconsole: cleaning up\n");
@@ -793,7 +767,6 @@ static void __exit cleanup_netconsole(vo
unregister_console(&netconsole);
dynamic_netconsole_exit();
- unregister_netdevice_notifier(&netconsole_netdev_notifier);
/*
* Targets created via configfs pin references on our module
--- a/include/linux/netpoll.h 2007-11-03 09:35:45.000000000 -0700
+++ b/include/linux/netpoll.h 2007-11-03 09:41:57.000000000 -0700
@@ -14,7 +14,6 @@
struct netpoll {
struct net_device *dev;
- char dev_name[IFNAMSIZ];
const char *name;
void (*rx_hook)(struct netpoll *, int, char *, int);
@@ -35,8 +34,8 @@ struct netpoll_info {
void netpoll_poll(struct netpoll *np);
void netpoll_send_udp(struct netpoll *np, const char *msg, int len);
void netpoll_print_options(struct netpoll *np);
-int netpoll_parse_options(struct netpoll *np, char *opt);
-int netpoll_setup(struct netpoll *np);
+int netpoll_parse_options(struct netpoll *np, char *opt, char *name);
+int netpoll_setup(struct netpoll *np, struct net_device *dev);
int netpoll_trap(void);
void netpoll_set_trap(int trap);
void netpoll_cleanup(struct netpoll *np);
--- a/net/core/netpoll.c 2007-11-03 09:35:45.000000000 -0700
+++ b/net/core/netpoll.c 2007-11-03 09:42:43.000000000 -0700
@@ -552,7 +552,7 @@ void netpoll_print_options(struct netpol
printk(KERN_INFO "%s: local IP %d.%d.%d.%d\n",
np->name, HIPQUAD(np->local_ip));
printk(KERN_INFO "%s: interface %s\n",
- np->name, np->dev_name);
+ np->name, np->dev->name);
printk(KERN_INFO "%s: remote port %d\n",
np->name, np->remote_port);
printk(KERN_INFO "%s: remote IP %d.%d.%d.%d\n",
@@ -561,7 +561,7 @@ void netpoll_print_options(struct netpol
np->name, print_mac(mac, np->remote_mac));
}
-int netpoll_parse_options(struct netpoll *np, char *opt)
+int netpoll_parse_options(struct netpoll *np, char *opt, char *dev_name)
{
char *cur=opt, *delim;
@@ -588,7 +588,7 @@ int netpoll_parse_options(struct netpoll
if ((delim = strchr(cur, ',')) == NULL)
goto parse_failed;
*delim = 0;
- strlcpy(np->dev_name, cur, sizeof(np->dev_name));
+ strlcpy(dev_name, cur, IFNAMSIZ);
cur = delim;
}
cur++;
@@ -650,22 +650,13 @@ int netpoll_parse_options(struct netpoll
return -1;
}
-int netpoll_setup(struct netpoll *np)
+int netpoll_setup(struct netpoll *np, struct net_device *ndev)
{
- struct net_device *ndev = NULL;
struct in_device *in_dev;
struct netpoll_info *npinfo;
unsigned long flags;
int err;
- if (np->dev_name)
- ndev = dev_get_by_name(&init_net, np->dev_name);
- if (!ndev) {
- printk(KERN_ERR "%s: %s doesn't exist, aborting.\n",
- np->name, np->dev_name);
- return -ENODEV;
- }
-
np->dev = ndev;
if (!ndev->npinfo) {
npinfo = kmalloc(sizeof(*npinfo), GFP_KERNEL);
@@ -689,7 +680,7 @@ int netpoll_setup(struct netpoll *np)
if (!ndev->poll_controller) {
printk(KERN_ERR "%s: %s doesn't support polling, aborting.\n",
- np->name, np->dev_name);
+ np->name, ndev->name);
err = -ENOTSUPP;
goto release;
}
@@ -698,7 +689,7 @@ int netpoll_setup(struct netpoll *np)
unsigned long atmost, atleast;
printk(KERN_INFO "%s: device %s not up yet, forcing it\n",
- np->name, np->dev_name);
+ np->name, ndev->name);
rtnl_lock();
err = dev_open(ndev);
@@ -742,7 +733,7 @@ int netpoll_setup(struct netpoll *np)
if (!in_dev || !in_dev->ifa_list) {
rcu_read_unlock();
printk(KERN_ERR "%s: no IP address for %s, aborting\n",
- np->name, np->dev_name);
+ np->name, ndev->name);
err = -EDESTADDRREQ;
goto release;
}
@@ -774,7 +765,6 @@ int netpoll_setup(struct netpoll *np)
if (!ndev->npinfo)
kfree(npinfo);
np->dev = NULL;
- dev_put(ndev);
return err;
}
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* [PATCH 02/11] netpoll: netpoll_poll cleanup
From: Stephen Hemminger @ 2007-11-03 18:43 UTC (permalink / raw)
To: David Miller, Satyam Sharma; +Cc: netdev
In-Reply-To: <20071103184314.216145305@linux-foundation.org>
[-- Attachment #1: netpoll-poll-cleanup.patch --]
[-- Type: text/plain, Size: 2006 bytes --]
Restructure code slightly to improve readability:
* dereference device once
* change obvious while() loop
* let poll_napi() handle null list itself
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/net/core/netpoll.c 2007-11-03 10:02:50.000000000 -0700
+++ b/net/core/netpoll.c 2007-11-03 11:40:27.000000000 -0700
@@ -139,16 +139,15 @@ static int poll_one_napi(struct netpoll_
return budget - work;
}
-static void poll_napi(struct netpoll *np)
+static void poll_napi(struct net_device *dev)
{
- struct netpoll_info *npinfo = np->dev->npinfo;
- struct napi_struct *napi;
int budget = 16;
+ struct napi_struct *napi;
- list_for_each_entry(napi, &np->dev->napi_list, dev_list) {
+ list_for_each_entry(napi, &dev->napi_list, dev_list) {
if (napi->poll_owner != smp_processor_id() &&
spin_trylock(&napi->poll_lock)) {
- budget = poll_one_napi(npinfo, napi, budget);
+ budget = poll_one_napi(dev->npinfo, napi, budget);
spin_unlock(&napi->poll_lock);
if (!budget)
@@ -159,30 +158,27 @@ static void poll_napi(struct netpoll *np
static void service_arp_queue(struct netpoll_info *npi)
{
- struct sk_buff *skb;
+ if (npi) {
+ struct sk_buff *skb;
- if (unlikely(!npi))
- return;
-
- skb = skb_dequeue(&npi->arp_tx);
-
- while (skb != NULL) {
- arp_reply(skb);
- skb = skb_dequeue(&npi->arp_tx);
+ while ( (skb = skb_dequeue(&npi->arp_tx)) )
+ arp_reply(skb);
}
}
void netpoll_poll(struct netpoll *np)
{
- if (!np->dev || !netif_running(np->dev) || !np->dev->poll_controller)
+ struct net_device *dev = np->dev;
+
+ if (!dev || !netif_running(dev) || !dev->poll_controller)
return;
/* Process pending work on NIC */
- np->dev->poll_controller(np->dev);
- if (!list_empty(&np->dev->napi_list))
- poll_napi(np);
+ dev->poll_controller(dev);
+
+ poll_napi(dev);
- service_arp_queue(np->dev->npinfo);
+ service_arp_queue(dev->npinfo);
zap_completion_queue();
}
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* [PATCH 07/11] netpoll: get rid of name parameter
From: Stephen Hemminger @ 2007-11-03 18:43 UTC (permalink / raw)
To: David Miller, Satyam Sharma; +Cc: netdev
In-Reply-To: <20071103184314.216145305@linux-foundation.org>
[-- Attachment #1: netpoll-no-name.patch --]
[-- Type: text/plain, Size: 6229 bytes --]
The name was being stored and used only for error messages.
The same effect can be had by just passing it in where needed during
config.
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/drivers/net/netconsole.c 2007-11-03 10:03:40.000000000 -0700
+++ b/drivers/net/netconsole.c 2007-11-03 10:12:35.000000000 -0700
@@ -170,7 +170,6 @@ static struct netconsole_target *new_tar
nt = kzalloc(sizeof(*nt), GFP_KERNEL);
if (nt) {
- nt->np.name = "netconsole";
strlcpy(nt->dev_name, "eth0", IFNAMSIZ);
nt->np.local_port = 6665;
nt->np.remote_port = 6666;
@@ -186,8 +185,13 @@ static int start_target(struct netconsol
int err;
dev = dev_get_by_name(&init_net, nt->dev_name);
- if (!dev)
+ if (!dev) {
+ printk(KERN_ERR "netconsole: device '%s' does not exist.\n",
+ nt->dev_name);
return -ENODEV;
+ }
+
+ netpoll_print_options("netconsole", &nt->np);
err = netpoll_setup(&nt->np, dev);
if (err)
@@ -212,13 +216,16 @@ static struct netconsole_target *alloc_p
/* Parse parameters and setup netpoll */
err = netpoll_parse_options(&nt->np, target_config, nt->dev_name);
- if (err)
+ if (err) {
+ printk(KERN_ERR "netconsole: parse options '%s' failed %d\n",
+ target_config, err);
goto fail;
-
+ }
err = start_target(nt);
if (err)
goto fail;
+
return nt;
fail:
@@ -369,12 +376,6 @@ static ssize_t store_enabled(struct netc
return enabled;
if (enabled) { /* 1 */
- /*
- * Skip netpoll_parse_options() -- all the attributes are
- * already configured via configfs. Just print them out.
- */
- netpoll_print_options(&nt->np);
-
err = start_target(nt);
if (err)
return err;
--- a/include/linux/netpoll.h 2007-11-03 10:03:40.000000000 -0700
+++ b/include/linux/netpoll.h 2007-11-03 10:10:00.000000000 -0700
@@ -14,7 +14,6 @@
struct netpoll {
struct net_device *dev;
- const char *name;
void (*rx_hook)(struct netpoll *, int, char *, int);
u32 local_ip, remote_ip;
@@ -33,7 +32,7 @@ struct netpoll_info {
void netpoll_poll(struct netpoll *np);
void netpoll_send_udp(struct netpoll *np, const char *msg, int len);
-void netpoll_print_options(struct netpoll *np);
+void netpoll_print_options(const char *prefix, struct netpoll *np);
int netpoll_parse_options(struct netpoll *np, char *opt, char *name);
int netpoll_setup(struct netpoll *np, struct net_device *dev);
int netpoll_trap(void);
--- a/net/core/netpoll.c 2007-11-03 10:03:40.000000000 -0700
+++ b/net/core/netpoll.c 2007-11-03 10:22:44.000000000 -0700
@@ -544,21 +544,21 @@ out:
return 0;
}
-void netpoll_print_options(struct netpoll *np)
+void netpoll_print_options(const char *name, struct netpoll *np)
{
DECLARE_MAC_BUF(mac);
printk(KERN_INFO "%s: local port %d\n",
- np->name, np->local_port);
+ name, np->local_port);
printk(KERN_INFO "%s: local IP %d.%d.%d.%d\n",
- np->name, HIPQUAD(np->local_ip));
+ name, HIPQUAD(np->local_ip));
printk(KERN_INFO "%s: interface %s\n",
- np->name, np->dev->name);
+ name, np->dev->name);
printk(KERN_INFO "%s: remote port %d\n",
- np->name, np->remote_port);
+ name, np->remote_port);
printk(KERN_INFO "%s: remote IP %d.%d.%d.%d\n",
- np->name, HIPQUAD(np->remote_ip));
+ name, HIPQUAD(np->remote_ip));
printk(KERN_INFO "%s: remote ethernet address %s\n",
- np->name, print_mac(mac, np->remote_mac));
+ name, print_mac(mac, np->remote_mac));
}
int netpoll_parse_options(struct netpoll *np, char *opt, char *dev_name)
@@ -640,13 +640,9 @@ int netpoll_parse_options(struct netpoll
np->remote_mac[5] = simple_strtol(cur, NULL, 16);
}
- netpoll_print_options(np);
-
return 0;
parse_failed:
- printk(KERN_INFO "%s: couldn't parse config at %s!\n",
- np->name, cur);
return -1;
}
@@ -679,8 +675,8 @@ int netpoll_setup(struct netpoll *np, st
}
if (!ndev->poll_controller) {
- printk(KERN_ERR "%s: %s doesn't support polling, aborting.\n",
- np->name, ndev->name);
+ printk(KERN_ERR "netpoll: %s doesn't support polling, aborting.\n",
+ ndev->name);
err = -ENOTSUPP;
goto release;
}
@@ -688,16 +684,16 @@ int netpoll_setup(struct netpoll *np, st
if (!netif_running(ndev)) {
unsigned long atmost, atleast;
- printk(KERN_INFO "%s: device %s not up yet, forcing it\n",
- np->name, ndev->name);
+ printk(KERN_INFO "netpoll: device %s not up yet, forcing it\n",
+ ndev->name);
rtnl_lock();
err = dev_open(ndev);
rtnl_unlock();
if (err) {
- printk(KERN_ERR "%s: failed to open %s\n",
- np->name, ndev->name);
+ printk(KERN_ERR "netpoll: failed to open %s\n",
+ ndev->name);
goto release;
}
@@ -705,9 +701,9 @@ int netpoll_setup(struct netpoll *np, st
atmost = jiffies + 4*HZ;
while (!netif_carrier_ok(ndev)) {
if (time_after(jiffies, atmost)) {
- printk(KERN_NOTICE
- "%s: timeout waiting for carrier\n",
- np->name);
+ printk(KERN_NOTICE "netpoll:"
+ "timeout waiting for carrier on '%s'\n",
+ ndev->name);
break;
}
cond_resched();
@@ -719,9 +715,9 @@ int netpoll_setup(struct netpoll *np, st
*/
if (time_before(jiffies, atleast)) {
- printk(KERN_NOTICE "%s: carrier detect appears"
- " untrustworthy, waiting 4 seconds\n",
- np->name);
+ printk(KERN_NOTICE "netpoll: carrier detect appears"
+ " untrustworthy '%s', waiting 4 seconds\n",
+ ndev->name);
msleep(4000);
}
}
@@ -732,16 +728,16 @@ int netpoll_setup(struct netpoll *np, st
if (!in_dev || !in_dev->ifa_list) {
rcu_read_unlock();
- printk(KERN_ERR "%s: no IP address for %s, aborting\n",
- np->name, ndev->name);
+ printk(KERN_ERR "netpoll: no IP address for %s, aborting\n",
+ ndev->name);
err = -EDESTADDRREQ;
goto release;
}
np->local_ip = ntohl(in_dev->ifa_list->ifa_local);
rcu_read_unlock();
- printk(KERN_INFO "%s: local IP %d.%d.%d.%d\n",
- np->name, HIPQUAD(np->local_ip));
+ printk(KERN_INFO "netpoll: local IP %d.%d.%d.%d\n",
+ HIPQUAD(np->local_ip));
}
if (np->rx_hook) {
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* [PATCH 05/11] netpoll: dont need rx_flags
From: Stephen Hemminger @ 2007-11-03 18:43 UTC (permalink / raw)
To: David Miller, Satyam Sharma; +Cc: netdev
In-Reply-To: <20071103184314.216145305@linux-foundation.org>
[-- Attachment #1: netpoll-enable-flag.patch --]
[-- Type: text/plain, Size: 2071 bytes --]
The rx_flags variable is redundant. Turning rx on/off is done
via setting the rx_np pointer.
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/include/linux/netpoll.h 2007-11-03 09:32:54.000000000 -0700
+++ b/include/linux/netpoll.h 2007-11-03 09:35:45.000000000 -0700
@@ -25,7 +25,6 @@ struct netpoll {
struct netpoll_info {
atomic_t refcnt;
- int rx_flags;
spinlock_t rx_lock;
struct netpoll *rx_np; /* netpoll that registered an rx_hook */
struct sk_buff_head arp_tx; /* list of arp requests to reply to */
@@ -51,12 +50,11 @@ static inline int netpoll_rx(struct sk_b
unsigned long flags;
int ret = 0;
- if (!npinfo || (!npinfo->rx_np && !npinfo->rx_flags))
+ if (!npinfo || !npinfo->rx_np)
return 0;
spin_lock_irqsave(&npinfo->rx_lock, flags);
- /* check rx_flags again with the lock held */
- if (npinfo->rx_flags && __netpoll_rx(skb))
+ if (__netpoll_rx(skb))
ret = 1;
spin_unlock_irqrestore(&npinfo->rx_lock, flags);
--- a/net/core/netpoll.c 2007-11-03 09:33:31.000000000 -0700
+++ b/net/core/netpoll.c 2007-11-03 09:35:45.000000000 -0700
@@ -39,7 +39,6 @@ static struct sk_buff_head skb_pool;
static atomic_t trapped;
#define USEC_PER_POLL 50
-#define NETPOLL_RX_ENABLED 1
#define MAX_SKB_SIZE \
(MAX_UDP_CHUNK + sizeof(struct udphdr) + \
@@ -675,7 +674,6 @@ int netpoll_setup(struct netpoll *np)
goto release;
}
- npinfo->rx_flags = 0;
npinfo->rx_np = NULL;
spin_lock_init(&npinfo->rx_lock);
@@ -757,7 +755,6 @@ int netpoll_setup(struct netpoll *np)
if (np->rx_hook) {
spin_lock_irqsave(&npinfo->rx_lock, flags);
- npinfo->rx_flags |= NETPOLL_RX_ENABLED;
npinfo->rx_np = np;
spin_unlock_irqrestore(&npinfo->rx_lock, flags);
}
@@ -799,7 +796,6 @@ void netpoll_cleanup(struct netpoll *np)
if (npinfo->rx_np == np) {
spin_lock_irqsave(&npinfo->rx_lock, flags);
npinfo->rx_np = NULL;
- npinfo->rx_flags &= ~NETPOLL_RX_ENABLED;
spin_unlock_irqrestore(&npinfo->rx_lock, flags);
}
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* [PATCH 11/11] netpoll: rx use RCU
From: Stephen Hemminger @ 2007-11-03 18:43 UTC (permalink / raw)
To: David Miller, Satyam Sharma; +Cc: netdev
In-Reply-To: <20071103184314.216145305@linux-foundation.org>
[-- Attachment #1: netpoll-rcu.patch --]
[-- Type: text/plain, Size: 3049 bytes --]
Get rid of rx_lock and use Read-Copy-Update to make sure
that netpoll info and rx handle are not used after free.
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/include/linux/netpoll.h 2007-11-03 11:08:36.000000000 -0700
+++ b/include/linux/netpoll.h 2007-11-03 11:15:11.000000000 -0700
@@ -23,7 +23,6 @@ struct netpoll {
struct netpoll_info {
atomic_t refcnt;
- spinlock_t rx_lock;
struct netpoll *rx_np; /* netpoll that registered an rx_hook */
struct sk_buff_head arp_tx; /* list of arp requests to reply to */
struct sk_buff_head txq;
--- a/net/core/netpoll.c 2007-11-03 11:08:36.000000000 -0700
+++ b/net/core/netpoll.c 2007-11-03 11:15:29.000000000 -0700
@@ -463,15 +463,15 @@ bool __netpoll_rx(struct sk_buff *skb)
int proto, len, ulen;
struct iphdr *iph;
struct udphdr *uh;
- struct netpoll_info *npi = skb->dev->npinfo;
+ struct netpoll_info *npi;
struct netpoll *np;
- unsigned long flags;
+ rcu_read_lock();
+ npi = rcu_dereference(skb->dev->npinfo);
if (!npi)
- return false;
+ goto out;
- spin_lock_irqsave(&npi->rx_lock, flags);
- np = npi->rx_np;
+ np = rcu_dereference(npi->rx_np);
if (!np)
goto out;
@@ -535,13 +535,13 @@ bool __netpoll_rx(struct sk_buff *skb)
np->rx_hook(np, ntohs(uh->source),
(char *)(uh+1),
ulen - sizeof(struct udphdr));
- spin_unlock_irqrestore(&npi->rx_lock, flags);
+ rcu_read_unlock();
kfree_skb(skb);
return true;
out:
- spin_unlock_irqrestore(&npi->rx_lock, flags);
+ rcu_read_unlock();
/* If packet received while already in poll then just
* silently drop.
*/
@@ -678,7 +678,6 @@ int netpoll_setup(struct netpoll *np, st
npinfo->rx_np = NULL;
- spin_lock_init(&npinfo->rx_lock);
skb_queue_head_init(&npinfo->arp_tx);
skb_queue_head_init(&npinfo->txq);
INIT_DELAYED_WORK(&npinfo->tx_work, queue_process);
@@ -755,11 +754,8 @@ int netpoll_setup(struct netpoll *np, st
HIPQUAD(np->local_ip));
}
- if (np->rx_hook) {
- spin_lock_irqsave(&npinfo->rx_lock, flags);
- npinfo->rx_np = np;
- spin_unlock_irqrestore(&npinfo->rx_lock, flags);
- }
+ if (np->rx_hook)
+ rcu_assign_pointer(npinfo->rx_np, np);
/* fill up the skb queue */
refill_skbs();
@@ -794,21 +790,21 @@ void netpoll_cleanup(struct netpoll *np)
if (np->dev) {
npinfo = np->dev->npinfo;
if (npinfo) {
- if (npinfo->rx_np == np) {
- spin_lock_irqsave(&npinfo->rx_lock, flags);
- npinfo->rx_np = NULL;
- spin_unlock_irqrestore(&npinfo->rx_lock, flags);
- }
+ if (npinfo->rx_np == np)
+ rcu_assign_pointer(npinfo->rx_np, NULL);
if (atomic_dec_and_test(&npinfo->refcnt)) {
+
skb_queue_purge(&npinfo->arp_tx);
skb_queue_purge(&npinfo->txq);
cancel_rearming_delayed_work(&npinfo->tx_work);
/* clean after last, unfinished work */
__skb_queue_purge(&npinfo->txq);
+
+ rcu_assign_pointer(np->dev->npinfo, NULL);
+ synchronize_net();
kfree(npinfo);
- np->dev->npinfo = NULL;
}
}
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* [PATCH 10/11] netpoll: rx optimization
From: Stephen Hemminger @ 2007-11-03 18:43 UTC (permalink / raw)
To: David Miller, Satyam Sharma; +Cc: netdev
In-Reply-To: <20071103184314.216145305@linux-foundation.org>
[-- Attachment #1: netpoll-rx.patch --]
[-- Type: text/plain, Size: 3603 bytes --]
This patch makes netpoll work for non-NAPI devices that call
netif_receive_skb. Devices are allowed to call netif_receive_skb
if they are receiving packets in softirq (ie in tasklet).
One side effect of this is that received packets will be looked
at twice for the non-NAPI case, but this is harmless.
Move the locking out of the inline hook and into the internal
function.
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/include/linux/netpoll.h 2007-11-03 11:05:33.000000000 -0700
+++ b/include/linux/netpoll.h 2007-11-03 11:08:36.000000000 -0700
@@ -30,6 +30,9 @@ struct netpoll_info {
struct delayed_work tx_work;
};
+
+#ifdef CONFIG_NETPOLL
+
void netpoll_poll(struct netpoll *np);
void netpoll_send_udp(struct netpoll *np, const char *msg, int len);
void netpoll_print_options(const char *prefix, struct netpoll *np);
@@ -38,32 +41,18 @@ int netpoll_setup(struct netpoll *np, st
int netpoll_trap(void);
void netpoll_set_trap(int trap);
void netpoll_cleanup(struct netpoll *np);
-int __netpoll_rx(struct sk_buff *skb);
+bool __netpoll_rx(struct sk_buff *skb);
-#ifdef CONFIG_NETPOLL
-static inline int netpoll_rx(struct sk_buff *skb)
+/* Hijack incoming packet for use by netpoll.
+ * NB: may be called twice for NAPI case
+ */
+static inline bool netpoll_rx(struct sk_buff *skb)
{
- struct netpoll_info *npinfo = skb->dev->npinfo;
- unsigned long flags;
- int ret = 0;
-
- if (!npinfo || !npinfo->rx_np)
- return 0;
-
- spin_lock_irqsave(&npinfo->rx_lock, flags);
- if (__netpoll_rx(skb))
- ret = 1;
- spin_unlock_irqrestore(&npinfo->rx_lock, flags);
+ if (unlikely(skb->dev->npinfo))
+ return __netpoll_rx(skb);
- return ret;
-}
-
-static inline int netpoll_receive_skb(struct sk_buff *skb)
-{
- if (!list_empty(&skb->dev->napi_list))
- return netpoll_rx(skb);
- return 0;
+ return false;
}
static inline void *netpoll_poll_lock(struct napi_struct *napi)
--- a/net/core/dev.c 2007-11-03 11:05:33.000000000 -0700
+++ b/net/core/dev.c 2007-11-03 11:08:36.000000000 -0700
@@ -2020,8 +2020,7 @@ int netif_receive_skb(struct sk_buff *sk
int ret = NET_RX_DROP;
__be16 type;
- /* if we've gotten here through NAPI, check netpoll */
- if (netpoll_receive_skb(skb))
+ if (netpoll_rx(skb))
return NET_RX_DROP;
if (!skb->tstamp.tv64)
--- a/net/core/netpoll.c 2007-11-03 11:08:23.000000000 -0700
+++ b/net/core/netpoll.c 2007-11-03 11:08:36.000000000 -0700
@@ -458,16 +458,23 @@ static void arp_reply(struct sk_buff *sk
netpoll_send_skb(np, send_skb);
}
-int __netpoll_rx(struct sk_buff *skb)
+bool __netpoll_rx(struct sk_buff *skb)
{
int proto, len, ulen;
struct iphdr *iph;
struct udphdr *uh;
struct netpoll_info *npi = skb->dev->npinfo;
- struct netpoll *np = npi->rx_np;
+ struct netpoll *np;
+ unsigned long flags;
+
+ if (!npi)
+ return false;
+ spin_lock_irqsave(&npi->rx_lock, flags);
+ np = npi->rx_np;
if (!np)
goto out;
+
if (skb->dev->type != ARPHRD_ETHER)
goto out;
@@ -528,20 +535,22 @@ int __netpoll_rx(struct sk_buff *skb)
np->rx_hook(np, ntohs(uh->source),
(char *)(uh+1),
ulen - sizeof(struct udphdr));
+ spin_unlock_irqrestore(&npi->rx_lock, flags);
kfree_skb(skb);
- return 1;
+ return true;
out:
+ spin_unlock_irqrestore(&npi->rx_lock, flags);
/* If packet received while already in poll then just
* silently drop.
*/
if (atomic_read(&trapped)) {
kfree_skb(skb);
- return 1;
+ return true;
}
- return 0;
+ return false;
}
void netpoll_print_options(const char *name, struct netpoll *np)
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: Fwd: Problem accessing a Certain Remote IP with Kernel 2.6.24-rc1
From: Stephen Hemminger @ 2007-11-03 23:38 UTC (permalink / raw)
To: Sparkletone; +Cc: netdev, shlomif
In-Reply-To: <1855FBA6-5EFE-46E2-A334-AB31C5EDC404@gmail.com>
On Sat, 3 Nov 2007 15:34:18 -0500
Sparkletone <sparkletone@gmail.com> wrote:
> Forwarding this for someone who was getting bounces for some reason.
>
> Begin forwarded message:
>
> > From: Shlomi Fish <shlomif@iglu.org.il>
> > Date: November 3, 2007 3:31:56 PM CDT
> > To: sparkletone@gmail.com
> > Subject: Problem accessing a Certain Remote IP with Kernel 2.6.24-rc1
> >
> > Hi all!
> >
> > I hope this mail gets through because last time I checked, I got
> > bounces
> > whenever I tried to send email to LKML ("not liked source" for mail).
> >
> > I've been having a problem with kernel 2.6.24-rc1 on Mandriva
> > Cooker. The
> > problem is that after using it for a while connecting through TCP to
> > the
> > following remote, Internet IP - 212.143.218.31 - takes a very long
> > time.
> >
> > Here's what I've learned:
> >
> > 1. Kernel 2.6.23 seem to exhibit this problem tooa fter a while.
> >
> > 2. It already happened twice with kernel 2.6.24-rc1.
> >
> > 3. A different computer on the same Home LAN connected via a NAT/
> > router has no
> > problem with that IP. (At the same time, the Linux computer exhibits
> > the
> > problematic behaviour).
> >
> > 4. I could connect using telnet to port 80 eventually, but it took
> > an awfully
> > long time.
> >
> > 5. I have problem with both HTTP to port 80 and POP.
> >
> > 6. Restarting the network ("/etc/init.d/network restart") does not
> > help - only
> > a reboot.
> >
> > 7. The network as a whole (Google, etc.) works fine.
> >
> > I need this IP, because it hosts my POP account and my homepage.
> >
> > Per Rik van Riel's advice I prepared Ethereal (libpcap) dumps of the
> > conversations before ("good") and after ("bad") the problem surfaced:
> >
> > http://freehackers.org/~shlomif/files/files/www.sf.org-conn-problem/
> >
> > I'd appreciate if anyone would be able to shed more light on this
> > problem and
> > hopefully fix it. Please let me know if there's anything else you
> > need.
Sounds like another instance of the default window scaling causing problems.
Look up TCP window scaling on lwn.net. There is almost some
stupid middlebox/firewall that doesn't handle window scaling properly.
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: Endianness problem with u32 classifier hash masks
From: Jarek Poplawski @ 2007-11-03 23:58 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: hadi, Radu Rendec, netdev
In-Reply-To: <472D06B2.9040402@o2.pl>
Jarek Poplawski wrote, On 11/04/2007 12:39 AM:
...
OOPS!!! Went too early! I've tried to save not send. Probably my
bad pronunciation...
But, it seems this could be something like this (instead of Radu's
change in u32_classify()). The change of hmask is needed.
But it needs more checking...
Jarek P.
^ permalink raw reply
* Re: Endianness problem with u32 classifier hash masks
From: Jarek Poplawski @ 2007-11-04 0:30 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: hadi, Radu Rendec, netdev
In-Reply-To: <472D0B1C.7000209@o2.pl>
Jarek Poplawski wrote, On 11/04/2007 12:58 AM:
> Jarek Poplawski wrote, On 11/04/2007 12:39 AM:
> ...
>
> OOPS!!! Went too early! I've tried to save not send. Probably my
> bad pronunciation...
>
> But, it seems this could be something like this (instead of Radu's
> change in u32_classify()). The change of hmask is needed.
OK, not exactly... hmask should be ntohl'ed only for fshift:
u32 mask = ntohl(s->hmask);
...
Other changes seem to be not needed.
>
> But it needs more checking...
Jarek P.
^ permalink raw reply
* Re: [PATCH] INET : removes per bucket rwlock in tcp/dccp ehash table
From: Andi Kleen @ 2007-11-04 0:54 UTC (permalink / raw)
To: David Miller; +Cc: dada1, netdev, acme
In-Reply-To: <20071103.162337.83099185.davem@davemloft.net>
> > Also the EHASH_LOCK_SZ == 0 special case is a little strange. Why did
> > you add that?
>
> He explained this in another reply, because ifdefs are ugly.
I meant why having it at all?
> Any use that makes
> "sense" is a case where the code should be rewritten to decrease the
> lock hold time or convert to RCU.
I don't think RCU would be really needed for single entry buckets (which
are common) if they are special cased. After all it would be just
a single pointer. Perhaps one could distingush this case e.g. by
setting the low order bit of the hash bucket pointer.
In fact to optimize for this case it might be an interesting
experiment to go towards an closed hash table and allocate the sockets
in page sized objects and then remap them directly into a virtual
continuous table similar to the new vmemmap code (need to try this at some
point).
-Andi
^ permalink raw reply
* Re: Endianness problem with u32 classifier hash masks
From: Jarek Poplawski @ 2007-11-04 1:17 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: hadi, Radu Rendec, netdev
In-Reply-To: <472D128B.8030704@o2.pl>
Jarek Poplawski wrote, On 11/04/2007 01:30 AM:
> Jarek Poplawski wrote, On 11/04/2007 12:58 AM:
...
> Other changes seem to be not needed.
>
>> But it needs more checking...
But not much more: it's a piece of fshit!
So, even if not full ntohl(), some byte moving seems to be
necessary here.
Sorry for this mess,
Jarek P
^ permalink raw reply
* Re: [PATCH] net: Add 405EX support to new EMAC driver
From: Benjamin Herrenschmidt @ 2007-11-04 3:37 UTC (permalink / raw)
To: Olof Johansson; +Cc: Stefan Roese, netdev, linuxppc-dev, jwboyer
In-Reply-To: <20071102160304.GA5277@lixom.net>
On Fri, 2007-11-02 at 11:03 -0500, Olof Johansson wrote:
> On Fri, Nov 02, 2007 at 08:14:43AM +0100, Stefan Roese wrote:
> > This patch adds support for the 405EX to the new EMAC driver. Some as on
> > AXON, the 405EX handles the MDIO via the RGMII bridge.
>
> Hi,
>
> This isn't feedback on your patch as much as on "new-emac" in general:
>
> Isn't this the case where there should really be device tree properties
> instead? If you had an "ibm,emac-has-axon-stacr" property in the device
> node, then you don't have to modify the driver for every new board out
> there. Same for the other device properties, of course.
>
> I thought this was what having the device tree was all about. :(
Somewhat yeah. There are subtle variations here or there we haven't
totally indenfified... It might be a better option in our case here to
add "has-mdio" to the rgmii nodes indeed.
Part of the problem with those cells is that the chip folks keep
changing things subtly from one rev to another though, it's not even
totally clear to me yet whether the RGMII registers are totally
compatible betwee axon and 405ex, which is why I've pretty much stuck to
"compatible" properties to identify the variants.
The device-tree can do both. It's still better than no device-tree since
at least you know what cell variant is in there.
As for the STACR, Axon isn't the first one to have that bit flipped, I
think we should name the property differently, something like
"stacr-oc-inverted".
We can still use properties that way for new things in fact. As for EMAC
on cell, well, I can always put some fixup somewhere.
Ben.
^ permalink raw reply
* Re: [RFC][BNX2X] .h files rewrite
From: Eliezer Tamir @ 2007-11-04 7:47 UTC (permalink / raw)
To: Max Asbock; +Cc: davem, Michael Chan, jeff, netdev
In-Reply-To: <1194046554.6369.20.camel@w-amax.beaverton.ibm.com>
On Fri, 2007-11-02 at 16:35 -0700, Max Asbock wrote:
> I built the newest bnx2x code against the net-2.6 kernel and ran a
> number of stress tests with netperf and pktgen. I did not encounter
> any
> errors.
>
> Max
>
>
Thanks,
Eliezer
^ permalink raw reply
* Re: 2.6.23: TG3+VLAN: IPv6 router advertisments missed by kernel
From: Bruno Prémont @ 2007-11-04 9:30 UTC (permalink / raw)
To: Linux Kernel Mailing List; +Cc: Linux NetDev
In-Reply-To: <200711012145.43085.bonbons@linux-vserver.org>
The issue shows up reliably when starting the system though some
(re)configuration operations on the network interface makes the issue
disapear.
One way to get the kernel to see the advertisments is to restart the interface
with its vlans or (as below) keeping the interface in promiscuous mode.
Regards,
Bruno
On Thursday 01 November 2007 21:45:42 you wrote:
> I'm seeing unexpected behavior on my laptop since I updated kernel to
> 2.6.23.1 from 2.6.22.1.
>
> My setup:
> Cisco Router <--- [2 vlans] -----> Laptop
>
> On the link two VLANs are active, native vlan is not used.
> Laptop nic is:
> Tigon3 [partno(BCM95751m) rev 4201 PHY(5750)] (PCI Express)
> 10/100/1000Base-T Ethernet
>
> On laptop I have eth0.500 and eth0.658 as active interfaces (eth0 is just
> up - no address manually assigned) with IPv4 address assigned. IPv6 is only
> enabled on the router for one of both vlans (500).
>
> When booting with 2.6.23.1 the router advertisments coming from the router
> (vlan 500) seem to get ignored by the kernel (they are detected by 2.6.22)
> and only enabling promiscuous mode on eth0 makes the kernel detect the
> router advertisments. (I'm doing "tcpdump icmp6" on the vlan interface)
>
> This looks like it could be caused by changes in regard to handling vlans
> with Tigon3 nic.
> A different machine (other nic and no vlans) sees the router advertisments
> correctly with 2.6.23.1. (So I don't expect the cause to be on IPv6 side)
>
> Bruno
>
>
>
> Probably relevant .config extract for 2.6.23.1:
> CONFIG_PACKET=y
> CONFIG_UNIX=y
> CONFIG_INET=y
> CONFIG_IP_MULTICAST=y
> CONFIG_IP_ADVANCED_ROUTER=y
> CONFIG_ASK_IP_FIB_HASH=y
> CONFIG_IP_FIB_HASH=y
> CONFIG_IP_MULTIPLE_TABLES=y
> CONFIG_NET_IPGRE=m
> CONFIG_SYN_COOKIES=y
> CONFIG_INET_DIAG=y
> CONFIG_INET_TCP_DIAG=y
> CONFIG_TCP_CONG_CUBIC=y
> CONFIG_DEFAULT_TCP_CONG="cubic"
> CONFIG_IPV6=y
> CONFIG_INET6_TUNNEL=m
> CONFIG_IPV6_TUNNEL=m
> CONFIG_IPV6_MULTIPLE_TABLES=y
> CONFIG_IPV6_SUBTREES=y
> CONFIG_NETFILTER=y
>
> CONFIG_NETDEVICES=y
> CONFIG_NETDEVICES_MULTIQUEUE=y
> # CONFIG_MACVLAN is not set
> CONFIG_TUN=m
> CONFIG_PHYLIB=m
> CONFIG_BROADCOM_PHY=m
> CONFIG_NET_ETHERNET=y
> CONFIG_MII=y
> CONFIG_NET_PCI=y
> CONFIG_B44=m
> CONFIG_NETDEV_1000=y
> CONFIG_TIGON3=m
>
> Same extract for 2.6.22.1:
> CONFIG_PACKET=y
> CONFIG_UNIX=y
> CONFIG_INET=y
> CONFIG_IP_MULTICAST=y
> CONFIG_IP_ADVANCED_ROUTER=y
> CONFIG_ASK_IP_FIB_HASH=y
> CONFIG_IP_FIB_HASH=y
> CONFIG_IP_MULTIPLE_TABLES=y
> CONFIG_NET_IPGRE=m
> CONFIG_SYN_COOKIES=y
> CONFIG_INET_DIAG=y
> CONFIG_INET_TCP_DIAG=y
> CONFIG_TCP_CONG_CUBIC=y
> CONFIG_DEFAULT_TCP_CONG="cubic"
> CONFIG_IPV6=y
> CONFIG_INET6_TUNNEL=m
> CONFIG_IPV6_TUNNEL=m
> CONFIG_IPV6_MULTIPLE_TABLES=y
> CONFIG_IPV6_SUBTREES=y
> CONFIG_NETFILTER=y
>
> CONFIG_NETDEVICES=y
> CONFIG_DUMMY=m
> CONFIG_TUN=m
> CONFIG_PHYLIB=m
> CONFIG_BROADCOM_PHY=m
> CONFIG_NET_ETHERNET=y
> CONFIG_MII=y
> CONFIG_NET_PCI=y
> CONFIG_B44=m
> CONFIG_NETDEV_1000=y
> CONFIG_TIGON3=m
^ permalink raw reply
* Re: [PATCH] INET : removes per bucket rwlock in tcp/dccp ehash table
From: Eric Dumazet @ 2007-11-04 11:31 UTC (permalink / raw)
To: David Miller; +Cc: ak, netdev, acme, Jarek Poplawski
In-Reply-To: <20071103.162337.83099185.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 3657 bytes --]
David Miller a écrit :
> From: Andi Kleen <ak@suse.de>
> Date: Sun, 4 Nov 2007 00:18:14 +0100
>
>> On Thursday 01 November 2007 11:16:20 Eric Dumazet wrote:
>>
>> Some quick comments:
>>
>>> +#if defined(CONFIG_SMP) || defined(CONFIG_PROVE_LOCKING)
>>> +/*
>>> + * Instead of using one rwlock for each inet_ehash_bucket, we use a table of locks
>>> + * The size of this table is a power of two and depends on the number of CPUS.
>>> + */
>> This shouldn't be hard coded based on NR_CPUS, but be done on runtime
>> based on num_possible_cpus(). This is better for kernels with a large
>> NR_CPUS, but which typically run on much smaller systems (like
>> distribution kernels)
>
> I think this is a good idea. Eric, could you make this change?
Yes of course, since using a non constant value for masking is cheap.
But I suspect distributions kernels enable CONFIG_HOTPLUG_CPU so
num_possible_cpus() will be NR_CPUS.
>
>> Also the EHASH_LOCK_SZ == 0 special case is a little strange. Why did
>> you add that?
>
> He explained this in another reply, because ifdefs are ugly.
This will vanish if done on runtime anyway.
>
>> And as a unrelated node have you tried converting the rwlocks
>> into normal spinlocks? spinlocks should be somewhat cheaper
>> because they have less cache protocol overhead and with
>> the huge thash tables in Linux the chain walks should be short
>> anyways so not doing this in parallel is probably not a big issue.
>> At some point I also had a crazy idea of using a special locking
>> scheme that special cases the common case that a hash chain
>> has only one member and doesn't take a look for that at all.
>
> I agree.
>
> There was movement at one point to get rid of all rwlock's in the
> kernel, I personally think they are pointless. Any use that makes
> "sense" is a case where the code should be rewritten to decrease the
> lock hold time or convert to RCU.
>
I agree too, rwlocks are more expensive when contention is low, so let do this
rwlock->spinlock change on next step (separate patch), because it means
changing also lhash_lock.
Thanks to Jarek, I added locks cleanup in dccp_fini()
[PATCH] INET : removes per bucket rwlock in tcp/dccp ehash table
As done two years ago on IP route cache table (commit
22c047ccbc68fa8f3fa57f0e8f906479a062c426) , we can avoid using one lock per
hash bucket for the huge TCP/DCCP hash tables.
On a typical x86_64 platform, this saves about 2MB or 4MB of ram, for litle
performance differences. (we hit a different cache line for the rwlock, but
then the bucket cache line have a better sharing factor among cpus, since we
dirty it less often). For netstat or ss commands that want a full scan of hash
table, we perform fewer memory accesses.
Using a 'small' table of hashed rwlocks should be more than enough to provide
correct SMP concurrency between different buckets, without using too much
memory. Sizing of this table depends on num_possible_cpus() and various CONFIG
settings.
This patch provides some locking abstraction that may ease a future work using
a different model for TCP/DCCP table.
Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
include/net/inet_hashtables.h | 71 +++++++++++++++++++++++++++++---
net/dccp/proto.c | 9 +++-
net/ipv4/inet_diag.c | 9 ++--
net/ipv4/inet_hashtables.c | 7 +--
net/ipv4/inet_timewait_sock.c | 13 +++--
net/ipv4/tcp.c | 4 -
net/ipv4/tcp_ipv4.c | 11 ++--
net/ipv6/inet6_hashtables.c | 19 ++++----
8 files changed, 106 insertions(+), 37 deletions(-)
[-- Attachment #2: tcp_ehash_locks.patch --]
[-- Type: text/plain, Size: 14142 bytes --]
diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index 4427dcd..8461cda 100644
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -37,7 +37,6 @@
* I'll experiment with dynamic table growth later.
*/
struct inet_ehash_bucket {
- rwlock_t lock;
struct hlist_head chain;
struct hlist_head twchain;
};
@@ -100,6 +99,9 @@ struct inet_hashinfo {
* TIME_WAIT sockets use a separate chain (twchain).
*/
struct inet_ehash_bucket *ehash;
+ rwlock_t *ehash_locks;
+ unsigned int ehash_size;
+ unsigned int ehash_locks_mask;
/* Ok, let's try this, I give up, we do need a local binding
* TCP hash as well as the others for fast bind/connect.
@@ -107,7 +109,7 @@ struct inet_hashinfo {
struct inet_bind_hashbucket *bhash;
unsigned int bhash_size;
- unsigned int ehash_size;
+ /* Note : 4 bytes padding on 64 bit arches */
/* All sockets in TCP_LISTEN state will be in here. This is the only
* table where wildcard'd TCP sockets can exist. Hash function here
@@ -134,6 +136,62 @@ static inline struct inet_ehash_bucket *inet_ehash_bucket(
return &hashinfo->ehash[hash & (hashinfo->ehash_size - 1)];
}
+static inline rwlock_t *inet_ehash_lockp(
+ struct inet_hashinfo *hashinfo,
+ unsigned int hash)
+{
+ return &hashinfo->ehash_locks[hash & hashinfo->ehash_locks_mask];
+}
+
+static inline int inet_ehash_locks_alloc(struct inet_hashinfo *hashinfo)
+{
+ unsigned int i, size = 256;
+#if defined(CONFIG_PROVE_LOCKING)
+ unsigned int nr_pcpus = 2;
+#else
+ unsigned int nr_pcpus = num_possible_cpus();
+#endif
+ if (nr_pcpus >= 4)
+ size = 512;
+ if (nr_pcpus >= 8)
+ size = 1024;
+ if (nr_pcpus >= 16)
+ size = 2048;
+ if (nr_pcpus >= 32)
+ size = 4096;
+ if (sizeof(rwlock_t) != 0) {
+#ifdef CONFIG_NUMA
+ if (size * sizeof(rwlock_t) > PAGE_SIZE)
+ hashinfo->ehash_locks = vmalloc(size * sizeof(rwlock_t));
+ else
+#endif
+ hashinfo->ehash_locks = kmalloc(size * sizeof(rwlock_t),
+ GFP_KERNEL);
+ if (!hashinfo->ehash_locks)
+ return ENOMEM;
+ for (i = 0; i < size; i++)
+ rwlock_init(&hashinfo->ehash_locks[i]);
+ }
+ hashinfo->ehash_locks_mask = size - 1;
+ return 0;
+}
+
+static inline void inet_ehash_locks_free(struct inet_hashinfo *hashinfo)
+{
+ if (hashinfo->ehash_locks) {
+#ifdef CONFIG_NUMA
+ unsigned int size = (hashinfo->ehash_locks_mask + 1) *
+ sizeof(rwlock_t);
+ if (size > PAGE_SIZE)
+ vfree(hashinfo->ehash_locks);
+ else
+#else
+ kfree(hashinfo->ehash_locks);
+#endif
+ hashinfo->ehash_locks = NULL;
+ }
+}
+
extern struct inet_bind_bucket *
inet_bind_bucket_create(struct kmem_cache *cachep,
struct inet_bind_hashbucket *head,
@@ -222,7 +280,7 @@ static inline void __inet_hash(struct inet_hashinfo *hashinfo,
sk->sk_hash = inet_sk_ehashfn(sk);
head = inet_ehash_bucket(hashinfo, sk->sk_hash);
list = &head->chain;
- lock = &head->lock;
+ lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
write_lock(lock);
}
__sk_add_node(sk, list);
@@ -253,7 +311,7 @@ static inline void inet_unhash(struct inet_hashinfo *hashinfo, struct sock *sk)
inet_listen_wlock(hashinfo);
lock = &hashinfo->lhash_lock;
} else {
- lock = &inet_ehash_bucket(hashinfo, sk->sk_hash)->lock;
+ lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
write_lock_bh(lock);
}
@@ -354,9 +412,10 @@ static inline struct sock *
*/
unsigned int hash = inet_ehashfn(daddr, hnum, saddr, sport);
struct inet_ehash_bucket *head = inet_ehash_bucket(hashinfo, hash);
+ rwlock_t *lock = inet_ehash_lockp(hashinfo, hash);
prefetch(head->chain.first);
- read_lock(&head->lock);
+ read_lock(lock);
sk_for_each(sk, node, &head->chain) {
if (INET_MATCH(sk, hash, acookie, saddr, daddr, ports, dif))
goto hit; /* You sunk my battleship! */
@@ -369,7 +428,7 @@ static inline struct sock *
}
sk = NULL;
out:
- read_unlock(&head->lock);
+ read_unlock(lock);
return sk;
hit:
sock_hold(sk);
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index d849739..7a3bea9 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -1072,11 +1072,13 @@ static int __init dccp_init(void)
}
for (i = 0; i < dccp_hashinfo.ehash_size; i++) {
- rwlock_init(&dccp_hashinfo.ehash[i].lock);
INIT_HLIST_HEAD(&dccp_hashinfo.ehash[i].chain);
INIT_HLIST_HEAD(&dccp_hashinfo.ehash[i].twchain);
}
+ if (inet_ehash_locks_alloc(&dccp_hashinfo))
+ goto out_free_dccp_ehash;
+
bhash_order = ehash_order;
do {
@@ -1091,7 +1093,7 @@ static int __init dccp_init(void)
if (!dccp_hashinfo.bhash) {
DCCP_CRIT("Failed to allocate DCCP bind hash table");
- goto out_free_dccp_ehash;
+ goto out_free_dccp_locks;
}
for (i = 0; i < dccp_hashinfo.bhash_size; i++) {
@@ -1121,6 +1123,8 @@ out_free_dccp_mib:
out_free_dccp_bhash:
free_pages((unsigned long)dccp_hashinfo.bhash, bhash_order);
dccp_hashinfo.bhash = NULL;
+out_free_dccp_locks:
+ inet_ehash_locks_free(&dccp_hashinfo);
out_free_dccp_ehash:
free_pages((unsigned long)dccp_hashinfo.ehash, ehash_order);
dccp_hashinfo.ehash = NULL;
@@ -1139,6 +1143,7 @@ static void __exit dccp_fini(void)
free_pages((unsigned long)dccp_hashinfo.ehash,
get_order(dccp_hashinfo.ehash_size *
sizeof(struct inet_ehash_bucket)));
+ inet_ehash_locks_free(&dccp_hashinfo);
kmem_cache_destroy(dccp_hashinfo.bind_bucket_cachep);
dccp_ackvec_exit();
dccp_sysctl_exit();
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index dc429b6..b017073 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -747,13 +747,14 @@ skip_listen_ht:
for (i = s_i; i < hashinfo->ehash_size; i++) {
struct inet_ehash_bucket *head = &hashinfo->ehash[i];
+ rwlock_t *lock = inet_ehash_lockp(hashinfo, i);
struct sock *sk;
struct hlist_node *node;
if (i > s_i)
s_num = 0;
- read_lock_bh(&head->lock);
+ read_lock_bh(lock);
num = 0;
sk_for_each(sk, node, &head->chain) {
struct inet_sock *inet = inet_sk(sk);
@@ -769,7 +770,7 @@ skip_listen_ht:
r->id.idiag_dport)
goto next_normal;
if (inet_csk_diag_dump(sk, skb, cb) < 0) {
- read_unlock_bh(&head->lock);
+ read_unlock_bh(lock);
goto done;
}
next_normal:
@@ -791,14 +792,14 @@ next_normal:
r->id.idiag_dport)
goto next_dying;
if (inet_twsk_diag_dump(tw, skb, cb) < 0) {
- read_unlock_bh(&head->lock);
+ read_unlock_bh(lock);
goto done;
}
next_dying:
++num;
}
}
- read_unlock_bh(&head->lock);
+ read_unlock_bh(lock);
}
done:
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 16eecc7..67704da 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -204,12 +204,13 @@ static int __inet_check_established(struct inet_timewait_death_row *death_row,
const __portpair ports = INET_COMBINED_PORTS(inet->dport, lport);
unsigned int hash = inet_ehashfn(daddr, lport, saddr, inet->dport);
struct inet_ehash_bucket *head = inet_ehash_bucket(hinfo, hash);
+ rwlock_t *lock = inet_ehash_lockp(hinfo, hash);
struct sock *sk2;
const struct hlist_node *node;
struct inet_timewait_sock *tw;
prefetch(head->chain.first);
- write_lock(&head->lock);
+ write_lock(lock);
/* Check TIME-WAIT sockets first. */
sk_for_each(sk2, node, &head->twchain) {
@@ -239,7 +240,7 @@ unique:
BUG_TRAP(sk_unhashed(sk));
__sk_add_node(sk, &head->chain);
sock_prot_inc_use(sk->sk_prot);
- write_unlock(&head->lock);
+ write_unlock(lock);
if (twp) {
*twp = tw;
@@ -255,7 +256,7 @@ unique:
return 0;
not_unique:
- write_unlock(&head->lock);
+ write_unlock(lock);
return -EADDRNOTAVAIL;
}
diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index 4e189e2..a60b99e 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c
@@ -20,16 +20,16 @@ static void __inet_twsk_kill(struct inet_timewait_sock *tw,
struct inet_bind_hashbucket *bhead;
struct inet_bind_bucket *tb;
/* Unlink from established hashes. */
- struct inet_ehash_bucket *ehead = inet_ehash_bucket(hashinfo, tw->tw_hash);
+ rwlock_t *lock = inet_ehash_lockp(hashinfo, tw->tw_hash);
- write_lock(&ehead->lock);
+ write_lock(lock);
if (hlist_unhashed(&tw->tw_node)) {
- write_unlock(&ehead->lock);
+ write_unlock(lock);
return;
}
__hlist_del(&tw->tw_node);
sk_node_init(&tw->tw_node);
- write_unlock(&ehead->lock);
+ write_unlock(lock);
/* Disassociate with bind bucket. */
bhead = &hashinfo->bhash[inet_bhashfn(tw->tw_num, hashinfo->bhash_size)];
@@ -59,6 +59,7 @@ void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
const struct inet_sock *inet = inet_sk(sk);
const struct inet_connection_sock *icsk = inet_csk(sk);
struct inet_ehash_bucket *ehead = inet_ehash_bucket(hashinfo, sk->sk_hash);
+ rwlock_t *lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
struct inet_bind_hashbucket *bhead;
/* Step 1: Put TW into bind hash. Original socket stays there too.
Note, that any socket with inet->num != 0 MUST be bound in
@@ -71,7 +72,7 @@ void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
inet_twsk_add_bind_node(tw, &tw->tw_tb->owners);
spin_unlock(&bhead->lock);
- write_lock(&ehead->lock);
+ write_lock(lock);
/* Step 2: Remove SK from established hash. */
if (__sk_del_node_init(sk))
@@ -81,7 +82,7 @@ void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
inet_twsk_add_node(tw, &ehead->twchain);
atomic_inc(&tw->tw_refcnt);
- write_unlock(&ehead->lock);
+ write_unlock(lock);
}
EXPORT_SYMBOL_GPL(__inet_twsk_hashdance);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index c64072b..8e65182 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2456,11 +2456,11 @@ void __init tcp_init(void)
thash_entries ? 0 : 512 * 1024);
tcp_hashinfo.ehash_size = 1 << tcp_hashinfo.ehash_size;
for (i = 0; i < tcp_hashinfo.ehash_size; i++) {
- rwlock_init(&tcp_hashinfo.ehash[i].lock);
INIT_HLIST_HEAD(&tcp_hashinfo.ehash[i].chain);
INIT_HLIST_HEAD(&tcp_hashinfo.ehash[i].twchain);
}
-
+ if (inet_ehash_locks_alloc(&tcp_hashinfo))
+ panic("TCP: failed to alloc ehash_locks");
tcp_hashinfo.bhash =
alloc_large_system_hash("TCP bind",
sizeof(struct inet_bind_hashbucket),
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index eec02b2..cd82c0e 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2049,8 +2049,9 @@ static void *established_get_first(struct seq_file *seq)
struct sock *sk;
struct hlist_node *node;
struct inet_timewait_sock *tw;
+ rwlock_t *lock = inet_ehash_lockp(&tcp_hashinfo, st->bucket);
- read_lock_bh(&tcp_hashinfo.ehash[st->bucket].lock);
+ read_lock_bh(lock);
sk_for_each(sk, node, &tcp_hashinfo.ehash[st->bucket].chain) {
if (sk->sk_family != st->family) {
continue;
@@ -2067,7 +2068,7 @@ static void *established_get_first(struct seq_file *seq)
rc = tw;
goto out;
}
- read_unlock_bh(&tcp_hashinfo.ehash[st->bucket].lock);
+ read_unlock_bh(lock);
st->state = TCP_SEQ_STATE_ESTABLISHED;
}
out:
@@ -2094,11 +2095,11 @@ get_tw:
cur = tw;
goto out;
}
- read_unlock_bh(&tcp_hashinfo.ehash[st->bucket].lock);
+ read_unlock_bh(inet_ehash_lockp(&tcp_hashinfo, st->bucket));
st->state = TCP_SEQ_STATE_ESTABLISHED;
if (++st->bucket < tcp_hashinfo.ehash_size) {
- read_lock_bh(&tcp_hashinfo.ehash[st->bucket].lock);
+ read_lock_bh(inet_ehash_lockp(&tcp_hashinfo, st->bucket));
sk = sk_head(&tcp_hashinfo.ehash[st->bucket].chain);
} else {
cur = NULL;
@@ -2206,7 +2207,7 @@ static void tcp_seq_stop(struct seq_file *seq, void *v)
case TCP_SEQ_STATE_TIME_WAIT:
case TCP_SEQ_STATE_ESTABLISHED:
if (v)
- read_unlock_bh(&tcp_hashinfo.ehash[st->bucket].lock);
+ read_unlock_bh(inet_ehash_lockp(&tcp_hashinfo, st->bucket));
break;
}
}
diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index d6f1026..adc73ad 100644
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -37,9 +37,8 @@ void __inet6_hash(struct inet_hashinfo *hashinfo,
} else {
unsigned int hash;
sk->sk_hash = hash = inet6_sk_ehashfn(sk);
- hash &= (hashinfo->ehash_size - 1);
- list = &hashinfo->ehash[hash].chain;
- lock = &hashinfo->ehash[hash].lock;
+ list = &inet_ehash_bucket(hashinfo, hash)->chain;
+ lock = inet_ehash_lockp(hashinfo, hash);
write_lock(lock);
}
@@ -70,9 +69,10 @@ struct sock *__inet6_lookup_established(struct inet_hashinfo *hashinfo,
*/
unsigned int hash = inet6_ehashfn(daddr, hnum, saddr, sport);
struct inet_ehash_bucket *head = inet_ehash_bucket(hashinfo, hash);
+ rwlock_t *lock = inet_ehash_lockp(hashinfo, hash);
prefetch(head->chain.first);
- read_lock(&head->lock);
+ read_lock(lock);
sk_for_each(sk, node, &head->chain) {
/* For IPV6 do the cheaper port and family tests first. */
if (INET6_MATCH(sk, hash, saddr, daddr, ports, dif))
@@ -92,12 +92,12 @@ struct sock *__inet6_lookup_established(struct inet_hashinfo *hashinfo,
goto hit;
}
}
- read_unlock(&head->lock);
+ read_unlock(lock);
return NULL;
hit:
sock_hold(sk);
- read_unlock(&head->lock);
+ read_unlock(lock);
return sk;
}
EXPORT_SYMBOL(__inet6_lookup_established);
@@ -175,12 +175,13 @@ static int __inet6_check_established(struct inet_timewait_death_row *death_row,
const unsigned int hash = inet6_ehashfn(daddr, lport, saddr,
inet->dport);
struct inet_ehash_bucket *head = inet_ehash_bucket(hinfo, hash);
+ rwlock_t *lock = inet_ehash_lockp(hinfo, hash);
struct sock *sk2;
const struct hlist_node *node;
struct inet_timewait_sock *tw;
prefetch(head->chain.first);
- write_lock(&head->lock);
+ write_lock(lock);
/* Check TIME-WAIT sockets first. */
sk_for_each(sk2, node, &head->twchain) {
@@ -216,7 +217,7 @@ unique:
__sk_add_node(sk, &head->chain);
sk->sk_hash = hash;
sock_prot_inc_use(sk->sk_prot);
- write_unlock(&head->lock);
+ write_unlock(lock);
if (twp != NULL) {
*twp = tw;
@@ -231,7 +232,7 @@ unique:
return 0;
not_unique:
- write_unlock(&head->lock);
+ write_unlock(lock);
return -EADDRNOTAVAIL;
}
^ permalink raw reply related
* Re: [PATCH] INET : removes per bucket rwlock in tcp/dccp ehash table
From: Andi Kleen @ 2007-11-04 12:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, acme, Jarek Poplawski
In-Reply-To: <472DAD90.4050709@cosmosbay.com>
> But I suspect distributions kernels enable CONFIG_HOTPLUG_CPU so
> num_possible_cpus() will be NR_CPUS.
Nope, on x86 num_possible_cpus() is derived from BIOS tables these days.
-Andi
^ permalink raw reply
* Re: [PATCH] INET : removes per bucket rwlock in tcp/dccp ehash table
From: Eric Dumazet @ 2007-11-04 13:05 UTC (permalink / raw)
To: Andi Kleen; +Cc: David Miller, netdev, acme, Jarek Poplawski
In-Reply-To: <200711041326.38380.ak@suse.de>
Andi Kleen a écrit :
>> But I suspect distributions kernels enable CONFIG_HOTPLUG_CPU so
>> num_possible_cpus() will be NR_CPUS.
>
> Nope, on x86 num_possible_cpus() is derived from BIOS tables these days.
Good to know, thank you Andi for this clarification.
^ permalink raw reply
* problems with ib-bonding of 2.6.24-rc1
From: Moni Shoua @ 2007-11-04 16:15 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev
Hi,
I've been doing some tests for bonding of 2.6.24-rc1 and noticed some problems.
My first goal was to see how bonding works with IPoIB slaves but I also tried it
with Ethernet.
Basically, what I see is that after a while commands like ifconfig or ip stucks.
I only use sysfs to configure bonding (which also stucks after a while).
After stripping the list of commits below from the code I see no problems.
Does anybody else have the same problem?
thanks
MoniS
commit d0e81b7e2246a41d068ecaf15aac9de570816d63
Author: Jay Vosburgh <fubar@us.ibm.com>
Date: Wed Oct 17 17:37:51 2007 -0700
bonding: Acquire correct locks in alb for promisc change
--
commit 6603a6f25e4bca922a7dfbf0bf03072d98850176
Author: Jay Vosburgh <fubar@us.ibm.com>
Date: Wed Oct 17 17:37:50 2007 -0700
bonding: Convert more locks to _bh, acquire rtnl, for new locking
--
commit 059fe7a578fba5bbb0fdc0365bfcf6218fa25eb0
Author: Jay Vosburgh <fubar@us.ibm.com>
Date: Wed Oct 17 17:37:49 2007 -0700
bonding: Convert locks to _bh, rework alb locking for new locking
--
commit 0b0eef66419e9abe6fd62bc958ab7cd0a18f858e
Author: Jay Vosburgh <fubar@us.ibm.com>
Date: Wed Oct 17 17:37:48 2007 -0700
bonding: Convert miimon to new locking
--
commit cf5f9044934658dd3ffc628a60cd37c70f8168b1
Author: Jay Vosburgh <fubar@us.ibm.com>
Date: Wed Oct 17 17:37:47 2007 -0700
bonding: Convert balance-rr transmit to new locking
--
commit 1b76b31693d4a6088dec104ff6a6ead54081a3c2
Author: Jay Vosburgh <fubar@us.ibm.com>
Date: Wed Oct 17 17:37:45 2007 -0700
Convert bonding timers to workqueues
--
commit 3a4fa0a25da81600ea0bcd75692ae8ca6050d165
Author: Robert P. J. Day <rpjday@mindspring.com>
Date: Fri Oct 19 23:10:43 2007 +0200
Fix misspellings of "system", "controller", "interrupt" and "necessary".
--
commit 1c3f0b8e07de78a86f2dce911f5e245845ce40a8
Author: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Date: Thu Oct 18 23:41:04 2007 -0700
Change struct marker users
^ permalink raw reply
* [NETLINK]: Fix unicast timeouts
From: Patrick McHardy @ 2007-11-04 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: Manfred Spraul, Linux Netdev List
[-- Attachment #1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 3911 bytes --]
[NETLINK]: Fix unicast timeouts
Commit ed6dcf4a in the history.git tree broke netlink_unicast timeouts by
moving the schedule_timeout() call to a new function that doesn't propagate
the remaining timeout back to the caller. This means on each retry we start
with the full timeout again.
ipc/mqueue.c seems to actually want to wait indefinitely so this behaviour
is retained.
Cc: Manfred Spraul <manfred@colorfullife.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
commit 251299cd3683f06b5b690e6a3bdd14133303ab2a
tree 3fd85bdae19d5f29efe09c328fa2defac9facd6b
parent b4f555081fdd27d13e6ff39d455d5aefae9d2c0c
author Patrick McHardy <kaber@trash.net> Sun, 04 Nov 2007 17:52:19 +0100
committer Patrick McHardy <kaber@trash.net> Sun, 04 Nov 2007 17:52:19 +0100
include/linux/netlink.h | 2 +-
ipc/mqueue.c | 6 ++++--
net/netlink/af_netlink.c | 10 +++++-----
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 7c1f3b1..d5bfaba 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -192,7 +192,7 @@ extern int netlink_unregister_notifier(struct notifier_block *nb);
/* finegrained unicast helpers: */
struct sock *netlink_getsockbyfilp(struct file *filp);
int netlink_attachskb(struct sock *sk, struct sk_buff *skb, int nonblock,
- long timeo, struct sock *ssk);
+ long *timeo, struct sock *ssk);
void netlink_detachskb(struct sock *sk, struct sk_buff *skb);
int netlink_sendskb(struct sock *sk, struct sk_buff *skb);
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index bfa274b..1e04cd4 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -1010,6 +1010,8 @@ asmlinkage long sys_mq_notify(mqd_t mqdes,
return -EINVAL;
}
if (notification.sigev_notify == SIGEV_THREAD) {
+ long timeo;
+
/* create the notify skb */
nc = alloc_skb(NOTIFY_COOKIE_LEN, GFP_KERNEL);
ret = -ENOMEM;
@@ -1038,8 +1040,8 @@ retry:
goto out;
}
- ret = netlink_attachskb(sock, nc, 0,
- MAX_SCHEDULE_TIMEOUT, NULL);
+ timeo = MAX_SCHEDULE_TIMEOUT;
+ ret = netlink_attachskb(sock, nc, 0, &timeo, NULL);
if (ret == 1)
goto retry;
if (ret) {
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 2601712..415c972 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -752,7 +752,7 @@ struct sock *netlink_getsockbyfilp(struct file *filp)
* 1: repeat lookup - reference dropped while waiting for socket memory.
*/
int netlink_attachskb(struct sock *sk, struct sk_buff *skb, int nonblock,
- long timeo, struct sock *ssk)
+ long *timeo, struct sock *ssk)
{
struct netlink_sock *nlk;
@@ -761,7 +761,7 @@ int netlink_attachskb(struct sock *sk, struct sk_buff *skb, int nonblock,
if (atomic_read(&sk->sk_rmem_alloc) > sk->sk_rcvbuf ||
test_bit(0, &nlk->state)) {
DECLARE_WAITQUEUE(wait, current);
- if (!timeo) {
+ if (!*timeo) {
if (!ssk || netlink_is_kernel(ssk))
netlink_overrun(sk);
sock_put(sk);
@@ -775,7 +775,7 @@ int netlink_attachskb(struct sock *sk, struct sk_buff *skb, int nonblock,
if ((atomic_read(&sk->sk_rmem_alloc) > sk->sk_rcvbuf ||
test_bit(0, &nlk->state)) &&
!sock_flag(sk, SOCK_DEAD))
- timeo = schedule_timeout(timeo);
+ *timeo = schedule_timeout(*timeo);
__set_current_state(TASK_RUNNING);
remove_wait_queue(&nlk->wait, &wait);
@@ -783,7 +783,7 @@ int netlink_attachskb(struct sock *sk, struct sk_buff *skb, int nonblock,
if (signal_pending(current)) {
kfree_skb(skb);
- return sock_intr_errno(timeo);
+ return sock_intr_errno(*timeo);
}
return 1;
}
@@ -877,7 +877,7 @@ retry:
if (netlink_is_kernel(sk))
return netlink_unicast_kernel(sk, skb);
- err = netlink_attachskb(sk, skb, nonblock, timeo, ssk);
+ err = netlink_attachskb(sk, skb, nonblock, &timeo, ssk);
if (err == 1)
goto retry;
if (err)
^ permalink raw reply related
* Re: [PATCH] net: Add 405EX support to new EMAC driver
From: Olof Johansson @ 2007-11-04 17:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Stefan Roese, netdev, linuxppc-dev, jwboyer
In-Reply-To: <1194147479.6511.11.camel@pasglop>
On Sun, Nov 04, 2007 at 02:37:59PM +1100, Benjamin Herrenschmidt wrote:
>
> On Fri, 2007-11-02 at 11:03 -0500, Olof Johansson wrote:
> > On Fri, Nov 02, 2007 at 08:14:43AM +0100, Stefan Roese wrote:
> > > This patch adds support for the 405EX to the new EMAC driver. Some as on
> > > AXON, the 405EX handles the MDIO via the RGMII bridge.
> >
> > Hi,
> >
> > This isn't feedback on your patch as much as on "new-emac" in general:
> >
> > Isn't this the case where there should really be device tree properties
> > instead? If you had an "ibm,emac-has-axon-stacr" property in the device
> > node, then you don't have to modify the driver for every new board out
> > there. Same for the other device properties, of course.
> >
> > I thought this was what having the device tree was all about. :(
>
> Somewhat yeah. There are subtle variations here or there we haven't
> totally indenfified... It might be a better option in our case here to
> add "has-mdio" to the rgmii nodes indeed.
>
> Part of the problem with those cells is that the chip folks keep
> changing things subtly from one rev to another though, it's not even
> totally clear to me yet whether the RGMII registers are totally
> compatible betwee axon and 405ex, which is why I've pretty much stuck to
> "compatible" properties to identify the variants.
>
> The device-tree can do both. It's still better than no device-tree since
> at least you know what cell variant is in there.
Well, it's better than compile-time ifdefs. Providing what version of
the device you have CAN be done without a device tree too. :-)
> As for the STACR, Axon isn't the first one to have that bit flipped, I
> think we should name the property differently, something like
> "stacr-oc-inverted".
Sure, it was the habit of having to modify the driver for platforms that
don't add any new features I was against. I don't really care what the
properties are called :-)
> We can still use properties that way for new things in fact. As for EMAC
> on cell, well, I can always put some fixup somewhere.
Sounds good (with s/can still/should/).
-Olof
^ permalink raw reply
* Re: TCP_DEFER_ACCEPT issues
From: dean gaudet @ 2007-11-04 17:18 UTC (permalink / raw)
To: Felix von Leitner; +Cc: Eric Dumazet, linux-kernel, Linux Netdev List
In-Reply-To: <20071102221912.GB4354@codeblau.de>
fwiw i also brought the TCP_DEFER_ACCEPT problems up the end of last year:
http://www.mail-archive.com/netdev@vger.kernel.org/msg28916.html
it's possible the final message in that thread is how we should define the
behaviour, i haven't tried the TCP_SYNCNT idea though.
-dean
^ permalink raw reply
* Re: [PATCH] INET : removes per bucket rwlock in tcp/dccp ehash table
From: Jarek Poplawski @ 2007-11-04 17:58 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, ak, netdev, acme
In-Reply-To: <472DAD90.4050709@cosmosbay.com>
Eric Dumazet wrote, On 11/04/2007 12:31 PM:
> David Miller a écrit :
>> From: Andi Kleen <ak@suse.de>
>> Date: Sun, 4 Nov 2007 00:18:14 +0100
>>
>>> On Thursday 01 November 2007 11:16:20 Eric Dumazet wrote:
...
>>> Also the EHASH_LOCK_SZ == 0 special case is a little strange. Why did
>>> you add that?
>> He explained this in another reply, because ifdefs are ugly.
But I hope he was only joking, didn't he?
Let's make it clear: ifdefs are in K&R, so they are very nice! Just like
all C! (K, &, and R as well.)
You know, I can even imagine, there are people, who have K&R around their
beds, instead of some other book, so they could be serious about such
things. (But, don't worry, it's not me - happily I'm not serious!)
This patch looks OK now, but a bit of grumbling shouldn't harm?:
...
> [PATCH] INET : removes per bucket rwlock in tcp/dccp ehash table
>
> As done two years ago on IP route cache table (commit
> 22c047ccbc68fa8f3fa57f0e8f906479a062c426) , we can avoid using one lock per
> hash bucket for the huge TCP/DCCP hash tables.
>
> On a typical x86_64 platform, this saves about 2MB or 4MB of ram, for litle
- litle
+ little
...
> +static inline int inet_ehash_locks_alloc(struct inet_hashinfo *hashinfo)
> +{
> + unsigned int i, size = 256;
> +#if defined(CONFIG_PROVE_LOCKING)
> + unsigned int nr_pcpus = 2;
> +#else
> + unsigned int nr_pcpus = num_possible_cpus();
> +#endif
> + if (nr_pcpus >= 4)
> + size = 512;
> + if (nr_pcpus >= 8)
> + size = 1024;
> + if (nr_pcpus >= 16)
> + size = 2048;
> + if (nr_pcpus >= 32)
> + size = 4096;
It seems, maybe in the future this could look a bit nicer with some log
type shifting.
> + if (sizeof(rwlock_t) != 0) {
> +#ifdef CONFIG_NUMA
> + if (size * sizeof(rwlock_t) > PAGE_SIZE)
> + hashinfo->ehash_locks = vmalloc(size * sizeof(rwlock_t));
> + else
> +#endif
> + hashinfo->ehash_locks = kmalloc(size * sizeof(rwlock_t),
> + GFP_KERNEL);
> + if (!hashinfo->ehash_locks)
> + return ENOMEM;
Probably doesn't matter now, but maybe more common?:
return -ENOMEM;
> + for (i = 0; i < size; i++)
> + rwlock_init(&hashinfo->ehash_locks[i]);
This looks better now, but still is doubtful to me: even if it's safe with
current rwlock implementation, can't we imagine some new debugging or
statistical code added, which would be called from rwlock_init() without
using rwlock_t structure? IMHO, if read_lock() etc. are called in such a
case, rwlock_init() should be done as well.
Regards,
Jarek P.
^ permalink raw reply
* Re: [PATCH] INET : removes per bucket rwlock in tcp/dccp ehash table
From: Jarek Poplawski @ 2007-11-04 18:15 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: Eric Dumazet, David Miller, ak, netdev, acme
In-Reply-To: <472E0857.4080405@o2.pl>
Jarek Poplawski wrote, On 11/04/2007 06:58 PM:
> Eric Dumazet wrote, On 11/04/2007 12:31 PM:
...
>> +static inline int inet_ehash_locks_alloc(struct inet_hashinfo *hashinfo)
>> +{
...
>> + if (sizeof(rwlock_t) != 0) {
...
>> + for (i = 0; i < size; i++)
>> + rwlock_init(&hashinfo->ehash_locks[i]);
>
>
> This looks better now, but still is doubtful to me: even if it's safe with
> current rwlock implementation, can't we imagine some new debugging or
> statistical code added, which would be called from rwlock_init() without
> using rwlock_t structure? IMHO, if read_lock() etc. are called in such a
> case, rwlock_init() should be done as well.
Of course I mean: if sizeof(rwlock_t) == 0.
Jarek P
^ permalink raw reply
* Re: [PATCH] INET : removes per bucket rwlock in tcp/dccp ehash table
From: Eric Dumazet @ 2007-11-04 21:23 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: David Miller, ak, netdev, acme
In-Reply-To: <472E0C24.9040009@o2.pl>
Jarek Poplawski a écrit :
> Jarek Poplawski wrote, On 11/04/2007 06:58 PM:
>
>> Eric Dumazet wrote, On 11/04/2007 12:31 PM:
>
> ...
>
>>> +static inline int inet_ehash_locks_alloc(struct inet_hashinfo *hashinfo)
>>> +{
>
> ...
>
>>> + if (sizeof(rwlock_t) != 0) {
>
> ...
>
>>> + for (i = 0; i < size; i++)
>>> + rwlock_init(&hashinfo->ehash_locks[i]);
>>
>> This looks better now, but still is doubtful to me: even if it's safe with
>> current rwlock implementation, can't we imagine some new debugging or
>> statistical code added, which would be called from rwlock_init() without
>> using rwlock_t structure? IMHO, if read_lock() etc. are called in such a
>> case, rwlock_init() should be done as well.
>
>
> Of course I mean: if sizeof(rwlock_t) == 0.
Given those two choices :
#if defined(CONFIG_SMP) || defined(CONFIG_PROVE__LOCKING)
kmalloc(sizeof(rwlock_t) * size);
#endif
and
if (sizeof(rwlock_t) != 0) {
kmalloc(sizeof(rwlock_t) * size);
}
I prefer the 2nd one. Less error prone, and no need to remember how are
spelled the gazillions CONFIG_something we have.
^ 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