Netdev List
 help / color / mirror / Atom feed
* Re: tg3: issue for reboot/kexec
From: randy_dunlap @ 2005-06-25  0:46 UTC (permalink / raw)
  To: Haren Myneni; +Cc: netdev, fastboot, linux-pm
In-Reply-To: <42BC9F09.1060002@us.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]

On Fri, 24 Jun 2005 17:02:17 -0700 Haren Myneni wrote:

| 
| Hello,
|     The kexec boot on power blade (JS20) is not successfull without 
| doing 'ifdown <ethernet device>'.  Based on my initial look in tg3 code, 
| the driver does not have shutdown nor reboot notifier code unlike in 
| other drivers. Hence, I added the the following patch (based on e1000) 
| and it is working. Is it OK to include this patch? If not, any help to 
| track down the issue would be appreciated.
| 
| My system is having the following cards:
| 0000:11:01.0 Ethernet controller: Broadcom Corporation NetXtreme 
| BCM5704S Gigabit Ethernet (rev 03)
| 0000:11:01.1 Ethernet controller: Broadcom Corporation NetXtreme 
| BCM5704S Gigabit Ethernet (rev 03)
| 
| lspci -n -s 0000:11:01
| 0000:11:01.0 Class 0200: 14e4:16a8 (rev 03)
| 0000:11:01.1 Class 0200: 14e4:16a8 (rev 03)

[adding fastboot & linux-pm mailing lists]

There was just a SCSI driver patch that tried to use a reboot
notifier on shutdown and the patch got this comment:

Subject: Re: [PATCH] 2.6 aacraid: shutdown notification
Message-ID: <42BC6D6F.5090402@us.ibm.com>
From:	Mark Haverkamp <markh@osdl.org>

and reply From:	Brian King <brking@us.ibm.com>:
| The pci_driver->shutdown method should be used instead of registering
| a reboot notifier.

so is there a good reason that network drivers should use the
reboot notifier instead of pci_driver->shutdown,
or should we be converting drivers to use pci_driver->shutdown
instead?
My quick look favors pci_driver->shutdown.

---
~Randy

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* Re: Fwd: [Bug 4774] e1000 driver works on UP, but not SMP x86_64
From: David Lang @ 2005-06-24 22:15 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: linux-kernel, netdev
In-Reply-To: <200506250157.14499.adobriyan@gmail.com>

here's what happens.

happy1-p:~# ifconfig eth11 192.168.255.1
SIOCSIFFLAGS: Invalid argument
happy1-p:~# tail -3 /var/log/messages
Jun 24 15:14:01 happy1-p /USR/SBIN/CRON[392]: (root) CMD (touch 
/tmp/.crond_running >/dev/null 2>/dev/null)
Jun 24 15:14:22 happy1-p kernel: request_irq: IRQ requested is greater 
than NR_IRQs!
Jun 24 15:14:22 happy1-p kernel: e1000: eth11: e1000_up: Unable to 
allocate interrupt Error: -22

On Sat, 25 Jun 2005, Alexey Dobriyan wrote:

