Netdev List
 help / color / mirror / Atom feed
* Re: [patch 01/10] e1000e: make E1000E default to the same kconfig setting as E1000
From: Stephen Hemminger @ 2007-12-14 23:54 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jeff Garzik, netdev, randy.dunlap, auke-jan.h.kok, linux-kernel
In-Reply-To: <20071214152215.55ef46e8.akpm@linux-foundation.org>

Andrew Morton wrote:
> On Fri, 14 Dec 2007 15:39:26 -0500
> Jeff Garzik <jeff@garzik.org> wrote:
>
>   
>> akpm@linux-foundation.org wrote:
>>     
>>> From: Randy Dunlap <randy.dunlap@oracle.com>
>>>
>>> Make E1000E default to the same kconfig setting as E1000.  So people's
>>> machiens don't stop working when they use oldconfig.
>>>
>>> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
>>> Cc: Jeff Garzik <jeff@garzik.org>
>>> Cc: Auke Kok <auke-jan.h.kok@intel.com>
>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>> ---
>>>
>>>  drivers/net/Kconfig |    1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff -puN drivers/net/Kconfig~e1000e-make-e1000e-default-to-the-same-kconfig-setting-as-e1000 drivers/net/Kconfig
>>> --- a/drivers/net/Kconfig~e1000e-make-e1000e-default-to-the-same-kconfig-setting-as-e1000
>>> +++ a/drivers/net/Kconfig
>>> @@ -1986,6 +1986,7 @@ config E1000_DISABLE_PACKET_SPLIT
>>>  config E1000E
>>>  	tristate "Intel(R) PRO/1000 PCI-Express Gigabit Ethernet support"
>>>  	depends on PCI
>>> +	default E1000
>>>       
>> I am not inclined to apply this one.  This practice, applied over time, 
>> will tend to accumulate weird 'default' and 'select' statements.
>>
>> So I think the breakage that occurs is mitigated by two factors:
>> 1) kernel hackers that do their own configs are expected to be able to 
>> figure this stuff.
>> 2) kernel builders (read: distros, mainly) are expected to have put 
>> thought into the Kconfig selection and driver migration strategies.
>>
>> PCI IDs move across drivers from time, and we don't want to apply these 
>> sorts changes:  Viewed in the long term, the suggested patch is merely a 
>> temporary change to allow kernel experts to more easily deal with the 
>> PCI ID migration across drivers.
>>
>> I would prefer simply to communicate to kernel experts and builders 
>> about a Kconfig issue that could potentially their booting/networking... 
>>   because this patch is only needed if the kernel experts do not already 
>> know about a necessary config update.
>>     
>
> You can take it out again later on - most people's .configs will then have
> E1000E set.   People who still do `cp ancientconfig .config ; make oldconfig'
> remain screwed.
>   

Sounds like something build system should help with. Some more user 
friendly syntax for dealing
with issues of driver conversion.

^ permalink raw reply

* Re: 2.6.24-rc5-mm1 -- inconsistent {in-softirq-W} -> {softirq-on-R} usage.
From: Andrew Morton @ 2007-12-14 23:36 UTC (permalink / raw)
  To: Miles Lane; +Cc: linux-kernel, netdev, Ingo Molnar, Peter Zijlstra
In-Reply-To: <a44ae5cd0712141413u4ddbfd06gca259cfc3bc0ab1a@mail.gmail.com>

On Fri, 14 Dec 2007 17:13:21 -0500
"Miles Lane" <miles.lane@gmail.com> wrote:

> Sorry Andrew, I don't know who to forward this problem to.
> 
> I tried running:  find /proc | xargs cat
> and got this:
> 
> =================================
> [ INFO: inconsistent lock state ]
> 2.6.24-rc5-mm1 #26
> ---------------------------------
> inconsistent {in-softirq-W} -> {softirq-on-R} usage.
> cat/6944 [HC0[0]:SC0[0]:HE1:SE1] takes:
> BUG: unable to handle kernel paging request at virtual address 0f1eff0b
> printing ip: c01fe64d *pde = 00000000
> Oops: 0000 [#1] PREEMPT SMP
> last sysfs file: /sys/block/sda/sda3/stat
> Modules linked in: aes_generic i915 drm rfcomm l2cap bluetooth
> cpufreq_stats cpufreq_conservative cpufreq_performance sbs sbshc
> dm_crypt sbp2 parport_pc lp parport pcmcia arc4 ecb crypto_blkcipher
> cryptomgr crypto_algapi tifm_7xx1 tifm_core yenta_socket
> rsrc_nonstatic pcmcia_core iwl3945 iTCO_wdt iTCO_vendor_support
> watchdog_core watchdog_dev snd_hda_intel mac80211 snd_pcm_oss
> snd_mixer_oss cfg80211 snd_pcm sky2 snd_seq_dummy snd_seq_oss
> snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq snd_timer
> snd_seq_device snd soundcore snd_page_alloc shpchp pci_hotplug
> firewire_ohci firewire_core crc_itu_t ata_generic piix ide_core
> 
> Pid: 6944, comm: cat Not tainted (2.6.24-rc5-mm1 #26)
> EIP: 0060:[<c01fe64d>] EFLAGS: 00210097 CPU: 0
> EIP is at strnlen+0x9/0x1c
> EAX: 0f1eff0b EBX: 0f1eff0b ECX: 0f1eff0b EDX: fffffffe
> ESI: c05b74f6 EDI: d6267d94 EBP: d6267cc8 ESP: d6267cc8
>  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> Process cat (pid: 6944, ti=d6267000 task=d5a09000 task.ti=d6267000)
> Stack: d6267cfc c01fdd22 00000400 c05b74f4 00000001 c05b78f4 00000000 ffffffff
>        ffffffff c048f503 00000400 d5a09000 00000002 d6267d0c c01fdf41 d6267d94
>        db68c04a d6267d74 c012ae81 d6267d94 00000028 c05b89f7 00200046 00000000
> Call Trace:
>  [<c0108eb2>] show_trace_log_lvl+0x12/0x25
>  [<c0108f4f>] show_stack_log_lvl+0x8a/0x95
>  [<c0108fe4>] show_registers+0x8a/0x1bd
>  [<c010922f>] die+0x118/0x1dc
>  [<c03cf706>] do_page_fault+0x5a4/0x681
>  [<c03cdd72>] error_code+0x72/0x78
>  [<c01fdd22>] vsnprintf+0x277/0x40e
>  [<c01fdf41>] vscnprintf+0xe/0x1d
>  [<c012ae81>] vprintk+0xcb/0x2f3
>  [<c012b0be>] printk+0x15/0x17
>  [<c0145e55>] print_lock_name+0x4e/0xa2
>  [<c0146099>] print_lock+0xe/0x3a
>  [<c01464cf>] print_usage_bug+0xbc/0x117
>  [<c0146fb6>] mark_lock+0x2e7/0x3fe
>  [<c0147b9a>] __lock_acquire+0x498/0xbf4
>  [<c014836c>] lock_acquire+0x76/0x9d
>  [<c03cd6d2>] _read_lock+0x23/0x32
>  [<c03491ae>] sock_i_ino+0x14/0x30
>  [<c03c88ed>] packet_seq_show+0x22/0x75
>  [<c019b41a>] seq_read+0x19d/0x26f
>  [<c01b0ded>] proc_reg_read+0x60/0x74
>  [<c01854aa>] vfs_read+0x8a/0x106
>  [<c01858a8>] sys_read+0x3b/0x60
>  [<c0107cea>] sysenter_past_esp+0x6b/0xc1
>  =======================
> Code: 01 00 00 00 4f 89 fa 5f 89 d0 5d c3 55 85 c9 89 e5 57 89 c7 89
> d0 74 05 f2 ae 75 01 4f 89 f8 5f 5d c3 55 89 c1 89 e5 89 c8 eb 06 <80>
> 38 00 74 07 40 4a 83 fa ff 75 f4 29 c8 5d c3 90 90 90 55 83
> EIP: [<c01fe64d>] strnlen+0x9/0x1c SS:ESP 0068:d6267cc8
> note: cat[6944] exited with preempt_count 4

I'd say you hit a networking locking bug and then when trying to report
that bug, lockdep crashed.

The networking bug looks to be around sock_i_ino()'s taking of
sk_callback_lock with softirq's enabled.  Perhaps this will fix it.

diff -puN net/core/sock.c~a net/core/sock.c
--- a/net/core/sock.c~a
+++ a/net/core/sock.c
@@ -1115,9 +1115,9 @@ int sock_i_uid(struct sock *sk)
 {
 	int uid;
 
-	read_lock(&sk->sk_callback_lock);
+	read_lock_bh(&sk->sk_callback_lock);
 	uid = sk->sk_socket ? SOCK_INODE(sk->sk_socket)->i_uid : 0;
-	read_unlock(&sk->sk_callback_lock);
+	read_unlock_bh(&sk->sk_callback_lock);
 	return uid;
 }
 
@@ -1125,9 +1125,9 @@ unsigned long sock_i_ino(struct sock *sk
 {
 	unsigned long ino;
 
-	read_lock(&sk->sk_callback_lock);
+	read_lock_bh(&sk->sk_callback_lock);
 	ino = sk->sk_socket ? SOCK_INODE(sk->sk_socket)->i_ino : 0;
-	read_unlock(&sk->sk_callback_lock);
+	read_unlock_bh(&sk->sk_callback_lock);
 	return ino;
 }
 
_


^ permalink raw reply

* [PATCH] e1000: Dump the eeprom when a user encounters a bad checksum
From: Auke Kok @ 2007-12-14 23:35 UTC (permalink / raw)
  To: jeff; +Cc: netdev, davem, john.ronciak, jesse.brandeburg

To help supporting users with a bad eeprom checksum, dump the
eeprom info when such a situation is encountered by a user.

Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
---

 drivers/net/e1000/e1000_main.c |   90 +++++++++++++++++++++++++++++++++++-----
 1 files changed, 79 insertions(+), 11 deletions(-)

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index efd8c2d..2dab1a6 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -817,6 +817,69 @@ e1000_reset(struct e1000_adapter *adapter)
 }
 
 /**
+ *  Dump the eeprom for users having checksum issues
+ **/
+void e1000_dump_eeprom(struct e1000_adapter *adapter)
+{
+	struct net_device *netdev = adapter->netdev;
+	struct ethtool_eeprom eeprom;
+	const struct ethtool_ops *ops = netdev->ethtool_ops;
+	u8 *data;
+	int i;
+	u16 csum_old, csum_new = 0;
+
+	eeprom.len = ops->get_eeprom_len(netdev);
+	eeprom.offset = 0;
+
+	data = kmalloc(eeprom.len, GFP_KERNEL);
+	if (!data) {
+		printk(KERN_ERR "Unable to allocate memory to dump EEPROM"
+		       " data\n");
+		return;
+	}
+
+	ops->get_eeprom(netdev, &eeprom, data);
+
+	csum_old = (data[EEPROM_CHECKSUM_REG * 2]) +
+		   (data[EEPROM_CHECKSUM_REG * 2 + 1] << 8);
+	for (i = 0; i < EEPROM_CHECKSUM_REG * 2; i += 2)
+		csum_new += data[i] + (data[i + 1] << 8);
+	csum_new = EEPROM_SUM - csum_new;
+
+	printk(KERN_ERR "/*********************/\n");
+	printk(KERN_ERR "Current EEPROM: 0x%04x\nCalculated    : 0x%04x\n",
+	       csum_old, csum_new);
+
+	printk(KERN_ERR "Offset   Values\n");
+	printk(KERN_ERR "======   ======\n");
+	for (i = 0; i < eeprom.len; i += 16)
+		printk(KERN_ERR "0x%04x   "
+		       "%02x %02x %02x %02x %02x %02x %02x %02x "
+		       "%02x %02x %02x %02x %02x %02x %02x %02x\n",
+		       i, data[i], data[i + 1], data[i + 2], data[i + 3],
+		       data[i + 4], data[i + 5], data[i + 6], data[i + 7],
+		       data[i + 8], data[i + 9], data[i + 10], data[i + 11],
+		       data[i + 12], data[i + 13], data[i + 14], data[i + 15]);
+
+	printk(KERN_ERR "Include this output when contacting your support "
+	       "provider.\n\nThis is not a software error! Something bad "
+	       "happened to your hardware or\nEEPROM image. Ignoring this "
+	       "problem could result in further problems,\npossibly loss "
+	       "of data, corruption or system hangs!\n\n");
+	printk(KERN_ERR "The MAC Address will be reset to 00:00:00:00:00:00, "
+	       "which is invalid\nand requires you to set the proper MAC "
+	       "address manually before continuing\nto enable this network "
+	       "device.\n\n");
+	printk(KERN_ERR "Please inspect the EEPROM dump and report the issue "
+	       "to your hardware vendor\nor Intel Customer Support: "
+	       "linux-nics@intel.com\n");
+
+	printk(KERN_ERR "/*********************/\n");
+
+	kfree(data);
+}
+
+/**
  * e1000_probe - Device Initialization Routine
  * @pdev: PCI device information struct
  * @ent: entry in e1000_pci_tbl
@@ -967,7 +1030,6 @@ e1000_probe(struct pci_dev *pdev,
 	adapter->en_mng_pt = e1000_enable_mng_pass_thru(&adapter->hw);
 
 	/* initialize eeprom parameters */
-
 	if (e1000_init_eeprom_params(&adapter->hw)) {
 		E1000_ERR("EEPROM initialization failed\n");
 		goto err_eeprom;
@@ -979,23 +1041,29 @@ e1000_probe(struct pci_dev *pdev,
 	e1000_reset_hw(&adapter->hw);
 
 	/* make sure the EEPROM is good */
-
 	if (e1000_validate_eeprom_checksum(&adapter->hw) < 0) {
 		DPRINTK(PROBE, ERR, "The EEPROM Checksum Is Not Valid\n");
-		goto err_eeprom;
+		e1000_dump_eeprom(adapter);
+		/*
+		 * set MAC address to all zeroes to invalidate and temporary
+		 * disable this device for the user. This blocks regular
+		 * traffic while still permitting ethtool ioctls from reaching
+		 * the hardware as well as allowing the user to run the
+		 * interface after manually setting a hw addr using
+		 * `ip set address`
+		 */
+		memset(adapter->hw.mac_addr, 0, netdev->addr_len);
+	} else {
+		/* copy the MAC address out of the EEPROM */
+		if (e1000_read_mac_addr(&adapter->hw))
+			DPRINTK(PROBE, ERR, "EEPROM Read Error\n");
 	}
-
-	/* copy the MAC address out of the EEPROM */
-
-	if (e1000_read_mac_addr(&adapter->hw))
-		DPRINTK(PROBE, ERR, "EEPROM Read Error\n");
+	/* don't block initalization here due to bad MAC address */
 	memcpy(netdev->dev_addr, adapter->hw.mac_addr, netdev->addr_len);
 	memcpy(netdev->perm_addr, adapter->hw.mac_addr, netdev->addr_len);
 
-	if (!is_valid_ether_addr(netdev->perm_addr)) {
+	if (!is_valid_ether_addr(netdev->perm_addr))
 		DPRINTK(PROBE, ERR, "Invalid MAC Address\n");
-		goto err_eeprom;
-	}
 
 	e1000_get_bus_info(&adapter->hw);
 


^ permalink raw reply related

* Re: [patch 04/10] ucc_geth-fix-build-break-introduced-by-commit-09f75cd7bf13720738e6a196cc0107ce9a5bd5a0-checkpatch-fixes
From: Andrew Morton @ 2007-12-14 23:27 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, Emilian.Medve, davem, galak, leoli, paulus
In-Reply-To: <4762F218.7040904@garzik.org>

On Fri, 14 Dec 2007 16:14:00 -0500
Jeff Garzik <jeff@garzik.org> wrote:

> akpm@linux-foundation.org wrote:
> > From: Andrew Morton <akpm@linux-foundation.org>
> > 
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Emil Medve <Emilian.Medve@Freescale.com>
> > Cc: Jeff Garzik <jeff@garzik.org>
> > Cc: Kumar Gala <galak@gate.crashing.org>
> > Cc: Li Yang <leoli@freescale.com>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> > 
> >  drivers/net/ucc_geth.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff -puN drivers/net/ucc_geth.c~ucc_geth-fix-build-break-introduced-by-commit-09f75cd7bf13720738e6a196cc0107ce9a5bd5a0-checkpatch-fixes drivers/net/ucc_geth.c
> > --- a/drivers/net/ucc_geth.c~ucc_geth-fix-build-break-introduced-by-commit-09f75cd7bf13720738e6a196cc0107ce9a5bd5a0-checkpatch-fixes
> > +++ a/drivers/net/ucc_geth.c
> > @@ -3447,7 +3447,7 @@ static int ucc_geth_rx(struct ucc_geth_p
> >  	u16 length, howmany = 0;
> >  	u32 bd_status;
> >  	u8 *bdBuffer;
> > -	struct net_device * dev;
> > +	struct net_device *dev;
> >  
> >  	ugeth_vdbg("%s: IN", __FUNCTION__);
> 
> applied this crucial fix to #upstream-fixes with a suitable changelog
> 

heh.  I meant to drop that - the patch machine gets unreliable when it
needs human intervention.  

^ permalink raw reply

* Re: [patch 01/10] e1000e: make E1000E default to the same kconfig setting as E1000
From: Andrew Morton @ 2007-12-14 23:22 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, randy.dunlap, auke-jan.h.kok, linux-kernel
In-Reply-To: <4762E9FE.1070707@garzik.org>

On Fri, 14 Dec 2007 15:39:26 -0500
Jeff Garzik <jeff@garzik.org> wrote:

> akpm@linux-foundation.org wrote:
> > From: Randy Dunlap <randy.dunlap@oracle.com>
> > 
> > Make E1000E default to the same kconfig setting as E1000.  So people's
> > machiens don't stop working when they use oldconfig.
> > 
> > Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> > Cc: Jeff Garzik <jeff@garzik.org>
> > Cc: Auke Kok <auke-jan.h.kok@intel.com>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> > 
> >  drivers/net/Kconfig |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff -puN drivers/net/Kconfig~e1000e-make-e1000e-default-to-the-same-kconfig-setting-as-e1000 drivers/net/Kconfig
> > --- a/drivers/net/Kconfig~e1000e-make-e1000e-default-to-the-same-kconfig-setting-as-e1000
> > +++ a/drivers/net/Kconfig
> > @@ -1986,6 +1986,7 @@ config E1000_DISABLE_PACKET_SPLIT
> >  config E1000E
> >  	tristate "Intel(R) PRO/1000 PCI-Express Gigabit Ethernet support"
> >  	depends on PCI
> > +	default E1000
> 
> I am not inclined to apply this one.  This practice, applied over time, 
> will tend to accumulate weird 'default' and 'select' statements.
> 
> So I think the breakage that occurs is mitigated by two factors:
> 1) kernel hackers that do their own configs are expected to be able to 
> figure this stuff.
> 2) kernel builders (read: distros, mainly) are expected to have put 
> thought into the Kconfig selection and driver migration strategies.
> 
> PCI IDs move across drivers from time, and we don't want to apply these 
> sorts changes:  Viewed in the long term, the suggested patch is merely a 
> temporary change to allow kernel experts to more easily deal with the 
> PCI ID migration across drivers.
> 
> I would prefer simply to communicate to kernel experts and builders 
> about a Kconfig issue that could potentially their booting/networking... 
>   because this patch is only needed if the kernel experts do not already 
> know about a necessary config update.

You can take it out again later on - most people's .configs will then have
E1000E set.   People who still do `cp ancientconfig .config ; make oldconfig'
remain screwed.

I dunno.  I guess I'm not into causing people pain in an attempt to train
them to do what we want.  This is a popular driver and a *lot* of people
are going to:

- build new kernel

- install new kernel

- find it doesn't work, go through quite large amounts of hassle trying
  to work out why it stopped working.  Eventually work out that e1000
  stopped working.  Eventually work out that it stopped working because we
  forcibly switched them to a new driver which they didn't know about.

- reconfigure kernel

- rebuild, reinstall

Multiply that by 100s of people (at least).  All because Jeff wouldn't
apply a one-liner?


^ permalink raw reply

* Re: [patch 01/10] e1000e: make E1000E default to the same kconfig setting as E1000
From: Jeff Garzik @ 2007-12-14 23:17 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: akpm, netdev, randy.dunlap, auke-jan.h.kok,
	Linux Kernel Mailing List
In-Reply-To: <20071214223947.GA5403@stusta.de>

Adrian Bunk wrote:
> On Fri, Dec 14, 2007 at 03:39:26PM -0500, Jeff Garzik wrote:
>> akpm@linux-foundation.org wrote:
>>> From: Randy Dunlap <randy.dunlap@oracle.com>
>> ...
>> So I think the breakage that occurs is mitigated by two factors:
>> 1) kernel hackers that do their own configs are expected to be able to 
>> figure this stuff.
>> 2) kernel builders (read: distros, mainly) are expected to have put thought 
>> into the Kconfig selection and driver migration strategies.
>> ...
>> I would prefer simply to communicate to kernel experts and builders about a 
>> Kconfig issue that could potentially their booting/networking...  because 
>> this patch is only needed if the kernel experts do not already know about a 
>> necessary config update.
> 
> You miss the vast majority of kconfig users:
> 
> 3) system administrators etc. who for different reasons compile their 
> own kernels but neither are nor want to be kernel developers
> 
> There's a reason why e.g. LPI requires you to be able to compile your 
> own kernel even for getting a "Junior Level Linux Professional" 
> certificate.

Great!


> Or that one of the authors of "Linux Device drivers" has written a book 
> covering only how to build and run your own kernel.

Nonetheless, it will always be true that configuring your own kernel 
requires knowledge of the options you are setting.

	Jeff




^ permalink raw reply

* Re: [patch 2/4] net: use mutex_is_locked() for ASSERT_RTNL()
From: Andrew Morton @ 2007-12-14 23:11 UTC (permalink / raw)
  To: David Miller; +Cc: herbert, netdev
In-Reply-To: <20071214.111514.03773174.davem@davemloft.net>

On Fri, 14 Dec 2007 11:15:14 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Fri, 14 Dec 2007 16:30:37 +0800
> 
> > On Fri, Dec 14, 2007 at 12:22:09AM -0800, Andrew Morton wrote:
> > >
> > > I don't see how it could warn about that.  Nor should it - one might want
> > > to check that rtnl_lock is held inside preempt_disable() or spin_lock or
> > > whatever.
> > > 
> > > It might make sense to warn if ASSERT_RTNL is called in in_interrupt()
> > > contexts though.
> > 
> > Well the paths where ASSERT_RTNL is used should never be in an
> > atomic context.  In the past it has been quite useful in pointing
> > out bogus locking practices.
> > 
> > There is currently one path where it's known to warn because of
> > this and it (promiscuous mode) is on my todo list.
> > 
> > Oh and it only warns when you have mutex debugging enabled.
> 
> Right, this change is just totally bogus.
> 
> I'm all for using existing facilities to replace hand-crafted copies,
> but this case is removing useful debugging functionality so it's
> wrong.

I don't believe that ASSERT_RTNL() presently warns when called from atomic
contexts.  If it does then I missed it.


^ permalink raw reply

* Re: [PATCH 2/2] cxgb3 - Parity initialization for T3C adapters
From: Divy Le Ray @ 2007-12-14 23:05 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, linux-kernel, Steve Wise, wenxiong
In-Reply-To: <4759E200.3050607@chelsio.com>

Divy Le Ray wrote:
> Jeff Garzik wrote:
>>
>> Divy Le Ray wrote:
>> > Jeff Garzik wrote:
>> >> Divy Le Ray wrote:
>> >>> From: Divy Le Ray <divy@chelsio.com>
>> >>>
>> >>> Add parity initialization for T3C adapters.
>> >>>
>> >>> Signed-off-by: Divy Le Ray <divy@chelsio.com>
>> >>> ---
>> >>>
>> >>>  drivers/net/cxgb3/adapter.h       |    1
>> >>>  drivers/net/cxgb3/cxgb3_main.c    |   82 ++++++++++++
>> >>>  drivers/net/cxgb3/cxgb3_offload.c |   15 ++
>> >>>  drivers/net/cxgb3/regs.h          |  248
>> >>> +++++++++++++++++++++++++++++++++++++
>> >>>  drivers/net/cxgb3/sge.c           |   24 +++-
>> >>>  drivers/net/cxgb3/t3_hw.c         |  131 +++++++++++++++++---
>> >>>  6 files changed, 472 insertions(+), 29 deletions(-)
>> >>
>> >> dropped patches 2-3, did not apply
>> >>
>> >>
>> >
>> > Hi Jeff,
>> >
>> > I noticed that you applied the first one of this 3 patches series 
>> to the
>> > #upstream-fixes branch.
>> > These patches are intended to the #upstream (2.6.25) branch, as 
>> they are
>> > built on top of the
>> > last 10 patches committed - 9 from me, and the white space clean up
>> > (thanks!).
>> > May be this is the reason why they did not apply.
>>
>> Ah... you need to tell me these things.  I looked for a kernel version
>> in your messages but did not see one.
>>
> I had put it in the introduction mail, I should have added the kernel 
> version in the patch titles.
> I'll do from now on.
>
>>
>> Does the patch #1 need to be reverted for 2.6.24?
>>
> No, it can be applied to 2.6.24.
> The 2 next patches seem to apply cleanly on #upstream when patch #1 is 
> popped out the patch stack.
>
Hi Jeff,

Did you get a chance to apply these patches #upstream ?

Cheers,
Divy



^ permalink raw reply

* [PATCH] [ROSE] ax25_send_frame() called with a constant paclen = 260
From: Bernard Pidoux @ 2007-12-14 22:49 UTC (permalink / raw)
  To: David S. Miller; +Cc: Ralf Baechle DL5RB, Linux Netdev List

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

Hi,

In rose_link.c ax25_send_frame() was called with a constant paclen 
parameter of 260 bytes.
This value looked odd to me for it did not correspond to any defined or 
possible computed length.Replacing this value by 0 (zero) allowed 
ax25_send_frame() to substitute it by the default AX25 frame size, which 
in turn induced significant results on the AX25 frame fragmentation and 
removed some garbage trailing characters in AX25 frames sent.


signed off by Bernard Pidoux, f6bvp@amsat.org

[-- Attachment #2: rose-2.6.24-rc5.patch6 --]
[-- Type: text/plain, Size: 438 bytes --]

--- linux-2.6.24-rc5/net/rose/rose_link.c	2007-12-11 04:48:43.000000000 +0100
+++ b/net/rose/rose_link.c	2007-12-14 14:39:23.000000000 +0100
@@ -107,7 +107,7 @@
 	else
 		rose_call = &rose_callsign;
 
-	neigh->ax25 = ax25_send_frame(skb, 260, rose_call, &neigh->callsign, neigh->digipeat, neigh->dev);
+	neigh->ax25 = ax25_send_frame(skb, 0, rose_call, &neigh->callsign, neigh->digipeat, neigh->dev);
 
 	return (neigh->ax25 != NULL);
 }

^ permalink raw reply

* [PATCH] [ROSE] reverts commits d85838c55d836c33077344fab424f200f2827d84
From: Bernard Pidoux @ 2007-12-14 21:58 UTC (permalink / raw)
  To: David Miller; +Cc: adobriyan, ralf, netdev, Jarek Poplawski
In-Reply-To: <20071007.234433.21388460.davem@davemloft.net>

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

Hi,

This patch against 2.6.24-rc5 reverts commit 
d85838c55d836c33077344fab424f200f2827d84,
that was supposed to detect all connected ROSE node neighbors.
Actually the loop was misplaced and the node list was not completely
explored.Next sublitted patch gives a solution.

signed off by Bernard Pidoux, f6bvp@amsat.org



[-- Attachment #2: rose-2.6.24-rc5.patch1 --]
[-- Type: text/plain, Size: 738 bytes --]

--- linux-2.6.24-rc5/net/rose/af_rose.c	2007-12-11 04:48:43.000000000 +0100
+++ b/net/rose/af_rose.c	2007-12-14 14:09:40.000000000 +0100
@@ -782,7 +782,6 @@
 
 		rose_insert_socket(sk);		/* Finish the bind */
 	}
-rose_try_next_neigh:
 	rose->dest_addr   = addr->srose_addr;
 	rose->dest_call   = addr->srose_call;
 	rose->rand        = ((long)rose & 0xFFFF) + rose->lci;
@@ -844,12 +843,6 @@
 	}
 
 	if (sk->sk_state != TCP_ESTABLISHED) {
-	/* Try next neighbour */
-		rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause, &diagnostic);
-		if (rose->neighbour)
-			goto rose_try_next_neigh;
-
-		/* No more neighbours */
 		sock->state = SS_UNCONNECTED;
 		err = sock_error(sk);	/* Always set at this point */
 		goto out_release;

^ permalink raw reply

* [PATCH] [ROSE] static const struct proto_ops in af_rose.c
From: Bernard Pidoux @ 2007-12-14 22:33 UTC (permalink / raw)
  To: David Miller; +Cc: Ralf Baechle DL5RB, Linux Netdev List

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

This patch corrects the declaration of struct proto_ops as static const 
like in struct socket <linux/net.h>

signed off by Bernard Pidoux, f6bvp@amsat.org

[-- Attachment #2: rose-2.6.24-rc5.patch4 --]
[-- Type: text/plain, Size: 571 bytes --]

--- linux-2.6.24-rc5/net/rose/af_rose.c	2007-12-11 04:48:43.000000000 +0100
+++ b/net/rose/af_rose.c	2007-12-14 14:32:06.000000000 +0100
@@ -63,7 +63,7 @@
 static HLIST_HEAD(rose_list);
 static DEFINE_SPINLOCK(rose_list_lock);
 
-static struct proto_ops rose_proto_ops;
+static const struct proto_ops rose_proto_ops;
 
 ax25_address rose_callsign;
 
@@ -1483,7 +1483,7 @@
 	.owner		=	THIS_MODULE,
 };
 
-static struct proto_ops rose_proto_ops = {
+static const struct proto_ops rose_proto_ops = {
 	.family		=	PF_ROSE,
 	.owner		=	THIS_MODULE,
 	.release	=	rose_release,

^ permalink raw reply

* [PATCH] [ROSE] ROSE_PACLEN  definition used a number constant value
From: Bernard Pidoux @ 2007-12-14 22:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: Ralf Baechle DL5RB, Linux Netdev List

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

Hi,

ROSE_PACLEN was defined by a substraction using a fixed constant value 
of 256 in af_rose.c

#define ROSE_PACLEN (256-ROSE_MIN_LEN)

For more convenience 256 is replaced by AX25_DEF_PACLEN default size 
declared in ax25.h which is the actual AX25 MTU.


signed off by Bernard Pidoux, f6bvp@amsat.org


[-- Attachment #2: rose-2.6.24-rc5.patch5 --]
[-- Type: text/plain, Size: 384 bytes --]

--- linux-2.6.24-rc5/net/rose/af_rose.c	2007-12-11 04:48:43.000000000 +0100
+++ b/net/rose/af_rose.c	2007-12-14 14:35:16.000000000 +0100
@@ -1148,7 +1148,7 @@
 	}
 
 #ifdef M_BIT
-#define ROSE_PACLEN (256-ROSE_MIN_LEN)
+#define ROSE_PACLEN (AX25_DEF_PACLEN-ROSE_MIN_LEN)
 	if (skb->len - ROSE_MIN_LEN > ROSE_PACLEN) {
 		unsigned char header[ROSE_MIN_LEN];
 		struct sk_buff *skbn;
 

^ permalink raw reply

* [PATCH] [AX25] circular locking with AX25 connection timeout
From: Bernard Pidoux @ 2007-12-14 22:29 UTC (permalink / raw)
  To: David Miller; +Cc: Jarek Poplawski, Ralf Baechle DL5RB, Linux Netdev List

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

Hi,

This patch cancels a circular locking conflict that appeared with a 
timeout of an AX25 connection.

signed off by Jarek Poplawski

[-- Attachment #2: rose-2.6.24-rc5.patch3 --]
[-- Type: text/plain, Size: 472 bytes --]

--- linux-2.6.24-rc5/net/ax25/ax25_subr.c	2007-12-11 04:48:43.000000000 +0100
+++ b/net/ax25/ax25_subr.c	2007-12-14 14:30:02.000000000 +0100
@@ -279,6 +279,7 @@
 	ax25_link_failed(ax25, reason);
 
 	if (ax25->sk != NULL) {
+		local_bh_disable();
 		bh_lock_sock(ax25->sk);
 		ax25->sk->sk_state     = TCP_CLOSE;
 		ax25->sk->sk_err       = reason;
@@ -288,5 +289,6 @@
 			sock_set_flag(ax25->sk, SOCK_DEAD);
 		}
 		bh_unlock_sock(ax25->sk);
+		local_bh_enable(); 
 	}
 }

^ permalink raw reply

* [PATCH] [ROSE] finding a connected ROSE neighbor node
From: Bernard Pidoux @ 2007-12-14 22:23 UTC (permalink / raw)
  To: David Miller; +Cc: Alexey Dobriyan, Ralf Baechle DL5RB, Linux Netdev List

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

Hi,

This patch introduces a test to determine if a neighbor node with a 
given ROSE address is already connected and if so returns its address.
The previous test was only performed on timer t0 and not on parameter 
restarted that is true when a node is connected.
Also, previously the test was not performed on the whole node list.
A new function __rose_get_neigh() is introduced to cure a spin lock 
conflict related to rose_node_list_lock.

signed off by Alexey Dobriyan,adobriyan@gmail.com
signed off by Bernard Pidoux, f6bvp @ amsat.org



[-- Attachment #2: rose-2.6.24-rc5.patch2 --]
[-- Type: text/plain, Size: 2755 bytes --]

--- linux-2.6.24-rc5/include/net/rose.h	2007-12-11 04:48:43.000000000 +0100
+++ b/include/net/rose.h	2007-12-14 14:25:02.000000000 +0100
@@ -202,6 +202,7 @@
 extern struct net_device *rose_dev_get(rose_address *);
 extern struct rose_route *rose_route_free_lci(unsigned int, struct rose_neigh *);
 extern struct rose_neigh *rose_get_neigh(rose_address *, unsigned char *, unsigned char *);
+extern struct rose_neigh *__rose_get_neigh(rose_address *, unsigned char *, unsigned char *);
 extern int  rose_rt_ioctl(unsigned int, void __user *);
 extern void rose_link_failed(ax25_cb *, int);
 extern int  rose_route_frame(struct sk_buff *, ax25_cb *);
--- linux-2.6.24-rc5/net/rose/rose_route.c	2007-12-11 04:48:43.000000000 +0100
+++ b/net/rose/rose_route.c	2007-12-14 14:25:02.000000000 +0100
@@ -664,25 +664,22 @@
 /*
  *	Find a neighbour given a ROSE address.
  */
-struct rose_neigh *rose_get_neigh(rose_address *addr, unsigned char *cause,
+struct rose_neigh *__rose_get_neigh(rose_address *addr, unsigned char *cause,
 	unsigned char *diagnostic)
 {
-	struct rose_neigh *res = NULL;
 	struct rose_node *node;
 	int failed = 0;
 	int i;
 
-	spin_lock_bh(&rose_node_list_lock);
 	for (node = rose_node_list; node != NULL; node = node->next) {
 		if (rosecmpm(addr, &node->address, node->mask) == 0) {
 			for (i = 0; i < node->count; i++) {
-				if (!rose_ftimer_running(node->neighbour[i])) {
-					res = node->neighbour[i];
-					goto out;
-				} else
-					failed = 1;
+				if (node->neighbour[i]->restarted)
+					return node->neighbour[i];
+				if (!rose_ftimer_running(node->neighbour[i]))			
+					return node->neighbour[i];
+				failed = 1;
 			}
-			break;
 		}
 	}
 
@@ -694,7 +691,16 @@
 		*diagnostic = 0;
 	}
 
-out:
+	return NULL;
+}
+
+struct rose_neigh *rose_get_neigh(rose_address *addr, unsigned char *cause,
+	unsigned char *diagnostic)
+{
+	struct rose_neigh *res;
+
+	spin_lock_bh(&rose_node_list_lock);
+	res = __rose_get_neigh(addr, cause, diagnostic);
 	spin_unlock_bh(&rose_node_list_lock);
 
 	return res;
@@ -1019,7 +1025,7 @@
 		rose_route = rose_route->next;
 	}
 
-	if ((new_neigh = rose_get_neigh(dest_addr, &cause, &diagnostic)) == NULL) {
+	if ((new_neigh = __rose_get_neigh(dest_addr, &cause, &diagnostic)) == NULL) {
 		rose_transmit_clear_request(rose_neigh, lci, cause, diagnostic);
 		goto out;
 	}
--- linux-2.6.24-rc5/net/rose/af_rose.c	2007-12-11 04:48:43.000000000 +0100
+++ b/net/rose/af_rose.c	2007-12-14 14:25:02.000000000 +0100
@@ -750,7 +750,7 @@
 	sk->sk_state   = TCP_CLOSE;
 	sock->state = SS_UNCONNECTED;
 
-	rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause,
+	rose->neighbour = __rose_get_neigh(&addr->srose_addr, &cause,
 					 &diagnostic);
 	if (!rose->neighbour)
 		return -ENETUNREACH;

^ permalink raw reply

* Re: [PATCH 03/29] mm: slb: add knowledge of reserve pages
From: Daniel Phillips @ 2007-12-14 22:51 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
	trond.myklebust
In-Reply-To: <20071214154439.242195000@chello.nl>

On Friday 14 December 2007 07:39, Peter Zijlstra wrote:
> Restrict objects from reserve slabs (ALLOC_NO_WATERMARKS) to
> allocation contexts that are entitled to it. This is done to ensure
> reserve pages don't leak out and get consumed.

Tighter definitions of "leak out" and "get consumed" would be helpful 
here.  As I see it, the chain of reasoning is:

  * Any MEMALLOC mode allocation must have come from a properly
   throttled path and  has a finite lifetime that must eventually
   produce writeout  progress.

  * Since the transaction that made the allocation was throttled and
    must have a finite lifetime, we  know that it must eventually return
    the resources it consumed to the appropriate resource pool.

Now, I think what you mean by "get consumed" and "leak out" is: "become 
pinned by false sharing with other allocations that do not guarantee 
that they will be returned to the resource pool".  We can say "pinned" 
for short.

So you are attempting to prevent slab pages from becoming pinned by 
users that do not obey the reserve management rules, which I think your 
approach achieves.  However...

Note that false sharing of slab pages is still possible between two 
unrelated writeout processes, both of which obey rules for their own 
writeout path, but the pinned combination does not.  This still leaves 
a hole through which a deadlock may slip.

My original solution was simply to allocate a full page when drawing 
from the memaloc reserve, which may use a tad more reserve, but makes 
it possible to prove the algorithm correct.

Regards,

Daniel

^ permalink raw reply

* Re: Packet per Second
From: Glen Turner @ 2007-12-14 22:31 UTC (permalink / raw)
  To: Flávio Pires; +Cc: netdev, bridge
In-Reply-To: <nemoFri121407105527@flis.man.torun.pl>


On Fri, 2007-12-14 at 15:34 +0000, Flávio Pires wrote:

> Well, I work on an ISP and we have a linux box acting as a
> bridge+firewall. With this bridge+firewall we control the packet rate
> per second from each client and from our repeaters. But I can`t
> measure the packet rate per IP. Is there any tool for this?

The usual approach is to generate NetFlow records -- there are
a number of Linux tools for this. Collect them with a collector
(flow-tools being a common choice). Then have a Perl script
which reads the flow records, processes them whichever way you
desire, and drops the result into a rrdtool file (there are modules
for both reading the flow-tools data and outputing in the rrdtool
format). The rrdtool utilities have a limited range of graphs,
but there is a huge selection of graphing packages from other
authors for rrdtool-stored data (Drraw, etc).  Flow-tools also
has some third-party analysis tools, some of those have good
"top talker" statistics.

This is a lot of work, since you are really putting a complete
measurement infrastructure in place to get the one statistic
you desire.  But I'd encourage you to do that, since knowing
one statistic usually leads to further questions of the data.

-- 
Glen Turner, Senior Network Engineer
Australia's Academic & Research Network    www.aarnet.edu.au


^ permalink raw reply

* Re: [Bugme-new] [Bug 9543] New: RTNL: assertion failed at net/ipv6/addrconf.c (2164)/RTNL: assertion failed at net/ipv4/devinet.c (1055)
From: Andy Gospodarek @ 2007-12-14 22:47 UTC (permalink / raw)
  To: Krzysztof Oledzki
  Cc: Andy Gospodarek, Jay Vosburgh, Herbert Xu, Andrew Morton,
	bugme-daemon, shemminger, davem, netdev
In-Reply-To: <Pine.LNX.4.64.0712141955080.12202@bizon.gios.gov.pl>

On Fri, Dec 14, 2007 at 07:57:42PM +0100, Krzysztof Oledzki wrote:
> 
> 
> On Fri, 14 Dec 2007, Andy Gospodarek wrote:
> 
> >On Fri, Dec 14, 2007 at 05:14:57PM +0100, Krzysztof Oledzki wrote:
> >>
> >>
> >>On Wed, 12 Dec 2007, Jay Vosburgh wrote:
> >>
> >>>Herbert Xu <herbert@gondor.apana.org.au> wrote:
> >>>
> >>>>>diff -puN drivers/net/bonding/bond_sysfs.c~bonding-locking-fix
> >>>>>drivers/net/bonding/bond_sysfs.c
> >>>>>--- a/drivers/net/bonding/bond_sysfs.c~bonding-locking-fix
> >>>>>+++ a/drivers/net/bonding/bond_sysfs.c
> >>>>>@@ -1111,8 +1111,6 @@ static ssize_t bonding_store_primary(str
> >>>>>out:
> >>>>>      write_unlock_bh(&bond->lock);
> >>>>>
> >>>>>-       rtnl_unlock();
> >>>>>-
> >>>>
> >>>>Looking at the changeset that added this perhaps the intention
> >>>>is to hold the lock? If so we should add an rtnl_lock to the start
> >>>>of the function.
> >>>
> >>>	Yes, this function needs to hold locks, and more than just
> >>>what's there now.  I believe the following should be correct; I haven't
> >>>tested it, though (I'm supposedly on vacation right now).
> >>>
> >>>	The following change should be correct for the
> >>>bonding_store_primary case discussed in this thread, and also corrects
> >>>the bonding_store_active case which performs similar functions.
> >>>
> >>>	The bond_change_active_slave and bond_select_active_slave
> >>>functions both require rtnl, bond->lock for read and curr_slave_lock for
> >>>write_bh, and no other locks.  This is so that the lower level
> >>>mode-specific functions can release locks down to just rtnl in order to
> >>>call, e.g., dev_set_mac_address with the locks it expects (rtnl only).
> >>>
> >>>Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
> >>>
> >>>diff --git a/drivers/net/bonding/bond_sysfs.c
> >>>b/drivers/net/bonding/bond_sysfs.c
> >>>index 11b76b3..28a2d80 100644
> >>>--- a/drivers/net/bonding/bond_sysfs.c
> >>>+++ b/drivers/net/bonding/bond_sysfs.c
> >>>@@ -1075,7 +1075,10 @@ static ssize_t bonding_store_primary(struct device
> >>>*d,
> >>>	struct slave *slave;
> >>>	struct bonding *bond = to_bond(d);
> >>>
> >>>-	write_lock_bh(&bond->lock);
> >>>+	rtnl_lock();
> >>>+	read_lock(&bond->lock);
> >>>+	write_lock_bh(&bond->curr_slave_lock);
> >>>+
> >>>	if (!USES_PRIMARY(bond->params.mode)) {
> >>>		printk(KERN_INFO DRV_NAME
> >>>		       ": %s: Unable to set primary slave; %s is in mode
> >>>		       %d\n",
> >>>@@ -1109,8 +1112,8 @@ static ssize_t bonding_store_primary(struct device
> >>>*d,
> >>>		}
> >>>	}
> >>>out:
> >>>-	write_unlock_bh(&bond->lock);
> >>>-
> >>>+	write_unlock_bh(&bond->curr_slave_lock);
> >>>+	read_unlock(&bond->lock);
> >>>	rtnl_unlock();
> >>>
> >>>	return count;
> >>>@@ -1190,7 +1193,8 @@ static ssize_t bonding_store_active_slave(struct
> >>>device *d,
> >>>	struct bonding *bond = to_bond(d);
> >>>
> >>>	rtnl_lock();
> >>>-	write_lock_bh(&bond->lock);
> >>>+	read_lock(&bond->lock);
> >>>+	write_lock_bh(&bond->curr_slave_lock);
> >>>
> >>>	if (!USES_PRIMARY(bond->params.mode)) {
> >>>		printk(KERN_INFO DRV_NAME
> >>>@@ -1247,7 +1251,8 @@ static ssize_t bonding_store_active_slave(struct
> >>>device *d,
> >>>		}
> >>>	}
> >>>out:
> >>>-	write_unlock_bh(&bond->lock);
> >>>+	write_unlock_bh(&bond->curr_slave_lock);
> >>>+	read_unlock(&bond->lock);
> >>>	rtnl_unlock();
> >>>
> >>>	return count;
> >>
> >>Vanilla 2.6.24-rc5 plus this patch:
> >>
> >>=========================================================
> >>[ INFO: possible irq lock inversion dependency detected ]
> >>2.6.24-rc5 #1
> >>---------------------------------------------------------
> >>events/0/9 just changed the state of lock:
> >> (&mc->mca_lock){-+..}, at: [<c0411c7a>] mld_ifc_timer_expire+0x130/0x1fb
> >>but this lock took another, soft-read-irq-unsafe lock in the past:
> >> (&bond->lock){-.--}
> >>
> >>and interrupts could create inverse lock ordering between them.
> >>
> >>
> >
> >Grrr, I should have seen that -- sorry.  Try your luck with this instead:
> <CUT>
> 
> No luck.
> 


I'm guessing if we go back to using a write-lock for bond->lock this
will go back to working again, but I'm not totally convinced since there
are plenty of places where we used a read-lock with it.


diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 11b76b3..635b857 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1075,7 +1075,10 @@ static ssize_t bonding_store_primary(struct device *d,
 	struct slave *slave;
 	struct bonding *bond = to_bond(d);
 
+	rtnl_lock();
 	write_lock_bh(&bond->lock);
+	write_lock_bh(&bond->curr_slave_lock);
+
 	if (!USES_PRIMARY(bond->params.mode)) {
 		printk(KERN_INFO DRV_NAME
 		       ": %s: Unable to set primary slave; %s is in mode %d\n",
@@ -1109,8 +1112,8 @@ static ssize_t bonding_store_primary(struct device *d,
 		}
 	}
 out:
+	write_unlock_bh(&bond->curr_slave_lock);
 	write_unlock_bh(&bond->lock);
-
 	rtnl_unlock();
 
 	return count;
@@ -1191,6 +1194,7 @@ static ssize_t bonding_store_active_slave(struct device *d,
 
 	rtnl_lock();
 	write_lock_bh(&bond->lock);
+	write_lock_bh(&bond->curr_slave_lock);
 
 	if (!USES_PRIMARY(bond->params.mode)) {
 		printk(KERN_INFO DRV_NAME
@@ -1247,6 +1251,7 @@ static ssize_t bonding_store_active_slave(struct device *d,
 		}
 	}
 out:
+	write_unlock_bh(&bond->curr_slave_lock);
 	write_unlock_bh(&bond->lock);
 	rtnl_unlock();
 

^ permalink raw reply related

* Re: [patch 01/10] e1000e: make E1000E default to the same kconfig setting as E1000
From: Adrian Bunk @ 2007-12-14 22:39 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: akpm, netdev, randy.dunlap, auke-jan.h.kok,
	Linux Kernel Mailing List
In-Reply-To: <4762E9FE.1070707@garzik.org>

On Fri, Dec 14, 2007 at 03:39:26PM -0500, Jeff Garzik wrote:
> akpm@linux-foundation.org wrote:
>> From: Randy Dunlap <randy.dunlap@oracle.com>
>...
> So I think the breakage that occurs is mitigated by two factors:
> 1) kernel hackers that do their own configs are expected to be able to 
> figure this stuff.
> 2) kernel builders (read: distros, mainly) are expected to have put thought 
> into the Kconfig selection and driver migration strategies.
>...
> I would prefer simply to communicate to kernel experts and builders about a 
> Kconfig issue that could potentially their booting/networking...  because 
> this patch is only needed if the kernel experts do not already know about a 
> necessary config update.

You miss the vast majority of kconfig users:

3) system administrators etc. who for different reasons compile their 
own kernels but neither are nor want to be kernel developers

There's a reason why e.g. LPI requires you to be able to compile your 
own kernel even for getting a "Junior Level Linux Professional" 
certificate.

Or that one of the authors of "Linux Device drivers" has written a book 
covering only how to build and run your own kernel.

> 	Jeff

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply

* [PATCH 11/11] drivers/net/usb/catc.c: Use print_mac
From: Joe Perches @ 2007-12-14 22:34 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, Jeff Garzik, linux-usb, netdev
In-Reply-To: <9fc484fc034428ab68ecb2711c8456df9e98d56d.1197671367.git.joe@perches.com>


Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/usb/catc.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/catc.c b/drivers/net/usb/catc.c
index 76752d8..e20db50 100644
--- a/drivers/net/usb/catc.c
+++ b/drivers/net/usb/catc.c
@@ -757,6 +757,7 @@ static int catc_probe(struct usb_interface *intf, const struct usb_device_id *id
 	struct catc *catc;
 	u8 broadcast[6];
 	int i, pktsz;
+	DECLARE_MAC_BUF(mac);
 
 	if (usb_set_interface(usbdev,
 			intf->altsetting->desc.bInterfaceNumber, 1)) {
@@ -891,11 +892,10 @@ static int catc_probe(struct usb_interface *intf, const struct usb_device_id *id
 		f5u011_rxmode(catc, catc->rxmode);
 	}
 	dbg("Init done.");
-	printk(KERN_INFO "%s: %s USB Ethernet at usb-%s-%s, ",
+	printk(KERN_INFO "%s: %s USB Ethernet at usb-%s-%s, %s\n",
 	       netdev->name, (catc->is_f5u011) ? "Belkin F5U011" : "CATC EL1210A NetMate",
-	       usbdev->bus->bus_name, usbdev->devpath);
-	for (i = 0; i < 5; i++) printk("%2.2x:", netdev->dev_addr[i]);
-	printk("%2.2x.\n", netdev->dev_addr[i]);
+	       usbdev->bus->bus_name, usbdev->devpath,
+	       print_mac(mac, netdev->dev_addr));
 	usb_set_intfdata(intf, catc);
 
 	SET_NETDEV_DEV(netdev, &intf->dev);
-- 
1.5.3.7.949.g2221a6


^ permalink raw reply related

* [PATCH 10/11] drivers/net/tulip/de4x5.c: Use print_mac
From: Joe Perches @ 2007-12-14 22:34 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jeff Garzik, netdev, tulip-users
In-Reply-To: <9fc484fc034428ab68ecb2711c8456df9e98d56d.1197671367.git.joe@perches.com>


Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/tulip/de4x5.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/tulip/de4x5.c b/drivers/net/tulip/de4x5.c
index 41f34bb..288f994 100644
--- a/drivers/net/tulip/de4x5.c
+++ b/drivers/net/tulip/de4x5.c
@@ -5361,14 +5361,11 @@ de4x5_dbg_open(struct net_device *dev)
 {
     struct de4x5_private *lp = netdev_priv(dev);
     int i;
+    DECLARE_MAC_BUF(mac);
 
     if (de4x5_debug & DEBUG_OPEN) {
 	printk("%s: de4x5 opening with irq %d\n",dev->name,dev->irq);
-	printk("\tphysical address: ");
-	for (i=0;i<6;i++) {
-	    printk("%2.2x:",(short)dev->dev_addr[i]);
-	}
-	printk("\n");
+	printk("\tphysical address: %s\n", print_mac(mac, dev->dev_addr));
 	printk("Descriptor head addresses:\n");
 	printk("\t0x%8.8lx  0x%8.8lx\n",(u_long)lp->rx_ring,(u_long)lp->tx_ring);
 	printk("Descriptor addresses:\nRX: ");
-- 
1.5.3.7.949.g2221a6


^ permalink raw reply related

* [PATCH 09/11] drivers/net/tg3.c: Use print_mac
From: Joe Perches @ 2007-12-14 22:34 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jeff Garzik, Michael Chan, netdev
In-Reply-To: <9fc484fc034428ab68ecb2711c8456df9e98d56d.1197671367.git.joe@perches.com>


Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/tg3.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 4942f7d..eea7da9 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -12351,9 +12351,10 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
 	unsigned long tg3reg_base, tg3reg_len;
 	struct net_device *dev;
 	struct tg3 *tp;
-	int i, err, pm_cap;
+	int err, pm_cap;
 	char str[40];
 	u64 dma_mask, persist_dma_mask;
+	DECLARE_MAC_BUF(mac);
 
 	if (tg3_version_printed++ == 0)
 		printk(KERN_INFO "%s", version);
@@ -12632,7 +12633,8 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
 		goto err_out_apeunmap;
 	}
 
-	printk(KERN_INFO "%s: Tigon3 [partno(%s) rev %04x PHY(%s)] (%s) %s Ethernet ",
+	printk(KERN_INFO "%s: Tigon3 [partno(%s) rev %04x PHY(%s)] "
+	       "(%s) %s Ethernet %s\n",
 	       dev->name,
 	       tp->board_part_number,
 	       tp->pci_chip_rev_id,
@@ -12640,11 +12642,8 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
 	       tg3_bus_string(tp, str),
 	       ((tp->tg3_flags & TG3_FLAG_10_100_ONLY) ? "10/100Base-TX" :
 		((tp->tg3_flags2 & TG3_FLG2_ANY_SERDES) ? "1000Base-SX" :
-		 "10/100/1000Base-T")));
-
-	for (i = 0; i < 6; i++)
-		printk("%2.2x%c", dev->dev_addr[i],
-		       i == 5 ? '\n' : ':');
+		 "10/100/1000Base-T")),
+	       print_mac(mac, dev->dev_addr));
 
 	printk(KERN_INFO "%s: RXcsums[%d] LinkChgREG[%d] "
 	       "MIirq[%d] ASF[%d] WireSpeed[%d] TSOcap[%d]\n",
-- 
1.5.3.7.949.g2221a6


^ permalink raw reply related

* [PATCH 08/11] drivers/net/sunvnet.c: Use print_mac
From: Joe Perches @ 2007-12-14 22:34 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jeff Garzik, netdev
In-Reply-To: <9fc484fc034428ab68ecb2711c8456df9e98d56d.1197671367.git.joe@perches.com>


Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/sunvnet.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/sunvnet.c b/drivers/net/sunvnet.c
index ff1028a..4a0035f 100644
--- a/drivers/net/sunvnet.c
+++ b/drivers/net/sunvnet.c
@@ -1149,6 +1149,7 @@ static int __devinit vnet_port_probe(struct vio_dev *vdev,
 	struct vnet *vp;
 	const u64 *rmac;
 	int len, i, err, switch_port;
+	DECLARE_MAC_BUF(mac);
 
 	print_version();
 
@@ -1213,12 +1214,9 @@ static int __devinit vnet_port_probe(struct vio_dev *vdev,
 
 	dev_set_drvdata(&vdev->dev, port);
 
-	printk(KERN_INFO "%s: PORT ( remote-mac ", vp->dev->name);
-	for (i = 0; i < 6; i++)
-		printk("%2.2x%c", port->raddr[i], i == 5 ? ' ' : ':');
-	if (switch_port)
-		printk("switch-port ");
-	printk(")\n");
+	printk(KERN_INFO "%s: PORT ( remote-mac %s%s )\n",
+	       vp->dev->name, print_mac(mac, port->raddr),
+	       switch_port ? " switch-port" : "");
 
 	vio_port_up(&port->vio);
 
-- 
1.5.3.7.949.g2221a6


^ permalink raw reply related

* [PATCH 07/11] drivers/net/smc911x.c: Use print_mac
From: Joe Perches @ 2007-12-14 22:34 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jeff Garzik, netdev
In-Reply-To: <9fc484fc034428ab68ecb2711c8456df9e98d56d.1197671367.git.joe@perches.com>


Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/smc911x.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
index 76cc1d3..f503b73 100644
--- a/drivers/net/smc911x.c
+++ b/drivers/net/smc911x.c
@@ -2086,10 +2086,9 @@ static int __init smc911x_probe(struct net_device *dev, unsigned long ioaddr)
 					"set using ifconfig\n", dev->name);
 		} else {
 			/* Print the Ethernet address */
-			printk("%s: Ethernet addr: ", dev->name);
-			for (i = 0; i < 5; i++)
-				printk("%2.2x:", dev->dev_addr[i]);
-			printk("%2.2x\n", dev->dev_addr[5]);
+			DECLARE_MAC_BUF(mac);
+			printk("%s: Ethernet addr: %s\n",
+			       dev->name, print_mac(mac, dev->dev_addr));
 		}
 
 		if (lp->phy_type == 0) {
-- 
1.5.3.7.949.g2221a6


^ permalink raw reply related

* [PATCH 05/11] drivers/net/niu.c: Use print_mac
From: Joe Perches @ 2007-12-14 22:34 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jeff Garzik, netdev
In-Reply-To: <9fc484fc034428ab68ecb2711c8456df9e98d56d.1197671367.git.joe@perches.com>


Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/niu.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index abfc61c..8a0f8f8 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -7392,12 +7392,10 @@ static void __devinit niu_assign_netdev_ops(struct net_device *dev)
 static void __devinit niu_device_announce(struct niu *np)
 {
 	struct net_device *dev = np->dev;
-	int i;
+	DECLARE_MAC_BUF(mac);
 
-	pr_info("%s: NIU Ethernet ", dev->name);
-	for (i = 0; i < 6; i++)
-		printk("%2.2x%c", dev->dev_addr[i],
-		       i == 5 ? '\n' : ':');
+	pr_info("%s: NIU Ethernet %s\n",
+		dev->name, print_mac(mac, dev->dev_addr));
 
 	pr_info("%s: Port type[%s] mode[%s:%s] XCVR[%s] phy[%s]\n",
 		dev->name,
-- 
1.5.3.7.949.g2221a6


^ permalink raw reply related

* [PATCH 06/11] drivers/net/pcnet32.c: Use print_mac
From: Joe Perches @ 2007-12-14 22:34 UTC (permalink / raw)
  To: linux-kernel; +Cc: Don Fry, Jeff Garzik, netdev
In-Reply-To: <9fc484fc034428ab68ecb2711c8456df9e98d56d.1197671367.git.joe@perches.com>


Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/pcnet32.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/pcnet32.c b/drivers/net/pcnet32.c
index ff92aca..87edeb6 100644
--- a/drivers/net/pcnet32.c
+++ b/drivers/net/pcnet32.c
@@ -1769,8 +1769,8 @@ pcnet32_probe1(unsigned long ioaddr, int shared, struct pci_dev *pdev)
 		memset(dev->dev_addr, 0, sizeof(dev->dev_addr));
 
 	if (pcnet32_debug & NETIF_MSG_PROBE) {
-		for (i = 0; i < 6; i++)
-			printk(" %2.2x", dev->dev_addr[i]);
+		DECLARE_MAC_BUF(mac);
+		printk(" %s", print_mac(mac, dev->dev_addr));
 
 		/* Version 0x2623 and 0x2624 */
 		if (((chip_version + 1) & 0xfffe) == 0x2624) {
-- 
1.5.3.7.949.g2221a6


^ permalink raw reply related


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