Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Satyam Sharma @ 2007-08-18  3:33 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Christoph Lameter, Paul Mackerras, heiko.carstens, horms,
	Stefan Richter, Linux Kernel Mailing List, David Miller,
	Paul E. McKenney, Ilpo Järvinen, ak, cfriesen, rpjday,
	Netdev, jesper.juhl, linux-arch, zlynx, Andrew Morton,
	schwidefsky, Chris Snook, Herbert Xu, Linus Torvalds, wensong,
	wjiang
In-Reply-To: <67eca5ac43d3b1dbd1a04c7c36aebd5a@kernel.crashing.org>



On Sat, 18 Aug 2007, Segher Boessenkool wrote:

> > > > > The "asm volatile" implementation does have exactly the same
> > > > > reordering guarantees as the "volatile cast" thing,
> > > > 
> > > > I don't think so.
> > > 
> > > "asm volatile" creates a side effect.
> > 
> > Yeah.
> > 
> > > Side effects aren't
> > > allowed to be reordered wrt sequence points.
> > 
> > Yeah.
> > 
> > > This is exactly
> > > the same reason as why "volatile accesses" cannot be reordered.
> > 
> > No, the code in that sub-thread I earlier pointed you at *WAS* written
> > such that there was a sequence point after all the uses of that volatile
> > access cast macro, and _therefore_ we were safe from re-ordering
> > (behaviour guaranteed by the C standard).
> 
> And exactly the same is true for the "asm" version.
> 
> > Now, one cannot fantasize that "volatile asms" are also sequence points.
> 
> Sure you can do that.  I don't though.
> 
> > In fact such an argument would be sadly mistaken, because "sequence
> > points" are defined by the C standard and it'd be horribly wrong to
> > even _try_ claiming that the C standard knows about "volatile asms".
> 
> That's nonsense.  GCC can extend the C standard any way they
> bloody well please -- witness the fact that they added an
> extra class of side effects...
> 
> > > > Read the relevant GCC documentation.
> > > 
> > > I did, yes.
> > 
> > No, you didn't read:
> > 
> > http://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html
> > 
> > Read the bit about the need for artificial dependencies, and the example
> > given there:
> > 
> > 	asm volatile("mtfsf 255,%0" : : "f" (fpenv));
> > 	sum = x + y;
> > 
> > The docs explicitly say the addition can be moved before the "volatile
> > asm". Hopefully, as you know, (x + y) is an C "expression" and hence
> > a "sequence point" as defined by the standard.
> 
> The _end of a full expression_ is a sequence point, not every
> expression.  And that is irrelevant here anyway.
> 
> It is perfectly fine to compute x+y any time before the
> assignment; the C compiler is allowed to compute it _after_
> the assignment even, if it could figure out how ;-)
> 
> x+y does not contain a side effect, you know.
> 
> > I know there is also stuff written about "side-effects" there which
> > _could_ give the same semantic w.r.t. sequence points as the volatile
> > access casts,
> 
> s/could/does/
> 
> > but hey, it's GCC's own documentation, you obviously can't
> > find fault with _me_ if there's wrong stuff written in there. Say that
> > to GCC ...
> 
> There's nothing wrong there.
> 
> > See, "volatile" C keyword, for all it's ill-definition and dodgy
> > semantics, is still at least given somewhat of a treatment in the C
> > standard (whose quality is ... ummm, sadly not always good and clear,
> > but unsurprisingly, still about 5,482 orders-of-magnitude times
> > better than GCC docs).
> 
> If you find any problems/shortcomings in the GCC documentation,
> please file a PR, don't go whine on some unrelated mailing lists.
> Thank you.
> 
> > Semantics of "volatile" as applies to inline
> > asm, OTOH? You're completely relying on the compiler for that ...
> 
> Yes, and?  GCC promises the behaviour it has documented.

LOTS there, which obviously isn't correct, but which I'll reply to later,
easier stuff first. (call this "handwaving" if you want, but don't worry,
I /will/ bother myself to reply)


> > > > [ of course, if the (latest) GCC documentation is *yet again*
> > > >   wrong, then alright, not much I can do about it, is there. ]
> > > 
> > > There was (and is) nothing wrong about the "+m" documentation, if
> > > that is what you are talking about.  It could be extended now, to
> > > allow "+m" -- but that takes more than just "fixing" the documentation.
> > 
> > No, there was (and is) _everything_ wrong about the "+" documentation as
> > applies to memory-constrained operands. I don't give a whit if it's
> > some workaround in their gimplifier, or the other, that makes it possible
> > to use "+m" (like the current kernel code does). The docs suggest
> > otherwise, so there's obviously a clear disconnect between the docs and
> > actual GCC behaviour.
> 
> The documentation simply doesn't say "+m" is allowed.  The code to
> allow it was added for the benefit of people who do not read the
> documentation.  Documentation for "+m" might get added later if it
> is decided this [the code, not the documentation] is a sane thing
> to have (which isn't directly obvious).

Huh?

"If the (current) documentation doesn't match up with the (current)
code, then _at least one_ of them has to be (as of current) wrong."

I wonder how could you even try to disagree with that.

And I didn't go whining about this ... you asked me. (I think I'd said
something to the effect of GCC docs are often wrong, which is true,
but probably you feel saying that is "not allowed" on non-gcc lists?)

As for the "PR" you're requesting me to file with GCC for this, that
gcc-patches@ thread did precisely that and more (submitted a patch to
said documentation -- and no, saying "documentation might get added
later" is totally bogus and nonsensical -- documentation exists to
document current behaviour, not past). But come on, this is wholly
petty. I wouldn't have replied, really, if you weren't so provoking.

^ permalink raw reply

* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Segher Boessenkool @ 2007-08-18  2:15 UTC (permalink / raw)
  To: Satyam Sharma
  Cc: Christoph Lameter, Paul Mackerras, heiko.carstens, horms,
	Stefan Richter, Linux Kernel Mailing List, David Miller,
	Paul E. McKenney, Ilpo Järvinen, ak, cfriesen, rpjday,
	Netdev, jesper.juhl, linux-arch, zlynx, Andrew Morton,
	schwidefsky, Chris Snook, Herbert Xu, Linus Torvalds, wensong,
	wjiang
In-Reply-To: <alpine.LFD.0.999.0708180556330.3666@enigma.security.iitk.ac.in>

>>>> The "asm volatile" implementation does have exactly the same
>>>> reordering guarantees as the "volatile cast" thing,
>>>
>>> I don't think so.
>>
>> "asm volatile" creates a side effect.
>
> Yeah.
>
>> Side effects aren't
>> allowed to be reordered wrt sequence points.
>
> Yeah.
>
>> This is exactly
>> the same reason as why "volatile accesses" cannot be reordered.
>
> No, the code in that sub-thread I earlier pointed you at *WAS* written
> such that there was a sequence point after all the uses of that 
> volatile
> access cast macro, and _therefore_ we were safe from re-ordering
> (behaviour guaranteed by the C standard).

And exactly the same is true for the "asm" version.

> Now, one cannot fantasize that "volatile asms" are also sequence 
> points.

Sure you can do that.  I don't though.

> In fact such an argument would be sadly mistaken, because "sequence
> points" are defined by the C standard and it'd be horribly wrong to
> even _try_ claiming that the C standard knows about "volatile asms".

That's nonsense.  GCC can extend the C standard any way they
bloody well please -- witness the fact that they added an
extra class of side effects...

>>> Read the relevant GCC documentation.
>>
>> I did, yes.
>
> No, you didn't read:
>
> http://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html
>
> Read the bit about the need for artificial dependencies, and the 
> example
> given there:
>
> 	asm volatile("mtfsf 255,%0" : : "f" (fpenv));
> 	sum = x + y;
>
> The docs explicitly say the addition can be moved before the "volatile
> asm". Hopefully, as you know, (x + y) is an C "expression" and hence
> a "sequence point" as defined by the standard.

The _end of a full expression_ is a sequence point, not every
expression.  And that is irrelevant here anyway.

It is perfectly fine to compute x+y any time before the
assignment; the C compiler is allowed to compute it _after_
the assignment even, if it could figure out how ;-)

x+y does not contain a side effect, you know.

> I know there is also stuff written about "side-effects" there which
> _could_ give the same semantic w.r.t. sequence points as the volatile
> access casts,

s/could/does/

> but hey, it's GCC's own documentation, you obviously can't
> find fault with _me_ if there's wrong stuff written in there. Say that
> to GCC ...

There's nothing wrong there.

> See, "volatile" C keyword, for all it's ill-definition and dodgy
> semantics, is still at least given somewhat of a treatment in the C
> standard (whose quality is ... ummm, sadly not always good and clear,
> but unsurprisingly, still about 5,482 orders-of-magnitude times
> better than GCC docs).

If you find any problems/shortcomings in the GCC documentation,
please file a PR, don't go whine on some unrelated mailing lists.
Thank you.

> Semantics of "volatile" as applies to inline
> asm, OTOH? You're completely relying on the compiler for that ...

Yes, and?  GCC promises the behaviour it has documented.

>>> [ of course, if the (latest) GCC documentation is *yet again*
>>>   wrong, then alright, not much I can do about it, is there. ]
>>
>> There was (and is) nothing wrong about the "+m" documentation, if
>> that is what you are talking about.  It could be extended now, to
>> allow "+m" -- but that takes more than just "fixing" the 
>> documentation.
>
> No, there was (and is) _everything_ wrong about the "+" documentation 
> as
> applies to memory-constrained operands. I don't give a whit if it's
> some workaround in their gimplifier, or the other, that makes it 
> possible
> to use "+m" (like the current kernel code does). The docs suggest
> otherwise, so there's obviously a clear disconnect between the docs and
> actual GCC behaviour.

The documentation simply doesn't say "+m" is allowed.  The code to
allow it was added for the benefit of people who do not read the
documentation.  Documentation for "+m" might get added later if it
is decided this [the code, not the documentation] is a sane thing
to have (which isn't directly obvious).

> [ You seem to often take issue with _amazingly_ petty and pedantic 
> things,
>   by the way :-) ]

If you're talking details, you better get them right.  Handwaving is
fine with me as long as you're not purporting you're not.

And I simply cannot stand false assertions.

You can always ignore me if _you_ take issue with _that_ :-)


Segher


^ permalink raw reply

* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Satyam Sharma @ 2007-08-18  2:15 UTC (permalink / raw)
  To: Nick Piggin
  Cc: Stefan Richter, paulmck, Herbert Xu, Paul Mackerras,
	Christoph Lameter, Chris Snook, Linux Kernel Mailing List,
	linux-arch, Linus Torvalds, netdev, Andrew Morton, ak,
	heiko.carstens, davem, schwidefsky, wensong, horms, wjiang,
	cfriesen, zlynx, rpjday, jesper.juhl, segher
In-Reply-To: <46C59B09.8040004@cyberone.com.au>



On Fri, 17 Aug 2007, Nick Piggin wrote:

> Satyam Sharma wrote:
> 
> > I didn't quite understand what you said here, so I'll tell what I think:
> > 
> > * foo() is a compiler barrier if the definition of foo() is invisible to
> >  the compiler at a callsite.
> > 
> > * foo() is also a compiler barrier if the definition of foo() includes
> >  a barrier, and it is inlined at the callsite.
> > 
> > If the above is wrong, or if there's something else at play as well,
> > do let me know.
> 
> [...]
> If a function is not completely visible to the compiler (so it can't
> determine whether a barrier could be in it or not), then it must always
> assume it will contain a barrier so it always does the right thing.

Yup, that's what I'd said just a few sentences above, as you can see. I
was actually asking for "elaboration" on "how a compiler determines that
function foo() (say foo == schedule), even when it cannot see that it has
a barrier(), as you'd mentioned, is a 'sleeping' function" actually, and
whether compilers have a "notion of sleep to automatically assume a
compiler barrier whenever such a sleeping function foo() is called". But
I think you've already qualified the discussion to this kernel, so okay,
I shouldn't nit-pick anymore.

^ permalink raw reply

* Re: [PATCH] i386: optimize memset of 6 and 8 bytes
From: Arjan van de Ven @ 2007-08-18  1:57 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, Andrew Morton, linux-kernel, netdev
In-Reply-To: <20070817185452.5cbf60c4@freepuppy.rosehill.hemminger.net>


On Fri, 2007-08-17 at 18:54 -0700, Stephen Hemminger wrote:
> On Fri, 17 Aug 2007 18:49:34 -0700
> Arjan van de Ven <arjan@infradead.org> wrote:
> 
> > 
> > On Fri, 2007-08-17 at 16:50 -0700, Stephen Hemminger wrote:
> > > Tne network code does memset for 6 and 8 byte values, that can easily
> > > be optimized into simple assignments without string instructions.
> > 
> > 
> > so... question.
> > Why are we doing this by hand? Wouldn't gcc just generate this code in
> > the first place (when using __builtin_memset)? I very much suspect it
> > would (and if some version doesn't.... we really ought to get that
> > fixed)
> 
> i386 and x86_64 are not using __builtin_memset, as least from the
> code that I see generated.

.. maybe we should just fix it that way then?

-- 
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org


^ permalink raw reply

* Re: [PATCH] i386: optimize memset of 6 and 8 bytes
From: Stephen Hemminger @ 2007-08-18  1:54 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: David S. Miller, Andrew Morton, linux-kernel, netdev
In-Reply-To: <1187401774.2789.0.camel@laptopd505.fenrus.org>

On Fri, 17 Aug 2007 18:49:34 -0700
Arjan van de Ven <arjan@infradead.org> wrote:

> 
> On Fri, 2007-08-17 at 16:50 -0700, Stephen Hemminger wrote:
> > Tne network code does memset for 6 and 8 byte values, that can easily
> > be optimized into simple assignments without string instructions.
> 
> 
> so... question.
> Why are we doing this by hand? Wouldn't gcc just generate this code in
> the first place (when using __builtin_memset)? I very much suspect it
> would (and if some version doesn't.... we really ought to get that
> fixed)

i386 and x86_64 are not using __builtin_memset, as least from the
code that I see generated.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

^ permalink raw reply

* Re: [PATCH] i386: optimize memset of 6 and 8 bytes
From: Arjan van de Ven @ 2007-08-18  1:49 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, Andrew Morton, linux-kernel, netdev
In-Reply-To: <20070817165030.5455f761@freepuppy.rosehill.hemminger.net>


On Fri, 2007-08-17 at 16:50 -0700, Stephen Hemminger wrote:
> Tne network code does memset for 6 and 8 byte values, that can easily
> be optimized into simple assignments without string instructions.


so... question.
Why are we doing this by hand? Wouldn't gcc just generate this code in
the first place (when using __builtin_memset)? I very much suspect it
would (and if some version doesn't.... we really ought to get that
fixed)


^ permalink raw reply

* [PATCH] net/802: indentation cleanup
From: Stephen Hemminger @ 2007-08-18  1:53 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev

Run the 802 related protocols through Lindent (and hand cleanup)
to fix indentation and whitespace style issues.

--- a/net/802/fc.c	2007-08-17 14:39:56.000000000 -0400
+++ b/net/802/fc.c	2007-08-17 14:50:52.000000000 -0400
@@ -44,33 +44,29 @@ static int fc_header(struct sk_buff *skb
 	 * Add the 802.2 SNAP header if IP as the IPv4 code calls
 	 * dev->hard_header directly.
 	 */
-	if (type == ETH_P_IP || type == ETH_P_ARP)
-	{
+	if (type == ETH_P_IP || type == ETH_P_ARP) {
 		struct fcllc *fcllc;
 
 		hdr_len = sizeof(struct fch_hdr) + sizeof(struct fcllc);
 		fch = (struct fch_hdr *)skb_push(skb, hdr_len);
-		fcllc = (struct fcllc *)(fch+1);
+		fcllc = (struct fcllc *)(fch + 1);
 		fcllc->dsap = fcllc->ssap = EXTENDED_SAP;
 		fcllc->llc = UI_CMD;
 		fcllc->protid[0] = fcllc->protid[1] = fcllc->protid[2] = 0x00;
 		fcllc->ethertype = htons(type);
-	}
-	else
-	{
+	} else {
 		hdr_len = sizeof(struct fch_hdr);
 		fch = (struct fch_hdr *)skb_push(skb, hdr_len);
 	}
 
-	if(saddr)
-		memcpy(fch->saddr,saddr,dev->addr_len);
+	if (saddr)
+		memcpy(fch->saddr, saddr, dev->addr_len);
 	else
-		memcpy(fch->saddr,dev->dev_addr,dev->addr_len);
+		memcpy(fch->saddr, dev->dev_addr, dev->addr_len);
 
-	if(daddr)
-	{
-		memcpy(fch->daddr,daddr,dev->addr_len);
-		return(hdr_len);
+	if (daddr) {
+		memcpy(fch->daddr, daddr, dev->addr_len);
+		return hdr_len;
 	}
 	return -hdr_len;
 }
@@ -82,10 +78,13 @@ static int fc_header(struct sk_buff *skb
 
 static int fc_rebuild_header(struct sk_buff *skb)
 {
-	struct fch_hdr *fch=(struct fch_hdr *)skb->data;
-	struct fcllc *fcllc=(struct fcllc *)(skb->data+sizeof(struct fch_hdr));
-	if(fcllc->ethertype != htons(ETH_P_IP)) {
-		printk("fc_rebuild_header: Don't know how to resolve type %04X addresses ?\n", ntohs(fcllc->ethertype));
+	struct fch_hdr *fch = (struct fch_hdr *)skb->data;
+	struct fcllc *fcllc = (struct fcllc *)(skb->data + sizeof(struct fch_hdr));
+
+	if (fcllc->ethertype != htons(ETH_P_IP)) {
+		printk
+		    ("fc_rebuild_header: Don't know how to resolve type %04X addresses ?\n",
+		     ntohs(fcllc->ethertype));
 		return 0;
 	}
 #ifdef CONFIG_INET
--- a/net/802/tr.c	2007-08-17 14:39:56.000000000 -0400
+++ b/net/802/tr.c	2007-08-17 14:50:55.000000000 -0400
@@ -67,14 +67,13 @@ static struct rif_cache *rif_table[RIF_T
 
 static DEFINE_SPINLOCK(rif_lock);
 
-
 /*
  *	Garbage disposal timer.
  */
 
 static struct timer_list rif_timer;
 
-int sysctl_tr_rif_timeout = 60*10*HZ;
+int sysctl_tr_rif_timeout = 60 * 10 * HZ;
 
 static inline unsigned long rif_hash(const unsigned char *addr)
 {
@@ -108,41 +107,37 @@ static int tr_header(struct sk_buff *skb
 	 * Add the 802.2 SNAP header if IP as the IPv4/IPv6 code calls
 	 * dev->hard_header directly.
 	 */
-	if (type == ETH_P_IP || type == ETH_P_IPV6 || type == ETH_P_ARP)
-	{
+	if (type == ETH_P_IP || type == ETH_P_IPV6 || type == ETH_P_ARP) {
 		struct trllc *trllc;
 
 		hdr_len = sizeof(struct trh_hdr) + sizeof(struct trllc);
 		trh = (struct trh_hdr *)skb_push(skb, hdr_len);
-		trllc = (struct trllc *)(trh+1);
+		trllc = (struct trllc *)(trh + 1);
 		trllc->dsap = trllc->ssap = EXTENDED_SAP;
 		trllc->llc = UI_CMD;
 		trllc->protid[0] = trllc->protid[1] = trllc->protid[2] = 0x00;
 		trllc->ethertype = htons(type);
-	}
-	else
-	{
+	} else {
 		hdr_len = sizeof(struct trh_hdr);
 		trh = (struct trh_hdr *)skb_push(skb, hdr_len);
 	}
 
-	trh->ac=AC;
-	trh->fc=LLC_FRAME;
+	trh->ac = AC;
+	trh->fc = LLC_FRAME;
 
-	if(saddr)
-		memcpy(trh->saddr,saddr,dev->addr_len);
+	if (saddr)
+		memcpy(trh->saddr, saddr, dev->addr_len);
 	else
-		memcpy(trh->saddr,dev->dev_addr,dev->addr_len);
+		memcpy(trh->saddr, dev->dev_addr, dev->addr_len);
 
 	/*
-	 *	Build the destination and then source route the frame
+	 *      Build the destination and then source route the frame
 	 */
 
-	if(daddr)
-	{
-		memcpy(trh->daddr,daddr,dev->addr_len);
-		tr_source_route(skb,trh,dev);
-		return(hdr_len);
+	if (daddr) {
+		memcpy(trh->daddr, daddr, dev->addr_len);
+		tr_source_route(skb, trh, dev);
+		return hdr_len;
 	}
 
 	return -hdr_len;
@@ -155,27 +150,27 @@ static int tr_header(struct sk_buff *skb
 
 static int tr_rebuild_header(struct sk_buff *skb)
 {
-	struct trh_hdr *trh=(struct trh_hdr *)skb->data;
-	struct trllc *trllc=(struct trllc *)(skb->data+sizeof(struct trh_hdr));
+	struct trh_hdr *trh = (struct trh_hdr *)skb->data;
+	struct trllc *trllc = (struct trllc *)(skb->data + sizeof(struct trh_hdr));
 	struct net_device *dev = skb->dev;
 
 	/*
-	 *	FIXME: We don't yet support IPv6 over token rings
+	 *      FIXME: We don't yet support IPv6 over token rings
 	 */
 
-	if(trllc->ethertype != htons(ETH_P_IP)) {
-		printk("tr_rebuild_header: Don't know how to resolve type %04X addresses ?\n", ntohs(trllc->ethertype));
+	if (trllc->ethertype != htons(ETH_P_IP)) {
+		printk("tr_rebuild_header: Don't know how to resolve"
+		       " type %04X addresses ?\n",
+		       ntohs(trllc->ethertype));
 		return 0;
 	}
-
 #ifdef CONFIG_INET
-	if(arp_find(trh->daddr, skb)) {
-			return 1;
-	}
-	else
+	if (arp_find(trh->daddr, skb)) {
+		return 1;
+	} else
 #endif
 	{
-		tr_source_route(skb,trh,dev);
+		tr_source_route(skb, trh, dev);
 		return 0;
 	}
 }
@@ -186,44 +181,41 @@ static int tr_rebuild_header(struct sk_b
  *	it via SNAP.
  */
 
-__be16 tr_type_trans(struct sk_buff *skb, struct net_device *dev)
+__be16 tr_type_trans(struct sk_buff * skb, struct net_device * dev)
 {
 
 	struct trh_hdr *trh;
 	struct trllc *trllc;
-	unsigned riflen=0;
+	unsigned riflen = 0;
 
 	skb->dev = dev;
 	skb_reset_mac_header(skb);
 	trh = tr_hdr(skb);
 
-	if(trh->saddr[0] & TR_RII)
+	if (trh->saddr[0] & TR_RII)
 		riflen = (ntohs(trh->rcf) & TR_RCF_LEN_MASK) >> 8;
 
-	trllc = (struct trllc *)(skb->data+sizeof(struct trh_hdr)-TR_MAXRIFLEN+riflen);
+	trllc = (struct trllc *)(skb->data + sizeof(struct trh_hdr)
+				 - TR_MAXRIFLEN + riflen);
 
-	skb_pull(skb,sizeof(struct trh_hdr)-TR_MAXRIFLEN+riflen);
+	skb_pull(skb, sizeof(struct trh_hdr) - TR_MAXRIFLEN + riflen);
 
-	if(*trh->daddr & 0x80)
-	{
-		if(!memcmp(trh->daddr,dev->broadcast,TR_ALEN))
-			skb->pkt_type=PACKET_BROADCAST;
+	if (*trh->daddr & 0x80) {
+		if (!memcmp(trh->daddr, dev->broadcast, TR_ALEN))
+			skb->pkt_type = PACKET_BROADCAST;
 		else
-			skb->pkt_type=PACKET_MULTICAST;
-	}
-	else if ( (trh->daddr[0] & 0x01) && (trh->daddr[1] & 0x00) && (trh->daddr[2] & 0x5E))
-	{
-		skb->pkt_type=PACKET_MULTICAST;
-	}
-	else if(dev->flags & IFF_PROMISC)
-	{
-		if(memcmp(trh->daddr, dev->dev_addr, TR_ALEN))
-			skb->pkt_type=PACKET_OTHERHOST;
+			skb->pkt_type = PACKET_MULTICAST;
+	} else if ((trh->daddr[0] & 0x01) && (trh->daddr[1] & 0x00)
+		   && (trh->daddr[2] & 0x5E)) {
+		skb->pkt_type = PACKET_MULTICAST;
+	} else if (dev->flags & IFF_PROMISC) {
+		if (memcmp(trh->daddr, dev->dev_addr, TR_ALEN))
+			skb->pkt_type = PACKET_OTHERHOST;
 	}
 
 	if ((skb->pkt_type != PACKET_BROADCAST) &&
 	    (skb->pkt_type != PACKET_MULTICAST))
-		tr_add_rif_info(trh,dev) ;
+		tr_add_rif_info(trh, dev);
 
 	/*
 	 * Strip the SNAP header from ARP packets since we don't
@@ -233,8 +225,7 @@ __be16 tr_type_trans(struct sk_buff *skb
 	if (trllc->dsap == EXTENDED_SAP &&
 	    (trllc->ethertype == htons(ETH_P_IP) ||
 	     trllc->ethertype == htons(ETH_P_IPV6) ||
-	     trllc->ethertype == htons(ETH_P_ARP)))
-	{
+	     trllc->ethertype == htons(ETH_P_ARP))) {
 		skb_pull(skb, sizeof(struct trllc));
 		return trllc->ethertype;
 	}
@@ -246,7 +237,8 @@ __be16 tr_type_trans(struct sk_buff *skb
  *	We try to do source routing...
  */
 
-void tr_source_route(struct sk_buff *skb,struct trh_hdr *trh,struct net_device *dev)
+void tr_source_route(struct sk_buff *skb, struct trh_hdr *trh,
+		     struct net_device *dev)
 {
 	int slack;
 	unsigned int hash;
@@ -254,65 +246,63 @@ void tr_source_route(struct sk_buff *skb
 	unsigned char *olddata;
 	unsigned long flags;
 	static const unsigned char mcast_func_addr[]
-		= {0xC0,0x00,0x00,0x04,0x00,0x00};
+		= { 0xC0, 0x00, 0x00, 0x04, 0x00, 0x00 };
 
 	spin_lock_irqsave(&rif_lock, flags);
 
 	/*
-	 *	Broadcasts are single route as stated in RFC 1042
+	 *      Broadcasts are single route as stated in RFC 1042
 	 */
-	if( (!memcmp(&(trh->daddr[0]),&(dev->broadcast[0]),TR_ALEN)) ||
-	    (!memcmp(&(trh->daddr[0]),&(mcast_func_addr[0]), TR_ALEN))  )
-	{
-		trh->rcf=htons((((sizeof(trh->rcf)) << 8) & TR_RCF_LEN_MASK)
-			       | TR_RCF_FRAME2K | TR_RCF_LIMITED_BROADCAST);
-		trh->saddr[0]|=TR_RII;
-	}
-	else
-	{
+	if ((!memcmp(&(trh->daddr[0]), &(dev->broadcast[0]), TR_ALEN)) ||
+	    (!memcmp(&(trh->daddr[0]), &(mcast_func_addr[0]), TR_ALEN))) {
+		trh->rcf = htons((((sizeof(trh->rcf)) << 8) & TR_RCF_LEN_MASK)
+				 | TR_RCF_FRAME2K | TR_RCF_LIMITED_BROADCAST);
+		trh->saddr[0] |= TR_RII;
+	} else {
 		hash = rif_hash(trh->daddr);
 		/*
-		 *	Walk the hash table and look for an entry
+		 *      Walk the hash table and look for an entry
 		 */
-		for(entry=rif_table[hash];entry && memcmp(&(entry->addr[0]),&(trh->daddr[0]),TR_ALEN);entry=entry->next);
+		for (entry = rif_table[hash];
+		     entry && memcmp(&(entry->addr[0]), &(trh->daddr[0]), TR_ALEN);
+		     entry = entry->next) ;
 
 		/*
-		 *	If we found an entry we can route the frame.
+		 *      If we found an entry we can route the frame.
 		 */
-		if(entry)
-		{
+		if (entry) {
 #if TR_SR_DEBUG
-printk("source routing for %02X:%02X:%02X:%02X:%02X:%02X\n",trh->daddr[0],
-		  trh->daddr[1],trh->daddr[2],trh->daddr[3],trh->daddr[4],trh->daddr[5]);
+			printk("source routing for %02X:%02X:%02X:%02X:%02X:%02X\n",
+			       trh->daddr[0], trh->daddr[1], trh->daddr[2],
+			       trh->daddr[3], trh->daddr[4], trh->daddr[5]);
 #endif
-			if(!entry->local_ring && (ntohs(entry->rcf) & TR_RCF_LEN_MASK) >> 8)
-			{
-				trh->rcf=entry->rcf;
-				memcpy(&trh->rseg[0],&entry->rseg[0],8*sizeof(unsigned short));
-				trh->rcf^=htons(TR_RCF_DIR_BIT);
-				trh->rcf&=htons(0x1fff);	/* Issam Chehab <ichehab@madge1.demon.co.uk> */
+			if (!entry->local_ring
+			    && (ntohs(entry->rcf) & TR_RCF_LEN_MASK) >> 8) {
+				trh->rcf = entry->rcf;
+				memcpy(&trh->rseg[0], &entry->rseg[0],
+				       8 * sizeof(unsigned short));
+				trh->rcf ^= htons(TR_RCF_DIR_BIT);
+				trh->rcf &= htons(0x1fff);	/* Issam Chehab <ichehab@madge1.demon.co.uk> */
 
-				trh->saddr[0]|=TR_RII;
+				trh->saddr[0] |= TR_RII;
 #if TR_SR_DEBUG
-				printk("entry found with rcf %04x\n", entry->rcf);
-			}
-			else
-			{
-				printk("entry found but without rcf length, local=%02x\n", entry->local_ring);
+				printk("entry found with rcf %04x\n",
+				       entry->rcf);
+			} else {
+				printk("entry found but without rcf length,"
+				       "local=%02x\n", entry->local_ring);
 #endif
 			}
-			entry->last_used=jiffies;
-		}
-		else
-		{
+			entry->last_used = jiffies;
+		} else {
 			/*
-			 *	Without the information we simply have to shout
-			 *	on the wire. The replies should rapidly clean this
-			 *	situation up.
+			 *      Without the information we simply have to shout
+			 *      on the wire. The replies should rapidly clean this
+			 *      situation up.
 			 */
-			trh->rcf=htons((((sizeof(trh->rcf)) << 8) & TR_RCF_LEN_MASK)
-				       | TR_RCF_FRAME2K | TR_RCF_LIMITED_BROADCAST);
-			trh->saddr[0]|=TR_RII;
+			trh->rcf = htons((((sizeof(trh->rcf)) << 8) & TR_RCF_LEN_MASK)
+				  | TR_RCF_FRAME2K | TR_RCF_LIMITED_BROADCAST);
+			trh->saddr[0] |= TR_RII;
 #if TR_SR_DEBUG
 			printk("no entry in rif table found - broadcasting frame\n");
 #endif
@@ -323,7 +313,7 @@ printk("source routing for %02X:%02X:%02
 	if (!(trh->saddr[0] & 0x80))
 		slack = 18;
 	else
-		slack = 18 - ((ntohs(trh->rcf) & TR_RCF_LEN_MASK)>>8);
+		slack = 18 - ((ntohs(trh->rcf) & TR_RCF_LEN_MASK) >> 8);
 	olddata = skb->data;
 	spin_unlock_irqrestore(&rif_lock, flags);
 
@@ -347,83 +337,84 @@ static void tr_add_rif_info(struct trh_h
 	saddr0 = trh->saddr[0];
 
 	/*
-	 *	Firstly see if the entry exists
+	 *      Firstly see if the entry exists
 	 */
 
-	if(trh->saddr[0] & TR_RII)
-	{
-		trh->saddr[0]&=0x7f;
-		if (((ntohs(trh->rcf) & TR_RCF_LEN_MASK) >> 8) > 2)
-		{
+	if (trh->saddr[0] & TR_RII) {
+		trh->saddr[0] &= 0x7f;
+		if (((ntohs(trh->rcf) & TR_RCF_LEN_MASK) >> 8) > 2) {
 			rii_p = 1;
 		}
 	}
 
 	hash = rif_hash(trh->saddr);
-	for(entry=rif_table[hash];entry && memcmp(&(entry->addr[0]),&(trh->saddr[0]),TR_ALEN);entry=entry->next);
+	for (entry = rif_table[hash];
+	     entry && memcmp(&(entry->addr[0]), &(trh->saddr[0]), TR_ALEN);
+	     entry = entry->next) ;
 
-	if(entry==NULL)
-	{
+	if (entry == NULL) {
 #if TR_SR_DEBUG
-printk("adding rif_entry: addr:%02X:%02X:%02X:%02X:%02X:%02X rcf:%04X\n",
-		trh->saddr[0],trh->saddr[1],trh->saddr[2],
-		trh->saddr[3],trh->saddr[4],trh->saddr[5],
-		ntohs(trh->rcf));
+		printk("adding rif_entry: addr:%02X:%02X:%02X:%02X:%02X:%02X"
+		       " rcf:%04X\n",
+		       trh->saddr[0], trh->saddr[1], trh->saddr[2], trh->saddr[3],
+		       trh->saddr[4], trh->saddr[5], ntohs(trh->rcf));
 #endif
 		/*
-		 *	Allocate our new entry. A failure to allocate loses
-		 *	use the information. This is harmless.
+		 *      Allocate our new entry. A failure to allocate loses
+		 *      use the information. This is harmless.
 		 *
-		 *	FIXME: We ought to keep some kind of cache size
-		 *	limiting and adjust the timers to suit.
+		 *      FIXME: We ought to keep some kind of cache size
+		 *      limiting and adjust the timers to suit.
 		 */
-		entry=kmalloc(sizeof(struct rif_cache),GFP_ATOMIC);
+		entry = kmalloc(sizeof(struct rif_cache), GFP_ATOMIC);
 
-		if(!entry)
-		{
-			printk(KERN_DEBUG "tr.c: Couldn't malloc rif cache entry !\n");
+		if (!entry) {
+			printk(KERN_DEBUG
+			       "tr.c: Couldn't malloc rif cache entry !\n");
 			spin_unlock_irqrestore(&rif_lock, flags);
 			return;
 		}
 
-		memcpy(&(entry->addr[0]),&(trh->saddr[0]),TR_ALEN);
+		memcpy(&(entry->addr[0]), &(trh->saddr[0]), TR_ALEN);
 		entry->iface = dev->ifindex;
-		entry->next=rif_table[hash];
-		entry->last_used=jiffies;
-		rif_table[hash]=entry;
-
-		if (rii_p)
-		{
-			entry->rcf = trh->rcf & htons((unsigned short)~TR_RCF_BROADCAST_MASK);
-			memcpy(&(entry->rseg[0]),&(trh->rseg[0]),8*sizeof(unsigned short));
+		entry->next = rif_table[hash];
+		entry->last_used = jiffies;
+		rif_table[hash] = entry;
+
+		if (rii_p) {
+			entry->rcf = trh->rcf
+				& htons((unsigned short)~TR_RCF_BROADCAST_MASK);
+			memcpy(&(entry->rseg[0]), &(trh->rseg[0]),
+			       8 * sizeof(unsigned short));
 			entry->local_ring = 0;
-		}
-		else
-		{
+		} else {
 			entry->local_ring = 1;
 		}
-	}
-	else	/* Y. Tahara added */
-	{
+	} else {		/* Y. Tahara added */
+
 		/*
-		 *	Update existing entries
+		 *      Update existing entries
 		 */
 		if (!entry->local_ring)
 		    if (entry->rcf != (trh->rcf & htons((unsigned short)~TR_RCF_BROADCAST_MASK)) &&
-			 !(trh->rcf & htons(TR_RCF_BROADCAST_MASK)))
-		    {
+			    && !(trh->rcf & htons(TR_RCF_BROADCAST_MASK))) {
 #if TR_SR_DEBUG
-printk("updating rif_entry: addr:%02X:%02X:%02X:%02X:%02X:%02X rcf:%04X\n",
-		trh->saddr[0],trh->saddr[1],trh->saddr[2],
-		trh->saddr[3],trh->saddr[4],trh->saddr[5],
-		ntohs(trh->rcf));
+				printk("updating rif_entry:"
+				       " addr:%02X:%02X:%02X:%02X:%02X:%02X"
+				       " rcf:%04X\n",
+				       trh->saddr[0], trh->saddr[1],
+				       trh->saddr[2], trh->saddr[3],
+				       trh->saddr[4], trh->saddr[5],
+				       ntohs(trh->rcf));
 #endif
-			    entry->rcf = trh->rcf & htons((unsigned short)~TR_RCF_BROADCAST_MASK);
-			    memcpy(&(entry->rseg[0]),&(trh->rseg[0]),8*sizeof(unsigned short));
-		    }
-		entry->last_used=jiffies;
+				entry->rcf = trh->rcf
+					& htons((unsigned short)~TR_RCF_BROADCAST_MASK);
+				memcpy(&(entry->rseg[0]), &(trh->rseg[0]),
+				       8 * sizeof(unsigned short));
+			}
+		entry->last_used = jiffies;
 	}
-	trh->saddr[0]=saddr0; /* put the routing indicator back for tcpdump */
+	trh->saddr[0] = saddr0;	/* put the routing indicator back for tcpdump */
 	spin_unlock_irqrestore(&rif_lock, flags);
 }
 
@@ -434,17 +425,17 @@ printk("updating rif_entry: addr:%02X:%0
 static void rif_check_expire(unsigned long dummy)
 {
 	int i;
-	unsigned long flags, next_interval = jiffies + sysctl_tr_rif_timeout/2;
+	unsigned long flags, next_interval = jiffies + sysctl_tr_rif_timeout / 2;
 
 	spin_lock_irqsave(&rif_lock, flags);
 
-	for(i =0; i < RIF_TABLE_SIZE; i++) {
+	for (i = 0; i < RIF_TABLE_SIZE; i++) {
 		struct rif_cache *entry, **pentry;
 
-		pentry = rif_table+i;
-		while((entry=*pentry) != NULL) {
+		pentry = rif_table + i;
+		while ((entry = *pentry) != NULL) {
 			unsigned long expires
-				= entry->last_used + sysctl_tr_rif_timeout;
+			    = entry->last_used + sysctl_tr_rif_timeout;
 
 			if (time_before_eq(expires, jiffies)) {
 				*pentry = entry->next;
@@ -477,8 +468,8 @@ static struct rif_cache *rif_get_idx(lof
 	struct rif_cache *entry;
 	loff_t off = 0;
 
-	for(i = 0; i < RIF_TABLE_SIZE; i++)
-		for(entry = rif_table[i]; entry; entry = entry->next) {
+	for (i = 0; i < RIF_TABLE_SIZE; i++)
+		for (entry = rif_table[i]; entry; entry = entry->next) {
 			if (off == pos)
 				return entry;
 			++off;
@@ -487,14 +478,14 @@ static struct rif_cache *rif_get_idx(lof
 	return NULL;
 }
 
-static void *rif_seq_start(struct seq_file *seq, loff_t *pos)
+static void *rif_seq_start(struct seq_file *seq, loff_t * pos)
 {
 	spin_lock_irq(&rif_lock);
 
 	return *pos ? rif_get_idx(*pos - 1) : SEQ_START_TOKEN;
 }
 
-static void *rif_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+static void *rif_seq_next(struct seq_file *seq, void *v, loff_t * pos)
 {
 	int i;
 	struct rif_cache *ent = v;
@@ -530,39 +521,39 @@ static int rif_seq_show(struct seq_file 
 
 	if (v == SEQ_START_TOKEN)
 		seq_puts(seq,
-		     "if     TR address       TTL   rcf   routing segments\n");
+			 "if     TR address       TTL   rcf   routing segments\n");
 	else {
 		struct net_device *dev = dev_get_by_index(entry->iface);
 		long ttl = (long) (entry->last_used + sysctl_tr_rif_timeout)
-				- (long) jiffies;
+			- (long) jiffies;
 
 		seq_printf(seq, "%s %02X:%02X:%02X:%02X:%02X:%02X %7li ",
 			   dev?dev->name:"?",
-			   entry->addr[0],entry->addr[1],entry->addr[2],
-			   entry->addr[3],entry->addr[4],entry->addr[5],
+			   entry->addr[0], entry->addr[1], entry->addr[2],
+			   entry->addr[3], entry->addr[4], entry->addr[5],
 			   ttl/HZ);
 
-			if (entry->local_ring)
-				seq_puts(seq, "local\n");
-			else {
-
-				seq_printf(seq, "%04X", ntohs(entry->rcf));
-				rcf_len = ((ntohs(entry->rcf) & TR_RCF_LEN_MASK)>>8)-2;
-				if (rcf_len)
-					rcf_len >>= 1;
-				for(j = 1; j < rcf_len; j++) {
-					if(j==1) {
-						segment=ntohs(entry->rseg[j-1])>>4;
-						seq_printf(seq,"  %03X",segment);
-					}
-
-					segment=ntohs(entry->rseg[j])>>4;
-					brdgnmb=ntohs(entry->rseg[j-1])&0x00f;
-					seq_printf(seq,"-%01X-%03X",brdgnmb,segment);
+		if (entry->local_ring)
+			seq_puts(seq, "local\n");
+		else {
+
+			seq_printf(seq, "%04X", ntohs(entry->rcf));
+			rcf_len = ((ntohs(entry->rcf) & TR_RCF_LEN_MASK)>>8)-2;
+			if (rcf_len)
+				rcf_len >>= 1;
+			for(j = 1; j < rcf_len; j++) {
+				if(j==1) {
+					segment = ntohs(entry->rseg[j-1])>>4;
+					seq_printf(seq,"  %03X", segment);
 				}
-				seq_putc(seq, '\n');
+
+				segment = ntohs(entry->rseg[j]) >> 4;
+				brdgnmb = ntohs(entry->rseg[j-1]) & 0x00f;
+				seq_printf(seq,"-%01X-%03X", brdgnmb, segment);
 			}
+			seq_putc(seq, '\n');
 		}
+	}
 	return 0;
 }
 
--- a/net/802/fddi.c	2007-08-17 14:30:18.000000000 -0400
+++ b/net/802/fddi.c	2007-08-17 14:49:51.000000000 -0400
@@ -57,12 +57,11 @@ static int fddi_header(struct sk_buff *s
 	int hl = FDDI_K_SNAP_HLEN;
 	struct fddihdr *fddi;
 
-	if(type != ETH_P_IP && type != ETH_P_IPV6 && type != ETH_P_ARP)
-		hl=FDDI_K_8022_HLEN-3;
+	if (type != ETH_P_IP && type != ETH_P_IPV6 && type != ETH_P_ARP)
+		hl = FDDI_K_8022_HLEN - 3;
 	fddi = (struct fddihdr *)skb_push(skb, hl);
-	fddi->fc			 = FDDI_FC_K_ASYNC_LLC_DEF;
-	if(type == ETH_P_IP || type == ETH_P_IPV6 || type == ETH_P_ARP)
-	{
+	fddi->fc = FDDI_FC_K_ASYNC_LLC_DEF;
+	if (type == ETH_P_IP || type == ETH_P_IPV6 || type == ETH_P_ARP) {
 		fddi->hdr.llc_snap.dsap		 = FDDI_EXTENDED_SAP;
 		fddi->hdr.llc_snap.ssap		 = FDDI_EXTENDED_SAP;
 		fddi->hdr.llc_snap.ctrl		 = FDDI_UI_CMD;
@@ -79,23 +78,21 @@ static int fddi_header(struct sk_buff *s
 	else
 		memcpy(fddi->saddr, dev->dev_addr, dev->addr_len);
 
-	if (daddr != NULL)
-	{
+	if (daddr != NULL) {
 		memcpy(fddi->daddr, daddr, dev->addr_len);
-		return(hl);
+		return hl;
 	}
 
-	return(-hl);
+	return -hl;
 }
 
-
 /*
  * Rebuild the FDDI MAC header. This is called after an ARP
  * (or in future other address resolution) has completed on
  * this sk_buff.  We now let ARP fill in the other fields.
  */
 
-static int fddi_rebuild_header(struct sk_buff	*skb)
+static int fddi_rebuild_header(struct sk_buff *skb)
 {
 	struct fddihdr *fddi = (struct fddihdr *)skb->data;
 
@@ -108,11 +105,10 @@ static int fddi_rebuild_header(struct sk
 	{
 		printk("%s: Don't know how to resolve type %04X addresses.\n",
 		       skb->dev->name, ntohs(fddi->hdr.llc_snap.ethertype));
-		return(0);
+		return 0;
 	}
 }
 
-
 /*
  * Determine the packet's protocol ID and fill in skb fields.
  * This routine is called before an incoming packet is passed
@@ -120,7 +116,7 @@ static int fddi_rebuild_header(struct sk
  * the proper pointer to the start of packet data (skb->data).
  */
 
-__be16 fddi_type_trans(struct sk_buff *skb, struct net_device *dev)
+__be16 fddi_type_trans(struct sk_buff * skb, struct net_device * dev)
 {
 	struct fddihdr *fddi = (struct fddihdr *)skb->data;
 	__be16 type;
@@ -133,36 +129,31 @@ __be16 fddi_type_trans(struct sk_buff *s
 	skb->dev = dev;
 	skb_reset_mac_header(skb);	/* point to frame control (FC) */
 
-	if(fddi->hdr.llc_8022_1.dsap==0xe0)
-	{
-		skb_pull(skb, FDDI_K_8022_HLEN-3);
+	if (fddi->hdr.llc_8022_1.dsap == 0xe0) {
+		skb_pull(skb, FDDI_K_8022_HLEN - 3);
 		type = htons(ETH_P_802_2);
-	}
-	else
-	{
-		skb_pull(skb, FDDI_K_SNAP_HLEN);		/* adjust for 21 byte header */
-		type=fddi->hdr.llc_snap.ethertype;
+	} else {
+		skb_pull(skb, FDDI_K_SNAP_HLEN);	/* adjust for 21 byte header */
+		type = fddi->hdr.llc_snap.ethertype;
 	}
 
 	/* Set packet type based on destination address and flag settings */
 
-	if (*fddi->daddr & 0x01)
-	{
+	if (*fddi->daddr & 0x01) {
 		if (memcmp(fddi->daddr, dev->broadcast, FDDI_K_ALEN) == 0)
 			skb->pkt_type = PACKET_BROADCAST;
 		else
 			skb->pkt_type = PACKET_MULTICAST;
 	}
 
-	else if (dev->flags & IFF_PROMISC)
-	{
+	else if (dev->flags & IFF_PROMISC) {
 		if (memcmp(fddi->daddr, dev->dev_addr, FDDI_K_ALEN))
 			skb->pkt_type = PACKET_OTHERHOST;
 	}
 
 	/* Assume 802.2 SNAP frames, for now */
 
-	return(type);
+	return type;
 }
 
 EXPORT_SYMBOL(fddi_type_trans);
@@ -170,9 +161,9 @@ EXPORT_SYMBOL(fddi_type_trans);
 static int fddi_change_mtu(struct net_device *dev, int new_mtu)
 {
 	if ((new_mtu < FDDI_K_SNAP_HLEN) || (new_mtu > FDDI_K_SNAP_DLEN))
-		return(-EINVAL);
+		return -EINVAL;
 	dev->mtu = new_mtu;
-	return(0);
+	return 0;
 }
 
 static void fddi_setup(struct net_device *dev)
@@ -206,4 +197,5 @@ struct net_device *alloc_fddidev(int siz
 {
 	return alloc_netdev(sizeof_priv, "fddi%d", fddi_setup);
 }
+
 EXPORT_SYMBOL(alloc_fddidev);
--- a/net/802/hippi.c	2007-08-17 14:30:18.000000000 -0400
+++ b/net/802/hippi.c	2007-08-17 14:45:39.000000000 -0400
@@ -49,9 +49,9 @@ static int hippi_header(struct sk_buff *
 			unsigned len)
 {
 	struct hippi_hdr *hip = (struct hippi_hdr *)skb_push(skb, HIPPI_HLEN);
-	struct hippi_cb *hcb = (struct hippi_cb *) skb->cb;
+	struct hippi_cb *hcb = (struct hippi_cb *)skb->cb;
 
-	if (!len){
+	if (!len) {
 		len = skb->len - HIPPI_HLEN;
 		printk("hippi_header(): length not supplied\n");
 	}
@@ -80,8 +80,7 @@ static int hippi_header(struct sk_buff *
 	hip->snap.oui[2]	= 0x00;
 	hip->snap.ethertype	= htons(type);
 
-	if (daddr)
-	{
+	if (daddr) {
 		memcpy(hip->le.dest_switch_addr, daddr + 3, 3);
 		memcpy(&hcb->ifield, daddr + 2, 4);
 		return HIPPI_HLEN;
@@ -90,7 +89,6 @@ static int hippi_header(struct sk_buff *
 	return -((int)HIPPI_HLEN);
 }
 
-
 /*
  * Rebuild the HIPPI MAC header. This is called after an ARP has
  * completed on this sk_buff. We now let ARP fill in the other fields.
@@ -104,9 +102,9 @@ static int hippi_rebuild_header(struct s
 	 * Only IP is currently supported
 	 */
 
-	if(hip->snap.ethertype != htons(ETH_P_IP))
-	{
-		printk(KERN_DEBUG "%s: unable to resolve type %X addresses.\n",skb->dev->name,ntohs(hip->snap.ethertype));
+	if (hip->snap.ethertype != htons(ETH_P_IP)) {
+		printk(KERN_DEBUG "%s: unable to resolve type %X addresses.\n",
+		       skb->dev->name, ntohs(hip->snap.ethertype));
 		return 0;
 	}
 
@@ -117,12 +115,11 @@ static int hippi_rebuild_header(struct s
 	return arp_find(hip->le.daddr, skb);
 }
 
-
 /*
  *	Determine the packet's protocol ID.
  */
 
-__be16 hippi_type_trans(struct sk_buff *skb, struct net_device *dev)
+__be16 hippi_type_trans(struct sk_buff * skb, struct net_device * dev)
 {
 	struct hippi_hdr *hip;
 
@@ -152,7 +149,7 @@ static int hippi_change_mtu(struct net_d
 	if ((new_mtu < 68) || (new_mtu > 65280))
 		return -EINVAL;
 	dev->mtu = new_mtu;
-	return(0);
+	return 0;
 }
 
 /*
@@ -174,9 +171,9 @@ static int hippi_neigh_setup_dev(struct 
 	p->mcast_probes = 0;
 
 	/* In IPv6 unicast probes are valid even on NBMA,
-	* because they are encapsulated in normal IPv6 protocol.
-	* Should be a generic flag.
-	*/
+	 * because they are encapsulated in normal IPv6 protocol.
+	 * Should be a generic flag.
+	 */
 	if (p->tbl->family != AF_INET6)
 		p->ucast_probes = 0;
 	return 0;
@@ -206,7 +203,6 @@ static void hippi_setup(struct net_devic
 	dev->tx_queue_len	= 25 /* 5 */;
 	memset(dev->broadcast, 0xFF, HIPPI_ALEN);
 
-
 	/*
 	 * HIPPI doesn't support broadcast+multicast and we only use
 	 * static ARP tables. ARP is disabled by hippi_neigh_setup_dev.

^ permalink raw reply

* Re: [RFC] restore netdev_priv optimization (planb)
From: Kok, Auke @ 2007-08-18  1:48 UTC (permalink / raw)
  To: David Miller; +Cc: auke-jan.h.kok, shemminger, netdev
In-Reply-To: <20070817.183008.38712695.davem@davemloft.net>

David Miller wrote:
> From: "Kok, Auke" <auke-jan.h.kok@intel.com>
> Date: Fri, 17 Aug 2007 18:21:25 -0700
> 
>> this sounds highly optimistic ("64 queues is enough for everyone"?)
>> and probably will be quickly outdated by both hardware and demand...
> 
> As such drivers appear in the tree we can adjust the value.
> 
> Even the most aggressively multi-queued virtualization and 10GB
> ethernet chips I am aware of, both in production and in development,
> do not exceed this limit.
> 
> Since you think this is worth complaining about, you must know of some
> exceptions? :-)

I actually don't, but I assume that demand for queues will quickly increase once 
the feature becomes available. e.g. in e1000 hardware (pci-e) we support 2, and 
this is really old hardware already (laugh), 82575 has 4.

the ixgbe driver me and Ayyappan posted already supports 64 rx queues and 32 tx...

I can only expect the next generation to take a bigger jump and implement at 
least 128 queues... :)

Auke

^ permalink raw reply

* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Satyam Sharma @ 2007-08-18  1:56 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Christoph Lameter, Paul Mackerras, heiko.carstens, horms,
	Stefan Richter, Linux Kernel Mailing List, David Miller,
	Paul E. McKenney, Ilpo Järvinen, ak, cfriesen, rpjday,
	Netdev, jesper.juhl, linux-arch, zlynx, Andrew Morton,
	schwidefsky, Chris Snook, Herbert Xu, Linus Torvalds, wensong,
	wjiang
In-Reply-To: <c0115c88da5d6756502e0c0499faa71f@kernel.crashing.org>



On Sat, 18 Aug 2007, Segher Boessenkool wrote:

> > > > > atomic_dec() writes
> > > > > to memory, so it _does_ have "volatile semantics", implicitly, as
> > > > > long as the compiler cannot optimise the atomic variable away
> > > > > completely -- any store counts as a side effect.
> > > > 
> > > > I don't think an atomic_dec() implemented as an inline "asm volatile"
> > > > or one that uses a "forget" macro would have the same re-ordering
> > > > guarantees as an atomic_dec() that uses a volatile access cast.
> > > 
> > > The "asm volatile" implementation does have exactly the same
> > > reordering guarantees as the "volatile cast" thing,
> > 
> > I don't think so.
> 
> "asm volatile" creates a side effect.

Yeah.

> Side effects aren't
> allowed to be reordered wrt sequence points.

Yeah.

> This is exactly
> the same reason as why "volatile accesses" cannot be reordered.

No, the code in that sub-thread I earlier pointed you at *WAS* written
such that there was a sequence point after all the uses of that volatile
access cast macro, and _therefore_ we were safe from re-ordering
(behaviour guaranteed by the C standard).

But you seem to be missing the simple and basic fact that:

	(something_that_has_side_effects || statement)
			!= something_that_is_a_sequence_point

Now, one cannot fantasize that "volatile asms" are also sequence points.
In fact such an argument would be sadly mistaken, because "sequence
points" are defined by the C standard and it'd be horribly wrong to
even _try_ claiming that the C standard knows about "volatile asms".


> > > if that is
> > > implemented by GCC in the "obvious" way.  Even a "plain" asm()
> > > will do the same.
> > 
> > Read the relevant GCC documentation.
> 
> I did, yes.

No, you didn't read:

http://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html

Read the bit about the need for artificial dependencies, and the example
given there:

	asm volatile("mtfsf 255,%0" : : "f" (fpenv));
	sum = x + y;

The docs explicitly say the addition can be moved before the "volatile
asm". Hopefully, as you know, (x + y) is an C "expression" and hence
a "sequence point" as defined by the standard. So the "volatile asm"
should've happened before it, right? Wrong.

I know there is also stuff written about "side-effects" there which
_could_ give the same semantic w.r.t. sequence points as the volatile
access casts, but hey, it's GCC's own documentation, you obviously can't
find fault with _me_ if there's wrong stuff written in there. Say that
to GCC ...

See, "volatile" C keyword, for all it's ill-definition and dodgy
semantics, is still at least given somewhat of a treatment in the C
standard (whose quality is ... ummm, sadly not always good and clear,
but unsurprisingly, still about 5,482 orders-of-magnitude times
better than GCC docs). Semantics of "volatile" as applies to inline
asm, OTOH? You're completely relying on the compiler for that ...


> > [ of course, if the (latest) GCC documentation is *yet again*
> >   wrong, then alright, not much I can do about it, is there. ]
> 
> There was (and is) nothing wrong about the "+m" documentation, if
> that is what you are talking about.  It could be extended now, to
> allow "+m" -- but that takes more than just "fixing" the documentation.

No, there was (and is) _everything_ wrong about the "+" documentation as
applies to memory-constrained operands. I don't give a whit if it's
some workaround in their gimplifier, or the other, that makes it possible
to use "+m" (like the current kernel code does). The docs suggest
otherwise, so there's obviously a clear disconnect between the docs and
actual GCC behaviour.


[ You seem to often take issue with _amazingly_ petty and pedantic things,
  by the way :-) ]

^ permalink raw reply

* [PATCH] atm: replace DPRINTK() with pr_debug
From: Stephen Hemminger @ 2007-08-18  1:31 UTC (permalink / raw)
  To: chas williams; +Cc: linux-atm-general, netdev

Get rid of using DPRINTK macro in ATM and use pr_debug (in kernel.h).
Using the standard macro is cleaner and forces code to check for bad arguments
and formatting.

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

--- a/net/atm/clip.c	2007-08-17 15:05:49.000000000 -0400
+++ b/net/atm/clip.c	2007-08-17 15:06:26.000000000 -0400
@@ -40,14 +40,6 @@
 #include "resources.h"
 #include <net/atmclip.h>
 
-
-#if 0
-#define DPRINTK(format,args...) printk(format,##args)
-#else
-#define DPRINTK(format,args...)
-#endif
-
-
 static struct net_device *clip_devs;
 static struct atm_vcc *atmarpd;
 static struct neigh_table clip_tbl;
@@ -59,7 +51,7 @@ static int to_atmarpd(enum atmarp_ctrl_t
 	struct atmarp_ctrl *ctrl;
 	struct sk_buff *skb;
 
-	DPRINTK("to_atmarpd(%d)\n", type);
+	pr_debug("to_atmarpd(%d)\n", type);
 	if (!atmarpd)
 		return -EUNATCH;
 	skb = alloc_skb(sizeof(struct atmarp_ctrl),GFP_ATOMIC);
@@ -79,7 +71,7 @@ static int to_atmarpd(enum atmarp_ctrl_t
 
 static void link_vcc(struct clip_vcc *clip_vcc, struct atmarp_entry *entry)
 {
-	DPRINTK("link_vcc %p to entry %p (neigh %p)\n", clip_vcc, entry,
+	pr_debug("link_vcc %p to entry %p (neigh %p)\n", clip_vcc, entry,
 		entry->neigh);
 	clip_vcc->entry = entry;
 	clip_vcc->xoff = 0;	/* @@@ may overrun buffer by one packet */
@@ -134,7 +126,7 @@ static int neigh_check_cb(struct neighbo
 		unsigned long exp = cv->last_use + cv->idle_timeout;
 
 		if (cv->idle_timeout && time_after(jiffies, exp)) {
-			DPRINTK("releasing vcc %p->%p of entry %p\n",
+			pr_debug("releasing vcc %p->%p of entry %p\n",
 				cv, cv->vcc, entry);
 			vcc_release_async(cv->vcc, -ETIMEDOUT);
 		}
@@ -146,7 +138,7 @@ static int neigh_check_cb(struct neighbo
 	if (atomic_read(&n->refcnt) > 1) {
 		struct sk_buff *skb;
 
-		DPRINTK("destruction postponed with ref %d\n",
+		pr_debug("destruction postponed with ref %d\n",
 			atomic_read(&n->refcnt));
 
 		while ((skb = skb_dequeue(&n->arp_queue)) != NULL)
@@ -155,7 +147,7 @@ static int neigh_check_cb(struct neighbo
 		return 0;
 	}
 
-	DPRINTK("expired neigh %p\n", n);
+	pr_debug("expired neigh %p\n", n);
 	return 1;
 }
 
@@ -171,14 +163,14 @@ static int clip_arp_rcv(struct sk_buff *
 {
 	struct atm_vcc *vcc;
 
-	DPRINTK("clip_arp_rcv\n");
+	pr_debug("clip_arp_rcv\n");
 	vcc = ATM_SKB(skb)->vcc;
 	if (!vcc || !atm_charge(vcc, skb->truesize)) {
 		dev_kfree_skb_any(skb);
 		return 0;
 	}
-	DPRINTK("pushing to %p\n", vcc);
-	DPRINTK("using %p\n", CLIP_VCC(vcc)->old_push);
+	pr_debug("pushing to %p\n", vcc);
+	pr_debug("using %p\n", CLIP_VCC(vcc)->old_push);
 	CLIP_VCC(vcc)->old_push(vcc, skb);
 	return 0;
 }
@@ -196,9 +188,9 @@ static void clip_push(struct atm_vcc *vc
 {
 	struct clip_vcc *clip_vcc = CLIP_VCC(vcc);
 
-	DPRINTK("clip push\n");
+	pr_debug("clip push\n");
 	if (!skb) {
-		DPRINTK("removing VCC %p\n", clip_vcc);
+		pr_debug("removing VCC %p\n", clip_vcc);
 		if (clip_vcc->entry)
 			unlink_clip_vcc(clip_vcc);
 		clip_vcc->old_push(vcc, NULL);	/* pass on the bad news */
@@ -247,7 +239,7 @@ static void clip_pop(struct atm_vcc *vcc
 	int old;
 	unsigned long flags;
 
-	DPRINTK("clip_pop(vcc %p)\n", vcc);
+	pr_debug("clip_pop(vcc %p)\n", vcc);
 	clip_vcc->old_pop(vcc, skb);
 	/* skb->dev == NULL in outbound ARP packets */
 	if (!dev)
@@ -263,7 +255,7 @@ static void clip_pop(struct atm_vcc *vcc
 
 static void clip_neigh_solicit(struct neighbour *neigh, struct sk_buff *skb)
 {
-	DPRINTK("clip_neigh_solicit (neigh %p, skb %p)\n", neigh, skb);
+	pr_debug("clip_neigh_solicit (neigh %p, skb %p)\n", neigh, skb);
 	to_atmarpd(act_need, PRIV(neigh->dev)->number, NEIGH2ENTRY(neigh)->ip);
 }
 
@@ -292,7 +284,7 @@ static int clip_constructor(struct neigh
 	struct in_device *in_dev;
 	struct neigh_parms *parms;
 
-	DPRINTK("clip_constructor (neigh %p, entry %p)\n", neigh, entry);
+	pr_debug("clip_constructor (neigh %p, entry %p)\n", neigh, entry);
 	neigh->type = inet_addr_type(entry->ip);
 	if (neigh->type != RTN_UNICAST)
 		return -EINVAL;
@@ -376,7 +368,7 @@ static int clip_start_xmit(struct sk_buf
 	int old;
 	unsigned long flags;
 
-	DPRINTK("clip_start_xmit (skb %p)\n", skb);
+	pr_debug("clip_start_xmit (skb %p)\n", skb);
 	if (!skb->dst) {
 		printk(KERN_ERR "clip_start_xmit: skb->dst == NULL\n");
 		dev_kfree_skb(skb);
@@ -412,9 +404,9 @@ static int clip_start_xmit(struct sk_buf
 		}
 		return 0;
 	}
-	DPRINTK("neigh %p, vccs %p\n", entry, entry->vccs);
+	pr_debug("neigh %p, vccs %p\n", entry, entry->vccs);
 	ATM_SKB(skb)->vcc = vcc = entry->vccs->vcc;
-	DPRINTK("using neighbour %p, vcc %p\n", skb->dst->neighbour, vcc);
+	pr_debug("using neighbour %p, vcc %p\n", skb->dst->neighbour, vcc);
 	if (entry->vccs->encap) {
 		void *here;
 
@@ -425,7 +417,7 @@ static int clip_start_xmit(struct sk_buf
 	atomic_add(skb->truesize, &sk_atm(vcc)->sk_wmem_alloc);
 	ATM_SKB(skb)->atm_options = vcc->atm_options;
 	entry->vccs->last_use = jiffies;
-	DPRINTK("atm_skb(%p)->vcc(%p)->dev(%p)\n", skb, vcc, vcc->dev);
+	pr_debug("atm_skb(%p)->vcc(%p)->dev(%p)\n", skb, vcc, vcc->dev);
 	old = xchg(&entry->vccs->xoff, 1);	/* assume XOFF ... */
 	if (old) {
 		printk(KERN_WARNING "clip_start_xmit: XOFF->XOFF transition\n");
@@ -468,7 +460,7 @@ static int clip_mkip(struct atm_vcc *vcc
 	clip_vcc = kmalloc(sizeof(struct clip_vcc), GFP_KERNEL);
 	if (!clip_vcc)
 		return -ENOMEM;
-	DPRINTK("mkip clip_vcc %p vcc %p\n", clip_vcc, vcc);
+	pr_debug("mkip clip_vcc %p vcc %p\n", clip_vcc, vcc);
 	clip_vcc->vcc = vcc;
 	vcc->user_back = clip_vcc;
 	set_bit(ATM_VF_IS_CLIP, &vcc->flags);
@@ -538,7 +530,7 @@ static int clip_setentry(struct atm_vcc 
 			printk(KERN_ERR "hiding hidden ATMARP entry\n");
 			return 0;
 		}
-		DPRINTK("setentry: remove\n");
+		pr_debug("setentry: remove\n");
 		unlink_clip_vcc(clip_vcc);
 		return 0;
 	}
@@ -552,9 +544,9 @@ static int clip_setentry(struct atm_vcc 
 	entry = NEIGH2ENTRY(neigh);
 	if (entry != clip_vcc->entry) {
 		if (!clip_vcc->entry)
-			DPRINTK("setentry: add\n");
+			pr_debug("setentry: add\n");
 		else {
-			DPRINTK("setentry: update\n");
+			pr_debug("setentry: update\n");
 			unlink_clip_vcc(clip_vcc);
 		}
 		link_vcc(clip_vcc, entry);
@@ -611,7 +603,7 @@ static int clip_create(int number)
 	}
 	clip_priv->next = clip_devs;
 	clip_devs = dev;
-	DPRINTK("registered (net:%s)\n", dev->name);
+	pr_debug("registered (net:%s)\n", dev->name);
 	return number;
 }
 
@@ -631,16 +623,16 @@ static int clip_device_event(struct noti
 
 	switch (event) {
 	case NETDEV_UP:
-		DPRINTK("clip_device_event NETDEV_UP\n");
+		pr_debug("clip_device_event NETDEV_UP\n");
 		to_atmarpd(act_up, PRIV(dev)->number, 0);
 		break;
 	case NETDEV_GOING_DOWN:
-		DPRINTK("clip_device_event NETDEV_DOWN\n");
+		pr_debug("clip_device_event NETDEV_DOWN\n");
 		to_atmarpd(act_down, PRIV(dev)->number, 0);
 		break;
 	case NETDEV_CHANGE:
 	case NETDEV_CHANGEMTU:
-		DPRINTK("clip_device_event NETDEV_CHANGE*\n");
+		pr_debug("clip_device_event NETDEV_CHANGE*\n");
 		to_atmarpd(act_change, PRIV(dev)->number, 0);
 		break;
 	}
@@ -681,14 +673,14 @@ static struct notifier_block clip_inet_n
 
 static void atmarpd_close(struct atm_vcc *vcc)
 {
-	DPRINTK("atmarpd_close\n");
+	pr_debug("atmarpd_close\n");
 
 	rtnl_lock();
 	atmarpd = NULL;
 	skb_queue_purge(&sk_atm(vcc)->sk_receive_queue);
 	rtnl_unlock();
 
-	DPRINTK("(done)\n");
+	pr_debug("(done)\n");
 	module_put(THIS_MODULE);
 }
 
--- a/net/atm/br2684.c	2007-08-17 15:06:14.000000000 -0400
+++ b/net/atm/br2684.c	2007-08-17 15:07:22.000000000 -0400
@@ -24,12 +24,6 @@ Author: Marcell GAL, 2000, XDSL Ltd, Hun
 
 #include "common.h"
 
-#ifdef DEBUG
-#define DPRINTK(format, args...) printk(KERN_DEBUG "br2684: " format, ##args)
-#else
-#define DPRINTK(format, args...)
-#endif
-
 #ifdef SKB_DEBUG
 static void skb_debug(const struct sk_buff *skb)
 {
@@ -162,7 +156,7 @@ static int br2684_xmit_vcc(struct sk_buf
 	skb_debug(skb);
 
 	ATM_SKB(skb)->vcc = atmvcc = brvcc->atmvcc;
-	DPRINTK("atm_skb(%p)->vcc(%p)->dev(%p)\n", skb, atmvcc, atmvcc->dev);
+	pr_debug("atm_skb(%p)->vcc(%p)->dev(%p)\n", skb, atmvcc, atmvcc->dev);
 	if (!atm_may_send(atmvcc, skb->truesize)) {
 		/* we free this here for now, because we cannot know in a higher
 			layer whether the skb point it supplied wasn't freed yet.
@@ -191,11 +185,11 @@ static int br2684_start_xmit(struct sk_b
 	struct br2684_dev *brdev = BRPRIV(dev);
 	struct br2684_vcc *brvcc;
 
-	DPRINTK("br2684_start_xmit, skb->dst=%p\n", skb->dst);
+	pr_debug("br2684_start_xmit, skb->dst=%p\n", skb->dst);
 	read_lock(&devs_lock);
 	brvcc = pick_outgoing_vcc(skb, brdev);
 	if (brvcc == NULL) {
-		DPRINTK("no vcc attached to dev %s\n", dev->name);
+		pr_debug("no vcc attached to dev %s\n", dev->name);
 		brdev->stats.tx_errors++;
 		brdev->stats.tx_carrier_errors++;
 		/* netif_stop_queue(dev); */
@@ -221,7 +215,7 @@ static int br2684_start_xmit(struct sk_b
 
 static struct net_device_stats *br2684_get_stats(struct net_device *dev)
 {
-	DPRINTK("br2684_get_stats\n");
+	pr_debug("br2684_get_stats\n");
 	return &BRPRIV(dev)->stats;
 }
 
@@ -290,7 +284,7 @@ packet_fails_filter(__be16 type, struct 
 
 static void br2684_close_vcc(struct br2684_vcc *brvcc)
 {
-	DPRINTK("removing VCC %p from dev %p\n", brvcc, brvcc->device);
+	pr_debug("removing VCC %p from dev %p\n", brvcc, brvcc->device);
 	write_lock_irq(&devs_lock);
 	list_del(&brvcc->brvccs);
 	write_unlock_irq(&devs_lock);
@@ -308,7 +302,7 @@ static void br2684_push(struct atm_vcc *
 	struct br2684_dev *brdev = BRPRIV(net_dev);
 	int plen = sizeof(llc_oui_pid_pad) + ETH_HLEN;
 
-	DPRINTK("br2684_push\n");
+	pr_debug("br2684_push\n");
 
 	if (unlikely(skb == NULL)) {
 		/* skb==NULL means VCC is being destroyed */
@@ -325,7 +319,7 @@ static void br2684_push(struct atm_vcc *
 
 	skb_debug(skb);
 	atm_return(atmvcc, skb->truesize);
-	DPRINTK("skb from brdev %p\n", brdev);
+	pr_debug("skb from brdev %p\n", brdev);
 	if (brvcc->encaps == e_llc) {
 		/* let us waste some time for checking the encapsulation.
 		   Note, that only 7 char is checked so frames with a valid FCS
@@ -366,7 +360,7 @@ static void br2684_push(struct atm_vcc *
 #endif /* CONFIG_ATM_BR2684_IPFILTER */
 	skb->dev = net_dev;
 	ATM_SKB(skb)->vcc = atmvcc;	/* needed ? */
-	DPRINTK("received packet's protocol: %x\n", ntohs(skb->protocol));
+	pr_debug("received packet's protocol: %x\n", ntohs(skb->protocol));
 	skb_debug(skb);
 	if (unlikely(!(net_dev->flags & IFF_UP))) {
 		/* sigh, interface is down */
@@ -424,7 +418,7 @@ Note: we do not have explicit unassign, 
 		err = -EINVAL;
 		goto error;
 	}
-	DPRINTK("br2684_regvcc vcc=%p, encaps=%d, brvcc=%p\n", atmvcc, be.encaps,
+	pr_debug("br2684_regvcc vcc=%p, encaps=%d, brvcc=%p\n", atmvcc, be.encaps,
 		brvcc);
 	if (list_empty(&brdev->brvccs) && !brdev->mac_was_set) {
 		unsigned char *esi = atmvcc->dev->esi;
@@ -496,7 +490,7 @@ static int br2684_create(void __user *ar
 	struct br2684_dev *brdev;
 	struct atm_newif_br2684 ni;
 
-	DPRINTK("br2684_create\n");
+	pr_debug("br2684_create\n");
 
 	if (copy_from_user(&ni, arg, sizeof ni)) {
 		return -EFAULT;
@@ -513,7 +507,7 @@ static int br2684_create(void __user *ar
 
 	brdev = BRPRIV(netdev);
 
-	DPRINTK("registered netdev %s\n", netdev->name);
+	pr_debug("registered netdev %s\n", netdev->name);
 	/* open, stop, do_ioctl ? */
 	err = register_netdev(netdev);
 	if (err < 0) {
--- a/net/atm/common.c	2007-08-06 04:26:48.000000000 -0400
+++ b/net/atm/common.c	2007-08-17 15:08:31.000000000 -0400
@@ -30,13 +30,6 @@
 #include "addr.h"		/* address registry */
 #include "signaling.h"		/* for WAITING and sigd_attach */
 
-
-#if 0
-#define DPRINTK(format,args...) printk(KERN_DEBUG format,##args)
-#else
-#define DPRINTK(format,args...)
-#endif
-
 struct hlist_head vcc_hash[VCC_HTABLE_SIZE];
 DEFINE_RWLOCK(vcc_sklist_lock);
 
@@ -70,13 +63,13 @@ static struct sk_buff *alloc_tx(struct a
 	struct sock *sk = sk_atm(vcc);
 
 	if (atomic_read(&sk->sk_wmem_alloc) && !atm_may_send(vcc, size)) {
-		DPRINTK("Sorry: wmem_alloc = %d, size = %d, sndbuf = %d\n",
+		pr_debug("Sorry: wmem_alloc = %d, size = %d, sndbuf = %d\n",
 			atomic_read(&sk->sk_wmem_alloc), size,
 			sk->sk_sndbuf);
 		return NULL;
 	}
 	while (!(skb = alloc_skb(size,GFP_KERNEL))) schedule();
-	DPRINTK("AlTx %d += %d\n", atomic_read(&sk->sk_wmem_alloc),
+	pr_debug("AlTx %d += %d\n", atomic_read(&sk->sk_wmem_alloc),
 		skb->truesize);
 	atomic_add(skb->truesize, &sk->sk_wmem_alloc);
 	return skb;
@@ -392,10 +385,10 @@ static int __vcc_connect(struct atm_vcc 
 	if (!error) error = adjust_tp(&vcc->qos.rxtp,vcc->qos.aal);
 	if (error)
 		goto fail;
-	DPRINTK("VCC %d.%d, AAL %d\n",vpi,vci,vcc->qos.aal);
-	DPRINTK("  TX: %d, PCR %d..%d, SDU %d\n",vcc->qos.txtp.traffic_class,
+	pr_debug("VCC %d.%d, AAL %d\n",vpi,vci,vcc->qos.aal);
+	pr_debug("  TX: %d, PCR %d..%d, SDU %d\n",vcc->qos.txtp.traffic_class,
 	    vcc->qos.txtp.min_pcr,vcc->qos.txtp.max_pcr,vcc->qos.txtp.max_sdu);
-	DPRINTK("  RX: %d, PCR %d..%d, SDU %d\n",vcc->qos.rxtp.traffic_class,
+	pr_debug("  RX: %d, PCR %d..%d, SDU %d\n",vcc->qos.rxtp.traffic_class,
 	    vcc->qos.rxtp.min_pcr,vcc->qos.rxtp.max_pcr,vcc->qos.rxtp.max_sdu);
 
 	if (dev->ops->open) {
@@ -420,7 +413,7 @@ int vcc_connect(struct socket *sock, int
 	struct atm_vcc *vcc = ATM_SD(sock);
 	int error;
 
-	DPRINTK("vcc_connect (vpi %d, vci %d)\n",vpi,vci);
+	pr_debug("vcc_connect (vpi %d, vci %d)\n",vpi,vci);
 	if (sock->state == SS_CONNECTED)
 		return -EISCONN;
 	if (sock->state != SS_UNCONNECTED)
@@ -433,7 +426,7 @@ int vcc_connect(struct socket *sock, int
 	else
 		if (test_bit(ATM_VF_PARTIAL,&vcc->flags))
 			return -EINVAL;
-	DPRINTK("vcc_connect (TX: cl %d,bw %d-%d,sdu %d; "
+	pr_debug("vcc_connect (TX: cl %d,bw %d-%d,sdu %d; "
 	    "RX: cl %d,bw %d-%d,sdu %d,AAL %s%d)\n",
 	    vcc->qos.txtp.traffic_class,vcc->qos.txtp.min_pcr,
 	    vcc->qos.txtp.max_pcr,vcc->qos.txtp.max_sdu,
@@ -504,7 +497,7 @@ int vcc_recvmsg(struct kiocb *iocb, stru
 	if (error)
 		return error;
 	sock_recv_timestamp(msg, sk, skb);
-	DPRINTK("RcvM %d -= %d\n", atomic_read(&sk->rmem_alloc), skb->truesize);
+	pr_debug("RcvM %d -= %d\n", atomic_read(&sk->rmem_alloc), skb->truesize);
 	atm_return(vcc, skb->truesize);
 	skb_free_datagram(sk, skb);
 	return copied;
--- a/net/atm/lec.c	2007-08-06 04:26:48.000000000 -0400
+++ b/net/atm/lec.c	2007-08-17 15:08:48.000000000 -0400
@@ -49,12 +49,6 @@ static unsigned char bridge_ula_lec[] = 
 #include "lec_arpc.h"
 #include "resources.h"
 
-#if 0
-#define DPRINTK printk
-#else
-#define DPRINTK(format,args...)
-#endif
-
 #define DUMP_PACKETS 0		/*
 				 * 0 = None,
 				 * 1 = 30 first bytes
@@ -274,7 +268,7 @@ static int lec_start_xmit(struct sk_buff
 	int i = 0;
 #endif /* DUMP_PACKETS >0 */
 
-	DPRINTK("lec_start_xmit called\n");
+	pr_debug("lec_start_xmit called\n");
 	if (!priv->lecd) {
 		printk("%s:No lecd attached\n", dev->name);
 		priv->stats.tx_errors++;
@@ -282,7 +276,7 @@ static int lec_start_xmit(struct sk_buff
 		return -EUNATCH;
 	}
 
-	DPRINTK("skbuff head:%lx data:%lx tail:%lx end:%lx\n",
+	pr_debug("skbuff head:%lx data:%lx tail:%lx end:%lx\n",
 		(long)skb->head, (long)skb->data, (long)skb_tail_pointer(skb),
 		(long)skb_end_pointer(skb));
 #if defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)
@@ -293,7 +287,7 @@ static int lec_start_xmit(struct sk_buff
 	/* Make sure we have room for lec_id */
 	if (skb_headroom(skb) < 2) {
 
-		DPRINTK("lec_start_xmit: reallocating skb\n");
+		pr_debug("lec_start_xmit: reallocating skb\n");
 		skb2 = skb_realloc_headroom(skb, LEC_HEADER_LEN);
 		kfree_skb(skb);
 		if (skb2 == NULL)
@@ -374,22 +368,22 @@ static int lec_start_xmit(struct sk_buff
 #endif
 	entry = NULL;
 	vcc = lec_arp_resolve(priv, dst, is_rdesc, &entry);
-	DPRINTK("%s:vcc:%p vcc_flags:%x, entry:%p\n", dev->name,
+	pr_debug("%s:vcc:%p vcc_flags:%x, entry:%p\n", dev->name,
 		vcc, vcc ? vcc->flags : 0, entry);
 	if (!vcc || !test_bit(ATM_VF_READY, &vcc->flags)) {
 		if (entry && (entry->tx_wait.qlen < LEC_UNRES_QUE_LEN)) {
-			DPRINTK("%s:lec_start_xmit: queuing packet, ",
+			pr_debug("%s:lec_start_xmit: queuing packet, ",
 				dev->name);
-			DPRINTK("MAC address 0x%02x:%02x:%02x:%02x:%02x:%02x\n",
+			pr_debug("MAC address 0x%02x:%02x:%02x:%02x:%02x:%02x\n",
 				lec_h->h_dest[0], lec_h->h_dest[1],
 				lec_h->h_dest[2], lec_h->h_dest[3],
 				lec_h->h_dest[4], lec_h->h_dest[5]);
 			skb_queue_tail(&entry->tx_wait, skb);
 		} else {
-			DPRINTK
+			pr_debug
 			    ("%s:lec_start_xmit: tx queue full or no arp entry, dropping, ",
 			     dev->name);
-			DPRINTK("MAC address 0x%02x:%02x:%02x:%02x:%02x:%02x\n",
+			pr_debug("MAC address 0x%02x:%02x:%02x:%02x:%02x:%02x\n",
 				lec_h->h_dest[0], lec_h->h_dest[1],
 				lec_h->h_dest[2], lec_h->h_dest[3],
 				lec_h->h_dest[4], lec_h->h_dest[5]);
@@ -403,8 +397,8 @@ static int lec_start_xmit(struct sk_buff
 #endif /* DUMP_PACKETS > 0 */
 
 	while (entry && (skb2 = skb_dequeue(&entry->tx_wait))) {
-		DPRINTK("lec.c: emptying tx queue, ");
-		DPRINTK("MAC address 0x%02x:%02x:%02x:%02x:%02x:%02x\n",
+		pr_debug("lec.c: emptying tx queue, ");
+		pr_debug("MAC address 0x%02x:%02x:%02x:%02x:%02x:%02x\n",
 			lec_h->h_dest[0], lec_h->h_dest[1], lec_h->h_dest[2],
 			lec_h->h_dest[3], lec_h->h_dest[4], lec_h->h_dest[5]);
 		lec_send(vcc, skb2, priv);
@@ -465,7 +459,7 @@ static int lec_atm_send(struct atm_vcc *
 	mesg = (struct atmlec_msg *)skb->data;
 	tmp = skb->data;
 	tmp += sizeof(struct atmlec_msg);
-	DPRINTK("%s: msg from zeppelin:%d\n", dev->name, mesg->type);
+	pr_debug("%s: msg from zeppelin:%d\n", dev->name, mesg->type);
 	switch (mesg->type) {
 	case l_set_mac_addr:
 		for (i = 0; i < 6; i++) {
@@ -501,9 +495,9 @@ static int lec_atm_send(struct atm_vcc *
 			       mesg->content.normal.atm_addr,
 			       mesg->content.normal.flag,
 			       mesg->content.normal.targetless_le_arp);
-		DPRINTK("lec: in l_arp_update\n");
+		pr_debug("lec: in l_arp_update\n");
 		if (mesg->sizeoftlvs != 0) {	/* LANE2 3.1.5 */
-			DPRINTK("lec: LANE2 3.1.5, got tlvs, size %d\n",
+			pr_debug("lec: LANE2 3.1.5, got tlvs, size %d\n",
 				mesg->sizeoftlvs);
 			lane2_associate_ind(dev, mesg->content.normal.mac_addr,
 					    tmp, mesg->sizeoftlvs);
@@ -545,7 +539,7 @@ static int lec_atm_send(struct atm_vcc *
 		{
 			struct net_bridge_fdb_entry *f;
 
-			DPRINTK
+			pr_debug
 			    ("%s: bridge zeppelin asks about 0x%02x:%02x:%02x:%02x:%02x:%02x\n",
 			     dev->name, mesg->content.proxy.mac_addr[0],
 			     mesg->content.proxy.mac_addr[1],
@@ -565,7 +559,7 @@ static int lec_atm_send(struct atm_vcc *
 				struct sk_buff *skb2;
 				struct sock *sk;
 
-				DPRINTK
+				pr_debug
 				    ("%s: entry found, responding to zeppelin\n",
 				     dev->name);
 				skb2 =
@@ -671,7 +665,7 @@ send_to_lecd(struct lec_priv *priv, atml
 	sk->sk_data_ready(sk, skb->len);
 
 	if (data != NULL) {
-		DPRINTK("lec: about to send %d bytes of data\n", data->len);
+		pr_debug("lec: about to send %d bytes of data\n", data->len);
 		atm_force_charge(priv->lecd, data->truesize);
 		skb_queue_tail(&sk->sk_receive_queue, data);
 		sk->sk_data_ready(sk, skb->len);
@@ -743,7 +737,7 @@ static void lec_push(struct atm_vcc *vcc
 	       vcc->vpi, vcc->vci);
 #endif
 	if (!skb) {
-		DPRINTK("%s: null skb\n", dev->name);
+		pr_debug("%s: null skb\n", dev->name);
 		lec_vcc_close(priv, vcc);
 		return;
 	}
@@ -767,7 +761,7 @@ static void lec_push(struct atm_vcc *vcc
 	if (memcmp(skb->data, lec_ctrl_magic, 4) == 0) {	/* Control frame, to daemon */
 		struct sock *sk = sk_atm(vcc);
 
-		DPRINTK("%s: To daemon\n", dev->name);
+		pr_debug("%s: To daemon\n", dev->name);
 		skb_queue_tail(&sk->sk_receive_queue, skb);
 		sk->sk_data_ready(sk, skb->len);
 	} else {		/* Data frame, queue to protocol handlers */
@@ -781,7 +775,7 @@ static void lec_push(struct atm_vcc *vcc
 			 * Probably looping back, or if lecd is missing,
 			 * lecd has gone down
 			 */
-			DPRINTK("Ignoring frame...\n");
+			pr_debug("Ignoring frame...\n");
 			dev_kfree_skb(skb);
 			return;
 		}
@@ -1443,9 +1437,9 @@ static void lane2_associate_ind(struct n
 #include <net/route.h>
 
 #if 0
-#define DPRINTK(format,args...)
+#define pr_debug(format,args...)
 /*
-#define DPRINTK printk
+#define pr_debug printk
 */
 #endif
 #define DEBUG_ARP_TABLE 0
@@ -1514,7 +1508,7 @@ lec_arp_add(struct lec_priv *priv, struc
 	tmp = &priv->lec_arp_tables[HASH(entry->mac_addr[ETH_ALEN - 1])];
 	hlist_add_head(&entry->next, tmp);
 
-	DPRINTK("LEC_ARP: Added entry:%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
+	pr_debug("LEC_ARP: Added entry:%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
 		0xff & entry->mac_addr[0], 0xff & entry->mac_addr[1],
 		0xff & entry->mac_addr[2], 0xff & entry->mac_addr[3],
 		0xff & entry->mac_addr[4], 0xff & entry->mac_addr[5]);
@@ -1556,7 +1550,7 @@ lec_arp_remove(struct lec_priv *priv, st
 	}
 	skb_queue_purge(&to_remove->tx_wait);	/* FIXME: good place for this? */
 
-	DPRINTK("LEC_ARP: Removed entry:%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
+	pr_debug("LEC_ARP: Removed entry:%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
 		0xff & to_remove->mac_addr[0], 0xff & to_remove->mac_addr[1],
 		0xff & to_remove->mac_addr[2], 0xff & to_remove->mac_addr[3],
 		0xff & to_remove->mac_addr[4], 0xff & to_remove->mac_addr[5]);
@@ -1778,7 +1772,7 @@ static struct lec_arp_table *lec_arp_fin
 	struct hlist_head *head;
 	struct lec_arp_table *entry;
 
-	DPRINTK("LEC_ARP: lec_arp_find :%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
+	pr_debug("LEC_ARP: lec_arp_find :%2.2x %2.2x %2.2x %2.2x %2.2x %2.2x\n",
 		mac_addr[0] & 0xff, mac_addr[1] & 0xff, mac_addr[2] & 0xff,
 		mac_addr[3] & 0xff, mac_addr[4] & 0xff, mac_addr[5] & 0xff);
 
@@ -1820,7 +1814,7 @@ static void lec_arp_expire_arp(unsigned 
 
 	entry = (struct lec_arp_table *)data;
 
-	DPRINTK("lec_arp_expire_arp\n");
+	pr_debug("lec_arp_expire_arp\n");
 	if (entry->status == ESI_ARP_PENDING) {
 		if (entry->no_tries <= entry->priv->max_retry_count) {
 			if (entry->is_rdesc)
@@ -1844,7 +1838,7 @@ static void lec_arp_expire_vcc(unsigned 
 
 	del_timer(&to_remove->timer);
 
-	DPRINTK("LEC_ARP %p %p: lec_arp_expire_vcc vpi:%d vci:%d\n",
+	pr_debug("LEC_ARP %p %p: lec_arp_expire_vcc vpi:%d vci:%d\n",
 		to_remove, priv,
 		to_remove->vcc ? to_remove->recv_vcc->vpi : 0,
 		to_remove->vcc ? to_remove->recv_vcc->vci : 0);
@@ -1884,7 +1878,7 @@ static void lec_arp_check_expire(struct 
 	unsigned long time_to_check;
 	int i;
 
-	DPRINTK("lec_arp_check_expire %p\n", priv);
+	pr_debug("lec_arp_check_expire %p\n", priv);
 	now = jiffies;
 restart:
 	spin_lock_irqsave(&priv->lec_arp_lock, flags);
@@ -1896,13 +1890,13 @@ restart:
 			else
 				time_to_check = priv->aging_time;
 
-			DPRINTK("About to expire: %lx - %lx > %lx\n",
+			pr_debug("About to expire: %lx - %lx > %lx\n",
 				now, entry->last_used, time_to_check);
 			if (time_after(now, entry->last_used + time_to_check)
 			    && !(entry->flags & LEC_PERMANENT_FLAG)
 			    && !(entry->mac_addr[0] & 0x01)) {	/* LANE2: 7.1.20 */
 				/* Remove entry */
-				DPRINTK("LEC:Entry timed out\n");
+				pr_debug("LEC:Entry timed out\n");
 				lec_arp_remove(priv, entry);
 				lec_arp_put(entry);
 			} else {
@@ -2000,7 +1994,7 @@ static struct atm_vcc *lec_arp_resolve(s
 		    entry->packets_flooded <
 		    priv->maximum_unknown_frame_count) {
 			entry->packets_flooded++;
-			DPRINTK("LEC_ARP: Flooding..\n");
+			pr_debug("LEC_ARP: Flooding..\n");
 			found = priv->mcast_vcc;
 			goto out;
 		}
@@ -2011,13 +2005,13 @@ static struct atm_vcc *lec_arp_resolve(s
 		 */
 		lec_arp_hold(entry);
 		*ret_entry = entry;
-		DPRINTK("lec: entry->status %d entry->vcc %p\n", entry->status,
+		pr_debug("lec: entry->status %d entry->vcc %p\n", entry->status,
 			entry->vcc);
 		found = NULL;
 	} else {
 		/* No matching entry was found */
 		entry = make_entry(priv, mac_to_find);
-		DPRINTK("LEC_ARP: Making entry\n");
+		pr_debug("LEC_ARP: Making entry\n");
 		if (!entry) {
 			found = priv->mcast_vcc;
 			goto out;
@@ -2054,7 +2048,7 @@ lec_addr_delete(struct lec_priv *priv, u
 	struct lec_arp_table *entry;
 	int i;
 
-	DPRINTK("lec_addr_delete\n");
+	pr_debug("lec_addr_delete\n");
 	spin_lock_irqsave(&priv->lec_arp_lock, flags);
 	for (i = 0; i < LEC_ARP_TABLE_SIZE; i++) {
 		hlist_for_each_entry_safe(entry, node, next, &priv->lec_arp_tables[i], next) {
@@ -2085,8 +2079,8 @@ lec_arp_update(struct lec_priv *priv, un
 	struct lec_arp_table *entry, *tmp;
 	int i;
 
-	DPRINTK("lec:%s", (targetless_le_arp) ? "targetless " : " ");
-	DPRINTK("lec_arp_update mac:%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x\n",
+	pr_debug("lec:%s", (targetless_le_arp) ? "targetless " : " ");
+	pr_debug("lec_arp_update mac:%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x\n",
 		mac_addr[0], mac_addr[1], mac_addr[2], mac_addr[3],
 		mac_addr[4], mac_addr[5]);
 
@@ -2123,7 +2117,7 @@ lec_arp_update(struct lec_priv *priv, un
 					entry->flags |= LEC_REMOTE_FLAG;
 				else
 					entry->flags &= ~LEC_REMOTE_FLAG;
-				DPRINTK("After update\n");
+				pr_debug("After update\n");
 				dump_arp_table(priv);
 				goto out;
 			}
@@ -2167,7 +2161,7 @@ lec_arp_update(struct lec_priv *priv, un
 		entry->status = ESI_VC_PENDING;
 		send_to_lecd(priv, l_svc_setup, entry->mac_addr, atm_addr, NULL);
 	}
-	DPRINTK("After update2\n");
+	pr_debug("After update2\n");
 	dump_arp_table(priv);
 out:
 	spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
@@ -2190,7 +2184,7 @@ lec_vcc_added(struct lec_priv *priv, str
 	if (ioc_data->receive == 2) {
 		/* Vcc for Multicast Forward. No timer, LANEv2 7.1.20 and 2.3.5.3 */
 
-		DPRINTK("LEC_ARP: Attaching mcast forward\n");
+		pr_debug("LEC_ARP: Attaching mcast forward\n");
 #if 0
 		entry = lec_arp_find(priv, bus_mac);
 		if (!entry) {
@@ -2215,7 +2209,7 @@ lec_vcc_added(struct lec_priv *priv, str
 		 * Vcc which we don't want to make default vcc,
 		 * attach it anyway.
 		 */
-		DPRINTK
+		pr_debug
 		    ("LEC_ARP:Attaching data direct, not default: "
 		     "%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x\n",
 		     ioc_data->atm_addr[0], ioc_data->atm_addr[1],
@@ -2243,7 +2237,7 @@ lec_vcc_added(struct lec_priv *priv, str
 		dump_arp_table(priv);
 		goto out;
 	}
-	DPRINTK
+	pr_debug
 	    ("LEC_ARP:Attaching data direct, default: "
 	     "%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x\n",
 	     ioc_data->atm_addr[0], ioc_data->atm_addr[1],
@@ -2261,8 +2255,8 @@ lec_vcc_added(struct lec_priv *priv, str
 			if (memcmp
 			    (ioc_data->atm_addr, entry->atm_addr,
 			     ATM_ESA_LEN) == 0) {
-				DPRINTK("LEC_ARP: Attaching data direct\n");
-				DPRINTK("Currently -> Vcc: %d, Rvcc:%d\n",
+				pr_debug("LEC_ARP: Attaching data direct\n");
+				pr_debug("Currently -> Vcc: %d, Rvcc:%d\n",
 					entry->vcc ? entry->vcc->vci : 0,
 					entry->recv_vcc ? entry->recv_vcc->
 					vci : 0);
@@ -2304,7 +2298,7 @@ lec_vcc_added(struct lec_priv *priv, str
 		}
 	}
 	if (found_entry) {
-		DPRINTK("After vcc was added\n");
+		pr_debug("After vcc was added\n");
 		dump_arp_table(priv);
 		goto out;
 	}
@@ -2324,7 +2318,7 @@ lec_vcc_added(struct lec_priv *priv, str
 	entry->timer.expires = jiffies + priv->vcc_timeout_period;
 	entry->timer.function = lec_arp_expire_vcc;
 	add_timer(&entry->timer);
-	DPRINTK("After vcc was added\n");
+	pr_debug("After vcc was added\n");
 	dump_arp_table(priv);
 out:
 	spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
@@ -2337,7 +2331,7 @@ static void lec_flush_complete(struct le
 	struct lec_arp_table *entry;
 	int i;
 
-	DPRINTK("LEC:lec_flush_complete %lx\n", tran_id);
+	pr_debug("LEC:lec_flush_complete %lx\n", tran_id);
 restart:
 	spin_lock_irqsave(&priv->lec_arp_lock, flags);
 	for (i = 0; i < LEC_ARP_TABLE_SIZE; i++) {
@@ -2354,7 +2348,7 @@ restart:
 				entry->last_used = jiffies;
 				entry->status = ESI_FORWARD_DIRECT;
 				lec_arp_put(entry);
-				DPRINTK("LEC_ARP: Flushed\n");
+				pr_debug("LEC_ARP: Flushed\n");
 				goto restart;
 			}
 		}
@@ -2377,7 +2371,7 @@ lec_set_flush_tran_id(struct lec_priv *p
 		hlist_for_each_entry(entry, node, &priv->lec_arp_tables[i], next) {
 			if (!memcmp(atm_addr, entry->atm_addr, ATM_ESA_LEN)) {
 				entry->flush_tran_id = tran_id;
-				DPRINTK("Set flush transaction id to %lx for %p\n",
+				pr_debug("Set flush transaction id to %lx for %p\n",
 					tran_id, entry);
 			}
 		}
@@ -2428,7 +2422,7 @@ static void lec_vcc_close(struct lec_pri
 	struct lec_arp_table *entry;
 	int i;
 
-	DPRINTK("LEC_ARP: lec_vcc_close vpi:%d vci:%d\n", vcc->vpi, vcc->vci);
+	pr_debug("LEC_ARP: lec_vcc_close vpi:%d vci:%d\n", vcc->vpi, vcc->vci);
 	dump_arp_table(priv);
 
 	spin_lock_irqsave(&priv->lec_arp_lock, flags);
@@ -2511,7 +2505,7 @@ lec_arp_check_empties(struct lec_priv *p
 			goto out;
 		}
 	}
-	DPRINTK("LEC_ARP: Arp_check_empties: entry not found!\n");
+	pr_debug("LEC_ARP: Arp_check_empties: entry not found!\n");
 out:
 	spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
 }
--- a/net/atm/pppoatm.c	2007-08-06 04:26:48.000000000 -0400
+++ b/net/atm/pppoatm.c	2007-08-17 15:09:26.000000000 -0400
@@ -46,13 +46,6 @@
 
 #include "common.h"
 
-#if 0
-#define DPRINTK(format, args...) \
-	printk(KERN_DEBUG "pppoatm: " format, ##args)
-#else
-#define DPRINTK(format, args...)
-#endif
-
 enum pppoatm_encaps {
 	e_autodetect = PPPOATM_ENCAPS_AUTODETECT,
 	e_vc = PPPOATM_ENCAPS_VC,
@@ -139,9 +132,9 @@ static void pppoatm_unassign_vcc(struct 
 static void pppoatm_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
 {
 	struct pppoatm_vcc *pvcc = atmvcc_to_pvcc(atmvcc);
-	DPRINTK("pppoatm push\n");
+	pr_debug("pppoatm push\n");
 	if (skb == NULL) {			/* VCC was closed */
-		DPRINTK("removing ATMPPP VCC %p\n", pvcc);
+		pr_debug("removing ATMPPP VCC %p\n", pvcc);
 		pppoatm_unassign_vcc(atmvcc);
 		atmvcc->push(atmvcc, NULL);	/* Pass along bad news */
 		return;
@@ -172,7 +165,7 @@ static void pppoatm_push(struct atm_vcc 
 			pvcc->chan.mtu += LLC_LEN;
 			break;
 		}
-		DPRINTK("(unit %d): Couldn't autodetect yet "
+		pr_debug("(unit %d): Couldn't autodetect yet "
 		    "(skb: %02X %02X %02X %02X %02X %02X)\n",
 		    pvcc->chan.unit,
 		    skb->data[0], skb->data[1], skb->data[2],
@@ -202,7 +195,7 @@ static int pppoatm_send(struct ppp_chann
 {
 	struct pppoatm_vcc *pvcc = chan_to_pvcc(chan);
 	ATM_SKB(skb)->vcc = pvcc->atmvcc;
-	DPRINTK("(unit %d): pppoatm_send (skb=0x%p, vcc=0x%p)\n",
+	pr_debug("(unit %d): pppoatm_send (skb=0x%p, vcc=0x%p)\n",
 	    pvcc->chan.unit, skb, pvcc->atmvcc);
 	if (skb->data[0] == '\0' && (pvcc->flags & SC_COMP_PROT))
 		(void) skb_pull(skb, 1);
@@ -228,7 +221,7 @@ static int pppoatm_send(struct ppp_chann
 			goto nospace;
 		break;
 	case e_autodetect:
-		DPRINTK("(unit %d): Trying to send without setting encaps!\n",
+		pr_debug("(unit %d): Trying to send without setting encaps!\n",
 		    pvcc->chan.unit);
 		kfree_skb(skb);
 		return 1;
@@ -236,7 +229,7 @@ static int pppoatm_send(struct ppp_chann
 
 	atomic_add(skb->truesize, &sk_atm(ATM_SKB(skb)->vcc)->sk_wmem_alloc);
 	ATM_SKB(skb)->atm_options = ATM_SKB(skb)->vcc->atm_options;
-	DPRINTK("(unit %d): atm_skb(%p)->vcc(%p)->dev(%p)\n",
+	pr_debug("(unit %d): atm_skb(%p)->vcc(%p)->dev(%p)\n",
 	    pvcc->chan.unit, skb, ATM_SKB(skb)->vcc,
 	    ATM_SKB(skb)->vcc->dev);
 	return ATM_SKB(skb)->vcc->send(ATM_SKB(skb)->vcc, skb)
--- a/net/atm/raw.c	2007-08-06 04:26:48.000000000 -0400
+++ b/net/atm/raw.c	2007-08-17 15:09:41.000000000 -0400
@@ -13,14 +13,6 @@
 #include "common.h"
 #include "protocols.h"
 
-
-#if 0
-#define DPRINTK(format,args...) printk(KERN_DEBUG format,##args)
-#else
-#define DPRINTK(format,args...)
-#endif
-
-
 /*
  * SKB == NULL indicates that the link is being closed
  */
@@ -40,7 +32,7 @@ static void atm_pop_raw(struct atm_vcc *
 {
 	struct sock *sk = sk_atm(vcc);
 
-	DPRINTK("APopR (%d) %d -= %d\n", vcc->vci, sk->sk_wmem_alloc,
+	pr_debug("APopR (%d) %d -= %d\n", vcc->vci, sk->sk_wmem_alloc,
 		skb->truesize);
 	atomic_sub(skb->truesize, &sk->sk_wmem_alloc);
 	dev_kfree_skb_any(skb);
--- a/net/atm/signaling.c	2007-08-17 15:06:24.000000000 -0400
+++ b/net/atm/signaling.c	2007-08-17 15:10:02.000000000 -0400
@@ -23,13 +23,6 @@
 				   Danger: may cause nasty hangs if the demon
 				   crashes. */
 
-#if 0
-#define DPRINTK(format,args...) printk(KERN_DEBUG format,##args)
-#else
-#define DPRINTK(format,args...)
-#endif
-
-
 struct atm_vcc *sigd = NULL;
 #ifdef WAIT_FOR_DEMON
 static DECLARE_WAIT_QUEUE_HEAD(sigd_sleep);
@@ -44,14 +37,14 @@ static void sigd_put_skb(struct sk_buff 
 	add_wait_queue(&sigd_sleep,&wait);
 	while (!sigd) {
 		set_current_state(TASK_UNINTERRUPTIBLE);
-		DPRINTK("atmsvc: waiting for signaling demon...\n");
+		pr_debug("atmsvc: waiting for signaling demon...\n");
 		schedule();
 	}
 	current->state = TASK_RUNNING;
 	remove_wait_queue(&sigd_sleep,&wait);
 #else
 	if (!sigd) {
-		DPRINTK("atmsvc: no signaling demon\n");
+		pr_debug("atmsvc: no signaling demon\n");
 		kfree_skb(skb);
 		return;
 	}
@@ -96,7 +89,7 @@ static int sigd_send(struct atm_vcc *vcc
 
 	msg = (struct atmsvc_msg *) skb->data;
 	atomic_sub(skb->truesize, &sk_atm(vcc)->sk_wmem_alloc);
-	DPRINTK("sigd_send %d (0x%lx)\n",(int) msg->type,
+	pr_debug("sigd_send %d (0x%lx)\n",(int) msg->type,
 	  (unsigned long) msg->vcc);
 	vcc = *(struct atm_vcc **) &msg->vcc;
 	sk = sk_atm(vcc);
@@ -130,7 +123,7 @@ static int sigd_send(struct atm_vcc *vcc
 		case as_indicate:
 			vcc = *(struct atm_vcc **) &msg->listen_vcc;
 			sk = sk_atm(vcc);
-			DPRINTK("as_indicate!!!\n");
+			pr_debug("as_indicate!!!\n");
 			lock_sock(sk);
 			if (sk_acceptq_is_full(sk)) {
 				sigd_enq(NULL,as_reject,vcc,NULL,NULL);
@@ -139,7 +132,7 @@ static int sigd_send(struct atm_vcc *vcc
 			}
 			sk->sk_ack_backlog++;
 			skb_queue_tail(&sk->sk_receive_queue, skb);
-			DPRINTK("waking sk->sk_sleep 0x%p\n", sk->sk_sleep);
+			pr_debug("waking sk->sk_sleep 0x%p\n", sk->sk_sleep);
 			sk->sk_state_change(sk);
 as_indicate_complete:
 			release_sock(sk);
@@ -176,7 +169,7 @@ void sigd_enq2(struct atm_vcc *vcc,enum 
 	struct atmsvc_msg *msg;
 	static unsigned session = 0;
 
-	DPRINTK("sigd_enq %d (0x%p)\n",(int) type,vcc);
+	pr_debug("sigd_enq %d (0x%p)\n",(int) type,vcc);
 	while (!(skb = alloc_skb(sizeof(struct atmsvc_msg),GFP_KERNEL)))
 		schedule();
 	msg = (struct atmsvc_msg *) skb_put(skb,sizeof(struct atmsvc_msg));
@@ -226,7 +219,7 @@ static void sigd_close(struct atm_vcc *v
 	struct sock *s;
 	int i;
 
-	DPRINTK("sigd_close\n");
+	pr_debug("sigd_close\n");
 	sigd = NULL;
 	if (skb_peek(&sk_atm(vcc)->sk_receive_queue))
 		printk(KERN_ERR "sigd_close: closing with requests pending\n");
@@ -260,7 +253,7 @@ static struct atm_dev sigd_dev = {
 int sigd_attach(struct atm_vcc *vcc)
 {
 	if (sigd) return -EADDRINUSE;
-	DPRINTK("sigd_attach\n");
+	pr_debug("sigd_attach\n");
 	sigd = vcc;
 	vcc->dev = &sigd_dev;
 	vcc_insert_socket(sk_atm(vcc));
--- a/net/atm/svc.c	2007-08-06 04:26:48.000000000 -0400
+++ b/net/atm/svc.c	2007-08-17 15:09:11.000000000 -0400
@@ -25,17 +25,8 @@
 #include "signaling.h"
 #include "addr.h"
 
-
-#if 0
-#define DPRINTK(format,args...) printk(KERN_DEBUG format,##args)
-#else
-#define DPRINTK(format,args...)
-#endif
-
-
 static int svc_create(struct socket *sock,int protocol);
 
-
 /*
  * Note: since all this is still nicely synchronized with the signaling demon,
  *       there's no need to protect sleep loops with clis. If signaling is
@@ -55,7 +46,7 @@ static void svc_disconnect(struct atm_vc
 	struct sk_buff *skb;
 	struct sock *sk = sk_atm(vcc);
 
-	DPRINTK("svc_disconnect %p\n",vcc);
+	pr_debug("svc_disconnect %p\n",vcc);
 	if (test_bit(ATM_VF_REGIS,&vcc->flags)) {
 		prepare_to_wait(sk->sk_sleep, &wait, TASK_UNINTERRUPTIBLE);
 		sigd_enq(vcc,as_close,NULL,NULL,NULL);
@@ -69,7 +60,7 @@ static void svc_disconnect(struct atm_vc
 	   as_indicate has been answered */
 	while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
 		atm_return(vcc, skb->truesize);
-		DPRINTK("LISTEN REL\n");
+		pr_debug("LISTEN REL\n");
 		sigd_enq2(NULL,as_reject,vcc,NULL,NULL,&vcc->qos,0);
 		dev_kfree_skb(skb);
 	}
@@ -85,7 +76,7 @@ static int svc_release(struct socket *so
 
 	if (sk)  {
 		vcc = ATM_SD(sock);
-		DPRINTK("svc_release %p\n", vcc);
+		pr_debug("svc_release %p\n", vcc);
 		clear_bit(ATM_VF_READY, &vcc->flags);
 		/* VCC pointer is used as a reference, so we must not free it
 		   (thereby subjecting it to re-use) before all pending connections
@@ -162,7 +153,7 @@ static int svc_connect(struct socket *so
 	struct atm_vcc *vcc = ATM_SD(sock);
 	int error;
 
-	DPRINTK("svc_connect %p\n",vcc);
+	pr_debug("svc_connect %p\n",vcc);
 	lock_sock(sk);
 	if (sockaddr_len != sizeof(struct sockaddr_atmsvc)) {
 		error = -EINVAL;
@@ -224,7 +215,7 @@ static int svc_connect(struct socket *so
 				prepare_to_wait(sk->sk_sleep, &wait, TASK_INTERRUPTIBLE);
 				continue;
 			}
-			DPRINTK("*ABORT*\n");
+			pr_debug("*ABORT*\n");
 			/*
 			 * This is tricky:
 			 *   Kernel ---close--> Demon
@@ -295,7 +286,7 @@ static int svc_listen(struct socket *soc
 	struct atm_vcc *vcc = ATM_SD(sock);
 	int error;
 
-	DPRINTK("svc_listen %p\n",vcc);
+	pr_debug("svc_listen %p\n",vcc);
 	lock_sock(sk);
 	/* let server handle listen on unbound sockets */
 	if (test_bit(ATM_VF_SESSION,&vcc->flags)) {
@@ -341,7 +332,7 @@ static int svc_accept(struct socket *soc
 
 	new_vcc = ATM_SD(newsock);
 
-	DPRINTK("svc_accept %p -> %p\n",old_vcc,new_vcc);
+	pr_debug("svc_accept %p -> %p\n",old_vcc,new_vcc);
 	while (1) {
 		DEFINE_WAIT(wait);
 
@@ -545,7 +536,7 @@ static int svc_addparty(struct socket *s
 		error = -EINPROGRESS;
 		goto out;
 	}
-	DPRINTK("svc_addparty added wait queue\n");
+	pr_debug("svc_addparty added wait queue\n");
 	while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) {
 		schedule();
 		prepare_to_wait(sk->sk_sleep, &wait, TASK_INTERRUPTIBLE);

^ permalink raw reply

* Re: [RFC] restore netdev_priv optimization (planb)
From: David Miller @ 2007-08-18  1:31 UTC (permalink / raw)
  To: shemminger; +Cc: auke-jan.h.kok, netdev
In-Reply-To: <20070817182807.7591878d@freepuppy.rosehill.hemminger.net>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 17 Aug 2007 18:28:07 -0700

> Plan C was replacing MULTIQUEUE boolean with a int value 1 ... 256.
> All this was a one day "what if" exercise, not really a big churn..

Yes, that's another reasonable approach.

^ permalink raw reply

* Re: [RFC] restore netdev_priv optimization (planb)
From: David Miller @ 2007-08-18  1:30 UTC (permalink / raw)
  To: auke-jan.h.kok; +Cc: shemminger, netdev
In-Reply-To: <46C64995.4030401@intel.com>

From: "Kok, Auke" <auke-jan.h.kok@intel.com>
Date: Fri, 17 Aug 2007 18:21:25 -0700

> this sounds highly optimistic ("64 queues is enough for everyone"?)
> and probably will be quickly outdated by both hardware and demand...

As such drivers appear in the tree we can adjust the value.

Even the most aggressively multi-queued virtualization and 10GB
ethernet chips I am aware of, both in production and in development,
do not exceed this limit.

Since you think this is worth complaining about, you must know of some
exceptions? :-)


^ permalink raw reply

* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Satyam Sharma @ 2007-08-18  1:41 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Paul E. McKenney, Herbert Xu, Linus Torvalds, Nick Piggin,
	Paul Mackerras, Segher Boessenkool, heiko.carstens, horms,
	linux-kernel, rpjday, ak, netdev, cfriesen, akpm, jesper.juhl,
	linux-arch, zlynx, schwidefsky, Chris Snook, davem, wensong,
	wjiang
In-Reply-To: <Pine.LNX.4.64.0708171817050.15427@schroedinger.engr.sgi.com>



On Fri, 17 Aug 2007, Christoph Lameter wrote:

> On Fri, 17 Aug 2007, Paul E. McKenney wrote:
> 
> > On Sat, Aug 18, 2007 at 08:09:13AM +0800, Herbert Xu wrote:
> > > On Fri, Aug 17, 2007 at 04:59:12PM -0700, Paul E. McKenney wrote:
> > > >
> > > > gcc bugzilla bug #33102, for whatever that ends up being worth.  ;-)
> > > 
> > > I had totally forgotten that I'd already filed that bug more
> > > than six years ago until they just closed yours as a duplicate
> > > of mine :)
> > > 
> > > Good luck in getting it fixed!
> > 
> > Well, just got done re-opening it for the third time.  And a local
> > gcc community member advised me not to give up too easily.  But I
> > must admit that I am impressed with the speed that it was identified
> > as duplicate.
> > 
> > Should be entertaining!  ;-)
> 
> Right. ROTFL... volatile actually breaks atomic_t instead of making it 
> safe. x++ becomes a register load, increment and a register store. Without 
> volatile we can increment the memory directly.

No code does (or would do, or should do):

	x.counter++;

on an "atomic_t x;" anyway.

^ permalink raw reply

* Re: [RFC] restore netdev_priv optimization (planb)
From: Stephen Hemminger @ 2007-08-18  1:28 UTC (permalink / raw)
  To: Kok, Auke; +Cc: David Miller, netdev
In-Reply-To: <46C64995.4030401@intel.com>

On Fri, 17 Aug 2007 18:21:25 -0700
"Kok, Auke" <auke-jan.h.kok@intel.com> wrote:

> David Miller wrote:
> > From: Stephen Hemminger <shemminger@linux-foundation.org>
> > Date: Fri, 17 Aug 2007 17:49:09 -0700
> > 
> >> Fix optimization of netdev_priv() lost by the addition of multiqueue.
> >>
> >> Only configurations that define MULITQUEUE need the extra overhead in
> >> netdevice structure and the loss of the netdev_priv optimization.
> >>
> >> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> > 
> > Every distribution vendor is going to turn MULTIQUEUE on.  Therefore
> > %99 of Linux users will not see any gain from your patch.
> > 
> > You're walking around in a dark room and hitting walls every
> > few seconds.  Take a moment and think about how to really deal
> > with this generically for a second before churning out another
> > patch.
> > 
> > For example, how about making the multiqueue limit fixed, say at 64
> > queues, and declare the egress_queues as an array of 64 entries.  Then
> > you can get constant pointer formation for both the netdev priv and
> > the queues.
> 
> this sounds highly optimistic ("64 queues is enough for everyone"?) and probably 
> will be quickly outdated by both hardware and demand...
> 

Plan C was replacing MULTIQUEUE boolean with a int value 1 ... 256.
All this was a one day "what if" exercise, not really a big churn..

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

^ permalink raw reply

* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Christoph Lameter @ 2007-08-18  1:24 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Herbert Xu, Linus Torvalds, Nick Piggin, Paul Mackerras,
	Segher Boessenkool, heiko.carstens, horms, linux-kernel, rpjday,
	ak, netdev, cfriesen, akpm, jesper.juhl, linux-arch, zlynx,
	satyam, schwidefsky, Chris Snook, davem, wensong, wjiang
In-Reply-To: <20070818010818.GQ8464@linux.vnet.ibm.com>

On Fri, 17 Aug 2007, Paul E. McKenney wrote:

> On Sat, Aug 18, 2007 at 08:09:13AM +0800, Herbert Xu wrote:
> > On Fri, Aug 17, 2007 at 04:59:12PM -0700, Paul E. McKenney wrote:
> > >
> > > gcc bugzilla bug #33102, for whatever that ends up being worth.  ;-)
> > 
> > I had totally forgotten that I'd already filed that bug more
> > than six years ago until they just closed yours as a duplicate
> > of mine :)
> > 
> > Good luck in getting it fixed!
> 
> Well, just got done re-opening it for the third time.  And a local
> gcc community member advised me not to give up too easily.  But I
> must admit that I am impressed with the speed that it was identified
> as duplicate.
> 
> Should be entertaining!  ;-)

Right. ROTFL... volatile actually breaks atomic_t instead of making it 
safe. x++ becomes a register load, increment and a register store. Without 
volatile we can increment the memory directly. It seems that volatile 
requires that the variable is loaded into a register first and then 
operated upon. Understandable when you think about volatile being used to 
access memory mapped I/O registers where a RMW operation could be 
problematic.

See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=3506

^ permalink raw reply

* Re: [RFC] restore netdev_priv optimization (planb)
From: Kok, Auke @ 2007-08-18  1:21 UTC (permalink / raw)
  To: David Miller; +Cc: shemminger, netdev
In-Reply-To: <20070817.180006.98552435.davem@davemloft.net>

David Miller wrote:
> From: Stephen Hemminger <shemminger@linux-foundation.org>
> Date: Fri, 17 Aug 2007 17:49:09 -0700
> 
>> Fix optimization of netdev_priv() lost by the addition of multiqueue.
>>
>> Only configurations that define MULITQUEUE need the extra overhead in
>> netdevice structure and the loss of the netdev_priv optimization.
>>
>> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> Every distribution vendor is going to turn MULTIQUEUE on.  Therefore
> %99 of Linux users will not see any gain from your patch.
> 
> You're walking around in a dark room and hitting walls every
> few seconds.  Take a moment and think about how to really deal
> with this generically for a second before churning out another
> patch.
> 
> For example, how about making the multiqueue limit fixed, say at 64
> queues, and declare the egress_queues as an array of 64 entries.  Then
> you can get constant pointer formation for both the netdev priv and
> the queues.

this sounds highly optimistic ("64 queues is enough for everyone"?) and probably 
will be quickly outdated by both hardware and demand...

Auke





^ permalink raw reply

* RE: e1000 autotuning doesn't get along with itself
From: Brandeburg, Jesse @ 2007-08-18  1:16 UTC (permalink / raw)
  To: Rick Jones, Linux Network Development list
In-Reply-To: <46C4A888.9090006@hp.com>

Rick Jones wrote:
Hi Rick, allow me to respond on my way out on a Friday... :-)

> hpcpc109:~/netperf2_trunk# src/netperf -t TCP_RR -H 192.168.2.105 -D
> 1.0 -l 15 TCP REQUEST/RESPONSE TEST from 0.0.0.0 (0.0.0.0) port 0
> AF_INET to 192.168.2.105 (192.168.2.105) port 0 AF_INET : demo :
> first burst 0 
> Interim result: 10014.93 Trans/s over 1.00 seconds
> Interim result: 10015.79 Trans/s over 1.00 seconds
> Interim result: 10014.30 Trans/s over 1.00 seconds
> Interim result: 10016.29 Trans/s over 1.00 seconds
> Interim result: 10085.80 Trans/s over 1.00 seconds
> Interim result: 17526.61 Trans/s over 1.00 seconds
> Interim result: 20007.60 Trans/s over 1.00 seconds
> Interim result: 19626.46 Trans/s over 1.02 seconds
> Interim result: 10616.44 Trans/s over 1.85 seconds
> Interim result: 10014.88 Trans/s over 1.06 seconds
> Interim result: 10015.79 Trans/s over 1.00 seconds
> Interim result: 10014.80 Trans/s over 1.00 seconds
> Interim result: 10035.30 Trans/s over 1.00 seconds
> Interim result: 13974.69 Trans/s over 1.00 seconds
> Local /Remote
> Socket Size   Request  Resp.   Elapsed  Trans.
> Send   Recv   Size     Size    Time     Rate
> bytes  Bytes  bytes    bytes   secs.    per sec
> 
> 16384  87380  1        1       15.00    12225.77
> 16384  87380

This is a pretty well expected behavior from this algorithm.  You're
basically getting a sine wave of the two ITR clocks.  At least your
average is 12,000 now.  Before this "dynamic tuning" your throughput
would have been 4,000-4,500 between two interfaces locked at 8000 ints a
sec.  You can test this by setting InterrruptThrottleRate=8000,8000,...
on both sides.

> On a slightly informed whim I tried disabling the interrupt thottle
> on both sides (modprobe e1000 InterruptThrottleRate=0,0,0,0,0,0,0,0)
> and re-ran: 
> 
> hpcpc109:~/netperf2_trunk# src/netperf -t TCP_RR -H 192.168.2.105 -D
> 1.0 -l 15TCP REQUEST/RESPONSE TEST from 0.0.0.0 (0.0.0.0) port 0
> AF_INET to 192.168.2.105 (192.168.2.105) port 0 AF_INET : demo :
> first burst 0 
> Interim result: 18673.68 Trans/s over 1.00 seconds
> Interim result: 18685.01 Trans/s over 1.00 seconds
> Interim result: 18682.30 Trans/s over 1.00 seconds
> Interim result: 18681.05 Trans/s over 1.00 seconds
> Interim result: 18680.25 Trans/s over 1.00 seconds
> Interim result: 18742.44 Trans/s over 1.00 seconds
> Interim result: 18739.45 Trans/s over 1.00 seconds
> Interim result: 18723.52 Trans/s over 1.00 seconds
> Interim result: 18736.53 Trans/s over 1.00 seconds
> Interim result: 18737.61 Trans/s over 1.00 seconds
> Interim result: 18744.76 Trans/s over 1.00 seconds
> Interim result: 18728.54 Trans/s over 1.00 seconds
> Interim result: 18738.91 Trans/s over 1.00 seconds
> Interim result: 18735.53 Trans/s over 1.00 seconds
> Interim result: 18741.03 Trans/s over 1.00 seconds
> Local /Remote
> Socket Size   Request  Resp.   Elapsed  Trans.
> Send   Recv   Size     Size    Time     Rate
> bytes  Bytes  bytes    bytes   secs.    per sec
> 
> 16384  87380  1        1       15.00    18717.94
> 16384  87380

and I'll bet that your systems max out in this test about 36000
interrupts per second on each side, giving you 1 interrupt per tx, and 1
interrupt per rx.
 
> and then just for grins I tried just disabling it on one side,
> leaving the other at defaults:
> 
> hpcpc109:~/netperf2_trunk# src/netperf -t TCP_RR -H 192.168.2.105 -D
> 1.0 -l 15TCP REQUEST/RESPONSE TEST from 0.0.0.0 (0.0.0.0) port 0
> AF_INET to 192.168.2.105 (192.168.2.105) port 0 AF_INET : demo :
> first burst 0 
> Interim result: 19980.84 Trans/s over 1.00 seconds
> Interim result: 19997.60 Trans/s over 1.00 seconds
> Interim result: 19995.60 Trans/s over 1.00 seconds
> Interim result: 20002.60 Trans/s over 1.00 seconds
> Interim result: 20011.58 Trans/s over 1.00 seconds
> Interim result: 19985.66 Trans/s over 1.00 seconds
> Interim result: 20002.60 Trans/s over 1.00 seconds
> Interim result: 20010.58 Trans/s over 1.00 seconds
> Interim result: 20012.60 Trans/s over 1.00 seconds
> Interim result: 19993.63 Trans/s over 1.00 seconds
> Interim result: 19979.63 Trans/s over 1.00 seconds
> Interim result: 19991.58 Trans/s over 1.00 seconds
> Interim result: 20011.60 Trans/s over 1.00 seconds
> Interim result: 19948.84 Trans/s over 1.00 seconds
> Local /Remote
> Socket Size   Request  Resp.   Elapsed  Trans.
> Send   Recv   Size     Size    Time     Rate
> bytes  Bytes  bytes    bytes   secs.    per sec
> 
> 16384  87380  1        1       15.00    19990.14
> 16384  87380

This is tied directly to the default of 20000 ints/second of the upper
limit of the default "dynamic" tuning.

> It looks like the e1000 interrupt throttle autotuning works very
> nicely when the other side isn't doing any, but if the other side is
> also trying to autotune it doesn't seem to stablize.  At least not
> during a netperf TCP_RR test. 

one of the side effects of the algorithm's fast response to bursty
traffic is that it ping pongs around quite a bit as your test transmits
and receives packets.  If you put the driver into
InterruptThrottleRate=1,1,1.... then the upper limit of interrupts goes
up to 70,000 ints/s, which may give you a still different result.
 
> Does anyone else see this?  To try to eliminate netperf demo mode I
> re-ran without it and got the same end results.

yes, we saw this and consider it normal.  Even one might call it an
improvement, since your latency is quite a bit lower (transactions is
quite a bit higher) than it used to be in this same test.

Should you need absolute lowest latency, InterruptThrottleRate=0 is the
way to go, but we hoped that the regular user using the default settings
would be pleased with a 10,000-20,000 transactions per second rate.  In
your case I would suggest InterruptThrottleRate=1 will still provide
better performance in this latency sensitive test while still allowing
for some interrupt mitigation if you all of a sudden decide to test bulk
traffic without reloading the driver.

Thanks for your numbers, its good to see things working expectedly.  If
you have suggestions for improvements please let me know,

  Jesse

^ permalink raw reply

* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Paul E. McKenney @ 2007-08-18  1:08 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Linus Torvalds, Nick Piggin, Paul Mackerras, Segher Boessenkool,
	heiko.carstens, horms, linux-kernel, rpjday, ak, netdev, cfriesen,
	akpm, jesper.juhl, linux-arch, zlynx, satyam, clameter,
	schwidefsky, Chris Snook, davem, wensong, wjiang
In-Reply-To: <20070818000913.GA25585@gondor.apana.org.au>

On Sat, Aug 18, 2007 at 08:09:13AM +0800, Herbert Xu wrote:
> On Fri, Aug 17, 2007 at 04:59:12PM -0700, Paul E. McKenney wrote:
> >
> > gcc bugzilla bug #33102, for whatever that ends up being worth.  ;-)
> 
> I had totally forgotten that I'd already filed that bug more
> than six years ago until they just closed yours as a duplicate
> of mine :)
> 
> Good luck in getting it fixed!

Well, just got done re-opening it for the third time.  And a local
gcc community member advised me not to give up too easily.  But I
must admit that I am impressed with the speed that it was identified
as duplicate.

Should be entertaining!  ;-)

						Thanx, Paul

^ permalink raw reply

* Re: [RFC] restore netdev_priv optimization (planb)
From: David Miller @ 2007-08-18  1:00 UTC (permalink / raw)
  To: shemminger; +Cc: netdev
In-Reply-To: <20070817174909.5e890ad6@freepuppy.rosehill.hemminger.net>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 17 Aug 2007 17:49:09 -0700

> Fix optimization of netdev_priv() lost by the addition of multiqueue.
> 
> Only configurations that define MULITQUEUE need the extra overhead in
> netdevice structure and the loss of the netdev_priv optimization.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

Every distribution vendor is going to turn MULTIQUEUE on.  Therefore
%99 of Linux users will not see any gain from your patch.

You're walking around in a dark room and hitting walls every
few seconds.  Take a moment and think about how to really deal
with this generically for a second before churning out another
patch.

For example, how about making the multiqueue limit fixed, say at 64
queues, and declare the egress_queues as an array of 64 entries.  Then
you can get constant pointer formation for both the netdev priv and
the queues.

This will also basically eliminate all of the alloc_mqueue() logic,
since struct netdev will now always be a fixed size.

And, amazingly, this deals with the loopback issue transparently as
well. :-)

It would also be nice to just get rid of netdev->priv, it will prevent
more misuses like the cxgb3 case, for example.

^ permalink raw reply

* Re: [PATCH] e1000e: Update e1000e driver to use devres
From: Tejun Heo @ 2007-08-18  0:51 UTC (permalink / raw)
  To: Brandon Philips; +Cc: Kok, Auke, jgarzik, e1000-devel, netdev
In-Reply-To: <20070817202502.GD4525@ifup.org>

Brandon Philips wrote:
> Conversion of e1000e probe() and remove() to devres.
> 
> Depends on "[patch 1/4] Update net core to use devres."
> 
> Signed-off-by: Brandon Philips <bphilips@suse.de>

Acked-by: Tejun Heo <htejun@gmail.com>

-- 
tejun

^ permalink raw reply

* [RFC] restore netdev_priv optimization (planb)
From: Stephen Hemminger @ 2007-08-18  0:49 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20070817.165625.23014105.davem@davemloft.net>

Fix optimization of netdev_priv() lost by the addition of multiqueue.

Only configurations that define MULITQUEUE need the extra overhead in
netdevice structure and the loss of the netdev_priv optimization.

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>


--- a/include/linux/netdevice.h	2007-08-17 16:45:39.000000000 -0700
+++ b/include/linux/netdevice.h	2007-08-17 17:31:51.000000000 -0700
@@ -574,17 +574,22 @@ struct net_device
 	const struct rtnl_link_ops *rtnl_link_ops;
 
 	/* The TX queue control structures */
-	unsigned int			egress_subqueue_count;
+#ifdef CONFIG_NETDEVICES_MULTIQUEUE
+	unsigned short			egress_subqueue_count;
 	struct net_device_subqueue	egress_subqueue[1];
+#endif
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
 #define	NETDEV_ALIGN		32
-#define	NETDEV_ALIGN_CONST	(NETDEV_ALIGN - 1)
 
 static inline void *netdev_priv(const struct net_device *dev)
 {
+#ifdef CONFIG_NETDEVICES_MULTIQUEUE
 	return dev->priv;
+#else
+	return (void *)dev + ALIGN(sizeof(struct net_device), NETDEV_ALIGN);
+#endif
 }
 
 #define SET_MODULE_OWNER(dev) do { } while (0)
--- a/net/core/dev.c	2007-08-17 16:45:39.000000000 -0700
+++ b/net/core/dev.c	2007-08-17 17:44:02.000000000 -0700
@@ -3706,7 +3706,7 @@ EXPORT_SYMBOL(dev_get_stats);
  *	@queue_count:	the number of subqueues to allocate
  *
  *	Allocates a struct net_device with private data area for driver use
- *	and performs basic initialization.  Also allocates subquue structs
+ *	and performs basic initialization.  Also allocates subqueue structs
  *	for each queue on the device at the end of the netdevice.
  */
 struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
@@ -3714,15 +3714,20 @@ struct net_device *alloc_netdev_mq(int s
 {
 	void *p;
 	struct net_device *dev;
-	int alloc_size;
+	size_t dev_size, alloc_size;
 
 	BUG_ON(strlen(name) >= sizeof(dev->name));
 
+#ifndef CONFIG_NETDEVICES_MULTIQUEUE
+	BUG_ON(queue_count > 1);
+#endif
+
 	/* ensure 32-byte alignment of both the device and private area */
-	alloc_size = (sizeof(*dev) + NETDEV_ALIGN_CONST +
-		     (sizeof(struct net_device_subqueue) * (queue_count - 1))) &
-		     ~NETDEV_ALIGN_CONST;
-	alloc_size += sizeof_priv + NETDEV_ALIGN_CONST;
+	dev_size = sizeof(*dev)
+		+ (queue_count - 1) * sizeof(struct net_device_subqueue);
+
+	alloc_size = roundup(dev_size, NETDEV_ALIGN);
+	alloc_size += sizeof_priv + NETDEV_ALIGN - 1;
 
 	p = kzalloc(alloc_size, GFP_KERNEL);
 	if (!p) {
@@ -3730,19 +3735,17 @@ struct net_device *alloc_netdev_mq(int s
 		return NULL;
 	}
 
-	dev = (struct net_device *)
-		(((long)p + NETDEV_ALIGN_CONST) & ~NETDEV_ALIGN_CONST);
+	dev = (struct net_device *) ALIGN((unsigned long) p, NETDEV_ALIGN);
 	dev->padded = (char *)dev - (char *)p;
 
-	if (sizeof_priv) {
-		dev->priv = ((char *)dev +
-			     ((sizeof(struct net_device) +
-			       (sizeof(struct net_device_subqueue) *
-				(queue_count - 1)) + NETDEV_ALIGN_CONST)
-			      & ~NETDEV_ALIGN_CONST));
-	}
-
+#ifdef CONFIG_NETDEVICES_MULTIQUEUE
 	dev->egress_subqueue_count = queue_count;
+	if (sizeof_priv)
+		dev->priv = ALIGN((unsigned long)dev + dev_size, NETDEV_ALIGN);
+#else
+	if (sizeof_priv)
+		dev->priv = netdev_priv(dev);
+#endif
 
 	setup(dev);
 	strcpy(dev->name, name);


^ permalink raw reply

* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Herbert Xu @ 2007-08-18  0:09 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Linus Torvalds, Nick Piggin, Paul Mackerras, Segher Boessenkool,
	heiko.carstens, horms, linux-kernel, rpjday, ak, netdev, cfriesen,
	akpm, jesper.juhl, linux-arch, zlynx, satyam, clameter,
	schwidefsky, Chris Snook, davem, wensong, wjiang
In-Reply-To: <20070817235912.GA24314@linux.vnet.ibm.com>

On Fri, Aug 17, 2007 at 04:59:12PM -0700, Paul E. McKenney wrote:
>
> gcc bugzilla bug #33102, for whatever that ends up being worth.  ;-)

I had totally forgotten that I'd already filed that bug more
than six years ago until they just closed yours as a duplicate
of mine :)

Good luck in getting it fixed!

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: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Segher Boessenkool @ 2007-08-18  0:04 UTC (permalink / raw)
  To: Satyam Sharma
  Cc: Christoph Lameter, Paul Mackerras, heiko.carstens, horms,
	Stefan Richter, Linux Kernel Mailing List, David Miller,
	Paul E. McKenney, Ilpo Järvinen, ak, cfriesen, rpjday,
	Netdev, jesper.juhl, linux-arch, zlynx, Andrew Morton,
	schwidefsky, Chris Snook, Herbert Xu, Linus Torvalds, wensong,
	wjiang
In-Reply-To: <alpine.LFD.0.999.0708180522120.3666@enigma.security.iitk.ac.in>

>>>> atomic_dec() writes
>>>> to memory, so it _does_ have "volatile semantics", implicitly, as
>>>> long as the compiler cannot optimise the atomic variable away
>>>> completely -- any store counts as a side effect.
>>>
>>> I don't think an atomic_dec() implemented as an inline "asm volatile"
>>> or one that uses a "forget" macro would have the same re-ordering
>>> guarantees as an atomic_dec() that uses a volatile access cast.
>>
>> The "asm volatile" implementation does have exactly the same
>> reordering guarantees as the "volatile cast" thing,
>
> I don't think so.

"asm volatile" creates a side effect.  Side effects aren't
allowed to be reordered wrt sequence points.  This is exactly
the same reason as why "volatile accesses" cannot be reordered.

>> if that is
>> implemented by GCC in the "obvious" way.  Even a "plain" asm()
>> will do the same.
>
> Read the relevant GCC documentation.

I did, yes.

> [ of course, if the (latest) GCC documentation is *yet again*
>   wrong, then alright, not much I can do about it, is there. ]

There was (and is) nothing wrong about the "+m" documentation, if
that is what you are talking about.  It could be extended now, to
allow "+m" -- but that takes more than just "fixing" the documentation.


Segher


^ permalink raw reply

* Re: Marvell 88E8056 gigabit ethernet controller
From: Stephen Hemminger @ 2007-08-18  0:03 UTC (permalink / raw)
  To: Kevin E; +Cc: netdev
In-Reply-To: <424491.59139.qm@web38906.mail.mud.yahoo.com>

On Fri, 17 Aug 2007 05:42:13 -0700 (PDT)
Kevin E <kevin360@yahoo.com> wrote:

> Hi all,
> 
> 	I've read where the onboard Marvell lan controller on
> some Gigabyte boards don't work.  I've got two systems
> using the same Gigabyte board, on one the LAN works on
> the other it dies like described by others.  Here's
> the systems:
> 
> 
> Working system:
> Gigabyte 965P-DS3 rev 3.3  (BIOS F10)
> Core2 Q6600
> 2GB Corsair XMS2 memory
> kernel 2.6.22.3
> 
> lspci for LAN controller:
> 04:00.0 Ethernet controller: Marvell Technology Group
> Ltd. 88E8056 PCI-E Gigabit Ethernet Controller (rev
> 14)
> 
> 
> Broken system:
> Gigabyte 965P-DS3 rev 3.3  (BIOS F10)
> Core2 E4400
> 2GB Corsair XMS2 memory
> kernel 2.6.22.3
> 
> lspci for LAN controller:
> 03:00.0 Ethernet controller: Marvell Technology Group
> Ltd. Unknown device 4364 (rev 12)
> 
> 
> 	The BIOS for the two systems are setup the same and
> the config for the kernels are the same too.  I've
> actually tried taking the kernel from the working
> system and booting it on the broken one but still the
> LAN dies after a couple of seconds.  The working
> system has one card plugged in (nvidia based PCI-X
> video card), I've taken that card and plugged into the
> broken system, booted the same kernel, and it still
> dies after a while.
> 
> 	I will gladly provide any info needed if it can help
> in getting this chipset working on the Gigabyte
> boards.
> 
> 	Thanks,
> 	Kevin

I maintain the sky2 driver, and have one of the (buggy) Gigabyte motherboards.
It is interesting that the problem seems to track with video card.
Are you using the Nvidia binary driver?
The video card in the system I have troubles with is:
	ATI Technologies Inc RV370 [Radeon X300SE]

Surprisingly, using other PCI-E cards with same driver (different Marvell chips)
has no problem.  Vendor version of sk98lin driver has same failure mode
on the buggy hardware.

You might want to look at lspci -vvv output on two system to see if there
are differences. Perhaps there is a CPU speed dependency?


^ permalink raw reply

* Re: [ofa-general] Re: [PATCH RFC] RDMA/CMA: Allocate PS_TCP ports from the host TCP port space.
From: David Miller @ 2007-08-18  0:00 UTC (permalink / raw)
  To: rdreier; +Cc: jeff, netdev, linux-kernel, general
In-Reply-To: <adafy2hyc04.fsf@cisco.com>

From: Roland Dreier <rdreier@cisco.com>
Date: Fri, 17 Aug 2007 16:31:07 -0700

>  > >  > When using RDMA you lose the capability to do packet shaping,
>  > >  > classification, and all the other wonderful networking facilities
>  > >  > you've grown to love and use over the years.
>  > > 
>  > > Same thing with TSO and LRO and who knows what else.
>  > 
>  > Not true at all.  Full classification and filtering still is usable
>  > with TSO and LRO.
> 
> Well, obviously with TSO and LRO the packets that the stack sends or
> receives are not the same as what's on the wire.  Whether that breaks
> your wonderful networking facilities or not depends on the specifics
> of the particular facility I guess -- for example shaping is clearly
> broken by TSO.  (And people can wonder what the packet trains TSO
> creates do to congestion control on the internet, but the netdev crowd
> has already decided that TSO is "good" and RDMA is "bad")

This is also a series of falsehoods.  All packet filtering,
queue management, and packet scheduling facilities work perfectly
fine and as designed with both LRO and TSO.

When problems come up, they are bugs, and we fix them.

Please stop spreading this FUD about TSO and LRO.

The fact is that RDMA bypasses the whole stack so that supporting
these facilities is not even _POSSIBLE_.  With stateless offloads it
is possible to support all of these facilities, and we do.

^ 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