> Date: Sat, 25 Jun 2005 01:57:13 +0400
> From: Alexey Dobriyan <adobriyan@gmail.com>
> To: David Lang <david.lang@digitalinsight.com>
> Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
> Subject: Fwd: [Bug 4774] e1000 driver works on UP, but not SMP x86_64
> 
> David, please try this debugging patch.
>
> You can also register at http://bugme.osdl.org/createaccount.cgi and add
> yourself to CC list.
>
> ----------  Forwarded Message  ----------
>
> Subject: [Bug 4774] e1000 driver works on UP, but not SMP x86_64
> Date: Saturday 25 June 2005 01:27
> From: bugme-daemon@kernel-bugs.osdl.org
> To: adobriyan@gmail.com
>
> http://bugzilla.kernel.org/show_bug.cgi?id=4774
>
>
> ------- Additional Comments From nacc@us.ibm.com  2005-06-24 14:27 -------
> Created an attachment (id=5211)
> --> (http://bugzilla.kernel.org/attachment.cgi?id=5211&action=view)
> Debugging patch
>
> That e1000 error message indicates an EINVAL error code, which is from this
> code:
>
> 	if ((irqflags & SA_SHIRQ) && !dev_id)
> 		return -EINVAL;
> 	if (irq >= NR_IRQS)
> 		return -EINVAL;
> 	if (!handler)
> 		return -EINVAL;
>
> I don't think it's the last one, because e1000_intr (which is sent in to
> request_irq() from e1000) is prototyped/defined. I spun up a patch to spit out
> some debugging here which simply inserts some printks (if the only driver which
> gets this warning is e1000, then it shouldn't flood your logs) -- basically
> narrowing down which error condition is causing the failure. I'm guessing it's
> probably the first case, but let's be sure.
>

-- 
There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies. And the other way is to make it so complicated that there are no obvious deficiencies.
  -- C.A.R. Hoare

^ permalink raw reply

* Fwd: [Bug 4774] e1000 driver works on UP, but not SMP x86_64
From: Alexey Dobriyan @ 2005-06-24 21:57 UTC (permalink / raw)
  To: David Lang; +Cc: linux-kernel, netdev

David, please try this debugging patch.

You can also register at http://bugme.osdl.org/createaccount.cgi and add
yourself to CC list.

----------  Forwarded Message  ----------

Subject: [Bug 4774] e1000 driver works on UP, but not SMP x86_64
Date: Saturday 25 June 2005 01:27
From: bugme-daemon@kernel-bugs.osdl.org
To: adobriyan@gmail.com

http://bugzilla.kernel.org/show_bug.cgi?id=4774


------- Additional Comments From nacc@us.ibm.com  2005-06-24 14:27 -------
Created an attachment (id=5211)
 --> (http://bugzilla.kernel.org/attachment.cgi?id=5211&action=view)
Debugging patch

That e1000 error message indicates an EINVAL error code, which is from this
code:

	if ((irqflags & SA_SHIRQ) && !dev_id)
		return -EINVAL;
	if (irq >= NR_IRQS)
		return -EINVAL;
	if (!handler)
		return -EINVAL;

I don't think it's the last one, because e1000_intr (which is sent in to
request_irq() from e1000) is prototyped/defined. I spun up a patch to spit out
some debugging here which simply inserts some printks (if the only driver which
gets this warning is e1000, then it shouldn't flood your logs) -- basically
narrowing down which error condition is causing the failure. I'm guessing it's
probably the first case, but let's be sure.

^ permalink raw reply

* Re: [2.6 patch] drivers/net/ne3210.c: cleanups
From: Jean Delvare @ 2005-06-24 20:36 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: Andrew Morton, Jeff Garzik, linux-kernel, netdev
In-Reply-To: <20050624200919.GK6656@stusta.de>

Hi Adrian,

>  	if (ei_debug > 0)
> -		printk(version);
> +		printk("ne3210 loaded.\n");

No KERN_DEBUG?

Thanks,
-- 
Jean Delvare

^ permalink raw reply

* [2.6 patch] drivers/net/ne3210.c: cleanups
From: Adrian Bunk @ 2005-06-24 20:09 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Jeff Garzik, linux-kernel, netdev

This patch contains the following cleanups:
- make two needlessly global functions static
- kill an ancient version variable

Signed-off-by: Adrian Bunk <bunk@stusta.de>

---

This patch was already sent on:
- 30 May 2005
- 2 May 2005
- 19 Apr 2005

 drivers/net/ne3210.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

--- linux-2.6.11-rc3-mm2-full/drivers/net/ne3210.c.old	2005-02-16 16:09:39.000000000 +0100
+++ linux-2.6.11-rc3-mm2-full/drivers/net/ne3210.c	2005-02-21 15:10:02.000000000 +0100
@@ -26,9 +26,6 @@
 	Updated to EISA probing API 5/2003 by Marc Zyngier.
 */
 
-static const char *version =
-	"ne3210.c: Driver revision v0.03, 30/09/98\n";
-
 #include <linux/module.h>
 #include <linux/eisa.h>
 #include <linux/kernel.h>
@@ -197,7 +194,7 @@
 	ei_status.priv = phys_mem;
 
 	if (ei_debug > 0)
-		printk(version);
+		printk("ne3210 loaded.\n");
 
 	ei_status.reset_8390 = &ne3210_reset_8390;
 	ei_status.block_input = &ne3210_block_input;
@@ -359,12 +356,12 @@
 MODULE_DESCRIPTION("NE3210 EISA Ethernet driver");
 MODULE_LICENSE("GPL");
 
-int ne3210_init(void)
+static int ne3210_init(void)
 {
 	return eisa_driver_register (&ne3210_eisa_driver);
 }
 
-void ne3210_cleanup(void)
+static void ne3210_cleanup(void)
 {
 	eisa_driver_unregister (&ne3210_eisa_driver);
 }

^ permalink raw reply

* [PATCH] ipconfig.c: fix dhcp timeout behaviour
From: Maxime Bizon @ 2005-06-24 18:53 UTC (permalink / raw)
  To: netdev

Hello,

I think there is a small bug in ipconfig.c in case IPCONFIG_DHCP is set
and dhcp is used.

When a DHCPOFFER is received, ip address is kept until we get DHCPACK.
If no ack is received, ic_dynamic() returns negatively, but leaves the
offered ip address in ic_myaddr.

This makes the main loop in ip_auto_config() break and uses the maybe
incomplete configuration.

Not sure if it's the best way to do, but the following trivial patch
correct this. 


Signed-off-by: Maxime Bizon <mbizon@freebox.fr>

--- linux-2.6.12.1/net/ipv4/ipconfig.c.orig	2005-06-22 21:33:05.000000000 +0200
+++ linux-2.6.12.1/net/ipv4/ipconfig.c	2005-06-24 17:55:11.000000000 +0200
@@ -1149,8 +1149,10 @@ static int __init ic_dynamic(void)
 		ic_rarp_cleanup();
 #endif
 
-	if (!ic_got_reply)
+	if (!ic_got_reply) {
+		ic_myaddr = INADDR_NONE;
 		return -1;
+	}
 
 	printk("IP-Config: Got %s answer from %u.%u.%u.%u, ",
 		((ic_got_reply & IC_RARP) ? "RARP" 


-- 
Maxime

^ permalink raw reply

* Re: [Patch] ipvs: close race conditions on ip_vs_conn_tab list modification
From: Neil Horman @ 2005-06-24 17:40 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: Neil Horman, linux-kernel, Wensong Zhang, akpm, netdev, davem
In-Reply-To: <Pine.LNX.4.44.0506241808150.2776-100000@l>

On Fri, Jun 24, 2005 at 06:09:40PM +0300, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Fri, 24 Jun 2005, Neil Horman wrote:
> 
> >  			if (ct) {
> >  				IP_VS_DBG(4, "del conn template\n");
> >  				ip_vs_conn_expire_now(ct);
> >  			}
> 
> 	Don't forget to use cp->control instead of ct, ct is not needed
> anymore.
> 
> Regards
> 
> --
> Julian Anastasov <ja@ssi.bg>
> 


Good catch.  Sorry, should have seen that earlier.  New patch attached with
corrections.  When you're comfortable with this, I'll post the 2.4 version of
the patch.

Regards 
Neil

Signed-off-by: Neil Horman <nhorman@redhat.com>

 ip_vs_conn.c |   24 ++++--------------------
 1 files changed, 4 insertions(+), 20 deletions(-)


--- linux-2.6.git/net/ipv4/ipvs/ip_vs_conn.c.orig	2005-06-23 13:11:00.000000000 -0400
+++ linux-2.6.git/net/ipv4/ipvs/ip_vs_conn.c	2005-06-24 13:33:03.000000000 -0400
@@ -548,7 +548,6 @@
 {
 	if (del_timer(&cp->timer))
 		mod_timer(&cp->timer, jiffies);
-	__ip_vs_conn_put(cp);
 }
 
 
@@ -801,21 +800,12 @@
 					continue;
 			}
 
-			/*
-			 * Drop the entry, and drop its ct if not referenced
-			 */
-			atomic_inc(&cp->refcnt);
-			ct_write_unlock(hash);
-
-			if ((ct = cp->control))
-				atomic_inc(&ct->refcnt);
 			IP_VS_DBG(4, "del connection\n");
 			ip_vs_conn_expire_now(cp);
-			if (ct) {
+			if (cp->control) {
 				IP_VS_DBG(4, "del conn template\n");
-				ip_vs_conn_expire_now(ct);
+				ip_vs_conn_expire_now(cp->control);
 			}
-			ct_write_lock(hash);
 		}
 		ct_write_unlock(hash);
 	}
@@ -829,7 +819,6 @@
 {
 	int idx;
 	struct ip_vs_conn *cp;
-	struct ip_vs_conn *ct;
 
   flush_again:
 	for (idx=0; idx<IP_VS_CONN_TAB_SIZE; idx++) {
@@ -839,18 +828,13 @@
 		ct_write_lock_bh(idx);
 
 		list_for_each_entry(cp, &ip_vs_conn_tab[idx], c_list) {
-			atomic_inc(&cp->refcnt);
-			ct_write_unlock(idx);
 
-			if ((ct = cp->control))
-				atomic_inc(&ct->refcnt);
 			IP_VS_DBG(4, "del connection\n");
 			ip_vs_conn_expire_now(cp);
-			if (ct) {
+			if (cp->control) {
 				IP_VS_DBG(4, "del conn template\n");
-				ip_vs_conn_expire_now(ct);
+				ip_vs_conn_expire_now(cp->control);
 			}
-			ct_write_lock(idx);
 		}
 		ct_write_unlock_bh(idx);
 	}
-- 
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *nhorman@redhat.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/

^ permalink raw reply

* Re: [git patch] urgent e1000 fix
From: Linus Torvalds @ 2005-06-24 16:40 UTC (permalink / raw)
  To: Tomasz K³oczko
  Cc: Keith Owens, Denis Vlasenko, Jeff Garzik, David Lang,
	Andrew Morton, Linux Kernel, Netdev List
In-Reply-To: <Pine.BSO.4.62.0506241140280.19853@rudy.mif.pg.gda.pl>



On Fri, 24 Jun 2005, Tomasz K³oczko wrote:
> 
> Linus .. why for kernel tree can't be used indent or other source 
> code formater ?

Sure, we do it, but then we try to make it obvious to all sides.

It's the "small and non-obvious" differences that are really poisonous. 
You don't see them in the soruces, yet patches don't apply.

So don't do subtle whitespace "fixups" by default. It just makes everybody
unhappy down the line.

That's not to say that we don't do whitespace fixups _occasionally_. When 
it's ugly enough to be noticeable, I sure as hell fix up whitespace. But 
not by default.

		Linus

^ permalink raw reply

* IPv6 and QoS
From: Didier Barvaux @ 2005-06-24 15:48 UTC (permalink / raw)
  To: netdev

Hello,


I would like to know if it's possible to do some QoS with IPv6. The LARTC howto says no [1], but perhaps the situation has evolved since the writing of this page.

What is the current situation ?


Regards,

Didier Barvaux


[1] http://lartc.org/howto/lartc.adv-filter.ipv6.html

^ permalink raw reply

* Re: [git patch] urgent e1000 fix
From: Horst von Brand @ 2005-06-24 15:11 UTC (permalink / raw)
  To: Denis Vlasenko
  Cc: Linus Torvalds, Jeff Garzik, David Lang, Andrew Morton,
	Linux Kernel, Netdev List
In-Reply-To: <200506240949.05620.vda@ilport.com.ua>

Denis Vlasenko <vda@ilport.com.ua> wrote:
> On Friday 24 June 2005 02:33, Linus Torvalds wrote:
> > To actually allow real fuzz or to allow real whitespace differences in the
> > patch data itself is a _much_ bigger issue than this trivial patch
> > corruption, and I'd prefer to avoid going there if at all possible.
> 
> How about automatic stripping of _trailing_ whitespace on all incoming
> patches? IIRC no file type (C, sh, Makefile, you name it) depends on
> conservation of it, thus it's 100% safe.

Works iff the patched code is similarly mangled first... I can hear a
distant howling on LKML on the bare thought of proposing this.

You also can't assume that spaces at the end of lines make no difference
for all uses people might want to put git to.
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513

^ permalink raw reply

* Re: [Patch] ipvs: close race conditions on ip_vs_conn_tab list modification
From: Julian Anastasov @ 2005-06-24 15:09 UTC (permalink / raw)
  To: Neil Horman; +Cc: linux-kernel, Wensong Zhang, akpm, netdev
In-Reply-To: <20050624144822.GD21499@hmsendeavour.rdu.redhat.com>


	Hello,

On Fri, 24 Jun 2005, Neil Horman wrote:

>  			if (ct) {
>  				IP_VS_DBG(4, "del conn template\n");
>  				ip_vs_conn_expire_now(ct);
>  			}

	Don't forget to use cp->control instead of ct, ct is not needed
anymore.

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [Patch] ipvs: close race conditions on ip_vs_conn_tab list modification
From: Neil Horman @ 2005-06-24 14:48 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: Neil Horman, linux-kernel, Wensong Zhang, akpm, netdev
In-Reply-To: <Pine.LNX.4.58.0506241046280.1690@u.domain.uli>

On Fri, Jun 24, 2005 at 11:46:21AM +0300, Julian Anastasov wrote:
> 
> 	Hello,
> 
> 	adding netdev to CC
> 
> On Thu, 23 Jun 2005, Neil Horman wrote:
><snip> 
> 	Looks ok but can you test an extended version:
> 
> - remove these atomic_inc for cp and ct and the corresponding
> __ip_vs_conn_put from ip_vs_conn_expire_now
> 
> - do the same for ip_vs_random_dropentry, it looks wrong in the same
> way because it is not running anymore together with the connection 
> expiration in same sltimer_handler
> 
> 	Also, 2.4 needs the same changes, I hope you can continue?
> 
> Regards
> 
> --
> Julian Anastasov <ja@ssi.bg>

No problem.  New patch attached with the above corrections/enhancements made.
I've tested them here, and had good results.

Signed-off-by: Neil Horman <nhorman@redhat.com>

 ip_vs_conn.c |   15 ---------------
 1 files changed, 15 deletions(-)


--- linux-2.6.git/net/ipv4/ipvs/ip_vs_conn.c.orig	2005-06-23 13:11:00.000000000 -0400
+++ linux-2.6.git/net/ipv4/ipvs/ip_vs_conn.c	2005-06-24 08:57:30.000000000 -0400
@@ -548,7 +548,6 @@
 {
 	if (del_timer(&cp->timer))
 		mod_timer(&cp->timer, jiffies);
-	__ip_vs_conn_put(cp);
 }
 
 
@@ -801,21 +800,12 @@
 					continue;
 			}
 
-			/*
-			 * Drop the entry, and drop its ct if not referenced
-			 */
-			atomic_inc(&cp->refcnt);
-			ct_write_unlock(hash);
-
-			if ((ct = cp->control))
-				atomic_inc(&ct->refcnt);
 			IP_VS_DBG(4, "del connection\n");
 			ip_vs_conn_expire_now(cp);
 			if (ct) {
 				IP_VS_DBG(4, "del conn template\n");
 				ip_vs_conn_expire_now(ct);
 			}
-			ct_write_lock(hash);
 		}
 		ct_write_unlock(hash);
 	}
@@ -839,18 +829,13 @@
 		ct_write_lock_bh(idx);
 
 		list_for_each_entry(cp, &ip_vs_conn_tab[idx], c_list) {
-			atomic_inc(&cp->refcnt);
-			ct_write_unlock(idx);
 
-			if ((ct = cp->control))
-				atomic_inc(&ct->refcnt);
 			IP_VS_DBG(4, "del connection\n");
 			ip_vs_conn_expire_now(cp);
 			if (ct) {
 				IP_VS_DBG(4, "del conn template\n");
 				ip_vs_conn_expire_now(ct);
 			}
-			ct_write_lock(idx);
 		}
 		ct_write_unlock_bh(idx);
 	}
-- 
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *nhorman@redhat.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/

^ permalink raw reply

* [PATCH] bugfix and scalability changes in net/ipv4/route.c
From: Eric Dumazet @ 2005-06-24 10:57 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: linux-net, herbert
In-Reply-To: <20050623.211140.131918815.davem@davemloft.net>

[-- Attachment #1: Type: text/plain, Size: 1117 bytes --]

Hi David

This is based on a previous patch I sent 3 months ago, reduced to only the essential part, since
the previous version got no success in netdev (some people said they were working on this stuff but still no progress)

reminder of the bugfix :

The rt_check_expire() has a serious problem on machines with large
route caches, and a standard HZ value of 1000.

With default values, ie ip_rt_gc_interval = 60*HZ = 60000 ;

the loop count :

     for (t = ip_rt_gc_interval << rt_hash_log; t >= 0;


overflows (t is a 31 bit value) as soon rt_hash_log is >= 16  (65536
slots in route cache hash table).

In this case, rt_check_expire() does nothing at all


Thank you

Eric Dumazet


[NET] Scalability fixes in net/ipv4/route.c, bugfix in rt_check_expire

  - Locking abstraction
  - Spinlocks moved out of rt hash table : Less memory (50%) used by rt hash table. it's a win even on UP.
  - Sizing of spinlocks table depends on NR_CPUS
  - rt hash table allocated using alloc_large_system_hash() function
  - rt_check_expire() fixes (an overflow was possible)

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>


[-- Attachment #2: patch.route --]
[-- Type: text/plain, Size: 7698 bytes --]

--- linux-2.6.12/net/ipv4/route.c	2005-06-17 21:48:29.000000000 +0200
+++ linux-2.6.12-ed/net/ipv4/route.c	2005-06-24 11:10:06.000000000 +0200
@@ -54,6 +54,7 @@
  *		Marc Boucher	:	routing by fwmark
  *	Robert Olsson		:	Added rt_cache statistics
  *	Arnaldo C. Melo		:	Convert proc stuff to seq_file
+ *	Eric Dumazet		:	hashed spinlocks and rt_check_expire() fixes.
  *
  *		This program is free software; you can redistribute it and/or
  *		modify it under the terms of the GNU General Public License
@@ -70,6 +71,7 @@
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/mm.h>
+#include <linux/bootmem.h>
 #include <linux/string.h>
 #include <linux/socket.h>
 #include <linux/sockios.h>
@@ -201,8 +203,37 @@
 
 struct rt_hash_bucket {
 	struct rtable	*chain;
-	spinlock_t	lock;
-} __attribute__((__aligned__(8)));
+};
+#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
+/*
+ * Instead of using one spinlock for each rt_hash_bucket, we use a table of spinlocks
+ * The size of this table is a power of two and depends on the number of CPUS.
+ */
+#if NR_CPUS >= 32
+#define RT_HASH_LOCK_SZ	4096
+#elif NR_CPUS >= 16
+#define RT_HASH_LOCK_SZ	2048
+#elif NR_CPUS >= 8
+#define RT_HASH_LOCK_SZ	1024
+#elif NR_CPUS >= 4
+#define RT_HASH_LOCK_SZ	512
+#else
+#define RT_HASH_LOCK_SZ	256
+#endif
+
+	static spinlock_t	*rt_hash_locks;
+# define rt_hash_lock_addr(slot) &rt_hash_locks[slot & (RT_HASH_LOCK_SZ - 1)]
+# define rt_hash_lock_init()	{ \
+		int i; \
+		rt_hash_locks = kmalloc(sizeof(spinlock_t) * RT_HASH_LOCK_SZ, GFP_KERNEL); \
+		if (!rt_hash_locks) panic("IP: failed to allocate rt_hash_locks\n"); \
+		for (i = 0; i < RT_HASH_LOCK_SZ; i++) \
+			spin_lock_init(&rt_hash_locks[i]); \
+		}
+#else
+# define rt_hash_lock_addr(slot) NULL
+# define rt_hash_lock_init()
+#endif
 
 static struct rt_hash_bucket 	*rt_hash_table;
 static unsigned			rt_hash_mask;
@@ -575,19 +606,24 @@
 /* This runs via a timer and thus is always in BH context. */
 static void rt_check_expire(unsigned long dummy)
 {
-	static int rover;
-	int i = rover, t;
+	static unsigned int rover;
+	int unsigned i = rover, goal;
 	struct rtable *rth, **rthp;
 	unsigned long now = jiffies;
-
-	for (t = ip_rt_gc_interval << rt_hash_log; t >= 0;
-	     t -= ip_rt_gc_timeout) {
+	u64 mult ;
+	mult = ((u64)ip_rt_gc_interval) << rt_hash_log;
+	if (ip_rt_gc_timeout > 1)
+		do_div(mult, ip_rt_gc_timeout);
+	goal = (unsigned int)mult;
+	if (goal > rt_hash_mask) goal = rt_hash_mask + 1;
+	for ( ; goal > 0 ; goal--) {
 		unsigned long tmo = ip_rt_gc_timeout;
 
 		i = (i + 1) & rt_hash_mask;
 		rthp = &rt_hash_table[i].chain;
 
-		spin_lock(&rt_hash_table[i].lock);
+		if (*rthp == 0) continue ;
+		spin_lock(rt_hash_lock_addr(i));
 		while ((rth = *rthp) != NULL) {
 			if (rth->u.dst.expires) {
 				/* Entry is expired even if it is in use */
@@ -620,8 +656,7 @@
  			rt_free(rth);
 #endif /* CONFIG_IP_ROUTE_MULTIPATH_CACHED */
 		}
-		spin_unlock(&rt_hash_table[i].lock);
-
+		spin_unlock(rt_hash_lock_addr(i));
 		/* Fallback loop breaker. */
 		if (time_after(jiffies, now))
 			break;
@@ -643,11 +678,11 @@
 	get_random_bytes(&rt_hash_rnd, 4);
 
 	for (i = rt_hash_mask; i >= 0; i--) {
-		spin_lock_bh(&rt_hash_table[i].lock);
+		spin_lock_bh(rt_hash_lock_addr(i));
 		rth = rt_hash_table[i].chain;
 		if (rth)
 			rt_hash_table[i].chain = NULL;
-		spin_unlock_bh(&rt_hash_table[i].lock);
+		spin_unlock_bh(rt_hash_lock_addr(i));
 
 		for (; rth; rth = next) {
 			next = rth->u.rt_next;
@@ -780,7 +815,7 @@
 
 			k = (k + 1) & rt_hash_mask;
 			rthp = &rt_hash_table[k].chain;
-			spin_lock_bh(&rt_hash_table[k].lock);
+			spin_lock_bh(rt_hash_lock_addr(k));
 			while ((rth = *rthp) != NULL) {
 				if (!rt_may_expire(rth, tmo, expire)) {
 					tmo >>= 1;
@@ -812,7 +847,7 @@
 				goal--;
 #endif /* CONFIG_IP_ROUTE_MULTIPATH_CACHED */
 			}
-			spin_unlock_bh(&rt_hash_table[k].lock);
+			spin_unlock_bh(rt_hash_lock_addr(k));
 			if (goal <= 0)
 				break;
 		}
@@ -882,7 +917,7 @@
 
 	rthp = &rt_hash_table[hash].chain;
 
-	spin_lock_bh(&rt_hash_table[hash].lock);
+	spin_lock_bh(rt_hash_lock_addr(hash));
 	while ((rth = *rthp) != NULL) {
 #ifdef CONFIG_IP_ROUTE_MULTIPATH_CACHED
 		if (!(rth->u.dst.flags & DST_BALANCED) &&
@@ -908,7 +943,7 @@
 			rth->u.dst.__use++;
 			dst_hold(&rth->u.dst);
 			rth->u.dst.lastuse = now;
-			spin_unlock_bh(&rt_hash_table[hash].lock);
+			spin_unlock_bh(rt_hash_lock_addr(hash));
 
 			rt_drop(rt);
 			*rp = rth;
@@ -949,7 +984,7 @@
 	if (rt->rt_type == RTN_UNICAST || rt->fl.iif == 0) {
 		int err = arp_bind_neighbour(&rt->u.dst);
 		if (err) {
-			spin_unlock_bh(&rt_hash_table[hash].lock);
+			spin_unlock_bh(rt_hash_lock_addr(hash));
 
 			if (err != -ENOBUFS) {
 				rt_drop(rt);
@@ -990,7 +1025,7 @@
 	}
 #endif
 	rt_hash_table[hash].chain = rt;
-	spin_unlock_bh(&rt_hash_table[hash].lock);
+	spin_unlock_bh(rt_hash_lock_addr(hash));
 	*rp = rt;
 	return 0;
 }
@@ -1058,7 +1093,7 @@
 {
 	struct rtable **rthp;
 
-	spin_lock_bh(&rt_hash_table[hash].lock);
+	spin_lock_bh(rt_hash_lock_addr(hash));
 	ip_rt_put(rt);
 	for (rthp = &rt_hash_table[hash].chain; *rthp;
 	     rthp = &(*rthp)->u.rt_next)
@@ -1067,7 +1102,7 @@
 			rt_free(rt);
 			break;
 		}
-	spin_unlock_bh(&rt_hash_table[hash].lock);
+	spin_unlock_bh(rt_hash_lock_addr(hash));
 }
 
 void ip_rt_redirect(u32 old_gw, u32 daddr, u32 new_gw,
@@ -3069,12 +3104,14 @@
 
 int __init ip_rt_init(void)
 {
-	int i, order, goal, rc = 0;
+	int rc = 0;
 
 	rt_hash_rnd = (int) ((num_physpages ^ (num_physpages>>8)) ^
 			     (jiffies ^ (jiffies >> 7)));
 
 #ifdef CONFIG_NET_CLS_ROUTE
+	{
+	int order;
 	for (order = 0;
 	     (PAGE_SIZE << order) < 256 * sizeof(struct ip_rt_acct) * NR_CPUS; order++)
 		/* NOTHING */;
@@ -3082,6 +3119,7 @@
 	if (!ip_rt_acct)
 		panic("IP: failed to allocate ip_rt_acct\n");
 	memset(ip_rt_acct, 0, PAGE_SIZE << order);
+	}
 #endif
 
 	ipv4_dst_ops.kmem_cachep = kmem_cache_create("ip_dst_cache",
@@ -3092,39 +3130,22 @@
 	if (!ipv4_dst_ops.kmem_cachep)
 		panic("IP: failed to allocate ip_dst_cache\n");
 
-	goal = num_physpages >> (26 - PAGE_SHIFT);
-	if (rhash_entries)
-		goal = (rhash_entries * sizeof(struct rt_hash_bucket)) >> PAGE_SHIFT;
-	for (order = 0; (1UL << order) < goal; order++)
-		/* NOTHING */;
-
-	do {
-		rt_hash_mask = (1UL << order) * PAGE_SIZE /
-			sizeof(struct rt_hash_bucket);
-		while (rt_hash_mask & (rt_hash_mask - 1))
-			rt_hash_mask--;
-		rt_hash_table = (struct rt_hash_bucket *)
-			__get_free_pages(GFP_ATOMIC, order);
-	} while (rt_hash_table == NULL && --order > 0);
-
-	if (!rt_hash_table)
-		panic("Failed to allocate IP route cache hash table\n");
-
-	printk(KERN_INFO "IP: routing cache hash table of %u buckets, %ldKbytes\n",
-	       rt_hash_mask,
-	       (long) (rt_hash_mask * sizeof(struct rt_hash_bucket)) / 1024);
-
-	for (rt_hash_log = 0; (1 << rt_hash_log) != rt_hash_mask; rt_hash_log++)
-		/* NOTHING */;
-
+	rt_hash_table = (struct rt_hash_bucket *)
+		alloc_large_system_hash("IP route cache",
+					sizeof(struct rt_hash_bucket),
+					rhash_entries,
+					(num_physpages >= 128 * 1024) ?
+						(27 - PAGE_SHIFT) :
+						(29 - PAGE_SHIFT),
+					HASH_HIGHMEM,
+					&rt_hash_log,
+					&rt_hash_mask,
+					0);
+	memset(rt_hash_table, 0, rt_hash_mask * sizeof(struct rt_hash_bucket));
+	rt_hash_lock_init();
+	ipv4_dst_ops.gc_thresh = rt_hash_mask;
+	ip_rt_max_size = rt_hash_mask * 16;
 	rt_hash_mask--;
-	for (i = 0; i <= rt_hash_mask; i++) {
-		spin_lock_init(&rt_hash_table[i].lock);
-		rt_hash_table[i].chain = NULL;
-	}
-
-	ipv4_dst_ops.gc_thresh = (rt_hash_mask + 1);
-	ip_rt_max_size = (rt_hash_mask + 1) * 16;
 
 	rt_cache_stat = alloc_percpu(struct rt_cache_stat);
 	if (!rt_cache_stat)

^ permalink raw reply

* Re: [git patch] urgent e1000 fix
From: Tomasz Kłoczko @ 2005-06-24 10:14 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Keith Owens, Denis Vlasenko, Jeff Garzik, David Lang,
	Andrew Morton, Linux Kernel, Netdev List
In-Reply-To: <Pine.LNX.4.58.0506240149440.11175@ppc970.osdl.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2715 bytes --]

On Fri, 24 Jun 2005, Linus Torvalds wrote:

>
>
> On Fri, 24 Jun 2005, Keith Owens wrote:
>
>> On Fri, 24 Jun 2005 09:49:05 +0300,
>> Denis Vlasenko <vda@ilport.com.ua> wrote:
>>> On Friday 24 June 2005 02:33, Linus Torvalds wrote:
>>>> To actually allow real fuzz or to allow real whitespace differences in the
>>>> patch data itself is a _much_ bigger issue than this trivial patch
>>>> corruption, and I'd prefer to avoid going there if at all possible.
>>>
>>> How about automatic stripping of _trailing_ whitespace on all incoming
>>> patches? IIRC no file type (C, sh, Makefile, you name it) depends on
>>> conservation of it, thus it's 100% safe.
>>
>> One (admittedly rare) case - adding a text file that contains an
>> embedded patch, so you have a patch that includes a patch.  This is
>> sometimes done in Documentation files when an external file has to be
>> changed.  In embedded patch, empty lines are converted to a single
>> space, which then appears as trailing whitespace.  Not sure if that is
>> a big enough reason not to strip whitespace.
>
> There's a much more important reason never _ever_ to mess with whitespace
> in patches: it by definition measn that the resulting whitespace now does
> not match the thing at the other end, and that _will_ mean merge problems
> later (ie subsequent patches will have increasingly incorrect whitespace,
> and now everybody has to live with whitespace not being reliable).
>
> So no. The only reliable way to handle whitespace is to never corrupt it.
> Don't make excuses for broken email clients etc.

Linus .. why for kernel tree can't be used indent or other source 
code formater ?

If indent tool can't feet all what is neccessary or have some not ease 
solveable bugs or can't be adopted other now avalaible tool IMO *it is* 
*time* for start project with special formater for kernel source tree. 
Using it can cut all this kind dissusions :>

If indent can handle correcly kernel source tree it will be posiible place
in source root tree one .indent.pro file and add small modificatiom to 
make suit with additional "indent" target.
In case using indent neccessary patch can take only few lines :>

After this all developers before generate patches will must only pass 
"make indent" .. nothing more.
As result can be also removed Documentation/CodingStyle file (description 
about passing "make indent" before generate patches can be moved to 
Documentation/SubmittingPatches)

kloczek
-- 
-----------------------------------------------------------
*Ludzie nie mają problemów, tylko sobie sami je stwarzają*
-----------------------------------------------------------
Tomasz Kłoczko, sys adm @zie.pg.gda.pl|*e-mail: kloczek@rudy.mif.pg.gda.pl*

^ permalink raw reply

* Re: [git patch] urgent e1000 fix
From: Linus Torvalds @ 2005-06-24  8:51 UTC (permalink / raw)
  To: Keith Owens
  Cc: Denis Vlasenko, Jeff Garzik, David Lang, Andrew Morton,
	Linux Kernel, Netdev List
In-Reply-To: <13661.1119601379@kao2.melbourne.sgi.com>



On Fri, 24 Jun 2005, Keith Owens wrote:

> On Fri, 24 Jun 2005 09:49:05 +0300, 
> Denis Vlasenko <vda@ilport.com.ua> wrote:
> >On Friday 24 June 2005 02:33, Linus Torvalds wrote:
> >> To actually allow real fuzz or to allow real whitespace differences in the
> >> patch data itself is a _much_ bigger issue than this trivial patch
> >> corruption, and I'd prefer to avoid going there if at all possible.
> >
> >How about automatic stripping of _trailing_ whitespace on all incoming
> >patches? IIRC no file type (C, sh, Makefile, you name it) depends on
> >conservation of it, thus it's 100% safe.
> 
> One (admittedly rare) case - adding a text file that contains an
> embedded patch, so you have a patch that includes a patch.  This is
> sometimes done in Documentation files when an external file has to be
> changed.  In embedded patch, empty lines are converted to a single
> space, which then appears as trailing whitespace.  Not sure if that is
> a big enough reason not to strip whitespace.

There's a much more important reason never _ever_ to mess with whitespace
in patches: it by definition measn that the resulting whitespace now does
not match the thing at the other end, and that _will_ mean merge problems
later (ie subsequent patches will have increasingly incorrect whitespace,
and now everybody has to live with whitespace not being reliable).

So no. The only reliable way to handle whitespace is to never corrupt it. 
Don't make excuses for broken email clients etc.

		Linus

^ permalink raw reply

* Re: [Patch] ipvs: close race conditions on ip_vs_conn_tab list modification
From: Julian Anastasov @ 2005-06-24  8:46 UTC (permalink / raw)
  To: Neil Horman; +Cc: linux-kernel, Wensong Zhang, akpm, netdev
In-Reply-To: <20050623183926.GI16783@hmsendeavour.rdu.redhat.com>


	Hello,

	adding netdev to CC

On Thu, 23 Jun 2005, Neil Horman wrote:

> Hello there-
> 	Patch to close a race condition in ip_vs_conn_flush.  In an smp system,
> it is possible for an connection timer to expire, calling ip_vs_conn_expire
> while the connection table is being flushed, before ct_write_lock_bh is
> acquired.  Since the list iterator loop in ip_vs_con_flush releases and
> re-acquires the spinlock (even though it doesn't re-enable softirqs), it is
> possible for the expiration function to modify the connection list, while it is
> being traversed in ip_vs_conn_flush.  The result is that the next pointer gets
> set to NULL, and subsequently dereferenced, resulting in an oops.  This patch
> removes the lock release and re-aquisition from the loop, closing the race
> window.  Tested by myself, and those who origionally experienced the crash and
> reported it to me, with successful results.
>
> Signed-off-by: Neil Horman <nhorman@redhat.com>
>
>  ip_vs_conn.c |    2 --
>  1 files changed, 2 deletions(-)
>
>
> --- linux-2.6.git/net/ipv4/ipvs/ip_vs_conn.c.orig	2005-06-23 13:11:00.910372471 -0400
> +++ linux-2.6.git/net/ipv4/ipvs/ip_vs_conn.c	2005-06-23 13:15:54.459852393 -0400
> @@ -840,7 +838,6 @@
>
>  		list_for_each_entry(cp, &ip_vs_conn_tab[idx], c_list) {
>  			atomic_inc(&cp->refcnt);
> -			ct_write_unlock(idx);
>
>  			if ((ct = cp->control))
>  				atomic_inc(&ct->refcnt);
> @@ -850,7 +847,6 @@
>  				IP_VS_DBG(4, "del conn template\n");
>  				ip_vs_conn_expire_now(ct);
>  			}
> -			ct_write_lock(idx);
>  		}
>  		ct_write_unlock_bh(idx);
>  	}

	Looks ok but can you test an extended version:

- remove these atomic_inc for cp and ct and the corresponding
__ip_vs_conn_put from ip_vs_conn_expire_now

- do the same for ip_vs_random_dropentry, it looks wrong in the same
way because it is not running anymore together with the connection 
expiration in same sltimer_handler

	Also, 2.4 needs the same changes, I hope you can continue?

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [git patch] urgent e1000 fix
From: Keith Owens @ 2005-06-24  8:22 UTC (permalink / raw)
  To: Denis Vlasenko
  Cc: Linus Torvalds, Jeff Garzik, David Lang, Andrew Morton,
	Linux Kernel, Netdev List
In-Reply-To: <200506240949.05620.vda@ilport.com.ua>

On Fri, 24 Jun 2005 09:49:05 +0300, 
Denis Vlasenko <vda@ilport.com.ua> wrote:
>On Friday 24 June 2005 02:33, Linus Torvalds wrote:
>> To actually allow real fuzz or to allow real whitespace differences in the
>> patch data itself is a _much_ bigger issue than this trivial patch
>> corruption, and I'd prefer to avoid going there if at all possible.
>
>How about automatic stripping of _trailing_ whitespace on all incoming
>patches? IIRC no file type (C, sh, Makefile, you name it) depends on
>conservation of it, thus it's 100% safe.

One (admittedly rare) case - adding a text file that contains an
embedded patch, so you have a patch that includes a patch.  This is
sometimes done in Documentation files when an external file has to be
changed.  In embedded patch, empty lines are converted to a single
space, which then appears as trailing whitespace.  Not sure if that is
a big enough reason not to strip whitespace.

^ permalink raw reply

* Re: [PATCH] dst numa: Avoid dst counter cacheline bouncing
From: Dipankar Sarma @ 2005-06-24  7:29 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: linux-kernel, netdev, davem, shai, akpm
In-Reply-To: <Pine.LNX.4.62.0506232204320.30382@graphe.net>

On Thu, Jun 23, 2005 at 10:05:09PM -0700, Christoph Lameter wrote:
> On Fri, 24 Jun 2005, Dipankar Sarma wrote:
> 
> > Do we really need to do a distributed reference counter implementation
> > inside dst cache code ? If you are willing to wait for a while,
> > we should have modified Rusty's bigref implementation on top of the 
> > interleaving dynamic per-cpu allocator. We can look at distributed 
> > reference counter for dst refcount then and see how that can be 
> > worked out.
> 
> Is that code available somewhere?

Various places in lkml discussions. Search for discussions on dynamic
per-cpu allocator. Currently Bharata is adding
cpu hotplug awareness in it, but the basic patches work.

BTW, I am not saying that bigref has what you need. What I am trying
to say is that you should see if something like bigref can
be tweaked to use in your case before implementing a new type of
ref counting wholly in dst code.

Thanks
Dipankar

^ permalink raw reply

* Re: [git patch] urgent e1000 fix
From: Denis Vlasenko @ 2005-06-24  6:49 UTC (permalink / raw)
  To: Linus Torvalds, Jeff Garzik
  Cc: David Lang, Andrew Morton, Linux Kernel, Netdev List
In-Reply-To: <Pine.LNX.4.58.0506231625310.11175@ppc970.osdl.org>

On Friday 24 June 2005 02:33, Linus Torvalds wrote:
> To actually allow real fuzz or to allow real whitespace differences in the
> patch data itself is a _much_ bigger issue than this trivial patch
> corruption, and I'd prefer to avoid going there if at all possible.

How about automatic stripping of _trailing_ whitespace on all incoming
patches? IIRC no file type (C, sh, Makefile, you name it) depends on
conservation of it, thus it's 100% safe.
--
vda

^ permalink raw reply

* Re: [PATCH] dst_entry structure use,lastuse and refcnt abstraction
From: David S. Miller @ 2005-06-24  6:16 UTC (permalink / raw)
  To: clameter; +Cc: linux-kernel, linux-net, shai, akpm, netdev, herbert
In-Reply-To: <Pine.LNX.4.62.0506232256360.17993@schroedinger.engr.sgi.com>

From: Christoph Lameter <clameter@engr.sgi.com>
Date: Thu, 23 Jun 2005 23:03:45 -0700 (PDT)

> Ok. Then we are done. With 58 Itanium processors and 200G Ram I get 
> more than 10% improvement ;-). With 500 tasks we have 453 vs. 499 j/m/t.
> That is 9.21%. For 300 tasks we have 9.4% etc. I am sure that I can push 
> this some more with bigger counts of processors and also some other NUMA 
> related performance issues.

So it took 7 times more processors to increase the performance gain by
just over 3 on a microscopic synthetic benchmark.  That's not
impressive at all.

And you still haven't shown what happens for the workloads I
suggested.  A web benchmark, with say a thousand unique clients, would
be sufficient for one of those btw.  That case has very low dst
locality, yet dsts are useful because you'll have about 2 or 3
concurrent connections per dst.

^ permalink raw reply

* Re: [PATCH] dst_entry structure use,lastuse and refcnt abstraction
From: Christoph Lameter @ 2005-06-24  6:03 UTC (permalink / raw)
  To: David S. Miller; +Cc: linux-kernel, linux-net, shai, akpm, netdev, herbert
In-Reply-To: <20050623.211140.131918815.davem@davemloft.net>

On Thu, 23 Jun 2005, David S. Miller wrote:

> > outweigh the complexity added by these patches. What would be 
> > the performance benefit that we would need to see to feel that is 
> > is right to get such a change in?
> 
> Something on the order of %10.  If I recall correctly, that's
> basically what we got on SMP from the RCU changes to the routing
> cache.

Ok. Then we are done. With 58 Itanium processors and 200G Ram I get 
more than 10% improvement ;-). With 500 tasks we have 453 vs. 499 j/m/t.
That is 9.21%. For 300 tasks we have 9.4% etc. I am sure that I can push 
this some more with bigger counts of processors and also some other NUMA 
related performance issues.

58p SGI Altix 200G Ram running 2.6.12-mm1.

with patch:

AIM Multiuser Benchmark - Suite VII Run Beginning

Tasks    jobs/min  jti  jobs/min/task      real       cpu
    1     2439.02  100      2439.0244      2.46      0.02   Fri Jun 24 00:28:01 2005
  100   138664.20   92      1386.6420      4.33    106.04   Fri Jun 24 00:28:19 2005
  200   191113.23   87       955.5662      6.28    218.09   Fri Jun 24 00:28:45 2005
  300   219968.23   85       733.2274      8.18    329.09   Fri Jun 24 00:29:19 2005
  400   237037.04   85       592.5926     10.12    442.07   Fri Jun 24 00:30:01 2005
  500   249418.02   82       498.8360     12.03    551.93   Fri Jun 24 00:30:51 2005
  600   257879.66   80       429.7994     13.96    663.44   Fri Jun 24 00:31:49 2005
  700   264550.26   84       377.9289     15.88    774.88   Fri Jun 24 00:32:54 2005
  800   269587.19   81       336.9840     17.80    886.85   Fri Jun 24 00:34:07 2005
  900   273736.50   83       304.1517     19.73    997.90   Fri Jun 24 00:35:28 2005
 1000   277225.89   82       277.2259     21.64   1109.18   Fri Jun 24 00:36:57 2005

without patch

AIM Multiuser Benchmark - Suite VII Run Beginning

Tasks    jobs/min  jti  jobs/min/task      real       cpu
    1     2439.02  100      2439.0244      2.46      0.02   Fri Jun 24 00:46:03 2005
  100   131955.14   92      1319.5514      4.55    114.52   Fri Jun 24 00:46:22 2005
  200   177409.82   88       887.0491      6.76    246.62   Fri Jun 24 00:46:50 2005
  300   201027.47   85       670.0916      8.95    373.49   Fri Jun 24 00:47:27 2005
  400   216313.65   87       540.7841     11.09    497.36   Fri Jun 24 00:48:13 2005
  500   226637.46   83       453.2749     13.24    620.68   Fri Jun 24 00:49:07 2005
  600   233781.41   85       389.6357     15.40    746.18   Fri Jun 24 00:50:10 2005
  700   239561.94   84       342.2313     17.53    869.34   Fri Jun 24 00:51:22 2005
  800   243630.09   85       304.5376     19.70    996.39   Fri Jun 24 00:52:43 2005
  900   246992.64   85       274.4363     21.86   1121.90   Fri Jun 24 00:54:13 2005
 1000   249979.17   84       249.9792     24.00   1245.55   Fri Jun 24 00:55:52 2005



^ permalink raw reply

* Re: [PATCH] dst numa: Avoid dst counter cacheline bouncing
From: Christoph Lameter @ 2005-06-24  5:05 UTC (permalink / raw)
  To: Dipankar Sarma; +Cc: linux-kernel, netdev, davem, shai, akpm
In-Reply-To: <20050624045854.GA6465@in.ibm.com>

On Fri, 24 Jun 2005, Dipankar Sarma wrote:

> Do we really need to do a distributed reference counter implementation
> inside dst cache code ? If you are willing to wait for a while,
> we should have modified Rusty's bigref implementation on top of the 
> interleaving dynamic per-cpu allocator. We can look at distributed 
> reference counter for dst refcount then and see how that can be 
> worked out.

Is that code available somewhere?

^ permalink raw reply

* Re: [PATCH] dst numa: Avoid dst counter cacheline bouncing
From: Dipankar Sarma @ 2005-06-24  4:58 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: linux-kernel, netdev, davem, shai, akpm
In-Reply-To: <Pine.LNX.4.62.0506232005030.28244@graphe.net>

On Thu, Jun 23, 2005 at 08:10:06PM -0700, Christoph Lameter wrote:
> The dst_entry structure contains an atomic counter that is incremented and
> decremented for each network packet being sent. If there is a high number of
> packets being sent from multiple processors then cacheline bouncing between
> NUMA nodes can limit the tcp performance in a NUMA system.
> 
> The following patch splits the usage counter into counters per node.
> 

Do we really need to do a distributed reference counter implementation
inside dst cache code ? If you are willing to wait for a while,
we should have modified Rusty's bigref implementation on top of the 
interleaving dynamic per-cpu allocator. We can look at distributed 
reference counter for dst refcount then and see how that can be 
worked out.

Thanks
Dipankar

^ permalink raw reply

* Re: [PATCH] dst_entry structure use,lastuse and refcnt abstraction
From: David S. Miller @ 2005-06-24  4:11 UTC (permalink / raw)
  To: christoph; +Cc: linux-kernel, linux-net, shai, akpm, netdev, herbert
In-Reply-To: <Pine.LNX.4.62.0506232057340.29222@graphe.net>

From: Christoph Lameter <christoph@lameter.com>
Date: Thu, 23 Jun 2005 21:06:30 -0700 (PDT)

> I am sure one can do better than that. The x445 is the smallest NUMA box 
> that I know of and the only one available in the context of that project. 
> But I am also not sure that this will reach proportions  that will 
> outweigh the complexity added by these patches. What would be 
> the performance benefit that we would need to see to feel that is 
> is right to get such a change in?

Something on the order of %10.  If I recall correctly, that's
basically what we got on SMP from the RCU changes to the routing
cache.

> Hmm. I like the idea of a separate routing cache per node. May actually 
> reach higher performance than splitting the counters. Lets think a bit 
> about that.

One thing that may happen down the line is that the flow cache
becomes a more unified thing.  Ie. local sockets get found there,
netfilter rules can match, as well as normal "routes".

The idea is that on a miss, you go down into the "slow" lookup path
where the data structure is less distributed (SMP and NUMA wise) and
slower to search.  And this populates the flow tables.

But that seems to work best on a per-cpu basis.

Unfortunately, it really doesn't address your problem in and of
itself.  This is because flow entries, especially when used on
sockets, would still get accessed by other cpus and thus nodes.
We would need to build something on top of the per-cpu flowcache,
such as the socket node array idea, to get something that helps
this NUMA issue as well.

Just tossing out some forward looking ideas :)

To be honest, the unified flow cache idea has been tossed around
a lot, and it's still intellectual masterbation.  But then again,
so was the design for the IPSEC stuff we have now for several years.

^ permalink raw reply

* Re: [PATCH] dst_entry structure use,lastuse and refcnt abstraction
From: Christoph Lameter @ 2005-06-24  4:06 UTC (permalink / raw)
  To: David S. Miller; +Cc: linux-kernel, linux-net, shai, akpm, netdev, herbert
In-Reply-To: <20050623.205447.66178303.davem@davemloft.net>

On Thu, 23 Jun 2005, David S. Miller wrote:

> Ok.  You're going to have to come up with something better
> than a %3 AIM benchmark increase with 5000 threads to justify
> those invasive NUMA changes, and thus this infrastructure for
> it.

I am sure one can do better than that. The x445 is the smallest NUMA box 
that I know of and the only one available in the context of that project. 
But I am also not sure that this will reach proportions  that will 
outweigh the complexity added by these patches. What would be 
the performance benefit that we would need to see to feel that is 
is right to get such a change in?

> I'm picking those examples, because I am rather certain your patches
> will hurt performance in those cases.  The data structure size
> expansion and extra memory allocations alone for the per-node dst
> stuff should be good about doing that.

Hmm. I like the idea of a separate routing cache per node. May actually 
reach higher performance than splitting the counters. Lets think a bit 
about that.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox