* Re: [linux-usb-devel] [PATCH RFC] ZyDAS ZD1211 USB-WLAN driver
From: David Brownell @ 2006-06-06 5:41 UTC (permalink / raw)
To: linux-usb-devel
Cc: Oliver Neukum, Daniel Drake, netdev, Ulrich Kunitz,
John W. Linville
In-Reply-To: <200606040025.32275.oliver@neukum.org>
On Saturday 03 June 2006 3:25 pm, Oliver Neukum wrote:
> Am Samstag, 3. Juni 2006 21:35 schrieb Daniel Drake:
> > Oliver Neukum wrote:
> > > +static int read_mac_addr(struct zd_chip *chip, u8 *mac_addr)
> > > +{
> > > + static const zd_addr_t addr[2] = { CR_MAC_ADDR_P1, CR_MAC_ADDR_P2 };
> > > + return _read_mac_addr(chip, mac_addr, (const zd_addr_t *)addr);
> > > +}
> > >
> > > Why on the stack?
> >
> > Technically it's not on the stack because it is static const (it goes in
> > rodata), but I don't think that this invalidates your point. What's the
> > alternative? kmalloc and kfree every time?
>
> In this case rodata will work. However, if you ever switch to direct DMA
> it will fail. I really did overlook the const keyword.
On some platforms rodata will work; but it's not guaranteed on any.
See Documentation/DMA-mapping.txt and read the little section at the
top explaining what types of memory can be DMA'd from ... neither stack,
nor BSS, nor data, nor rodata, nor text are OK.
- Dave
^ permalink raw reply
* Re: [PATCH 1/1] LSM-IPsec SELinux Authorize (with minor fix)
From: David Miller @ 2006-06-06 5:49 UTC (permalink / raw)
To: jmorris
Cc: cxzhang, netdev, chrisw, herbert, sds, tjaeger, latten, sergeh,
gcwilson, czhang.us
In-Reply-To: <Pine.LNX.4.64.0606060133220.9787@d.namei>
From: James Morris <jmorris@namei.org>
Date: Tue, 6 Jun 2006 01:37:04 -0400 (EDT)
> On Tue, 6 Jun 2006, Catherine Zhang wrote:
>
> > Minor fix per James' comment.
>
> Can you also add a Signed-off-by line?
>
> I can't recall if you were the original author. If not, we also need a
> From line (per Documentation/SubmittingPatches).
I'll apply this to my net-2.6.18 tree once this is all sorted
out. Thanks everyone.
^ permalink raw reply
* Re: [NET]: Add netif_tx_lock
From: Roland Dreier @ 2006-06-06 6:01 UTC (permalink / raw)
To: David Miller; +Cc: herbert, mchan, jgarzik, netdev
In-Reply-To: <20060605.221018.70218518.davem@davemloft.net>
David> As long as you never take priv->lock while ->xmit_lock is
David> held your patch should be OK.
Duh ... unfortunately priv->lock is taken from interrupt context so
that patch isn't safe. A correct fix would be the following, which
leads to a trivial conversion to using netif_tx_lock().
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 1dae4b2..7a8ca5d 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -821,8 +821,8 @@ void ipoib_mcast_restart_task(void *dev_
ipoib_mcast_stop_thread(dev, 0);
- spin_lock_irqsave(&dev->xmit_lock, flags);
- spin_lock(&priv->lock);
+ spin_lock_bh(&dev->xmit_lock);
+ spin_lock_irqsave(&priv->lock, flags);
/*
* Unfortunately, the networking core only gives us a list of all of
@@ -895,8 +895,8 @@ void ipoib_mcast_restart_task(void *dev_
}
}
- spin_unlock(&priv->lock);
- spin_unlock_irqrestore(&dev->xmit_lock, flags);
+ spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_bh(&dev->xmit_lock);
/* We have to cancel outside of the spinlock */
list_for_each_entry_safe(mcast, tmcast, &remove_list, list) {
^ permalink raw reply related
* Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device
From: Greg KH @ 2006-06-06 6:50 UTC (permalink / raw)
To: Jeff Garzik
Cc: Greg KH, Jiri Slaby, Linux Kernel Mailing List, linux-pci,
jgarzik, netdev, mb, st3, linville
In-Reply-To: <20060606011818.GA4135@havoc.gtf.org>
On Mon, Jun 05, 2006 at 09:18:18PM -0400, Jeff Garzik wrote:
> On Mon, Jun 05, 2006 at 01:53:09PM -0700, Greg KH wrote:
> > Why not just use the proper pci interface? Why poke around in another
> > pci device to steal an irq, when that irq might not even be valid?
> > (irqs are not valid until pci_enable_device() is called on them...)
>
> Answered this question the last time you asked.
>
> Answer: this is an embedded platform that needs such poking. The
> wireless device is _another_ device.
Ugh, sorry, too many patches, too many different threads... You are
right...
greg k-h
^ permalink raw reply
* Re: NETDEV WATCHDOG: eth2: transmit timed out with 3c905C-TX
From: Marco Berizzi @ 2006-06-06 9:12 UTC (permalink / raw)
To: klassert; +Cc: netdev
In-Reply-To: <20060523205842.GC18008@bayes.mathematik.tu-chemnitz.de>
Steffen Klassert wrote:
>On Tue, May 23, 2006 at 03:36:35PM +0200, Marco Berizzi wrote:
> > Steffen Klassert wrote:
> >
> > >On Wed, Apr 05, 2006 at 06:33:18PM +0200, Marco Berizzi wrote:
> > >> Hello everybody.
> > >> I'm getting these errors (with packet/connectivity loss) on
> > >> our firewall after I have plugged in a 3c905C nic. Linux is
> > >> Slackware 10.2 with vanilla 2.6.16.1.
> > >>
> > >> Hints?
> > >>
> > >> PS: I have temporary resolved the problem running 'ifconfig
> > >> eth2 down' and 'ifconfig eth2 up'
> > >>
> > >> Apr 5 17:47:07 Teti kernel: eth2: Resetting the Tx ring pointer.
> > >> Apr 5 17:47:47 Teti last message repeated 4 times
> > >> Apr 5 17:48:57 Teti last message repeated 7 times
> > >> Apr 5 17:49:57 Teti last message repeated 6 times
> > >> Apr 5 17:50:57 Teti last message repeated 6 times
> > >>
> > >> Apr 5 17:47:07 Teti kernel: NETDEV WATCHDOG: eth2: transmit timed
>out
> > >
> > >There were some problems of this kind with 10base2 networks in 2.6.16.
> > >Could you please try whether 2.6.17-rc1 has this problems too?
> >
> > [Sorry for the very huge delay, but after 2.6.17-rc1 upgrade
> > xfs filesystem crashed].
> >
> > Same problem here with 2.6.17-rc3-git18. Running ifconfig
> > eth2 down and ifconfig eth2 up resolves the problem for
> > a while.
>
>Actually I have not really an idea what is going on here,
>but increasing the debug level could give some more informations.
>Setting debug=4 is a good start.
I have moved this damn pc from the remote to my site and I have
placed it in production environment with 2.6.17-rc5
No problem after 24 hours (on the remote side the problem was
arising after a couple of hours). I have modprobed 3c59x with
debug=4. I see only these kind of messages (are they fine?):
Jun 5 14:31:25 Pleiadi kernel: eth2: vortex_error(), status=0x8081
Jun 5 14:31:40 Pleiadi last message repeated 3 times
Jun 5 14:31:47 Pleiadi kernel: eth2: vortex_error(), status=0x8281
Jun 5 14:31:47 Pleiadi kernel: eth2: Media selection timer tick happened,
Autonegotiate.
Jun 5 14:31:47 Pleiadi kernel: dev->watchdog_timeo=1250
Jun 5 14:31:47 Pleiadi kernel: eth2: MII transceiver has status 782d.
Jun 5 14:31:47 Pleiadi kernel: eth2: Media selection timer finished,
Autonegotiate.
Jun 5 14:31:51 Pleiadi kernel: eth2: vortex_error(), status=0x8081
Jun 5 14:32:03 Pleiadi last message repeated 2 times
Jun 5 14:32:10 Pleiadi kernel: eth2: vortex_error(), status=0x8481
Jun 5 14:32:15 Pleiadi kernel: eth2: vortex_error(), status=0x8081
Jun 5 14:32:46 Pleiadi last message repeated 7 times
The only relevant change, between the remote and my site, is a
different ethernet switch where the 3c905C is connected to.
Could it be an issue?
>Did you try older kernel versions too?
I started with 2.6.16.x because I need iptables policy match.
^ permalink raw reply
* Re: 2.6.17-rc5-mm3
From: Mel Gorman @ 2006-06-06 9:43 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, netdev
In-Reply-To: <20060605115456.c2b1e156.akpm@osdl.org>
On Mon, 5 Jun 2006, Andrew Morton wrote:
> On Mon, 5 Jun 2006 18:56:37 +0100
> mel@csn.ul.ie (Mel Gorman) wrote:
>
>>
>> I am seeing more networking-related funniness with 2.6.17-rc5-mm3 on the
>> same machine previously fixed by git-net-llc-fix.patch. The console log is
>> below. I've done no investigation work in case it's a known problem.
>
> It's not a known problem, afaik.
>
>> ...
>> Starting anacron: [ OK ]
>> Starting atd: [ OK ]
>> Starting Avahi daemon: [ OK ]
>> Starting cups-config-daemon: [ OK ]
>> Starting HAL daemon: [ OK ]
>> Fedora Core release 5 (Bordeaux)
>> Kernel 2.6.17-rc5-mm2-autokern1 on an x86_64
>> bl6-13.ltc.austin.ibm.com login: -- 0:conmux-control -- time-stamp -- Jun/05/06 10:47:46 --
>> -- 0:conmux-control -- time-stamp -- Jun/05/06 10:51:12 --
>> BUG: warning at include/net/dst.h:153/dst_release()
>> Call Trace:
>> <IRQ> [<ffffffff81228274>] __kfree_skb+0x3c/0xbd
>> [<ffffffff81199568>] tg3_poll+0x1a1/0x94f
>> [<ffffffff8122d80c>] net_rx_action+0xac/0x160
>> [<ffffffff81037904>] __do_softirq+0x48/0xb4
>> [<ffffffff8100a496>] call_softirq+0x1e/0x28
>> [<ffffffff8100b84e>] do_softirq+0x2c/0x7e
>> [<ffffffff8100b6c8>] do_IRQ+0x50/0x59
>> [<ffffffff81007807>] default_idle+0x0/0x54
>> [<ffffffff810097b8>] ret_from_intr+0x0/0xb
>> <EOI>
>> Attempt to release alive inet socket ffff81003f8b2780
>> BUG: warning at include/net/dst.h:153/dst_release()
>> Call Trace:
>> <IRQ> [<ffffffff81228274>] __kfree_skb+0x3c/0xbd
>> [<ffffffff81268fc4>] icmp_rcv+0x17c/0x184
>> [<ffffffff812484ca>] ip_local_deliver+0xfe/0x1bf
>> [<ffffffff812489bf>] ip_rcv+0x434/0x475
>> [<ffffffff8122d615>] netif_receive_skb+0x2c6/0x2e5
>> [<ffffffff81199add>] tg3_poll+0x716/0x94f
>> [<ffffffff8122d80c>] net_rx_action+0xac/0x160<7>Losing some ticks... checking if CPU frequency changed.
>> [<ffffffff81037904>] __do_softirq+0x48/0xb4
>> [<ffffffff8100a496>] call_softirq+0x1e/0x28
>> [<ffffffff8100b84e>] do_softirq+0x2c/0x7e
>> [<ffffffff8100b6c8>] do_IRQ+0x50/0x59
>> [<ffffffff81007807>] default_idle+0x0/0x54
>> [<ffffffff810097b8>] ret_from_intr+0x0/0xb
>
> There are quite a few changes in the net tree. I guess the first thing to
> investigate would be 2.6.17-rc5+origin.patch+git-net.patch.
>
That survived long enough to build a kernel, but backing out git-net on
top of mm like I did for the LLC bug also survived. Not sure what is going
on.
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
^ permalink raw reply
* Re: [PATCH 2.6.18 1/3] tg3: Remove unnecessary tx_lock
From: Herbert Xu @ 2006-06-06 10:17 UTC (permalink / raw)
To: Michael Chan; +Cc: davem, jgarzik, netdev
In-Reply-To: <1551EAE59135BE47B544934E30FC4FC041BCBA@NT-IRVA-0751.brcm.ad.broadcom.com>
On Mon, Jun 05, 2006 at 10:10:56PM -0700, Michael Chan wrote:
>
> In places where we don't call tg3_netif_stop() before tg3_full_lock(),
> we are typically doing one of the following:
>
> - changing tg3_flags or tg3_flags2
> - reprogramming MAC address
> - reprogramming interrupt coalescing values
> - reprogramming the PHY
> - setting the rx mode
OK, thanks for checking.
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [NET]: Add netif_tx_lock
From: Herbert Xu @ 2006-06-06 10:22 UTC (permalink / raw)
To: David Miller; +Cc: mchan, jgarzik, netdev
In-Reply-To: <20060605.213250.85688925.davem@davemloft.net>
On Mon, Jun 05, 2006 at 09:32:50PM -0700, David Miller wrote:
>
> IPOIB is going to BUG() with this change. Because now, in their
> multicast code, you're going to local_bh_disable() via
> netif_tx_unlock() with hw IRQs disabled which is illegal.
>
> It shows a bug here in the locking of the IPOIB driver.
You had me woried there.
> We need to think about this change some more. :)
I thought about it a bit more and I'm not worried anymore :)
Notice that the patch does netif_tx_lock/netif_tx_unlock
for IB instead of netif_tx_lock_bh/netif_tx_unlock_bh.
So there is no BH enabling going on at all.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: 2.6.17-rc5-mm3
From: Mel Gorman @ 2006-06-06 10:57 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, netdev
In-Reply-To: <20060605115456.c2b1e156.akpm@osdl.org>
On Mon, 5 Jun 2006, Andrew Morton wrote:
> On Mon, 5 Jun 2006 18:56:37 +0100
> mel@csn.ul.ie (Mel Gorman) wrote:
>
>>
>> I am seeing more networking-related funniness with 2.6.17-rc5-mm3 on the
>> same machine previously fixed by git-net-llc-fix.patch. The console log is
>> below. I've done no investigation work in case it's a known problem.
>
> It's not a known problem, afaik.
>
>> ...
>> Starting anacron: [ OK ]
>> Starting atd: [ OK ]
>> Starting Avahi daemon: [ OK ]
>> Starting cups-config-daemon: [ OK ]
>> Starting HAL daemon: [ OK ]
>> Fedora Core release 5 (Bordeaux)
>> Kernel 2.6.17-rc5-mm2-autokern1 on an x86_64
Bah, I'm a spanner. The patches I was testing were rebased to the latest
-mm, but the kernel version they were then tested on was not changed. This
was probably the LLC bug with a different shaped error and the first set
of tests are passing with -mm3. Sorry for the noise.
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
^ permalink raw reply
* Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device
From: Michael Buesch @ 2006-06-06 12:58 UTC (permalink / raw)
To: Jiri Slaby
Cc: Greg KH, Linux Kernel Mailing List, linux-pci, jgarzik, netdev,
mb, st3, linville
In-Reply-To: <20060605205309.GA31061@kroah.com>
On Monday 05 June 2006 22:53, Greg KH wrote:
> On Mon, Jun 05, 2006 at 10:20:07PM +0200, Jiri Slaby wrote:
> > bcm43xx avoid pci_find_device
> >
> > Change pci_find_device to safer pci_get_device with support for more
> > devices.
> >
> > Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> >
> > ---
> > commit 4b73c16f5411d97360d5f26f292ffddeb670ff75
> > tree 6e43c8bd02498eb1ceec6bdc64277fa8408da9e2
> > parent d59f9ea8489749f59cd0c7333a4784cab964daa8
> > author Jiri Slaby <ku@bellona.localdomain> Mon, 05 Jun 2006 22:01:03 +0159
> > committer Jiri Slaby <ku@bellona.localdomain> Mon, 05 Jun 2006 22:01:03 +0159
> >
> > drivers/net/wireless/bcm43xx/bcm43xx_main.c | 21 ++++++++++++++++-----
> > 1 files changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> > index 22b8fa6..d1a9975 100644
> > --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> > +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> > @@ -2133,6 +2133,13 @@ out:
> > return err;
> > }
> >
> > +#ifdef CONFIG_BCM947XX
> > +static struct pci_device_id bcm43xx_47xx_ids[] = {
> > + { PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4324) },
> > + { 0 }
> > +};
> > +#endif
> > +
> > static int bcm43xx_initialize_irq(struct bcm43xx_private *bcm)
> > {
> > int res;
> > @@ -2142,11 +2149,15 @@ static int bcm43xx_initialize_irq(struct
> > bcm->irq = bcm->pci_dev->irq;
> > #ifdef CONFIG_BCM947XX
> > if (bcm->pci_dev->bus->number == 0) {
> > - struct pci_dev *d = NULL;
> > - /* FIXME: we will probably need more device IDs here... */
> > - d = pci_find_device(PCI_VENDOR_ID_BROADCOM, 0x4324, NULL);
> > - if (d != NULL) {
> > - bcm->irq = d->irq;
> > + struct pci_dev *d;
> > + struct pci_device_id *id;
> > + for (id = bcm43xx_47xx_ids; id->vendor; id++) {
> > + d = pci_get_device(id->vendor, id->device, NULL);
> > + if (d != NULL) {
> > + bcm->irq = d->irq;
> > + pci_dev_put(d);
> > + break;
> > + }
>
> This will not work if you have more than one of the same devices in the
> system.
>
> Well, the original code will not either :(
>
> Why not just use the proper pci interface? Why poke around in another
> pci device to steal an irq, when that irq might not even be valid?
> (irqs are not valid until pci_enable_device() is called on them...)
Ok, if someone really wants to have this patch in mainline.
Signed-off-by: Michael Buesch <mb@bu3sch.de>
But the whole purpose of this patch is really questionable.
* This code is only compiled for the OpenWRT Router kernel.
This kernel does not use softmac, but dscape stack.
So nobody will ever actually use this code.
One could even argue, if the code should be removed from
the softmac driver, but I think openwrt people use it for
some kind of hacking, testing, whatever.
It's not so much code, so I don't care. It does not add
maintainance work.
* Do we really need to increment some reference counters?
I mean, we are asking for a bus here. This bus is not
hotpluggable or something and will never go away. Either it
is soldered on the board or not. That is what we test here.
So, Jiri, if you really want to have this patch upstream, you
have my Signed-off-by. If nobody else cares, it will get lost
in the deep black netdev hole. ;)
--
Greetings Michael.
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Neil Horman @ 2006-06-06 13:52 UTC (permalink / raw)
To: Kok, Auke; +Cc: Garzik, Jeff, netdev, Brandeburg, Jesse, Kok, Auke, jmoyer, mpm
In-Reply-To: <20060605231125.12584.17039.stgit@gitlost.site>
On Mon, Jun 05, 2006 at 04:11:25PM -0700, Kok, Auke wrote:
>
> Netpoll was broken due to the earlier addition of multiqueue.
>
> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> ---
>
> drivers/net/e1000/e1000_main.c | 9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index ed15fca..7103a0e 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -4629,10 +4629,17 @@ static void
> e1000_netpoll(struct net_device *netdev)
> {
> struct e1000_adapter *adapter = netdev_priv(netdev);
> +#ifdef CONFIG_E1000_NAPI
> + int budget = 0;
> +
> + disable_irq(adapter->pdev->irq);
> + e1000_clean_tx_irq(adapter, adapter->tx_ring);
> + adapter->clean_rx(adapter, adapter->rx_ring, &budget, netdev->weight);
> +#else
> +
I've been speaking about this fix with a Jeff Moyer, and we've come up with some
concerns regarding its implementation. Specifically the call to
adapter->clean_rx in the case of the e1000 driver is rather a layering
violation in the netpoll code, in the sense that the function pointed to by clean_rx
is functionality that is nominally used by the dev->poll method. In fact in
this case, it would appear possible since dev->poll is called under the
poll_lock, but dev->poll_controller is not, that is is possible to have cpus in
a system executing in e1000_clean_rx_irq_[ps] at the same time leading to data
corruption:
CPU0:
netpoll_poll_dev
dev->poll_controller (e1000_netpoll)
adapter->clean_rx (e1000_clean_rx_irq)
CPU1:
napi_poll
dev->poll (e1000_clean)
e1000_clean_rx_irq
I'm not sure what the right fix is here. A spinlock in e1000_clean_rx_irq[_ps]
would seem to be an easy and direct fix, but I don't know that thats the best
solution.
Something I don't understand is why the call to adapter->clean_rx is
required in the first place when the napi_poll routine calls it itself directly.
All other drivers schedule a napi poll and receive frames via that path. My
guess is that it has to do with the fact that we schedule polls on the device in
the polling_netdev array, rather than the actual registered netdev. Looking at
the driver code I note that while an entire array is allocated for polling
netdevs, we only ever use entry 0. Would it make sense to just remove the the
polling_netdev array and use the registered device like all the other drivers.
I expect that would eliminate the need for this patch as well.
Regards
Neil
> disable_irq(adapter->pdev->irq);
> e1000_intr(adapter->pdev->irq, netdev, NULL);
> e1000_clean_tx_irq(adapter, adapter->tx_ring);
> -#ifndef CONFIG_E1000_NAPI
> adapter->clean_rx(adapter, adapter->rx_ring);
> #endif
> enable_irq(adapter->pdev->irq);
>
>
>
> --
> Auke Kok <auke-jan.h.kok@intel.com>
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
/***************************************************
*Neil Horman
*Software Engineer
*gpg keyid: 1024D / 0x92A74FA1 - http://pgp.mit.edu
***************************************************/
^ permalink raw reply
* Re: [PATCH 1/1] LSM-IPsec SELinux Authorize (with minor fix)
From: Xiaolan Zhang @ 2006-06-06 14:55 UTC (permalink / raw)
To: James Morris
Cc: netdev, davem, chrisw, herbert, sds, tjaeger, latten,
Serge E Hallyn, George Wilson, czhang.us
In-Reply-To: <Pine.LNX.4.64.0606060133220.9787@d.namei>
Singned-off-by: Catherine Zhang <cxzhang@watson.ibm.com>
James, is this enough or do I need to modify the original patch to add the
above line? The code was taken from various pieces of patches originally
from Trent and merged/modified by me. Let me know what else I need to do.
thanks,
Catherine
James Morris <jmorris@namei.org> wrote on 06/06/2006 01:37:04 AM:
> On Tue, 6 Jun 2006, Catherine Zhang wrote:
>
> > Minor fix per James' comment.
>
> Can you also add a Signed-off-by line?
>
> I can't recall if you were the original author. If not, we also need a
> From line (per Documentation/SubmittingPatches).
>
>
> Thanks,
>
> --
> James Morris
> <jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH] hush noisy ieee80211 CCMP printks
From: Jason Lunz @ 2006-06-06 15:02 UTC (permalink / raw)
To: Jouni Malinen; +Cc: linville, netdev
In-Reply-To: <20060606034138.GB9568@jm.kir.nu>
On Mon, Jun 05, 2006 at 08:41:38PM -0700, Jouni Malinen wrote:
> Do you happen to have a wireless sniffer that you could use to capture
> the frames? It would be interesting to see whether such a capture log
> could be mapped into the dropped frames shown in the kernel debug log.
I don't know. Would booting something like knoppix on another laptop
with some flavor of Intel wireless do? The only two laptops I have
regular access to are the bcm43xx one in question and another Intel one
that ordinarily runs winxp.
> Would you be interested in testing this with net/d80211 code and
> wireless-dev.git?
sure, I can do that. I'm fairly clumsy with git, so it may take a while.
Jason
^ permalink raw reply
* Re: 2.6.17-rc5-mm3-lockdep -
From: Stefan Richter @ 2006-06-06 16:39 UTC (permalink / raw)
To: Valdis.Kletnieks
Cc: Jiri Slaby, Andrew Morton, arjan, mingo, linux-kernel,
linux1394-devel, netdev
In-Reply-To: <4485798B.4030007@s5r6.in-berlin.de>
>>Valdis.Kletnieks@vt.edu napsal(a):
...
>>>[ 464.687000] [ BUG: illegal lock usage! ]
>>>[ 464.687000] ----------------------------
>>>[ 464.687000] illegal {in-hardirq-W} -> {hardirq-on-W} usage.
>>>[ 464.687000] id/2700 [HC0[0]:SC0[1]:HE1:SE0] takes:
>>>[ 464.687000] (&list->lock){++..}, at: [<c0351a07>] unix_stream_connect+0x334/0x408
>>>[ 464.687000] {in-hardirq-W} state was registered at:
>>>[ 464.687000] [<c012dd45>] lockdep_acquire+0x67/0x7f
>>>[ 464.687000] [<c0383f11>] _spin_lock_irqsave+0x30/0x3f
>>>[ 464.687000] [<c02fa93f>] skb_dequeue+0x18/0x49
>>>[ 464.687000] [<f086b7f1>] hpsb_bus_reset+0x5e/0xa2 [ieee1394]
>>>[ 464.687000] [<f0887007>] ohci_irq_handler+0x370/0x726 [ohci1394]
>>>[ 464.687000] [<c013f9a8>] handle_IRQ_event+0x1d/0x52
>>>[ 464.687000] [<c0140bc4>] handle_level_irq+0x97/0xe3
>>>[ 464.687000] [<c01045d0>] do_IRQ+0x8b/0xaf
>>>[ 464.687000] irq event stamp: 2964
>>>[ 464.687000] hardirqs last enabled at (2963): [<c0384223>] _spin_unlock_irqrestore+0x3b/0x6d
>>>[ 464.687000] hardirqs last disabled at (2962): [<c0383ef5>] _spin_lock_irqsave+0x14/0x3f
>>>[ 464.687000] softirqs last enabled at (2956): [<c0119da0>] __do_softirq+0x9d/0xa5
>>>[ 464.687000] softirqs last disabled at (2964): [<c0383f6b>] _spin_lock_bh+0x10/0x3a
>>>[ 464.687000]
>>>[ 464.687000] other info that might help us debug this:
>>>[ 464.687000] 1 locks held by id/2700:
>>>[ 464.687000] #0: (&u->lock){--..}, at: [<c03517bb>] unix_stream_connect+0xe8/0x408
>>>[ 464.687000]
>>>[ 464.687000] stack backtrace:
>>>[ 464.687000] [<c01032d6>] show_trace_log_lvl+0x64/0x125
>>>[ 464.687000] [<c0103865>] show_trace+0x1b/0x20
>>>[ 464.687000] [<c010391c>] dump_stack+0x1f/0x24
>>>[ 464.687000] [<c012bfb1>] print_usage_bug+0x1a8/0x1b4
>>>[ 464.687000] [<c012c6c7>] mark_lock+0x2ba/0x4e5
>>>[ 464.687000] [<c012d2b8>] __lockdep_acquire+0x476/0xa91
>>>[ 464.687000] [<c012dd45>] lockdep_acquire+0x67/0x7f
>>>[ 464.687000] [<c0383f87>] _spin_lock_bh+0x2c/0x3a
>>>[ 464.687000] [<c0351a07>] unix_stream_connect+0x334/0x408
>>>[ 464.687000] [<c02f7236>] sys_connect+0x6e/0xa3
>>>[ 464.687000] [<c02f79da>] sys_socketcall+0x96/0x190
>>>[ 464.687000] [<c03845e2>] sysenter_past_esp+0x63/0xa1
On second look it doesn't seem to be a problem of the ieee1394 stack but
rather of underlying skb and net infrastructure.
BTW, the locking in -mm's net/unix/af_unix.c::unix_stream_connect()
differs a bit from stock unix_stream_connect(). I see spin_lock_bh() in
2.6.17-rc5-mm3 where 2.6.17-rc5 has spin_lock().
(added Cc: netdev)
--
Stefan Richter
-=====-=-==- -==- --==-
http://arcgraph.de/sr/
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Mitch Williams @ 2006-06-06 16:39 UTC (permalink / raw)
To: Neil Horman
Cc: Kok, Auke, Garzik, Jeff, netdev, Brandeburg, Jesse, Kok, Auke,
jmoyer, mpm
In-Reply-To: <20060606135217.GA21969@hmsreliant.homelinux.net>
On Tue, 2006-06-06 at 09:52 -0400, Neil Horman wrote:
> I've been speaking about this fix with a Jeff Moyer, and we've come up with some
> concerns regarding its implementation. Specifically the call to
> adapter->clean_rx in the case of the e1000 driver is rather a layering
> violation in the netpoll code, in the sense that the function pointed to by clean_rx
> is functionality that is nominally used by the dev->poll method. In fact in
> this case, it would appear possible since dev->poll is called under the
> poll_lock, but dev->poll_controller is not, that is is possible to have cpus in
> a system executing in e1000_clean_rx_irq_[ps] at the same time leading to data
> corruption:
>
> CPU0:
> netpoll_poll_dev
> dev->poll_controller (e1000_netpoll)
> adapter->clean_rx (e1000_clean_rx_irq)
>
> CPU1:
> napi_poll
> dev->poll (e1000_clean)
> e1000_clean_rx_irq
Hmmm. You may have a point. I don't think a spinlock is required, but
we do need to check if the poll is already scheduled on another CPU,
like netpoll does in poll_napi().
In practice, of course, we never see this. The only netpoll client in
the kernel is netconsole, which doesn't do receives. A few Major
Distros use netdump, which does do receives, but only after the system
has crashed. In that case, all other CPUs are stopped anyway.
However, just for the sake of correctness (and paranoia), I'll whip up
another patch that does this check.
Jeff, please do not commit this patch.
-Mitch
NB: The root of this problem is that e1000 uses a dummy netdev to
schedule NAPI polling. Since netpoll doesn't know about the dummy
netdev, it checks the "real" e1000 netdev struct to see if it's
scheduled for polling. Since this is never the case, netpoll fails when
e1000 is configured to use NAPI. Hence, this patch.
Why is the dummy netdev in place? To support multi-queue RX. Our PCIe
adapters support this, but the kernel's not _quite_ ready yet.
Hopefully, the VJ net channel stuff will enable this feature.
^ permalink raw reply
* [PATCH]TCP Westwood+
From: Luca De Cicco @ 2006-06-06 16:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, David S. Miller, Saverio Mascolo
In-Reply-To: <20060605093240.5d68e117@dxpl.pdx.osdl.net>
[-- Attachment #1: Type: text/plain, Size: 801 bytes --]
Dear all,
As requested you find attached patches (they should be orghogonal) to
tcp_westwood.c. Changes are listed below:
* westwood_comments.diff: Updated comments pointing to essential papers
about TCP Westwood
* westwood_bugfix.diff: Fixes a subtle bug that caused the first sample
to be wrong
* westwood_faster_filter.diff: The bandwidth estimate filter is now
initialized with the first bandwidth sample in order to have better
performances in the case of small file transfers.
* westwood_rtt_min_reset.diff: RTT_min is updated each time a timeout
event occurs (in order to cope with hard handovers in wireless
scenarios such as UMTS). A new inline function, update_rtt_min has been
added.
Signed-off-by: Luca De Cicco <ldecicco@gmail.com>
Best Regards,
Luca De Cicco
Politecnico di Bari
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: westwood_bugfix.diff --]
[-- Type: text/x-patch; name=westwood_bugfix.diff, Size: 1073 bytes --]
Index: ipv4/tcp_westwood.c
===================================================================
--- ipv4.orig/tcp_westwood.c 2006-06-06 18:08:03.000000000 +0200
+++ ipv4/tcp_westwood.c 2006-06-06 18:09:44.000000000 +0200
@@ -22,6 +22,7 @@
u32 accounted;
u32 rtt;
u32 rtt_min; /* minimum observed RTT */
+ u16 first_ack; /* flag which infers that this is the first ack */
};
@@ -52,6 +53,7 @@
w->rtt_min = w->rtt = TCP_WESTWOOD_INIT_RTT;
w->rtt_win_sx = tcp_time_stamp;
w->snd_una = tcp_sk(sk)->snd_una;
+ w->first_ack = 1;
}
/*
@@ -184,6 +186,15 @@
{
struct tcp_sock *tp = tcp_sk(sk);
struct westwood *w = inet_csk_ca(sk);
+
+ /* Initialise w->snd_una with the first acked sequence number in oder
+ * to fix mismatch between tp->snd_una and w->snd_una for the first
+ * bandwidth sample
+ */
+ if(w->first_ack && (event == CA_EVENT_FAST_ACK||event == CA_EVENT_SLOW_ACK)) {
+ w->snd_una = tp->snd_una;
+ w->first_ack = 0;
+ }
switch(event) {
case CA_EVENT_FAST_ACK:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: westwood_comments.diff --]
[-- Type: text/x-patch; name=westwood_comments.diff, Size: 1432 bytes --]
Index: ipv4/tcp_westwood.c
===================================================================
--- ipv4.orig/tcp_westwood.c 2006-06-06 17:56:13.000000000 +0200
+++ ipv4/tcp_westwood.c 2006-06-06 17:57:13.000000000 +0200
@@ -1,9 +1,29 @@
/*
- * TCP Westwood+
+ * TCP Westwood+: end-to-end bandwidth estimation for TCP
*
- * Angelo Dell'Aera: TCP Westwood+ support
+ * Angelo Dell'Aera: author of the first version of TCP Westwood+ in Linux 2.4
+ * Luca De Cicco: current support of Westwood+ and author of the last patch
+ * with: updated RTT_min and initial bandwidth estimate
+ *
+ * Support at http://c3lab.poliba.it/index.php/Westwood
+ * Main references in literature:
+ *
+ * - Mascolo S, Casetti, M. Gerla et al.
+ * "TCP Westwood: bandwidth estimation for TCP" Proc. ACM Mobicom 2001
+ *
+ * - A. Grieco, s. Mascolo
+ * "Performance evaluation of New Reno, Vegas, Westwood+ TCP" ACM Computer
+ * Comm. Review, 2004
+ *
+ * - A. Dell'Aera, L. Grieco, S. Mascolo.
+ * "Linux 2.4 Implementation of Westwood+ TCP with Rate-Halving :
+ * A Performance Evaluation Over the Internet" (ICC 2004), Paris, June 2004
+ *
+ * Westwood+ employs end-to-end bandwdidth measurement to set cwnd and
+ * ssthresh after packet loss. The probing phase is as the original Reno.
*/
+
#include <linux/config.h>
#include <linux/mm.h>
#include <linux/module.h>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: westwood_faster_filter.diff --]
[-- Type: text/x-patch; name=westwood_faster_filter.diff, Size: 799 bytes --]
Index: ipv4/tcp_westwood.c
===================================================================
--- ipv4.orig/tcp_westwood.c 2006-06-06 18:14:50.000000000 +0200
+++ ipv4/tcp_westwood.c 2006-06-06 18:16:49.000000000 +0200
@@ -65,8 +65,17 @@
static inline void westwood_filter(struct westwood *w, u32 delta)
{
- w->bw_ns_est = westwood_do_filter(w->bw_ns_est, w->bk / delta);
- w->bw_est = westwood_do_filter(w->bw_est, w->bw_ns_est);
+ /*
+ * If the filter is empty fill it with the first sample of bandwidth
+ */
+ if (w->bw_ns_est==0 && w->bw_est==0)
+ {
+ w->bw_ns_est = w->bk / delta;
+ w->bw_est = w->bw_ns_est ;
+ } else {
+ w->bw_ns_est = westwood_do_filter(w->bw_ns_est, w->bk / delta);
+ w->bw_est = westwood_do_filter(w->bw_est, w->bw_ns_est);
+ }
}
/*
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: westwood_rtt_min_reset.diff --]
[-- Type: text/x-patch; name=westwood_rtt_min_reset.diff, Size: 1820 bytes --]
Index: ipv4/tcp_westwood.c
===================================================================
--- ipv4.orig/tcp_westwood.c 2006-06-06 18:10:22.000000000 +0200
+++ ipv4/tcp_westwood.c 2006-06-06 18:13:07.000000000 +0200
@@ -15,6 +15,7 @@
struct westwood {
u32 bw_ns_est; /* first bandwidth estimation..not too smoothed 8) */
u32 bw_est; /* bandwidth estimate */
+ u16 reset_rtt_min; /* Please reset RTT min to RTT sample*/
u32 rtt_win_sx; /* here starts a new evaluation... */
u32 bk;
u32 snd_una; /* used for evaluating the number of acked bytes */
@@ -50,6 +51,7 @@
w->bw_est = 0;
w->accounted = 0;
w->cumul_ack = 0;
+ w->reset_rtt_min = 1;
w->rtt_min = w->rtt = TCP_WESTWOOD_INIT_RTT;
w->rtt_win_sx = tcp_time_stamp;
w->snd_una = tcp_sk(sk)->snd_una;
@@ -110,6 +112,19 @@
}
}
+static inline void update_rtt_min(struct sock *sk)
+{
+ struct westwood *w = inet_csk_ca(sk);
+
+ if (w->reset_rtt_min) {
+ w->rtt_min = w->rtt;
+ w->reset_rtt_min = 0;
+ } else {
+ w->rtt_min = min(w->rtt, w->rtt_min);
+ }
+}
+
+
/*
* @westwood_fast_bw
* It is called when we are in fast path. In particular it is called when
@@ -125,7 +140,7 @@
w->bk += tp->snd_una - w->snd_una;
w->snd_una = tp->snd_una;
- w->rtt_min = min(w->rtt, w->rtt_min);
+ update_rtt_min(sk);
}
/*
@@ -207,12 +222,14 @@
case CA_EVENT_FRTO:
tp->snd_ssthresh = westwood_bw_rttmin(sk);
+ /* Please update RTT_min when next ack arrives */
+ w->reset_rtt_min = 1;
break;
case CA_EVENT_SLOW_ACK:
westwood_update_window(sk);
w->bk += westwood_acked_count(sk);
- w->rtt_min = min(w->rtt, w->rtt_min);
+ update_rtt_min(sk);
break;
default:
^ permalink raw reply
* Re: [PATCH] TCP changes for 2.6.18
From: Stephen Hemminger @ 2006-06-06 16:46 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20060605.165859.104035231.davem@davemloft.net>
On Mon, 05 Jun 2006 16:58:59 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <shemminger@osdl.org>
> Date: Mon, 5 Jun 2006 11:03:31 -0700
>
> > Dave, please consider adding these for the 2.6.18.
>
> I'll pick these into my tree by extracting out the patches.
> Your tree wasn't based upon the net-2.6.18 tree, so if I just
> pull it into mine I'll get all the upstream changes since I
> cut the net-2.6.18 which at the current time I don't want :)
>
I will rebase it on your tree today.
> BTW, it seems we now have at least 3 instances "VEGAS + stuff"
> and thus the core vegas logic is duplicated that many times.
> Would be nice to have some core vegas infrastructure in the
> generic congestion avoidance layer at some point.
Agreed, and the vegas stuff needs some work as well. It should
have more smoothing like Westwood+
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Neil Horman @ 2006-06-06 17:05 UTC (permalink / raw)
To: Mitch Williams
Cc: Kok, Auke, Garzik, Jeff, netdev, Brandeburg, Jesse, Kok, Auke,
jmoyer, mpm
In-Reply-To: <1149611965.13635.19.camel@strongmad>
On Tue, Jun 06, 2006 at 09:39:25AM -0700, Mitch Williams wrote:
> On Tue, 2006-06-06 at 09:52 -0400, Neil Horman wrote:
> > I've been speaking about this fix with a Jeff Moyer, and we've come up with some
> > concerns regarding its implementation. Specifically the call to
> > adapter->clean_rx in the case of the e1000 driver is rather a layering
> > violation in the netpoll code, in the sense that the function pointed to by clean_rx
> > is functionality that is nominally used by the dev->poll method. In fact in
> > this case, it would appear possible since dev->poll is called under the
> > poll_lock, but dev->poll_controller is not, that is is possible to have cpus in
> > a system executing in e1000_clean_rx_irq_[ps] at the same time leading to data
> > corruption:
> >
> > CPU0:
> > netpoll_poll_dev
> > dev->poll_controller (e1000_netpoll)
> > adapter->clean_rx (e1000_clean_rx_irq)
> >
> > CPU1:
> > napi_poll
> > dev->poll (e1000_clean)
> > e1000_clean_rx_irq
>
> Hmmm. You may have a point. I don't think a spinlock is required, but
> we do need to check if the poll is already scheduled on another CPU,
> like netpoll does in poll_napi().
>
> In practice, of course, we never see this. The only netpoll client in
> the kernel is netconsole, which doesn't do receives. A few Major
> Distros use netdump, which does do receives, but only after the system
> has crashed. In that case, all other CPUs are stopped anyway.
>
You are probably right, this is a rare case, if it ever happens at all, but I
think there is (at least as Jeff explained it to me) a corner case, where
netconsole on transmit, notes an exhaustion of tx descriptors, and in response
calls the poll controller method of the driver to clean up and make some of
those descriptors available:
printk
release_console_sem
call_console_drivers
netconsole.c:write_msg
netpoll_send_udp
netpoll_send_skb
if (netif_queue_stopped(np->dev)) <--- out of descriptors ?
netpoll_poll(np); <--- trigger a poll to clean up
If this is being done at the same time as a napi_poll on another cpu, we have a
real set of conditions under which a corruption can occur.
> However, just for the sake of correctness (and paranoia), I'll whip up
> another patch that does this check.
>
Thanks for the quick feedback!
Regards
Neil
> Jeff, please do not commit this patch.
>
> -Mitch
>
> NB: The root of this problem is that e1000 uses a dummy netdev to
> schedule NAPI polling. Since netpoll doesn't know about the dummy
> netdev, it checks the "real" e1000 netdev struct to see if it's
> scheduled for polling. Since this is never the case, netpoll fails when
> e1000 is configured to use NAPI. Hence, this patch.
>
> Why is the dummy netdev in place? To support multi-queue RX. Our PCIe
> adapters support this, but the kernel's not _quite_ ready yet.
> Hopefully, the VJ net channel stuff will enable this feature.
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
/***************************************************
*Neil Horman
*Software Engineer
*gpg keyid: 1024D / 0x92A74FA1 - http://pgp.mit.edu
***************************************************/
^ permalink raw reply
* [PATCH 1/5] skge: use workq for PHY handling
From: Stephen Hemminger @ 2006-06-06 17:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20060606171110.592587000@zqx3.pdx.osdl.net>
[-- Attachment #1: skge-phy-workq.patch --]
[-- Type: text/plain, Size: 5675 bytes --]
Since accessing the PHY can take 100's of usecs, use a work queue to
allow spinning in outside of soft/hard irq.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- skge-2.6.orig/drivers/net/skge.c
+++ skge-2.6/drivers/net/skge.c
@@ -603,7 +603,7 @@ static void skge_led(struct skge_port *s
struct skge_hw *hw = skge->hw;
int port = skge->port;
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
if (hw->chip_id == CHIP_ID_GENESIS) {
switch (mode) {
case LED_MODE_OFF:
@@ -663,7 +663,7 @@ static void skge_led(struct skge_port *s
PHY_M_LED_MO_RX(MO_LED_ON));
}
}
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
}
/* blink LED's for finding board */
@@ -2038,7 +2038,7 @@ static void skge_phy_reset(struct skge_p
netif_stop_queue(skge->netdev);
netif_carrier_off(skge->netdev);
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
if (hw->chip_id == CHIP_ID_GENESIS) {
genesis_reset(hw, port);
genesis_mac_init(hw, port);
@@ -2046,7 +2046,7 @@ static void skge_phy_reset(struct skge_p
yukon_reset(hw, port);
yukon_init(hw, port);
}
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
}
/* Basic MII support */
@@ -2067,12 +2067,12 @@ static int skge_ioctl(struct net_device
/* fallthru */
case SIOCGMIIREG: {
u16 val = 0;
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
if (hw->chip_id == CHIP_ID_GENESIS)
err = __xm_phy_read(hw, skge->port, data->reg_num & 0x1f, &val);
else
err = __gm_phy_read(hw, skge->port, data->reg_num & 0x1f, &val);
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
data->val_out = val;
break;
}
@@ -2081,14 +2081,14 @@ static int skge_ioctl(struct net_device
if (!capable(CAP_NET_ADMIN))
return -EPERM;
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
if (hw->chip_id == CHIP_ID_GENESIS)
err = xm_phy_write(hw, skge->port, data->reg_num & 0x1f,
data->val_in);
else
err = gm_phy_write(hw, skge->port, data->reg_num & 0x1f,
data->val_in);
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
break;
}
return err;
@@ -2191,12 +2191,12 @@ static int skge_up(struct net_device *de
goto free_rx_ring;
/* Initialize MAC */
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
if (hw->chip_id == CHIP_ID_GENESIS)
genesis_mac_init(hw, port);
else
yukon_mac_init(hw, port);
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
/* Configure RAMbuffers */
chunk = hw->ram_size / ((hw->ports + 1)*2);
@@ -2847,16 +2847,16 @@ static void skge_error_irq(struct skge_h
}
/*
- * Interrupt from PHY are handled in tasklet (soft irq)
+ * Interrupt from PHY are handled in work queue
* because accessing phy registers requires spin wait which might
* cause excess interrupt latency.
*/
-static void skge_extirq(unsigned long data)
+static void skge_extirq(void *arg)
{
- struct skge_hw *hw = (struct skge_hw *) data;
+ struct skge_hw *hw = arg;
int port;
- spin_lock(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
for (port = 0; port < hw->ports; port++) {
struct net_device *dev = hw->dev[port];
struct skge_port *skge = netdev_priv(dev);
@@ -2868,7 +2868,7 @@ static void skge_extirq(unsigned long da
bcom_phy_intr(skge);
}
}
- spin_unlock(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
hw->intr_mask |= IS_EXT_REG;
skge_write32(hw, B0_IMSK, hw->intr_mask);
@@ -2886,7 +2886,7 @@ static irqreturn_t skge_intr(int irq, vo
if (status & IS_EXT_REG) {
hw->intr_mask &= ~IS_EXT_REG;
- tasklet_schedule(&hw->ext_tasklet);
+ schedule_work(&hw->phy_work);
}
if (status & (IS_R1_F|IS_XA1_F)) {
@@ -2957,7 +2957,7 @@ static int skge_set_mac_address(struct n
if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL;
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
memcpy_toio(hw->regs + B2_MAC_1 + port*8,
dev->dev_addr, ETH_ALEN);
@@ -2970,7 +2970,7 @@ static int skge_set_mac_address(struct n
gma_set_addr(hw, port, GM_SRC_ADDR_1L, dev->dev_addr);
gma_set_addr(hw, port, GM_SRC_ADDR_2L, dev->dev_addr);
}
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
return 0;
}
@@ -3150,14 +3150,14 @@ static int skge_reset(struct skge_hw *hw
skge_write32(hw, B0_IMSK, hw->intr_mask);
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
for (i = 0; i < hw->ports; i++) {
if (hw->chip_id == CHIP_ID_GENESIS)
genesis_reset(hw, i);
else
yukon_reset(hw, i);
}
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
return 0;
}
@@ -3305,8 +3305,8 @@ static int __devinit skge_probe(struct p
}
hw->pdev = pdev;
- spin_lock_init(&hw->phy_lock);
- tasklet_init(&hw->ext_tasklet, skge_extirq, (unsigned long) hw);
+ mutex_init(&hw->phy_mutex);
+ INIT_WORK(&hw->phy_work, skge_extirq, hw);
hw->regs = ioremap_nocache(pci_resource_start(pdev, 0), 0x4000);
if (!hw->regs) {
@@ -3392,7 +3392,7 @@ static void __devexit skge_remove(struct
skge_write16(hw, B0_LED, LED_STAT_OFF);
skge_write8(hw, B0_CTST, CS_RST_SET);
- tasklet_kill(&hw->ext_tasklet);
+ flush_scheduled_work();
free_irq(pdev->irq, hw);
pci_release_regions(pdev);
--- skge-2.6.orig/drivers/net/skge.h
+++ skge-2.6/drivers/net/skge.h
@@ -2399,9 +2399,8 @@ struct skge_hw {
u32 ram_size;
u32 ram_offset;
u16 phy_addr;
-
- struct tasklet_struct ext_tasklet;
- spinlock_t phy_lock;
+ struct work_struct phy_work;
+ struct mutex phy_mutex;
};
enum {
--
^ permalink raw reply
* [PATCH 4/5] skge: dont allow bad hardware address from ROM
From: Stephen Hemminger @ 2006-06-06 17:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20060606171110.592587000@zqx3.pdx.osdl.net>
[-- Attachment #1: skge-badaddr.patch --]
[-- Type: text/plain, Size: 709 bytes --]
Sometimes boards don't reset properly, and the address read out of the
EEPROM is zero. Stop the insanity before the device gets registered.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- skge-2.6.orig/drivers/net/skge.c
+++ skge-2.6/drivers/net/skge.c
@@ -3362,6 +3362,14 @@ static int __devinit skge_probe(struct p
if ((dev = skge_devinit(hw, 0, using_dac)) == NULL)
goto err_out_led_off;
+ if (!is_valid_ether_addr(dev->dev_addr)) {
+ printk(KERN_ERR PFX "%s: bad (zero?) ethernet address in rom\n",
+ pci_name(pdev));
+ err = -EIO;
+ goto err_out_free_netdev;
+ }
+
+
err = register_netdev(dev);
if (err) {
printk(KERN_ERR PFX "%s: cannot register net device\n",
--
^ permalink raw reply
* [PATCH 0/5] skge: new version 1.6
From: Stephen Hemminger @ 2006-06-06 17:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
Go back to handling transmit completion via IRQ not NAPI. This works
better and is cleaner. Handle PHY via work queue, not tasklet.
--
^ permalink raw reply
* [PATCH 2/5] skge: TX low water mark definition
From: Stephen Hemminger @ 2006-06-06 17:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20060606171110.592587000@zqx3.pdx.osdl.net>
[-- Attachment #1: skge-lowater.patch --]
[-- Type: text/plain, Size: 1430 bytes --]
Consolidate all usage of ring low water mark to one value.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- skge-2.6.orig/drivers/net/skge.c
+++ skge-2.6/drivers/net/skge.c
@@ -50,6 +50,7 @@
#define DEFAULT_TX_RING_SIZE 128
#define DEFAULT_RX_RING_SIZE 512
#define MAX_TX_RING_SIZE 1024
+#define TX_LOW_WATER (MAX_SKB_FRAGS + 1)
#define MAX_RX_RING_SIZE 4096
#define RX_COPY_THRESHOLD 128
#define RX_BUF_SIZE 1536
@@ -401,7 +402,7 @@ static int skge_set_ring_param(struct ne
int err;
if (p->rx_pending == 0 || p->rx_pending > MAX_RX_RING_SIZE ||
- p->tx_pending < MAX_SKB_FRAGS+1 || p->tx_pending > MAX_TX_RING_SIZE)
+ p->tx_pending < TX_LOW_WATER || p->tx_pending > MAX_TX_RING_SIZE)
return -EINVAL;
skge->rx_ring.count = p->rx_pending;
@@ -2394,7 +2395,7 @@ static int skge_xmit_frame(struct sk_buf
dev->name, e - ring->start, skb->len);
ring->to_use = e->next;
- if (skge_avail(&skge->tx_ring) <= MAX_SKB_FRAGS + 1) {
+ if (skge_avail(&skge->tx_ring) <= TX_LOW_WATER) {
pr_debug("%s: transmit queue full\n", dev->name);
netif_stop_queue(dev);
}
@@ -2689,7 +2690,7 @@ static void skge_tx_done(struct skge_por
skge_write8(skge->hw, Q_ADDR(txqaddr[skge->port], Q_CSR), CSR_IRQ_CL_F);
- if (skge_avail(&skge->tx_ring) > MAX_SKB_FRAGS + 1)
+ if (skge_avail(&skge->tx_ring) > TX_LOW_WATER)
netif_wake_queue(skge->netdev);
spin_unlock(&skge->tx_lock);
--
^ permalink raw reply
* [PATCH 3/5] skge: transmit complete via IRQ not NAPI
From: Stephen Hemminger @ 2006-06-06 17:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20060606171110.592587000@zqx3.pdx.osdl.net>
[-- Attachment #1: skge-txirq.patch --]
[-- Type: text/plain, Size: 9856 bytes --]
The transmit side code has a number of ring problems that caused some
of the Bugzilla reports. Rather than trying to fix the details, it is safer
to rewrite the code that handles transmit completion and freeing.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- skge-2.6.orig/drivers/net/skge.c
+++ skge-2.6/drivers/net/skge.c
@@ -2303,21 +2303,20 @@ static int skge_xmit_frame(struct sk_buf
{
struct skge_port *skge = netdev_priv(dev);
struct skge_hw *hw = skge->hw;
- struct skge_ring *ring = &skge->tx_ring;
struct skge_element *e;
struct skge_tx_desc *td;
int i;
u32 control, len;
u64 map;
+ unsigned long flags;
skb = skb_padto(skb, ETH_ZLEN);
if (!skb)
return NETDEV_TX_OK;
- if (!spin_trylock(&skge->tx_lock)) {
+ if (!spin_trylock_irqsave(&skge->tx_lock, flags))
/* Collision - tell upper layer to requeue */
return NETDEV_TX_LOCKED;
- }
if (unlikely(skge_avail(&skge->tx_ring) < skb_shinfo(skb)->nr_frags + 1)) {
if (!netif_queue_stopped(dev)) {
@@ -2326,12 +2325,13 @@ static int skge_xmit_frame(struct sk_buf
printk(KERN_WARNING PFX "%s: ring full when queue awake!\n",
dev->name);
}
- spin_unlock(&skge->tx_lock);
+ spin_unlock_irqrestore(&skge->tx_lock, flags);
return NETDEV_TX_BUSY;
}
- e = ring->to_use;
+ e = skge->tx_ring.to_use;
td = e->desc;
+ BUG_ON(td->control & BMU_OWN);
e->skb = skb;
len = skb_headlen(skb);
map = pci_map_single(hw->pdev, skb->data, len, PCI_DMA_TODEVICE);
@@ -2372,8 +2372,10 @@ static int skge_xmit_frame(struct sk_buf
frag->size, PCI_DMA_TODEVICE);
e = e->next;
- e->skb = NULL;
+ e->skb = skb;
tf = e->desc;
+ BUG_ON(tf->control & BMU_OWN);
+
tf->dma_lo = map;
tf->dma_hi = (u64) map >> 32;
pci_unmap_addr_set(e, mapaddr, map);
@@ -2390,56 +2392,68 @@ static int skge_xmit_frame(struct sk_buf
skge_write8(hw, Q_ADDR(txqaddr[skge->port], Q_CSR), CSR_START);
- if (netif_msg_tx_queued(skge))
+ if (unlikely(netif_msg_tx_queued(skge)))
printk(KERN_DEBUG "%s: tx queued, slot %td, len %d\n",
- dev->name, e - ring->start, skb->len);
+ dev->name, e - skge->tx_ring.start, skb->len);
- ring->to_use = e->next;
+ skge->tx_ring.to_use = e->next;
if (skge_avail(&skge->tx_ring) <= TX_LOW_WATER) {
pr_debug("%s: transmit queue full\n", dev->name);
netif_stop_queue(dev);
}
- mmiowb();
- spin_unlock(&skge->tx_lock);
+ spin_unlock_irqrestore(&skge->tx_lock, flags);
dev->trans_start = jiffies;
return NETDEV_TX_OK;
}
-static void skge_tx_complete(struct skge_port *skge, struct skge_element *last)
+
+/* Free resources associated with this reing element */
+static void skge_tx_free(struct skge_port *skge, struct skge_element *e,
+ u32 control)
{
struct pci_dev *pdev = skge->hw->pdev;
- struct skge_element *e;
- for (e = skge->tx_ring.to_clean; e != last; e = e->next) {
- struct sk_buff *skb = e->skb;
- int i;
+ BUG_ON(!e->skb);
- e->skb = NULL;
+ /* skb header vs. fragment */
+ if (control & BMU_STF)
pci_unmap_single(pdev, pci_unmap_addr(e, mapaddr),
- skb_headlen(skb), PCI_DMA_TODEVICE);
+ pci_unmap_len(e, maplen),
+ PCI_DMA_TODEVICE);
+ else
+ pci_unmap_page(pdev, pci_unmap_addr(e, mapaddr),
+ pci_unmap_len(e, maplen),
+ PCI_DMA_TODEVICE);
- for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
- e = e->next;
- pci_unmap_page(pdev, pci_unmap_addr(e, mapaddr),
- skb_shinfo(skb)->frags[i].size,
- PCI_DMA_TODEVICE);
- }
+ if (control & BMU_EOF) {
+ if (unlikely(netif_msg_tx_done(skge)))
+ printk(KERN_DEBUG PFX "%s: tx done slot %td\n",
+ skge->netdev->name, e - skge->tx_ring.start);
- dev_kfree_skb(skb);
+ dev_kfree_skb_any(e->skb);
}
- skge->tx_ring.to_clean = e;
+ e->skb = NULL;
}
+/* Free all buffers in transmit ring */
static void skge_tx_clean(struct skge_port *skge)
{
+ struct skge_element *e;
+ unsigned long flags;
- spin_lock_bh(&skge->tx_lock);
- skge_tx_complete(skge, skge->tx_ring.to_use);
+ spin_lock_irqsave(&skge->tx_lock, flags);
+ for (e = skge->tx_ring.to_clean; e != skge->tx_ring.to_use; e = e->next) {
+ struct skge_tx_desc *td = e->desc;
+ skge_tx_free(skge, e, td->control);
+ td->control = 0;
+ }
+
+ skge->tx_ring.to_clean = e;
netif_wake_queue(skge->netdev);
- spin_unlock_bh(&skge->tx_lock);
+ spin_unlock_irqrestore(&skge->tx_lock, flags);
}
static void skge_tx_timeout(struct net_device *dev)
@@ -2665,32 +2679,28 @@ resubmit:
return NULL;
}
-static void skge_tx_done(struct skge_port *skge)
+/* Free all buffers in Tx ring which are no longer owned by device */
+static void skge_txirq(struct net_device *dev)
{
+ struct skge_port *skge = netdev_priv(dev);
struct skge_ring *ring = &skge->tx_ring;
- struct skge_element *e, *last;
+ struct skge_element *e;
+
+ rmb();
spin_lock(&skge->tx_lock);
- last = ring->to_clean;
for (e = ring->to_clean; e != ring->to_use; e = e->next) {
struct skge_tx_desc *td = e->desc;
if (td->control & BMU_OWN)
break;
- if (td->control & BMU_EOF) {
- last = e->next;
- if (unlikely(netif_msg_tx_done(skge)))
- printk(KERN_DEBUG PFX "%s: tx done slot %td\n",
- skge->netdev->name, e - ring->start);
- }
+ skge_tx_free(skge, e, td->control);
}
+ skge->tx_ring.to_clean = e;
- skge_tx_complete(skge, last);
-
- skge_write8(skge->hw, Q_ADDR(txqaddr[skge->port], Q_CSR), CSR_IRQ_CL_F);
-
- if (skge_avail(&skge->tx_ring) > TX_LOW_WATER)
+ if (netif_queue_stopped(skge->netdev)
+ && skge_avail(&skge->tx_ring) > TX_LOW_WATER)
netif_wake_queue(skge->netdev);
spin_unlock(&skge->tx_lock);
@@ -2705,8 +2715,6 @@ static int skge_poll(struct net_device *
int to_do = min(dev->quota, *budget);
int work_done = 0;
- skge_tx_done(skge);
-
for (e = ring->to_clean; prefetch(e->next), work_done < to_do; e = e->next) {
struct skge_rx_desc *rd = e->desc;
struct sk_buff *skb;
@@ -2738,10 +2746,12 @@ static int skge_poll(struct net_device *
return 1; /* not done */
netif_rx_complete(dev);
- mmiowb();
- hw->intr_mask |= skge->port == 0 ? (IS_R1_F|IS_XA1_F) : (IS_R2_F|IS_XA2_F);
+ spin_lock_irq(&hw->hw_lock);
+ hw->intr_mask |= rxirqmask[skge->port];
skge_write32(hw, B0_IMSK, hw->intr_mask);
+ mmiowb();
+ spin_unlock_irq(&hw->hw_lock);
return 0;
}
@@ -2871,8 +2881,10 @@ static void skge_extirq(void *arg)
}
mutex_unlock(&hw->phy_mutex);
+ spin_lock_irq(&hw->hw_lock);
hw->intr_mask |= IS_EXT_REG;
skge_write32(hw, B0_IMSK, hw->intr_mask);
+ spin_unlock_irq(&hw->hw_lock);
}
static irqreturn_t skge_intr(int irq, void *dev_id, struct pt_regs *regs)
@@ -2885,54 +2897,68 @@ static irqreturn_t skge_intr(int irq, vo
if (status == 0)
return IRQ_NONE;
+ spin_lock(&hw->hw_lock);
+ status &= hw->intr_mask;
if (status & IS_EXT_REG) {
hw->intr_mask &= ~IS_EXT_REG;
schedule_work(&hw->phy_work);
}
- if (status & (IS_R1_F|IS_XA1_F)) {
- skge_write8(hw, Q_ADDR(Q_R1, Q_CSR), CSR_IRQ_CL_F);
- hw->intr_mask &= ~(IS_R1_F|IS_XA1_F);
- netif_rx_schedule(hw->dev[0]);
+ if (status & IS_XA1_F) {
+ skge_write8(hw, Q_ADDR(Q_XA1, Q_CSR), CSR_IRQ_CL_F);
+ skge_txirq(hw->dev[0]);
}
- if (status & (IS_R2_F|IS_XA2_F)) {
- skge_write8(hw, Q_ADDR(Q_R2, Q_CSR), CSR_IRQ_CL_F);
- hw->intr_mask &= ~(IS_R2_F|IS_XA2_F);
- netif_rx_schedule(hw->dev[1]);
+ if (status & IS_R1_F) {
+ skge_write8(hw, Q_ADDR(Q_R1, Q_CSR), CSR_IRQ_CL_F);
+ hw->intr_mask &= ~IS_R1_F;
+ netif_rx_schedule(hw->dev[0]);
}
- if (likely((status & hw->intr_mask) == 0))
- return IRQ_HANDLED;
+ if (status & IS_PA_TO_TX1)
+ skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_TX1);
if (status & IS_PA_TO_RX1) {
struct skge_port *skge = netdev_priv(hw->dev[0]);
+
++skge->net_stats.rx_over_errors;
skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_RX1);
}
- if (status & IS_PA_TO_RX2) {
- struct skge_port *skge = netdev_priv(hw->dev[1]);
- ++skge->net_stats.rx_over_errors;
- skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_RX2);
- }
-
- if (status & IS_PA_TO_TX1)
- skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_TX1);
-
- if (status & IS_PA_TO_TX2)
- skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_TX2);
if (status & IS_MAC1)
skge_mac_intr(hw, 0);
- if (status & IS_MAC2)
- skge_mac_intr(hw, 1);
+ if (hw->dev[1]) {
+ if (status & IS_XA2_F) {
+ skge_write8(hw, Q_ADDR(Q_XA2, Q_CSR), CSR_IRQ_CL_F);
+ skge_txirq(hw->dev[1]);
+ }
+
+ if (status & IS_R2_F) {
+ skge_write8(hw, Q_ADDR(Q_R2, Q_CSR), CSR_IRQ_CL_F);
+ hw->intr_mask &= ~IS_R2_F;
+ netif_rx_schedule(hw->dev[1]);
+ }
+
+ if (status & IS_PA_TO_RX2) {
+ struct skge_port *skge = netdev_priv(hw->dev[1]);
+ ++skge->net_stats.rx_over_errors;
+ skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_RX2);
+ }
+
+ if (status & IS_PA_TO_TX2)
+ skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_TX2);
+
+ if (status & IS_MAC2)
+ skge_mac_intr(hw, 1);
+ }
if (status & IS_HW_ERR)
skge_error_irq(hw);
skge_write32(hw, B0_IMSK, hw->intr_mask);
+ spin_unlock(&hw->hw_lock);
return IRQ_HANDLED;
}
@@ -3083,6 +3109,7 @@ static int skge_reset(struct skge_hw *hw
else
hw->ram_size = t8 * 4096;
+ spin_lock_init(&hw->hw_lock);
hw->intr_mask = IS_HW_ERR | IS_EXT_REG | IS_PORT_1;
if (hw->ports > 1)
hw->intr_mask |= IS_PORT_2;
@@ -3389,7 +3416,11 @@ static void __devexit skge_remove(struct
dev0 = hw->dev[0];
unregister_netdev(dev0);
+ spin_lock_irq(&hw->hw_lock);
+ hw->intr_mask = 0;
skge_write32(hw, B0_IMSK, 0);
+ spin_unlock_irq(&hw->hw_lock);
+
skge_write16(hw, B0_LED, LED_STAT_OFF);
skge_write8(hw, B0_CTST, CS_RST_SET);
--- skge-2.6.orig/drivers/net/skge.h
+++ skge-2.6/drivers/net/skge.h
@@ -2388,6 +2388,7 @@ struct skge_ring {
struct skge_hw {
void __iomem *regs;
struct pci_dev *pdev;
+ spinlock_t hw_lock;
u32 intr_mask;
struct net_device *dev[2];
--
^ permalink raw reply
* [PATCH 5/5] skge: version 1.6
From: Stephen Hemminger @ 2006-06-06 17:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20060606171110.592587000@zqx3.pdx.osdl.net>
[-- Attachment #1: skge-1.6.patch --]
[-- Type: text/plain, Size: 338 bytes --]
Update version string.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- skge-2.6.orig/drivers/net/skge.c
+++ skge-2.6/drivers/net/skge.c
@@ -44,7 +44,7 @@
#include "skge.h"
#define DRV_NAME "skge"
-#define DRV_VERSION "1.5"
+#define DRV_VERSION "1.6"
#define PFX DRV_NAME " "
#define DEFAULT_TX_RING_SIZE 128
--
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Auke Kok @ 2006-06-06 17:18 UTC (permalink / raw)
To: Garzik, Jeff
Cc: Neil Horman, Mitch Williams, netdev, Brandeburg, Jesse, Kok, Auke,
jmoyer, mpm
In-Reply-To: <20060606170513.GB21969@hmsreliant.homelinux.net>
Neil Horman wrote:
> On Tue, Jun 06, 2006 at 09:39:25AM -0700, Mitch Williams wrote:
>> On Tue, 2006-06-06 at 09:52 -0400, Neil Horman wrote:
[snip]
>> However, just for the sake of correctness (and paranoia), I'll whip up
>> another patch that does this check.
>>
> Thanks for the quick feedback!
>
> Regards
> Neil
>
>> Jeff, please do not commit this patch.
Jeff,
I've popped the patch off from our gitserver, so you can pull the two
outstanding patches while we revamp this one.
Auke
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox