Netdev List
 help / color / mirror / Atom feed
* [PATCH 0/7] AlacrityVM guest drivers
From: Gregory Haskins @ 2009-08-03 17:17 UTC (permalink / raw)
  To: linux-kernel; +Cc: alacrityvm-devel, netdev

(Applies to v2.6.31-rc5, proposed for linux-next after review is complete)

This series implements the guest-side drivers for accelerated IO
when running on top of the AlacrityVM hypervisor, the details of
which you can find here:

http://developer.novell.com/wiki/index.php/AlacrityVM

This series includes the basic plumbing, as well as the driver for
accelerated 802.x (ethernet) networking.

Regards,
-Greg

---

Gregory Haskins (7):
      venet: add scatter-gather/GSO support
      net: Add vbus_enet driver
      ioq: add driver-side vbus helpers
      vbus-proxy: add a pci-to-vbus bridge
      vbus: add a "vbus-proxy" bus model for vbus_driver objects
      ioq: Add basic definitions for a shared-memory, lockless queue
      shm-signal: shared-memory signals


 arch/x86/Kconfig            |    2 
 drivers/Makefile            |    1 
 drivers/net/Kconfig         |   14 +
 drivers/net/Makefile        |    1 
 drivers/net/vbus-enet.c     |  899 +++++++++++++++++++++++++++++++++++++++++++
 drivers/vbus/Kconfig        |   24 +
 drivers/vbus/Makefile       |    6 
 drivers/vbus/bus-proxy.c    |  216 ++++++++++
 drivers/vbus/pci-bridge.c   |  824 +++++++++++++++++++++++++++++++++++++++
 include/linux/Kbuild        |    4 
 include/linux/ioq.h         |  415 ++++++++++++++++++++
 include/linux/shm_signal.h  |  189 +++++++++
 include/linux/vbus_driver.h |   80 ++++
 include/linux/vbus_pci.h    |  127 ++++++
 include/linux/venet.h       |   84 ++++
 lib/Kconfig                 |   21 +
 lib/Makefile                |    2 
 lib/ioq.c                   |  294 ++++++++++++++
 lib/shm_signal.c            |  192 +++++++++
 19 files changed, 3395 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/vbus-enet.c
 create mode 100644 drivers/vbus/Kconfig
 create mode 100644 drivers/vbus/Makefile
 create mode 100644 drivers/vbus/bus-proxy.c
 create mode 100644 drivers/vbus/pci-bridge.c
 create mode 100644 include/linux/ioq.h
 create mode 100644 include/linux/shm_signal.h
 create mode 100644 include/linux/vbus_driver.h
 create mode 100644 include/linux/vbus_pci.h
 create mode 100644 include/linux/venet.h
 create mode 100644 lib/ioq.c
 create mode 100644 lib/shm_signal.c

-- 
Signature

^ permalink raw reply

* [RFC PATCH v1] tun: Cleanup error handling in tun_set_iff()
From: Paul Moore @ 2009-08-03 16:12 UTC (permalink / raw)
  To: netdev

Convert some pointless gotos into returns and ensure tun_attach() errors are
handled correctly.

Signed-off-by: Paul Moore <paul.moore@hp.com>

--

I'm sending this as an RFC patch because I'm not entirely certain that the
changes to the tun_attach() error handling code are 100% correct, although I
strongly suspect that the current behavor of simply returning an error code
without any cleanup is broken.  Can anyone comment on this?
---

 drivers/net/tun.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 4a0db7a..b6d06fd 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -938,13 +938,10 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 		err = tun_attach(tun, file);
 		if (err < 0)
 			return err;
-	}
-	else {
+	} else {
 		char *name;
 		unsigned long flags = 0;
 
-		err = -EINVAL;
-
 		if (!capable(CAP_NET_ADMIN))
 			return -EPERM;
 
@@ -958,7 +955,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 			flags |= TUN_TAP_DEV;
 			name = "tap%d";
 		} else
-			goto failed;
+			return -EINVAL;
 
 		if (*ifr->ifr_name)
 			name = ifr->ifr_name;
@@ -976,10 +973,11 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 		tun->flags = flags;
 		tun->txflt.count = 0;
 
-		err = -ENOMEM;
 		sk = sk_alloc(net, AF_UNSPEC, GFP_KERNEL, &tun_proto);
-		if (!sk)
+		if (!sk) {
+			err = -ENOMEM;
 			goto err_free_dev;
+		}
 
 		init_waitqueue_head(&tun->socket.wait);
 		sock_init_data(&tun->socket, sk);
@@ -1010,7 +1008,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 
 		err = tun_attach(tun, file);
 		if (err < 0)
-			goto failed;
+			goto err_unreg_dev;
 	}
 
 	DBG(KERN_INFO "%s: tun_set_iff\n", tun->dev->name);
@@ -1039,11 +1037,14 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 	strcpy(ifr->ifr_name, tun->dev->name);
 	return 0;
 
+ err_unreg_dev:
+	rtnl_lock();
+	unregister_netdevice(tun->dev);
+	rtnl_unlock();
  err_free_sk:
 	sock_put(sk);
  err_free_dev:
 	free_netdev(dev);
- failed:
 	return err;
 }
 


^ permalink raw reply related

* Re: [PATCH 1/5] sky2: Avoid rewinding sky2->tx_prod
From: Stephen Hemminger @ 2009-08-03 16:11 UTC (permalink / raw)
  To: Mike McCormack; +Cc: netdev
In-Reply-To: <392fb48f0908030407j69f32c6qed9529f1219ee474@mail.gmail.com>

On Mon, 3 Aug 2009 20:07:58 +0900
Mike McCormack <mikem@ring3k.org> wrote:

> ---
>  drivers/net/sky2.c |   34 ++++++++++++++++++----------------
>  1 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
> index 661abd0..16a359d 100644
> --- a/drivers/net/sky2.c
> +++ b/drivers/net/sky2.c
> @@ -989,11 +989,11 @@ static void sky2_prefetch_init(struct sky2_hw
> *hw, u32 qaddr,
>  	sky2_read32(hw, Y2_QADDR(qaddr, PREF_UNIT_CTRL));

Your mailer line wrapped the patch making it not apply!

^ permalink raw reply

* Re: 2.6.31-rc5: Reported regressions from 2.6.30
From: Rafael J. Wysocki @ 2009-08-03 16:09 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Linus Torvalds, Linux Kernel Mailing List, Adrian Bunk,
	Andrew Morton, Natalie Protasevich, Kernel Testers List,
	Network Development, Linux ACPI, Linux PM List, Linux SCSI List,
	Linux Wireless List, DRI, Mikael Pettersson
In-Reply-To: <20090803143108.GA12041-druUgvl0LCNAfugRpC6u6w@public.gmane.org>

On Monday 03 August 2009, Johannes Weiner wrote:
> On Sun, Aug 02, 2009 at 03:22:01PM -0700, Linus Torvalds wrote:
> 
> > > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=13815
> > > Subject		: emacs -nw compilation doesn't show the error message
> > > Submitter	: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> > > Date		: 2009-07-23 06:22 (11 days old)
> > > First-Bad-Commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d945cb9cce20ac7143c2de8d88b187f62db99bdc
> > 
> > Same old pty bug, same fix: commit e043e42bdb.
> > 
> > > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=13813
> > > Subject		: Hangups in n_tty_read()
> > > Submitter	: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
> > > Date		: 2009-07-16 18:48 (18 days old)
> > > References	: http://marc.info/?l=linux-kernel&m=124777019920579&w=4
> > 
> > Hmm. Worth testing that same fix, although the symptoms here are not 
> > exactly the same.
> 
> Hm, I had no debugging enabled, but I think a silent hang in
> 
> 	pty_write()
> 	  tty_flip_buffer_push()
> 	    flush_to_ldisc()
> 
> can cause the receiving side
> 
> 	tty_read()
> 	  n_tty_read()
> 
> to wait forever for updates.  I haven't looked closely.  It seems to
> be gone with your latest master in any case.

Thanks, closed (in fact I closed it earlier already).

Rafael

^ permalink raw reply

* Re: [PATCH] sky2: Avoid transmits during sky2_down()
From: Stephen Hemminger @ 2009-08-03 15:59 UTC (permalink / raw)
  To: David Miller; +Cc: mikem, netdev
In-Reply-To: <20090802.130912.180264634.davem@davemloft.net>

On Sun, 02 Aug 2009 13:09:12 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Mike McCormack <mikem@ring3k.org>
> Date: Fri, 31 Jul 2009 20:57:42 +0900
> 
> > I am aware you object to storing extra state, but I can't see a way
> > around this. Without remembering that we're restarting,
> > netif_wake_queue() is called in the ISR from sky2_tx_complete(), and
> > netif_tx_lock() is used in sky2_tx_done().  If anybody can see a way
> > around this, please let me know.
> 
> Stephen please do something about this, soon.
> 
> Michael has been trying to get this fixed for more than a month, and
> if you don't have a better fix like right now then we should put his
> fix in for the time being.
> 
> Sky2 patches are ones that consistently rot in patchwork and I'd
> appreciate if it that trend would cease, thanks.

Well, Mike's next set of patches is great, so the wait was worth it.

Some of us developer's get paid for doing things. There was a little
thing like a major release from our small engineering organization
at Vyatta to get done. Unlike the cushy life at other companies, in
a small company the engineers have to deal with customer issues,
bugzilla bugs.


^ permalink raw reply

* Re: [PATCH 0/5] sky2: Resolve sky2_down/transmit races, lockless transmit
From: Stephen Hemminger @ 2009-08-03 15:52 UTC (permalink / raw)
  To: Mike McCormack; +Cc: netdev
In-Reply-To: <392fb48f0908030407v6fa32f23pb10ca91ecfb2fd67@mail.gmail.com>

On Mon, 3 Aug 2009 20:07:49 +0900
Mike McCormack <mikem@ring3k.org> wrote:

> Hi Stephen,
> 
> Here is an alternate (and more intrusive) patch set to solve the
> transmit during restart issues.
> 
> These patches avoid introducing a new state variable by
> 
> * reworking the transmit queue to work without locks
> * tweaking sky2_down() to avoid invalid tx queue positions being
> received in the status queue
> 
> Please let me know if this looks better,

Looks good, I will checkout on a couple of (older) boards as well.


^ permalink raw reply

* Re: linux-next: Tree for August 3 (xfrm4)
From: Randy Dunlap @ 2009-08-03 15:49 UTC (permalink / raw)
  To: Stephen Rothwell, netdev; +Cc: linux-next, LKML
In-Reply-To: <20090803185203.a96063fd.sfr@canb.auug.org.au>

On Mon, 3 Aug 2009 18:52:03 +1000 Stephen Rothwell wrote:

> Hi all,
> 
> Changes since 20090731:


when PROC_FS=n, PROC_SYSCTL=n:

(.init.text+0x41ed): undefined reference to `net_ipv4_ctl_path'
(.init.text+0x420b): undefined reference to `register_net_sysctl_table'
xfrm4_policy.c:(.exit.text+0x3c3): undefined reference to `unregister_net_sysctl_table'


---
~Randy
LPC 2009, Sept. 23-25, Portland, Oregon
http://linuxplumbersconf.org/2009/

^ permalink raw reply

* Re: [PATCH 1/5] sky2: Avoid rewinding sky2->tx_prod
From: Stephen Hemminger @ 2009-08-03 15:48 UTC (permalink / raw)
  To: Mike McCormack; +Cc: netdev
In-Reply-To: <392fb48f0908030407j69f32c6qed9529f1219ee474@mail.gmail.com>

I agree this is good, but more explanation in the commit log would be helpful
as a record.

Note: the unwind code should not ever be called on x86, since dma_mapping always
succeeds. 


Acked-by: Stephen Hemminger <shemminger@vyatta.com>

^ permalink raw reply

* Re: 2.6.31-rc5: Reported regressions from 2.6.30
From: Rafael J. Wysocki @ 2009-08-03 15:36 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux Kernel Mailing List, Adrian Bunk, Andrew Morton,
	Natalie Protasevich, Kernel Testers List, Network Development,
	Linux ACPI, Linux PM List, Linux SCSI List, Linux Wireless List,
	DRI, Mikael Pettersson
In-Reply-To: <alpine.LFD.2.01.0908021456430.3352@localhost.localdomain>

On Monday 03 August 2009, Linus Torvalds wrote:
> 
> On Sun, 2 Aug 2009, Rafael J. Wysocki wrote:
> > 
> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=13896
> > Subject		: 2.6.31-rc4 broke expect and gcc's testsuite
> > Submitter	: Mikael Pettersson <mikpe@it.uu.se>
> > Date		: 2009-07-29 11:00 (5 days old)
> > References	: http://marc.info/?l=linux-kernel&m=124885806406520&w=4
> 
> I bet this is the same tty bug that got fixed by Ogawa in commit 
> e043e42bdb. -rc5 has that fix. Mikael?
> 
> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=13842
> > Subject		: Oops when writing to /sys/block/ram0/queue/max_sectors_kb
> > Submitter	: Jens Rosenboom <jens@mcbone.net>
> > Date		: 2009-07-23 15:30 (11 days old)
> > References	: http://marc.info/?l=linux-kernel&m=124836574403032&w=4
> 
> Commit a4e7d46407d73f35d217013b363b79a8f8eafcaa fixed this one.
> 
> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=13826
> > Subject		: thinkpad boots with backlight low
> > Submitter	: Pavel Machek <pavel@ucw.cz>
> > Date		: 2009-07-15 15:13 (19 days old)
> > References	: http://marc.info/?l=linux-kernel&m=124756359126830&w=4
> > Handled-By	: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> 
> This should be commit 59fe4fe34d7afdf63208124f313be9056feaa2f4.
> 
> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=13815
> > Subject		: emacs -nw compilation doesn't show the error message
> > Submitter	: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > Date		: 2009-07-23 06:22 (11 days old)
> > First-Bad-Commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d945cb9cce20ac7143c2de8d88b187f62db99bdc
> 
> Same old pty bug, same fix: commit e043e42bdb.
> 
> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=13813
> > Subject		: Hangups in n_tty_read()
> > Submitter	: Johannes Weiner <hannes@cmpxchg.org>
> > Date		: 2009-07-16 18:48 (18 days old)
> > References	: http://marc.info/?l=linux-kernel&m=124777019920579&w=4
> 
> Hmm. Worth testing that same fix, although the symptoms here are not 
> exactly the same.
> 
> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=13812
> > Subject		: Ooops on uplug
> > Submitter	: Daniel Mack <daniel@caiaq.de>
> > Date		: 2009-07-20 17:51 (14 days old)
> > References	: http://marc.info/?l=linux-kernel&m=124811234302786&w=4
> > Handled-By	: Alan Stern <stern@rowland.harvard.edu>
> > 		  Alan Cox <alan@linux.intel.com>
> 
> Commit c56d3000861 should fix this.
> 
> > Regressions with patches
> > ------------------------
> > 
> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=13891
> > Subject		: PCI resources allocation problem on HP nx6325
> > Submitter	: Rafael J. Wysocki <rjw@sisk.pl>
> > Date		: 2009-08-02 13:37 (1 days old)
> > Handled-By	: Linus Torvalds <torvalds@linux-foundation.org>
> > Patch		: http://patchwork.kernel.org/patch/38774/
> 
> Ok, this is committed as 79896cf42f6a96d7e14f2dc3473443d68d74031d.
> 
> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=13825
> > Subject		: eeepc-laptop: fix hot-unplug on resume
> > Submitter	: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > Date		: 2009-06-29 13:12 (35 days old)
> > References	: http://lkml.org/lkml/2009/6/29/150
> > Handled-By	: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > Patch		: http://patchwork.kernel.org/patch/32926/
> 
> commit 7334546a52c6764df120459509b1f803a073eacc

Thanks a lot!

All closed except for http://bugzilla.kernel.org/show_bug.cgi?id=13813.

Best,
Rafael

^ permalink raw reply

* Re: 2.6.31-rc5: Reported regressions 2.6.29 -> 2.6.30
From: Luis R. Rodriguez @ 2009-08-03 15:16 UTC (permalink / raw)
  To: Rafael J. Wysocki, rootkit85-whZMOeQn8C0
  Cc: Linux Kernel Mailing List, Andrew Morton, Linus Torvalds,
	Natalie Protasevich, Kernel Testers List, Network Development,
	Linux ACPI, Linux PM List, Linux SCSI List, Linux Wireless List,
	DRI
In-Reply-To: <upey9ZSfnaF.A.-oE.rpgdKB@chimera>

Here are the wireless ones:

On Sun, Aug 2, 2009 at 12:06 PM, Rafael J. Wysocki<rjw-KKrjLPT3xs0@public.gmane.org> wrote:

> Unresolved regressions
> ----------------------
>
> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=13898
> Subject         : Intel 3945ABG - problems on 2.6.30.X
> Submitter       : dienet <dienet-IjDXvh/HVVUAvxtiuMwx3w@public.gmane.org>
> Date            : 2009-07-31 15:17 (3 days old)
> References      : http://marc.info/?l=linux-kernel&m=124905346729959&w=4


> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=13638
> Subject         : rt2870 driver is broken for (some) cards
> Submitter       : jakob gruber <jakob.gruber@kabelnet.at>
> Date            : 2009-06-27 17:33 (37 days old)



> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=13581
> Subject         : ath9k doesn't work with newer kernels
> Submitter       : Matteo <rootkit85-whZMOeQn8C0@public.gmane.org>
> Date            : 2009-06-19 12:04 (45 days old)

Issue looks more like a the user has rfkill enabled rather than a
driver issue. Waiting on user feedback since 2009-07-27.

> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=13362
> Subject         : rt2x00: slow wifi with correct basic rate bitmap
> Submitter       : Alejandro Riveira <ariveira-Re5JQEeQqe8@public.gmane.orgm>
> Date            : 2009-05-22 13:32 (73 days old)



> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=13337
> Subject         : [post 2.6.29 regression] hang during suspend of b44/b43 modules
> Submitter       : Tomas Janousek <tomi-YoqI/XImC7s@public.gmane.org>
> Date            : 2009-05-18 10:59 (77 days old)
> Handled-By      : Johannes Berg <johannes@sipsolutions.net>
> Patch           : http://patchwork.kernel.org/patch/37837/


> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=13319
> Subject         : Page allocation failures with b43 and p54usb
> Submitter       : Larry Finger <Larry.Finger@lwfinger.net>
> Date            : 2009-04-29 21:01 (96 days old)
> References      : http://marc.info/?l=linux-kernel&m=124103897101088&w=4
>                  http://lkml.org/lkml/2009/6/7/136
>                  http://lkml.org/lkml/2009/7/26/213
> Handled-By      : Johannes Berg <johannes@sipsolutions.net>
>                  David Rientjes <rientjes-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Patch           : http://patchwork.kernel.org/patch/37655/
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: 2.6.31-rc5: Reported regressions from 2.6.30
From: Johannes Weiner @ 2009-08-03 14:31 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Rafael J. Wysocki, Linux Kernel Mailing List, Adrian Bunk,
	Andrew Morton, Natalie Protasevich, Kernel Testers List,
	Network Development, Linux ACPI, Linux PM List, Linux SCSI List,
	Linux Wireless List, DRI, Mikael Pettersson
In-Reply-To: <alpine.LFD.2.01.0908021456430.3352@localhost.localdomain>

On Sun, Aug 02, 2009 at 03:22:01PM -0700, Linus Torvalds wrote:

> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=13815
> > Subject		: emacs -nw compilation doesn't show the error message
> > Submitter	: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > Date		: 2009-07-23 06:22 (11 days old)
> > First-Bad-Commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d945cb9cce20ac7143c2de8d88b187f62db99bdc
> 
> Same old pty bug, same fix: commit e043e42bdb.
> 
> > Bug-Entry	: http://bugzilla.kernel.org/show_bug.cgi?id=13813
> > Subject		: Hangups in n_tty_read()
> > Submitter	: Johannes Weiner <hannes@cmpxchg.org>
> > Date		: 2009-07-16 18:48 (18 days old)
> > References	: http://marc.info/?l=linux-kernel&m=124777019920579&w=4
> 
> Hmm. Worth testing that same fix, although the symptoms here are not 
> exactly the same.

Hm, I had no debugging enabled, but I think a silent hang in

	pty_write()
	  tty_flip_buffer_push()
	    flush_to_ldisc()

can cause the receiving side

	tty_read()
	  n_tty_read()

to wait forever for updates.  I haven't looked closely.  It seems to
be gone with your latest master in any case.

	Hannes

^ permalink raw reply

* [PATCH]bluetooth: rfcomm_init bug fix
From: Dave Young @ 2009-08-03 14:26 UTC (permalink / raw)
  To: marcel; +Cc: Oliver Hartkopp, Alan Cox, netdev, linux-bluetooth, linux-kernel

rfcomm tty may be used before rfcomm_tty_driver initilized,
The problem is that now socket layer init before tty layer, if userspace
program do socket callback right here then oops will happen.

reporting in:
http://marc.info/?l=linux-bluetooth&m=124404919324542&w=2

make 3 changes:
1. remove #ifdef in rfcomm/core.c,
make it blank function when rfcomm tty not selected in rfcomm.h

2. tune the rfcomm_init error patch to ensure
tty driver initilized before rfcomm socket usage.

3. remove __exit for rfcomm_cleanup_sockets
because above change need call it in a __init function. 


CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
Reported-by: Oliver Hartkopp <oliver@hartkopp.net>
Tested-by: Oliver Hartkopp <oliver@hartkopp.net>
Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
--
include/net/bluetooth/rfcomm.h |   12 +++++++++++-
net/bluetooth/rfcomm/core.c    |   29 ++++++++++++++++++++---------
net/bluetooth/rfcomm/sock.c    |    2 +-
3 files changed, 32 insertions(+), 11 deletions(-)

--- linux-2.6.orig/include/net/bluetooth/rfcomm.h	2009-08-01 13:56:53.000000000 +0800
+++ linux-2.6/include/net/bluetooth/rfcomm.h	2009-08-01 17:24:59.000000000 +0800
@@ -355,7 +355,17 @@ struct rfcomm_dev_list_req {
 };
 
 int  rfcomm_dev_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
+
+#ifdef CONFIG_BT_RFCOMM_TTY
 int  rfcomm_init_ttys(void);
 void rfcomm_cleanup_ttys(void);
-
+#else
+static inline int rfcomm_init_ttys(void)
+{
+	return 0;
+}
+static inline void rfcomm_cleanup_ttys(void)
+{
+}
+#endif
 #endif /* __RFCOMM_H */
--- linux-2.6.orig/net/bluetooth/rfcomm/core.c	2009-08-01 13:56:53.000000000 +0800
+++ linux-2.6/net/bluetooth/rfcomm/core.c	2009-08-01 13:57:18.000000000 +0800
@@ -2080,28 +2080,41 @@ static CLASS_ATTR(rfcomm_dlc, S_IRUGO, r
 /* ---- Initialization ---- */
 static int __init rfcomm_init(void)
 {
+	int ret;
+
 	l2cap_load();
 
 	hci_register_cb(&rfcomm_cb);
 
 	rfcomm_thread = kthread_run(rfcomm_run, NULL, "krfcommd");
 	if (IS_ERR(rfcomm_thread)) {
-		hci_unregister_cb(&rfcomm_cb);
-		return PTR_ERR(rfcomm_thread);
+		ret = PTR_ERR(rfcomm_thread);
+		goto out_thread;
 	}
 
 	if (class_create_file(bt_class, &class_attr_rfcomm_dlc) < 0)
 		BT_ERR("Failed to create RFCOMM info file");
 
-	rfcomm_init_sockets();
-
-#ifdef CONFIG_BT_RFCOMM_TTY
-	rfcomm_init_ttys();
-#endif
+	ret = rfcomm_init_ttys();
+	if (ret)
+		goto out_tty;
+
+	ret = rfcomm_init_sockets();
+	if (ret)
+		goto out_sock;
 
 	BT_INFO("RFCOMM ver %s", VERSION);
 
 	return 0;
+
+out_sock:
+	rfcomm_cleanup_ttys();
+out_tty:
+	kthread_stop(rfcomm_thread);
+out_thread:
+	hci_unregister_cb(&rfcomm_cb);
+
+	return ret;
 }
 
 static void __exit rfcomm_exit(void)
@@ -2112,9 +2125,7 @@ static void __exit rfcomm_exit(void)
 
 	kthread_stop(rfcomm_thread);
 
-#ifdef CONFIG_BT_RFCOMM_TTY
 	rfcomm_cleanup_ttys();
-#endif
 
 	rfcomm_cleanup_sockets();
 }
--- linux-2.6.orig/net/bluetooth/rfcomm/sock.c	2009-08-01 13:56:53.000000000 +0800
+++ linux-2.6/net/bluetooth/rfcomm/sock.c	2009-08-01 13:57:18.000000000 +0800
@@ -1132,7 +1132,7 @@ error:
 	return err;
 }
 
-void __exit rfcomm_cleanup_sockets(void)
+void rfcomm_cleanup_sockets(void)
 {
 	class_remove_file(bt_class, &class_attr_rfcomm);
 

^ permalink raw reply

* radvd 1.4 released
From: Pekka Savola @ 2009-08-03 13:18 UTC (permalink / raw)
  To: netdev, radvd-announce-l; +Cc: radvd-devel-l

Hello,

A new version of radvd has been released.  This just fixes an 
unintentional change in 1.3: RAs were accidentally often unicast to 
solicitors instead of being multicast. This is still compliant with 
the specification but is not optimal.

Thanks to Ninad Adi, Elitecore Technologies/Cyberoam UTM for finding 
and reporting the problem found in IPv6 Ready testing.

Get it at: http://www.litech.org/radvd/

-- 
Pekka Savola                 "You each name yourselves king, yet the
Netcore Oy                    kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings


^ permalink raw reply

* Re: netpoll + xmit_lock == deadlock
From: Ben Hutchings @ 2009-08-03 12:05 UTC (permalink / raw)
  To: David Miller; +Cc: herbert, mpm, netdev, mcarlson
In-Reply-To: <20090802.130704.133993421.davem@davemloft.net>

On Sun, 2009-08-02 at 13:07 -0700, David Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Wed, 29 Jul 2009 15:35:23 +0800
> 
> > So if a driver is in the TX path, and a printk occurs, then a
> > recursive dead lock can occur if that driver tries to take the
> > xmit lock in its poll function to clean up descriptors.
> 
> My position has always been that such printk's are simply
> not allowed.  (check archives if you don't believe me :-)
>
> The locking is going to get rediculious if we start having
> to account for this.

I agree with that, but this does seem quite restrictive.  How can we be
sure that none of the kernel functions used by a driver's TX path (e.g.
kmalloc or DMA-mapping) will print debug or warning messages?  If such
guarantees exist, they do not seem to be documented.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: tty_register_device NULL pointer dereference in 2.6.31-rc4
From: Oliver Hartkopp @ 2009-08-03 12:03 UTC (permalink / raw)
  To: Dave Young; +Cc: Alan Cox, Marcel Holtmann, Linux Netdev List, linux-bluetooth
In-Reply-To: <20090801093207.GA2873@darkstar>

Dave Young wrote:
> On Sat, Aug 01, 2009 at 11:21:28AM +0200, Oliver Hartkopp wrote:
>>> --- linux-2.6.orig/include/net/bluetooth/rfcomm.h	2009-08-01 10:53:18.000000000 +0800
>>> +++ linux-2.6/include/net/bluetooth/rfcomm.h	2009-08-01 10:55:29.000000000 +0800
>>> @@ -355,7 +355,18 @@ struct rfcomm_dev_list_req {
>>>  };
>>>  
>>>  int  rfcomm_dev_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
>>> +
>>> +#ifdef CONFIG_BT_RFCOMM_TTY
>>>  int  rfcomm_init_ttys(void);
>>>  void rfcomm_cleanup_ttys(void);
>>> -
>>> +#else
>>> +static inline int rfcomm_init_ttys(void)
>>> +{
>>> +	return 0;
>>> +}
>>> +static inline int rfcomm_cleanup_ttys(void)
>>> +{
>>> +	return 0;
>>> +}
>>> +#endif
>> Just a minor thing:
>>
>> Does rfcomm_cleanup_ttys() return 'int' or is it 'void' ?
> 
> Yes it should be void. Thanks for pointing out, here update the patch:

Hi Dave,

the latest approach seems to fit.

I booted about 25+ times without any problems.
Even the upgrade from Debian Lenny to Squeeze did not cause any probs with
bluetooth.

Tested-by: Oliver Hartkopp <oliver@hartkopp.net>

Thanks!

> ---
> 
> rfcomm tty may be used before rfcomm_tty_driver initilized,
> The problem is that now socket layer init before tty layer, if userspace
> program do socket callback right here then oops will happen.
> 
> reporting in:
> http://marc.info/?l=linux-bluetooth&m=124404919324542&w=2
> 
> make 3 changes:
> 1. remove #ifdef in rfcomm/core.c,
> make it blank function when rfcomm tty not selected in rfcomm.h
> 
> 2. tune the rfcomm_init error patch to ensure
> tty driver initilized before rfcomm socket usage.
> 
> 3. remove __exit for rfcomm_cleanup_sockets
> because above change need call it in a __init function. 
> 
> 
> CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Reported-by: Oliver Hartkopp <oliver@hartkopp.net>
> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> --
> include/net/bluetooth/rfcomm.h |   12 +++++++++++-
> net/bluetooth/rfcomm/core.c    |   29 ++++++++++++++++++++---------
> net/bluetooth/rfcomm/sock.c    |    2 +-
> 3 files changed, 32 insertions(+), 11 deletions(-)
> 
> --- linux-2.6.orig/include/net/bluetooth/rfcomm.h	2009-08-01 13:56:53.000000000 +0800
> +++ linux-2.6/include/net/bluetooth/rfcomm.h	2009-08-01 17:24:59.000000000 +0800
> @@ -355,7 +355,17 @@ struct rfcomm_dev_list_req {
>  };
>  
>  int  rfcomm_dev_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
> +
> +#ifdef CONFIG_BT_RFCOMM_TTY
>  int  rfcomm_init_ttys(void);
>  void rfcomm_cleanup_ttys(void);
> -
> +#else
> +static inline int rfcomm_init_ttys(void)
> +{
> +	return 0;
> +}
> +static inline void rfcomm_cleanup_ttys(void)
> +{
> +}
> +#endif
>  #endif /* __RFCOMM_H */
> --- linux-2.6.orig/net/bluetooth/rfcomm/core.c	2009-08-01 13:56:53.000000000 +0800
> +++ linux-2.6/net/bluetooth/rfcomm/core.c	2009-08-01 13:57:18.000000000 +0800
> @@ -2080,28 +2080,41 @@ static CLASS_ATTR(rfcomm_dlc, S_IRUGO, r
>  /* ---- Initialization ---- */
>  static int __init rfcomm_init(void)
>  {
> +	int ret;
> +
>  	l2cap_load();
>  
>  	hci_register_cb(&rfcomm_cb);
>  
>  	rfcomm_thread = kthread_run(rfcomm_run, NULL, "krfcommd");
>  	if (IS_ERR(rfcomm_thread)) {
> -		hci_unregister_cb(&rfcomm_cb);
> -		return PTR_ERR(rfcomm_thread);
> +		ret = PTR_ERR(rfcomm_thread);
> +		goto out_thread;
>  	}
>  
>  	if (class_create_file(bt_class, &class_attr_rfcomm_dlc) < 0)
>  		BT_ERR("Failed to create RFCOMM info file");
>  
> -	rfcomm_init_sockets();
> -
> -#ifdef CONFIG_BT_RFCOMM_TTY
> -	rfcomm_init_ttys();
> -#endif
> +	ret = rfcomm_init_ttys();
> +	if (ret)
> +		goto out_tty;
> +
> +	ret = rfcomm_init_sockets();
> +	if (ret)
> +		goto out_sock;
>  
>  	BT_INFO("RFCOMM ver %s", VERSION);
>  
>  	return 0;
> +
> +out_sock:
> +	rfcomm_cleanup_ttys();
> +out_tty:
> +	kthread_stop(rfcomm_thread);
> +out_thread:
> +	hci_unregister_cb(&rfcomm_cb);
> +
> +	return ret;
>  }
>  
>  static void __exit rfcomm_exit(void)
> @@ -2112,9 +2125,7 @@ static void __exit rfcomm_exit(void)
>  
>  	kthread_stop(rfcomm_thread);
>  
> -#ifdef CONFIG_BT_RFCOMM_TTY
>  	rfcomm_cleanup_ttys();
> -#endif
>  
>  	rfcomm_cleanup_sockets();
>  }
> --- linux-2.6.orig/net/bluetooth/rfcomm/sock.c	2009-08-01 13:56:53.000000000 +0800
> +++ linux-2.6/net/bluetooth/rfcomm/sock.c	2009-08-01 13:57:18.000000000 +0800
> @@ -1132,7 +1132,7 @@ error:
>  	return err;
>  }
>  
> -void __exit rfcomm_cleanup_sockets(void)
> +void rfcomm_cleanup_sockets(void)
>  {
>  	class_remove_file(bt_class, &class_attr_rfcomm);
>  


^ permalink raw reply

* Re: Kernel oops on setting sky2 interfaces down
From: Rene Mayrhofer @ 2009-08-03 11:55 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Richard Leitner, mikem
In-Reply-To: <20090727153548.7c0d9f85@nehalam>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I have now tried again with the newest stable kernel (2.6.30.4), without
PaX and squashfs-lzma support. Still the same problem:

[~]# uname -a
Linux gibraltar3-esys-master 2.6.30.4 #9 SMP PREEMPT Fri Jul 31 15:32:55
UTC 2009 i686 GNU/Linux
[~]# /etc/init.d/networking restart
Reconfiguring network interfaces...[  277.816049] sky2 0000:01:00.0:
error interrupt status=0xffffffff
[  277.822124] sky2 0000:01:00.0: PCI hardware error (0xffff)
[  277.827656] sky2 0000:01:00.0: PCI Express error (0xffffffff)
[  277.833449] sky2 wan: ram data read parity error
[  277.838107] sky2 wan: ram data write parity error
[  277.842852] sky2 wan: MAC parity error
[  277.846643] sky2 wan: RX parity error
[  277.850345] sky2 wan: TCP segmentation error
[  277.854688] BUG: unable to handle kernel NULL pointer dereference at
0000038d
[  277.858653] IP: [<f8050ca5>] sky2_mac_intr+0x30/0xc1 [sky2]
[  277.858653] *pde = 00000000
[  277.858653] Oops: 0000 [#1] PREEMPT SMP
[  277.858653] last sysfs file:
/sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed
[  277.858653] Modules linked in: xt_multiport cpufreq_userspace xt_DSCP
xt_length xt_mark xt_dscp xt_MARK xt_CONNMARK xt_comment xt_policy
ipt_REDIRECT ip6t_LOG xt_tcpudp ip6table_mangle iptable_mangle
ip6table_filter ip6_tables sit tunnel4 8021q garp stp llc ipt_LOG
xt_limit xt_state iptable_nat iptable_filter ip_tables x_tables dm_mod
p4_clockmod speedstep_lib freq_table tun imq nf_nat_ftp nf_nat
nf_conntrack_ftp nf_conntrack_ipv6 nf_conntrack_ipv4 nf_conntrack
nf_defrag_ipv4 ipv6 evdev parport_pc parport serio_raw i2c_i801 pcspkr
i2c_core iTCO_wdt rng_core intel_agp loop aufs exportfs nls_utf8
nls_cp437 ide_generic sd_mod ide_gd_mod ata_generic pata_acpi skge
ata_piix piix ide_pci_generic ide_core sky2 thermal_sys
[  277.858653]
[  277.858653] Pid: 9423, comm: tlsmgr Not tainted (2.6.30.4 #9)
[  277.858653] EIP: 0060:[<f8050ca5>] EFLAGS: 00010286 CPU: 0
[  277.858653] EIP is at sky2_mac_intr+0x30/0xc1 [sky2]
[  277.858653] EAX: f8068f88 EBX: 00000001 ECX: 00000008 EDX: 000000ff
[  277.858653] ESI: 00000000 EDI: f6901b80 EBP: f6acfce4 ESP: f6acfccc
[  277.858653]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[  277.858653] Process tlsmgr (pid: 9423, ti=f6ace000 task=f7176e70
task.ti=f6ace000)
[  277.858653] Stack:
[  277.858653]  00000080 ff901b80 968c5f08 f71ed840 ffffffff ffffffff
f6acfd6c f80542d8
[  277.858653]  00000000 c181d260 00000040 f6901b88 f6acfd08 c04ee2b5
f6901b80 ffffffff
[  277.858653]  c022ded2 f71ef000 00000000 00000000 0000000f c181d260
00000000 00000246
[  277.858653] Call Trace:
[  277.858653]  [<f80542d8>] ? sky2_poll+0x1d2/0xb1e [sky2]
[  277.858653]  [<c04ee2b5>] ? _spin_unlock_irqrestore+0x31/0x44
[  277.858653]  [<c022ded2>] ? try_to_wake_up+0x291/0x2ac
[  277.858653]  [<c022df62>] ? wake_up_process+0x1b/0x2e
[  277.858653]  [<c04772f4>] ? __qdisc_run+0x73/0x1ca
[  277.858653]  [<c0463cc2>] ? net_rx_action+0x9e/0x1a2
[  277.858653]  [<c0237b5e>] ? __do_softirq+0xb2/0x188
[  277.858653]  [<c0237c73>] ? do_softirq+0x3f/0x5c
[  277.858653]  [<c0237dfd>] ? irq_exit+0x37/0x80
[  277.858653]  [<c0213cfd>] ? smp_apic_timer_interrupt+0x7c/0x9b
[  277.858653]  [<c02037dd>] ? apic_timer_interrupt+0x31/0x38
[  277.858653]  [<c0371524>] ? radix_tree_lookup_slot+0x34/0x79
[  277.858653]  [<c0284852>] ? find_get_page+0x34/0xc6
[  277.858653]  [<c0284c9e>] ? find_lock_page+0x21/0x67
[  277.858653]  [<c0285214>] ? filemap_fault+0x97/0x366
[  277.858653]  [<c0297054>] ? __do_fault+0x56/0x3b0
[  277.858653]  [<c02503a2>] ? getnstimeofday+0x5f/0xf3
[  277.858653]  [<c0252d85>] ? clockevents_program_event+0xe8/0x108
[  277.858653]  [<c0298f33>] ? handle_mm_fault+0x2b9/0x668
[  277.858653]  [<c024b121>] ? hrtimer_interrupt+0x13e/0x15f
[  277.858653]  [<c021d3f6>] ? do_page_fault+0x1fb/0x21b
[  277.858653]  [<c021d1fb>] ? do_page_fault+0x0/0x21b
[  277.858653]  [<c04ee72a>] ? error_code+0x7a/0x80
[  277.858653] Code: c7 56 53 89 d3 83 ec 0c 65 a1 14 00 00 00 89 45 f0
31 c0 8b 74 97 3c c1 e2 07 89 d0 05 08 0f 00 00 89 55 e8 03 07 8a 10 88
55 ef <f6> 86 8d 03 00 00 02 74 12 0f b6 c2 50 56 68 30 64 05 f8 e8 74
[  277.858653] EIP: [<f8050ca5>] sky2_mac_intr+0x30/0xc1 [sky2] SS:ESP
0068:f6acfccc
[  277.858653] CR2: 000000000000038d
[  278.173200] ---[ end trace bec12ce036036cbf ]---
[  278.177861] Kernel panic - not syncing: Fatal exception in interrupt
[  278.184259] Pid: 9423, comm: tlsmgr Tainted: G      D    2.6.30.4 #9
[  278.190654] Call Trace:
[  278.193140]  [<c04eb04e>] ? printk+0x1d/0x30
[  278.197452]  [<c04eaf8c>] panic+0x53/0xf8
[  278.201506]  [<c0206368>] oops_end+0x9f/0xbf
[  278.205817]  [<c021ceb4>] no_context+0x11a/0x135
[  278.210480]  [<c021d005>] __bad_area_nosemaphore+0x136/0x14f
[  278.216177]  [<c0374e68>] ? vsnprintf+0x91/0x332
[  278.220840]  [<c04ee2b5>] ? _spin_unlock_irqrestore+0x31/0x44
[  278.226622]  [<c04ee2b5>] ? _spin_unlock_irqrestore+0x31/0x44
[  278.232404]  [<c0232f3f>] ? release_console_sem+0x18b/0x1c9
[  278.238015]  [<c021d03b>] bad_area_nosemaphore+0x1d/0x34
[  278.243370]  [<c021d30b>] do_page_fault+0x110/0x21b
[  278.248287]  [<c021d1fb>] ? do_page_fault+0x0/0x21b
[  278.253209]  [<c04ee72a>] error_code+0x7a/0x80
[  278.257693]  [<c037007b>] ? kobject_uevent_env+0x42/0x387
[  278.263141]  [<f8050ca5>] ? sky2_mac_intr+0x30/0xc1 [sky2]
[  278.268673]  [<f80542d8>] sky2_poll+0x1d2/0xb1e [sky2]
[  278.273850]  [<c04ee2b5>] ? _spin_unlock_irqrestore+0x31/0x44
[  278.279632]  [<c022ded2>] ? try_to_wake_up+0x291/0x2ac
[  278.284818]  [<c022df62>] ? wake_up_process+0x1b/0x2e
[  278.289914]  [<c04772f4>] ? __qdisc_run+0x73/0x1ca
[  278.294750]  [<c0463cc2>] net_rx_action+0x9e/0x1a2
[  278.299578]  [<c0237b5e>] __do_softirq+0xb2/0x188
[  278.304321]  [<c0237c73>] do_softirq+0x3f/0x5c
[  278.308801]  [<c0237dfd>] irq_exit+0x37/0x80
[  278.313111]  [<c0213cfd>] smp_apic_timer_interrupt+0x7c/0x9b
[  278.318807]  [<c02037dd>] apic_timer_interrupt+0x31/0x38
[  278.324165]  [<c0371524>] ? radix_tree_lookup_slot+0x34/0x79
[  278.329869]  [<c0284852>] find_get_page+0x34/0xc6
[  278.334619]  [<c0284c9e>] find_lock_page+0x21/0x67
[  278.339447]  [<c0285214>] filemap_fault+0x97/0x366
[  278.344276]  [<c0297054>] __do_fault+0x56/0x3b0
[  278.348842]  [<c02503a2>] ? getnstimeofday+0x5f/0xf3
[  278.353847]  [<c0252d85>] ? clockevents_program_event+0xe8/0x108
[  278.359899]  [<c0298f33>] handle_mm_fault+0x2b9/0x668
[  278.364997]  [<c024b121>] ? hrtimer_interrupt+0x13e/0x15f
[  278.370445]  [<c021d3f6>] do_page_fault+0x1fb/0x21b
[  278.375364]  [<c021d1fb>] ? do_page_fault+0x0/0x21b
[  278.380287]  [<c04ee72a>] error_code+0x7a/0x80
[  278.384779] Rebooting in 30 seconds..

To allow easier debugging, I have now put our whole kernel tree up in a
public (read-only) git repository at
https://www.gibraltar.at/git/linux-2.6-gibraltar.git. The branch for
this kernel is origin/gibraltar-3.0, although the above dump was
produced by a version slightly "older" then HEAD, which did not yet have
the latest PaX patch applied (no PaX and no lzma-squashfs in this kernel).

Any hints/pointers/patches/etc. would be highly appreciated.

best regards,
Rene

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkp20DYACgkQq7SPDcPCS96R3QCdGTJsPiJGLfiWUZk67f6wms9Y
rVgAoPMO2hnT3jwRtY0Qz40NRp0DpKxT
=8NsP
-----END PGP SIGNATURE-----

^ permalink raw reply

* [PATCH 5/5] sky2: Lock tx queue when calling sky2_down locally
From: Mike McCormack @ 2009-08-03 11:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

Signed-off-by: Mike McCormack <mikem@ring3k.org>
---
 drivers/net/sky2.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index dbedee4..972c716 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -3078,10 +3078,13 @@ static void sky2_restart(struct work_struct *work)
 	int i, err;

 	rtnl_lock();
+
 	for (i = 0; i < hw->ports; i++) {
 		dev = hw->dev[i];
-		if (netif_running(dev))
+		if (netif_running(dev)) {
+			netif_tx_lock(dev);
 			sky2_down(dev);
+		}
 	}

 	napi_disable(&hw->napi);
@@ -3099,6 +3102,8 @@ static void sky2_restart(struct work_struct *work)
 				       dev->name, err);
 				dev_close(dev);
 			}
+			else
+				netif_tx_unlock(dev);
 		}
 	}

@@ -3697,8 +3702,10 @@ static int sky2_set_ringparam(struct net_device *dev,
 	    ering->tx_pending > TX_RING_SIZE - 1)
 		return -EINVAL;

-	if (netif_running(dev))
+	if (netif_running(dev)) {
+		netif_tx_lock(dev);
 		sky2_down(dev);
+	}

 	sky2->rx_pending = ering->rx_pending;
 	sky2->tx_pending = ering->tx_pending;
@@ -3707,6 +3714,8 @@ static int sky2_set_ringparam(struct net_device *dev,
 		err = sky2_up(dev);
 		if (err)
 			dev_close(dev);
+		else
+			netif_tx_unlock(dev);
 	}

 	return err;
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH 4/5] sky2: Remove tx locks
From: Mike McCormack @ 2009-08-03 11:08 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

Should no longer be necessary.

Signed-off-by: Mike McCormack <mikem@ring3k.org>
---
 drivers/net/sky2.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index ac303cb..dbedee4 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1799,9 +1799,7 @@ static void sky2_tx_clean(struct net_device *dev)
 {
 	struct sky2_port *sky2 = netdev_priv(dev);

-	netif_tx_lock_bh(dev);
 	sky2_tx_complete(sky2, sky2->tx_prod);
-	netif_tx_unlock_bh(dev);
 }

 static void sky2_tx_reset(struct sky2_port* sky2)
@@ -2369,11 +2367,7 @@ static inline void sky2_tx_done(struct
net_device *dev, u16 last)
 {
 	struct sky2_port *sky2 = netdev_priv(dev);

-	if (netif_running(dev)) {
-		netif_tx_lock(dev);
-		sky2_tx_complete(sky2, last);
-		netif_tx_unlock(dev);
-	}
+	sky2_tx_complete(sky2, last);
 }

 static inline void sky2_skb_rx(const struct sky2_port *sky2,
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH 3/5] sky2: Reset tx train after interrupts disabled.
From: Mike McCormack @ 2009-08-03 11:08 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

Reseting the tx chain too soon results in invalid tx queue positions
being delivered in the status queue.  This also makes sure there's no
overlap between the cleanup done by sky2_tx_clean() and
sky2_tx_done().

Signed-off-by: Mike McCormack <mikem@ring3k.org>
---
 drivers/net/sky2.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index a9bb3fa..ac303cb 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1866,8 +1866,6 @@ static int sky2_down(struct net_device *dev)
 	      && port == 0 && hw->dev[1] && netif_running(hw->dev[1])))
 		sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_RST_SET);

-	sky2_tx_reset(sky2);
-
 	sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_RST_SET);

 	/* Force any delayed status interrrupt and NAPI */
@@ -1892,6 +1890,8 @@ static int sky2_down(struct net_device *dev)
 	/* turn off LED's */
 	sky2_write16(hw, B0_Y2LED, LED_STAT_OFF);

+	sky2_tx_reset(sky2);
+
 	sky2_tx_clean(dev);
 	sky2_rx_clean(sky2);

-- 
1.5.6.5

^ permalink raw reply related

* [PATCH 2/5] sky2: Move tx reset functionality to sky2_tx_reset()
From: Mike McCormack @ 2009-08-03 11:08 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

Signed-off-by: Mike McCormack <mikem@ring3k.org>
---
 drivers/net/sky2.c |   44 ++++++++++++++++++++++++++------------------
 1 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 16a359d..a9bb3fa 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1804,6 +1804,31 @@ static void sky2_tx_clean(struct net_device *dev)
 	netif_tx_unlock_bh(dev);
 }

+static void sky2_tx_reset(struct sky2_port* sky2)
+{
+	unsigned port = sky2->port;
+	struct sky2_hw *hw = sky2->hw;
+
+	/* Disable Force Sync bit and Enable Alloc bit */
+	sky2_write8(hw, SK_REG(port, TXA_CTRL),
+		    TXA_DIS_FSYNC | TXA_DIS_ALLOC | TXA_STOP_RC);
+
+	/* Stop Interval Timer and Limit Counter of Tx Arbiter */
+	sky2_write32(hw, SK_REG(port, TXA_ITI_INI), 0L);
+	sky2_write32(hw, SK_REG(port, TXA_LIM_INI), 0L);
+
+	/* Reset the PCI FIFO of the async Tx queue */
+	sky2_write32(hw, Q_ADDR(txqaddr[port], Q_CSR),
+		     BMU_RST_SET | BMU_FIFO_RST);
+
+	/* Reset the Tx prefetch units */
+	sky2_write32(hw, Y2_QADDR(txqaddr[port], PREF_UNIT_CTRL),
+		     PREF_UNIT_RST_SET);
+
+	sky2_write32(hw, RB_ADDR(txqaddr[port], RB_CTRL), RB_RST_SET);
+	sky2_write8(hw, SK_REG(port, TX_GMF_CTRL_T), GMF_RST_SET);
+}
+
 /* Network shutdown */
 static int sky2_down(struct net_device *dev)
 {
@@ -1841,26 +1866,9 @@ static int sky2_down(struct net_device *dev)
 	      && port == 0 && hw->dev[1] && netif_running(hw->dev[1])))
 		sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_RST_SET);

-	/* Disable Force Sync bit and Enable Alloc bit */
-	sky2_write8(hw, SK_REG(port, TXA_CTRL),
-		    TXA_DIS_FSYNC | TXA_DIS_ALLOC | TXA_STOP_RC);
-
-	/* Stop Interval Timer and Limit Counter of Tx Arbiter */
-	sky2_write32(hw, SK_REG(port, TXA_ITI_INI), 0L);
-	sky2_write32(hw, SK_REG(port, TXA_LIM_INI), 0L);
-
-	/* Reset the PCI FIFO of the async Tx queue */
-	sky2_write32(hw, Q_ADDR(txqaddr[port], Q_CSR),
-		     BMU_RST_SET | BMU_FIFO_RST);
-
-	/* Reset the Tx prefetch units */
-	sky2_write32(hw, Y2_QADDR(txqaddr[port], PREF_UNIT_CTRL),
-		     PREF_UNIT_RST_SET);
-
-	sky2_write32(hw, RB_ADDR(txqaddr[port], RB_CTRL), RB_RST_SET);
+	sky2_tx_reset(sky2);

 	sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_RST_SET);
-	sky2_write8(hw, SK_REG(port, TX_GMF_CTRL_T), GMF_RST_SET);

 	/* Force any delayed status interrrupt and NAPI */
 	sky2_write32(hw, STAT_LEV_TIMER_CNT, 0);
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH 1/5] sky2: Avoid rewinding sky2->tx_prod
From: Mike McCormack @ 2009-08-03 11:07 UTC (permalink / raw)
  To: netdev

---
 drivers/net/sky2.c |   34 ++++++++++++++++++----------------
 1 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 661abd0..16a359d 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -989,11 +989,11 @@ static void sky2_prefetch_init(struct sky2_hw
*hw, u32 qaddr,
 	sky2_read32(hw, Y2_QADDR(qaddr, PREF_UNIT_CTRL));
 }

-static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2)
+static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2, u16 *slot)
 {
-	struct sky2_tx_le *le = sky2->tx_le + sky2->tx_prod;
+	struct sky2_tx_le *le = sky2->tx_le + *slot;

-	sky2->tx_prod = RING_NEXT(sky2->tx_prod, TX_RING_SIZE);
+	*slot = RING_NEXT(*slot, TX_RING_SIZE);
 	le->ctrl = 0;
 	return le;
 }
@@ -1006,7 +1006,7 @@ static void tx_init(struct sky2_port *sky2)
 	sky2->tx_tcpsum = 0;
 	sky2->tx_last_mss = 0;

-	le = get_tx_le(sky2);
+	le = get_tx_le(sky2, &sky2->tx_prod);
 	le->addr = 0;
 	le->opcode = OP_ADDR64 | HW_OWNER;
 }
@@ -1565,7 +1565,8 @@ static int sky2_xmit_frame(struct sk_buff *skb,
struct net_device *dev)
 	struct sky2_hw *hw = sky2->hw;
 	struct sky2_tx_le *le = NULL;
 	struct tx_ring_info *re;
-	unsigned i, len, first_slot;
+	unsigned i, len;
+	u16 slot;
 	dma_addr_t mapping;
 	u16 mss;
 	u8 ctrl;
@@ -1579,14 +1580,14 @@ static int sky2_xmit_frame(struct sk_buff
*skb, struct net_device *dev)
 	if (pci_dma_mapping_error(hw->pdev, mapping))
 		goto mapping_error;

-	first_slot = sky2->tx_prod;
+	slot = sky2->tx_prod;
 	if (unlikely(netif_msg_tx_queued(sky2)))
 		printk(KERN_DEBUG "%s: tx queued, slot %u, len %d\n",
-		       dev->name, first_slot, skb->len);
+		       dev->name, slot, skb->len);

 	/* Send high bits if needed */
 	if (sizeof(dma_addr_t) > sizeof(u32)) {
-		le = get_tx_le(sky2);
+		le = get_tx_le(sky2, &slot);
 		le->addr = cpu_to_le32(upper_32_bits(mapping));
 		le->opcode = OP_ADDR64 | HW_OWNER;
 	}
@@ -1599,7 +1600,7 @@ static int sky2_xmit_frame(struct sk_buff *skb,
struct net_device *dev)
 			mss += ETH_HLEN + ip_hdrlen(skb) + tcp_hdrlen(skb);

   		if (mss != sky2->tx_last_mss) {
-  			le = get_tx_le(sky2);
+			le = get_tx_le(sky2, &slot);
   			le->addr = cpu_to_le32(mss);

 			if (hw->flags & SKY2_HW_NEW_LE)
@@ -1615,7 +1616,7 @@ static int sky2_xmit_frame(struct sk_buff *skb,
struct net_device *dev)
 	/* Add VLAN tag, can piggyback on LRGLEN or ADDR64 */
 	if (sky2->vlgrp && vlan_tx_tag_present(skb)) {
 		if (!le) {
-			le = get_tx_le(sky2);
+			le = get_tx_le(sky2, &slot);
 			le->addr = 0;
 			le->opcode = OP_VLAN|HW_OWNER;
 		} else
@@ -1644,7 +1645,7 @@ static int sky2_xmit_frame(struct sk_buff *skb,
struct net_device *dev)
 			if (tcpsum != sky2->tx_tcpsum) {
 				sky2->tx_tcpsum = tcpsum;

-				le = get_tx_le(sky2);
+				le = get_tx_le(sky2, &slot);
 				le->addr = cpu_to_le32(tcpsum);
 				le->length = 0;	/* initial checksum value */
 				le->ctrl = 1;	/* one packet */
@@ -1653,7 +1654,7 @@ static int sky2_xmit_frame(struct sk_buff *skb,
struct net_device *dev)
 		}
 	}

-	le = get_tx_le(sky2);
+	le = get_tx_le(sky2, &slot);
 	le->addr = cpu_to_le32((u32) mapping);
 	le->length = cpu_to_le16(len);
 	le->ctrl = ctrl;
@@ -1674,13 +1675,13 @@ static int sky2_xmit_frame(struct sk_buff
*skb, struct net_device *dev)
 			goto mapping_unwind;

 		if (sizeof(dma_addr_t) > sizeof(u32)) {
-			le = get_tx_le(sky2);
+			le = get_tx_le(sky2, &slot);
 			le->addr = cpu_to_le32(upper_32_bits(mapping));
 			le->ctrl = 0;
 			le->opcode = OP_ADDR64 | HW_OWNER;
 		}

-		le = get_tx_le(sky2);
+		le = get_tx_le(sky2, &slot);
 		le->addr = cpu_to_le32((u32) mapping);
 		le->length = cpu_to_le16(frag->size);
 		le->ctrl = ctrl;
@@ -1694,6 +1695,8 @@ static int sky2_xmit_frame(struct sk_buff *skb,
struct net_device *dev)

 	le->ctrl |= EOP;

+	sky2->tx_prod = slot;
+
 	if (tx_avail(sky2) <= MAX_SKB_TX_LE)
 		netif_stop_queue(dev);

@@ -1702,7 +1705,7 @@ static int sky2_xmit_frame(struct sk_buff *skb,
struct net_device *dev)
 	return NETDEV_TX_OK;

 mapping_unwind:
-	for (i = first_slot; i != sky2->tx_prod; i = RING_NEXT(i, TX_RING_SIZE)) {
+	for (i = sky2->tx_prod; i != slot; i = RING_NEXT(i, TX_RING_SIZE)) {
 		le = sky2->tx_le + i;
 		re = sky2->tx_ring + i;

@@ -1722,7 +1725,6 @@ mapping_unwind:
 		}
 	}

-	sky2->tx_prod = first_slot;
 mapping_error:
 	if (net_ratelimit())
 		dev_warn(&hw->pdev->dev, "%s: tx mapping error\n", dev->name);
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH 0/5] sky2: Resolve sky2_down/transmit races, lockless transmit
From: Mike McCormack @ 2009-08-03 11:07 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

Hi Stephen,

Here is an alternate (and more intrusive) patch set to solve the
transmit during restart issues.

These patches avoid introducing a new state variable by

* reworking the transmit queue to work without locks
* tweaking sky2_down() to avoid invalid tx queue positions being
received in the status queue

Please let me know if this looks better,

thanks,

Mike

^ permalink raw reply

* Re: [RFC] [PATCH] Avoid enqueuing skb for default qdiscs
From: Jarek Poplawski @ 2009-08-03  9:58 UTC (permalink / raw)
  To: Krishna Kumar2; +Cc: davem, herbert, kaber, netdev
In-Reply-To: <OF1689973C.61968A07-ON65257607.003168D7-65257607.0033083B@in.ibm.com>

On Mon, Aug 03, 2009 at 02:47:24PM +0530, Krishna Kumar2 wrote:
> It is not going through for some reason though I sent the patch about
> 1.5 hours back. Is it OK if I attach the patch this one time while I figure
> out what needs to be done to fix my mailer to start working again?
> 
> Following attachment contains the description and the patch.
> 
> thanks,
> 
> - KK
> 
> 
> (See attached file: patch.dont_queue)
> 

I can see this file :-)

Thanks,
Jarek P.

^ permalink raw reply

* Re: [RFC] [PATCH] Avoid enqueuing skb for default qdiscs
From: Krishna Kumar2 @ 2009-08-03  9:17 UTC (permalink / raw)
  To: davem; +Cc: herbert, jarkao2, kaber, netdev
In-Reply-To: <20090803081016.32746.23400.sendpatchset@localhost.localdomain>

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

It is not going through for some reason though I sent the patch about
1.5 hours back. Is it OK if I attach the patch this one time while I figure
out what needs to be done to fix my mailer to start working again?

Following attachment contains the description and the patch.

thanks,

- KK


(See attached file: patch.dont_queue)


Krishna Kumar2/India/IBM@IBMIN wrote on 08/03/2009 01:40:16 PM:

> Krishna Kumar2/India/IBM@IBMIN
> 08/03/2009 01:40 PM
>
> To
>
> davem@davemloft.net
>
> cc
>
> Krishna Kumar2/India/IBM@IBMIN, herbert@gondor.apana.org.au,
kaber@trash.net,
> jarkao2@gmail.com, netdev@vger.kernel.org
>
> Subject
>
> [RFC] [PATCH] Avoid enqueuing skb for default qdiscs
>
> From: Krishna Kumar <krkumar2@in.ibm.com>
>
> dev_queue_xmit enqueue's a skb and calls qdisc_run which
> dequeue's the skb and xmits it. In most cases (after
> instrumenting the code), the skb that is enqueue'd is the
> same one that is dequeue'd (unless the queue gets stopped
> or multiple cpu's write to the same queue and ends in a
> race with qdisc_run). For default qdiscs, we can remove
> this path and simply xmit the skb since this is a work
> conserving queue.
>
> The patch uses a new flag - TCQ_F_CAN_BYPASS to identify
> the default fast queue. I plan to use this flag for the
> previous patch also (rename if required).  The controversial
> part of the patch is incrementing qlen when a skb is
> requeued, this is to avoid checks like the second line below:
>
> +    } else if ((q->flags & TCQ_F_CAN_BYPASS) && !qdisc_qlen(q) &&
> >> THIS LINE:   !q->gso_skb &&
> +               !test_and_set_bit(__QDISC_STATE_RUNNING, &q->state)) {
>
> Results of a 4 hour testing for multiple netperf sessions
> (1, 2, 4, 8, 12 sessions on a 4 cpu system-X and 1, 2, 4,
> 8, 16, 32 sessions on a 16 cpu P6). Aggregate Mb/s across
> the iterations:
>
> -----------------------------------------------------------------
>      |         System-X          |               P6
> -----------------------------------------------------------------
> Size |  ORG BW          NEW BW   |      ORG BW          NEW BW
> -----|---------------------------|-------------------------------
> 16K  |  154264          156234   |      155350          157569
> 64K  |  154364          154825   |      155790          158845
> 128K |  154644          154803   |      153418          155572
> 256K |  153882          152007   |      154784          154596
> -----------------------------------------------------------------
>
> Netperf reported Service demand reduced by 15% on the P6 but
> no noticeable difference on the system-X box.
>
> Please review.
>
> Thanks,
>
> - KK
>
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> ---
>
>  include/net/sch_generic.h |    9 +++
>  net/core/dev.c            |   46 ++++++++++++-----
>  net/sched/sch_generic.c   |   94 +++++++++++++++++++++---------------
>  3 files changed, 99 insertions(+), 50 deletions(-)
>
> diff -ruNp org/include/net/sch_generic.h new/include/net/sch_generic.h
> --- org/include/net/sch_generic.h   2009-03-24 08:54:16.000000000 +0530
> +++ new/include/net/sch_generic.h   2009-07-28 19:37:10.000000000 +0530
> @@ -45,6 +45,7 @@ struct Qdisc
>  #define TCQ_F_BUILTIN      1
>  #define TCQ_F_THROTTLED      2
>  #define TCQ_F_INGRESS      4
> +#define TCQ_F_CAN_BYPASS   8
>  #define TCQ_F_WARN_NONWC   (1 << 16)
>     int         padded;
>     struct Qdisc_ops   *ops;
> @@ -182,6 +183,11 @@ struct qdisc_skb_cb {
>     char         data[];
>  };
>
> +static inline int qdisc_qlen(struct Qdisc *q)
> +{
> +   return q->q.qlen;
> +}
> +
>  static inline struct qdisc_skb_cb *qdisc_skb_cb(struct sk_buff *skb)
>  {
>     return (struct qdisc_skb_cb *)skb->cb;
> @@ -306,6 +312,9 @@ extern struct Qdisc *qdisc_create_dflt(s
>                     struct Qdisc_ops *ops, u32 parentid);
>  extern void qdisc_calculate_pkt_len(struct sk_buff *skb,
>                 struct qdisc_size_table *stab);
> +extern int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
> +            struct net_device *dev, struct netdev_queue *txq,
> +            spinlock_t *root_lock);
>  extern void tcf_destroy(struct tcf_proto *tp);
>  extern void tcf_destroy_chain(struct tcf_proto **fl);
>
> diff -ruNp org2/net/core/dev.c new2/net/core/dev.c
> --- org2/net/core/dev.c   2009-07-27 09:08:24.000000000 +0530
> +++ new2/net/core/dev.c   2009-07-28 19:37:10.000000000 +0530
> @@ -1786,6 +1786,38 @@ static struct netdev_queue *dev_pick_tx(
>     return netdev_get_tx_queue(dev, queue_index);
>  }
>
> +static inline int __dev_hw_xmit(struct sk_buff *skb, struct Qdisc *q,
> +            struct net_device *dev,
> +            struct netdev_queue *txq)
> +{
> +   spinlock_t *root_lock = qdisc_lock(q);
> +   int rc;
> +
> +   spin_lock(root_lock);
> +   if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) {
> +      kfree_skb(skb);
> +      rc = NET_XMIT_DROP;
> +   } else if ((q->flags & TCQ_F_CAN_BYPASS) && !qdisc_qlen(q) &&
> +         !test_and_set_bit(__QDISC_STATE_RUNNING, &q->state)) {
> +      /*
> +       * This is the default work-conserving queue; there are no
> +       * old skbs waiting to be sent out; and the qdisc is not
> +       * running - xmit the skb directly.
> +       */
> +      if (sch_direct_xmit(skb, q, dev, txq, root_lock))
> +         __qdisc_run(q);
> +      else
> +         clear_bit(__QDISC_STATE_RUNNING, &q->state);
> +      rc = 0;
> +   } else {
> +      rc = qdisc_enqueue_root(skb, q);
> +      qdisc_run(q);
> +   }
> +   spin_unlock(root_lock);
> +
> +   return rc;
> +}
> +
>  /**
>   *   dev_queue_xmit - transmit a buffer
>   *   @skb: buffer to transmit
> @@ -1859,19 +1891,7 @@ gso:
>     skb->tc_verd = SET_TC_AT(skb->tc_verd,AT_EGRESS);
>  #endif
>     if (q->enqueue) {
> -      spinlock_t *root_lock = qdisc_lock(q);
> -
> -      spin_lock(root_lock);
> -
> -      if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) {
> -         kfree_skb(skb);
> -         rc = NET_XMIT_DROP;
> -      } else {
> -         rc = qdisc_enqueue_root(skb, q);
> -         qdisc_run(q);
> -      }
> -      spin_unlock(root_lock);
> -
> +      rc = __dev_hw_xmit(skb, q, dev, txq);
>        goto out;
>     }
>
> diff -ruNp org2/net/sched/sch_generic.c new2/net/sched/sch_generic.c
> --- org2/net/sched/sch_generic.c   2009-05-25 07:48:07.000000000 +0530
> +++ new2/net/sched/sch_generic.c   2009-07-28 19:40:39.000000000 +0530
> @@ -37,15 +37,11 @@
>   * - updates to tree and tree walking are only done under the rtnl
mutex.
>   */
>
> -static inline int qdisc_qlen(struct Qdisc *q)
> -{
> -   return q->q.qlen;
> -}
> -
>  static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
>  {
>     q->gso_skb = skb;
>     q->qstats.requeues++;
> +   q->q.qlen++;   /* it's still part of the queue */
>     __netif_schedule(q);
>
>     return 0;
> @@ -61,9 +57,11 @@ static inline struct sk_buff *dequeue_sk
>
>        /* check the reason of requeuing without tx lock first */
>        txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
> -      if (!netif_tx_queue_stopped(txq) && !netif_tx_queue_frozen(txq))
> +      if (!netif_tx_queue_stopped(txq) &&
> +          !netif_tx_queue_frozen(txq)) {
>           q->gso_skb = NULL;
> -      else
> +         q->q.qlen--;
> +      } else
>           skb = NULL;
>     } else {
>        skb = q->dequeue(q);
> @@ -102,45 +100,24 @@ static inline int handle_dev_cpu_collisi
>     return ret;
>  }
>
> -/*
> - * NOTE: Called under qdisc_lock(q) with locally disabled BH.
> - *
> - * __QDISC_STATE_RUNNING guarantees only one CPU can process
> - * this qdisc at a time. qdisc_lock(q) serializes queue accesses for
> - * this queue.
> - *
> - *  netif_tx_lock serializes accesses to device driver.
> - *
> - *  qdisc_lock(q) and netif_tx_lock are mutually exclusive,
> - *  if one is grabbed, another must be free.
> - *
> - * Note, that this procedure can be called by a watchdog timer
> +/*
> + * Transmit one skb, and handle the return status as required. Holding
the
> + * __QDISC_STATE_RUNNING bit guarantees that only one CPU can execute
this
> + * function.
>   *
>   * Returns to the caller:
>   *            0  - queue is empty or throttled.
>   *            >0 - queue is not empty.
> - *
>   */
> -static inline int qdisc_restart(struct Qdisc *q)
> +int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
> +          struct net_device *dev, struct netdev_queue *txq,
> +          spinlock_t *root_lock)
>  {
> -   struct netdev_queue *txq;
>     int ret = NETDEV_TX_BUSY;
> -   struct net_device *dev;
> -   spinlock_t *root_lock;
> -   struct sk_buff *skb;
> -
> -   /* Dequeue packet */
> -   if (unlikely((skb = dequeue_skb(q)) == NULL))
> -      return 0;
> -
> -   root_lock = qdisc_lock(q);
>
>     /* And release qdisc */
>     spin_unlock(root_lock);
>
> -   dev = qdisc_dev(q);
> -   txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
> -
>     HARD_TX_LOCK(dev, txq, smp_processor_id());
>     if (!netif_tx_queue_stopped(txq) &&
>         !netif_tx_queue_frozen(txq))
> @@ -177,6 +154,43 @@ static inline int qdisc_restart(struct Q
>     return ret;
>  }
>
> +/*
> + * NOTE: Called under qdisc_lock(q) with locally disabled BH.
> + *
> + * __QDISC_STATE_RUNNING guarantees only one CPU can process
> + * this qdisc at a time. qdisc_lock(q) serializes queue accesses for
> + * this queue.
> + *
> + *  netif_tx_lock serializes accesses to device driver.
> + *
> + *  qdisc_lock(q) and netif_tx_lock are mutually exclusive,
> + *  if one is grabbed, another must be free.
> + *
> + * Note, that this procedure can be called by a watchdog timer
> + *
> + * Returns to the caller:
> + *            0  - queue is empty or throttled.
> + *            >0 - queue is not empty.
> + *
> + */
> +static inline int qdisc_restart(struct Qdisc *q)
> +{
> +   struct netdev_queue *txq;
> +   struct net_device *dev;
> +   spinlock_t *root_lock;
> +   struct sk_buff *skb;
> +
> +   /* Dequeue packet */
> +   if (unlikely((skb = dequeue_skb(q)) == NULL))
> +      return 0;
> +
> +   root_lock = qdisc_lock(q);
> +   dev = qdisc_dev(q);
> +   txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
> +
> +   return sch_xmit_direct(skb, q, dev, txq, root_lock);
> +}
> +
>  void __qdisc_run(struct Qdisc *q)
>  {
>     unsigned long start_time = jiffies;
> @@ -547,8 +561,11 @@ void qdisc_reset(struct Qdisc *qdisc)
>     if (ops->reset)
>        ops->reset(qdisc);
>
> -   kfree_skb(qdisc->gso_skb);
> -   qdisc->gso_skb = NULL;
> +   if (qdisc->gso_skb) {
> +      kfree_skb(qdisc->gso_skb);
> +      qdisc->gso_skb = NULL;
> +      qdisc->q.qlen--;
> +   }
>  }
>  EXPORT_SYMBOL(qdisc_reset);
>
> @@ -605,6 +622,9 @@ static void attach_one_default_qdisc(str
>           printk(KERN_INFO "%s: activation failed\n", dev->name);
>           return;
>        }
> +
> +      /* Can by-pass the queue discipline for default qdisc */
> +      qdisc->flags = TCQ_F_CAN_BYPASS;
>     } else {
>        qdisc =  &noqueue_qdisc;
>     }

[-- Attachment #2: patch.dont_queue --]
[-- Type: application/octet-stream, Size: 9264 bytes --]

From: Krishna Kumar <krkumar2@in.ibm.com>

dev_queue_xmit enqueue's a skb and calls qdisc_run which
dequeue's the skb and xmits it. In most cases (after
instrumenting the code), the skb that is enqueue'd is the
same one that is dequeue'd (unless the queue gets stopped
or multiple cpu's write to the same queue and ends in a
race with qdisc_run). For default qdiscs, we can remove
this path and simply xmit the skb since this is a work
conserving queue.

The patch uses a new flag - TCQ_F_CAN_BYPASS to identify
the default fast queue. I plan to use this flag for the
previous patch also (rename if required).  The controversial
part of the patch is incrementing qlen when a skb is
requeued, this is to avoid checks like the second line below:

+    } else if ((q->flags & TCQ_F_CAN_BYPASS) && !qdisc_qlen(q) &&
>> THIS LINE:   !q->gso_skb &&
+               !test_and_set_bit(__QDISC_STATE_RUNNING, &q->state)) {

Results of a 4 hour testing for multiple netperf sessions
(1, 2, 4, 8, 12 sessions on a 4 cpu system-X and 1, 2, 4,
8, 16, 32 sessions on a 16 cpu P6). Aggregate Mb/s across
the iterations:

-----------------------------------------------------------------
     |         System-X          |               P6
-----------------------------------------------------------------
Size |  ORG BW          NEW BW   |      ORG BW          NEW BW
-----|---------------------------|-------------------------------
16K  |  154264          156234   |      155350          157569
64K  |  154364          154825   |      155790          158845
128K |  154644          154803   |      153418          155572
256K |  153882          152007   |      154784          154596
-----------------------------------------------------------------

Netperf reported Service demand reduced by 15% on the P6 but
no noticeable difference on the system-X box.

Please review.

Thanks,

- KK

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---

 include/net/sch_generic.h |    9 +++
 net/core/dev.c            |   46 ++++++++++++-----
 net/sched/sch_generic.c   |   94 +++++++++++++++++++++---------------
 3 files changed, 99 insertions(+), 50 deletions(-)

diff -ruNp org/include/net/sch_generic.h new/include/net/sch_generic.h
--- org/include/net/sch_generic.h	2009-03-24 08:54:16.000000000 +0530
+++ new/include/net/sch_generic.h	2009-07-28 19:37:10.000000000 +0530
@@ -45,6 +45,7 @@ struct Qdisc
 #define TCQ_F_BUILTIN		1
 #define TCQ_F_THROTTLED		2
 #define TCQ_F_INGRESS		4
+#define TCQ_F_CAN_BYPASS	8
 #define TCQ_F_WARN_NONWC	(1 << 16)
 	int			padded;
 	struct Qdisc_ops	*ops;
@@ -182,6 +183,11 @@ struct qdisc_skb_cb {
 	char			data[];
 };
 
+static inline int qdisc_qlen(struct Qdisc *q)
+{
+	return q->q.qlen;
+}
+
 static inline struct qdisc_skb_cb *qdisc_skb_cb(struct sk_buff *skb)
 {
 	return (struct qdisc_skb_cb *)skb->cb;
@@ -306,6 +312,9 @@ extern struct Qdisc *qdisc_create_dflt(s
 				       struct Qdisc_ops *ops, u32 parentid);
 extern void qdisc_calculate_pkt_len(struct sk_buff *skb,
 				   struct qdisc_size_table *stab);
+extern int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
+			   struct net_device *dev, struct netdev_queue *txq,
+			   spinlock_t *root_lock);
 extern void tcf_destroy(struct tcf_proto *tp);
 extern void tcf_destroy_chain(struct tcf_proto **fl);
 
diff -ruNp org2/net/core/dev.c new2/net/core/dev.c
--- org2/net/core/dev.c	2009-07-27 09:08:24.000000000 +0530
+++ new2/net/core/dev.c	2009-07-28 19:37:10.000000000 +0530
@@ -1786,6 +1786,38 @@ static struct netdev_queue *dev_pick_tx(
 	return netdev_get_tx_queue(dev, queue_index);
 }
 
+static inline int __dev_hw_xmit(struct sk_buff *skb, struct Qdisc *q,
+				struct net_device *dev,
+				struct netdev_queue *txq)
+{
+	spinlock_t *root_lock = qdisc_lock(q);
+	int rc;
+
+	spin_lock(root_lock);
+	if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) {
+		kfree_skb(skb);
+		rc = NET_XMIT_DROP;
+	} else if ((q->flags & TCQ_F_CAN_BYPASS) && !qdisc_qlen(q) &&
+		   !test_and_set_bit(__QDISC_STATE_RUNNING, &q->state)) {
+		/*
+		 * This is the default work-conserving queue; there are no
+		 * old skbs waiting to be sent out; and the qdisc is not
+		 * running - xmit the skb directly.
+		 */
+		if (sch_direct_xmit(skb, q, dev, txq, root_lock))
+			__qdisc_run(q);
+		else
+			clear_bit(__QDISC_STATE_RUNNING, &q->state);
+		rc = 0;
+	} else {
+		rc = qdisc_enqueue_root(skb, q);
+		qdisc_run(q);
+	}
+	spin_unlock(root_lock);
+
+	return rc;
+}
+
 /**
  *	dev_queue_xmit - transmit a buffer
  *	@skb: buffer to transmit
@@ -1859,19 +1891,7 @@ gso:
 	skb->tc_verd = SET_TC_AT(skb->tc_verd,AT_EGRESS);
 #endif
 	if (q->enqueue) {
-		spinlock_t *root_lock = qdisc_lock(q);
-
-		spin_lock(root_lock);
-
-		if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) {
-			kfree_skb(skb);
-			rc = NET_XMIT_DROP;
-		} else {
-			rc = qdisc_enqueue_root(skb, q);
-			qdisc_run(q);
-		}
-		spin_unlock(root_lock);
-
+		rc = __dev_hw_xmit(skb, q, dev, txq);
 		goto out;
 	}
 
diff -ruNp org2/net/sched/sch_generic.c new2/net/sched/sch_generic.c
--- org2/net/sched/sch_generic.c	2009-05-25 07:48:07.000000000 +0530
+++ new2/net/sched/sch_generic.c	2009-07-28 19:40:39.000000000 +0530
@@ -37,15 +37,11 @@
  * - updates to tree and tree walking are only done under the rtnl mutex.
  */
 
-static inline int qdisc_qlen(struct Qdisc *q)
-{
-	return q->q.qlen;
-}
-
 static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
 {
 	q->gso_skb = skb;
 	q->qstats.requeues++;
+	q->q.qlen++;	/* it's still part of the queue */
 	__netif_schedule(q);
 
 	return 0;
@@ -61,9 +57,11 @@ static inline struct sk_buff *dequeue_sk
 
 		/* check the reason of requeuing without tx lock first */
 		txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
-		if (!netif_tx_queue_stopped(txq) && !netif_tx_queue_frozen(txq))
+		if (!netif_tx_queue_stopped(txq) &&
+		    !netif_tx_queue_frozen(txq)) {
 			q->gso_skb = NULL;
-		else
+			q->q.qlen--;
+		} else
 			skb = NULL;
 	} else {
 		skb = q->dequeue(q);
@@ -102,45 +100,24 @@ static inline int handle_dev_cpu_collisi
 	return ret;
 }
 
-/*
- * NOTE: Called under qdisc_lock(q) with locally disabled BH.
- *
- * __QDISC_STATE_RUNNING guarantees only one CPU can process
- * this qdisc at a time. qdisc_lock(q) serializes queue accesses for
- * this queue.
- *
- *  netif_tx_lock serializes accesses to device driver.
- *
- *  qdisc_lock(q) and netif_tx_lock are mutually exclusive,
- *  if one is grabbed, another must be free.
- *
- * Note, that this procedure can be called by a watchdog timer
+/* 
+ * Transmit one skb, and handle the return status as required. Holding the
+ * __QDISC_STATE_RUNNING bit guarantees that only one CPU can execute this
+ * function.
  *
  * Returns to the caller:
  *				0  - queue is empty or throttled.
  *				>0 - queue is not empty.
- *
  */
-static inline int qdisc_restart(struct Qdisc *q)
+int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
+		    struct net_device *dev, struct netdev_queue *txq,
+		    spinlock_t *root_lock)
 {
-	struct netdev_queue *txq;
 	int ret = NETDEV_TX_BUSY;
-	struct net_device *dev;
-	spinlock_t *root_lock;
-	struct sk_buff *skb;
-
-	/* Dequeue packet */
-	if (unlikely((skb = dequeue_skb(q)) == NULL))
-		return 0;
-
-	root_lock = qdisc_lock(q);
 
 	/* And release qdisc */
 	spin_unlock(root_lock);
 
-	dev = qdisc_dev(q);
-	txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
-
 	HARD_TX_LOCK(dev, txq, smp_processor_id());
 	if (!netif_tx_queue_stopped(txq) &&
 	    !netif_tx_queue_frozen(txq))
@@ -177,6 +154,43 @@ static inline int qdisc_restart(struct Q
 	return ret;
 }
 
+/*
+ * NOTE: Called under qdisc_lock(q) with locally disabled BH.
+ *
+ * __QDISC_STATE_RUNNING guarantees only one CPU can process
+ * this qdisc at a time. qdisc_lock(q) serializes queue accesses for
+ * this queue.
+ *
+ *  netif_tx_lock serializes accesses to device driver.
+ *
+ *  qdisc_lock(q) and netif_tx_lock are mutually exclusive,
+ *  if one is grabbed, another must be free.
+ *
+ * Note, that this procedure can be called by a watchdog timer
+ *
+ * Returns to the caller:
+ *				0  - queue is empty or throttled.
+ *				>0 - queue is not empty.
+ *
+ */
+static inline int qdisc_restart(struct Qdisc *q)
+{
+	struct netdev_queue *txq;
+	struct net_device *dev;
+	spinlock_t *root_lock;
+	struct sk_buff *skb;
+
+	/* Dequeue packet */
+	if (unlikely((skb = dequeue_skb(q)) == NULL))
+		return 0;
+
+	root_lock = qdisc_lock(q);
+	dev = qdisc_dev(q);
+	txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
+
+	return sch_xmit_direct(skb, q, dev, txq, root_lock);
+}
+
 void __qdisc_run(struct Qdisc *q)
 {
 	unsigned long start_time = jiffies;
@@ -547,8 +561,11 @@ void qdisc_reset(struct Qdisc *qdisc)
 	if (ops->reset)
 		ops->reset(qdisc);
 
-	kfree_skb(qdisc->gso_skb);
-	qdisc->gso_skb = NULL;
+	if (qdisc->gso_skb) {
+		kfree_skb(qdisc->gso_skb);
+		qdisc->gso_skb = NULL;
+		qdisc->q.qlen--;
+	}
 }
 EXPORT_SYMBOL(qdisc_reset);
 
@@ -605,6 +622,9 @@ static void attach_one_default_qdisc(str
 			printk(KERN_INFO "%s: activation failed\n", dev->name);
 			return;
 		}
+
+		/* Can by-pass the queue discipline for default qdisc */
+		qdisc->flags = TCQ_F_CAN_BYPASS;
 	} else {
 		qdisc =  &noqueue_qdisc;
 	}

^ permalink raw reply

* reg IFF_RUNNING behaviour
From: durgam@it.iitb.ac.in phani @ 2009-08-03  9:26 UTC (permalink / raw)
  To: netdev

when the  interface operational status goes down (IFF_RUNNING)  why
linux ip stack is not  flushing routing/neighbour (arp) entries
related to the interface . I  have the following doubt .

If  there are two routes to the destiantion,   one route is more
specific and  points to the eth0 and another one points to the another
interface eth1.   If eth0 goes down operationally ,   linux kernel has
not removed the specific route which points eth0.  When the traffic
came for this destination,  specific route was getting exercised and
traffic was getting dropped . Why is this behaviour supported in
linux.


This is exampe.

eth0   Link encap:Ethernet  HWaddr 00:05:1E:76:42:A1
          inet addr:4.4.4.2  Bcast:4.4.4.255  Mask:255.255.255.0
          UP BROADCAST MULTICAST  MTU:9018  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:2 errors:5 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0

eth1     Link encap:Ethernet  HWaddr 00:05:1E:76:42:80
          inet addr:10.32.152.16  Bcast:10.32.159.255  Mask:255.255.240.0
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:115582 errors:1 dropped:0 overruns:0 frame:0
          TX packets:1472 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000


Routing table

Destination     Gateway         Genmask         Flags Metric Ref    Use Iface
10.2.2.52       0.0.0.0         255.255.255.255 UH    0      0        0  eth0
4.4.4.0         0.0.0.0         255.255.255.0   U     0      0        0  eth0
10.32.144.0     0.0.0.0         255.255.240.0   U     0      0        0 eth1
0.0.0.0         10.32.144.1     0.0.0.0         UG    0      0        0 eth1



Thanks
Pani.

^ permalink raw reply


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