* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Denys Vlasenko @ 2007-08-24 12:19 UTC (permalink / raw)
To: Linus Torvalds
Cc: Satyam Sharma, Christoph Lameter, Paul E. McKenney, Herbert Xu,
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: <alpine.LFD.0.999.0708172110400.30176@woody.linux-foundation.org>
On Saturday 18 August 2007 05:13, Linus Torvalds wrote:
> On Sat, 18 Aug 2007, Satyam Sharma wrote:
> > No code does (or would do, or should do):
> >
> > x.counter++;
> >
> > on an "atomic_t x;" anyway.
>
> That's just an example of a general problem.
>
> No, you don't use "x.counter++". But you *do* use
>
> if (atomic_read(&x) <= 1)
>
> and loading into a register is stupid and pointless, when you could just
> do it as a regular memory-operand to the cmp instruction.
It doesn't mean that (volatile int*) cast is bad, it means that current gcc
is bad (or "not good enough"). IOW: instead of avoiding volatile cast,
it's better to fix the compiler.
> And as far as the compiler is concerned, the problem is the 100% same:
> combining operations with the volatile memop.
>
> The fact is, a compiler that thinks that
>
> movl mem,reg
> cmpl $val,reg
>
> is any better than
>
> cmpl $val,mem
>
> is just not a very good compiler.
Linus, in all honesty gcc has many more cases of suboptimal code,
case of "volatile" is just one of many.
Off the top of my head:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28417
unsigned v;
void f(unsigned A) { v = ((unsigned long long)A) * 365384439 >> (27+32); }
gcc-4.1.1 -S -Os -fomit-frame-pointer t.c
f:
movl $365384439, %eax
mull 4(%esp)
movl %edx, %eax <===== ?
shrl $27, %eax
movl %eax, v
ret
Why is it moving %edx to %eax?
gcc-4.2.1 -S -Os -fomit-frame-pointer t.c
f:
movl $365384439, %eax
mull 4(%esp)
movl %edx, %eax <===== ?
xorl %edx, %edx <===== ??!
shrl $27, %eax
movl %eax, v
ret
Progress... Now we also zero out %edx afterwards for no apparent reason.
--
vda
^ permalink raw reply
* RE: [PATCH] i386: Fix a couple busy loops in mach_wakecpu.h:wait_for_init_deassert()
From: Kenn Humborg @ 2007-08-24 12:12 UTC (permalink / raw)
To: Denys Vlasenko, Satyam Sharma
Cc: Heiko Carstens, Herbert Xu, Chris Snook, clameter,
Linux Kernel Mailing List, linux-arch, Linus Torvalds, netdev,
Andrew Morton, ak, davem, schwidefsky, wensong, horms, wjiang,
cfriesen, zlynx, rpjday, jesper.juhl, segher
In-Reply-To: <200708241259.33659.vda.linux@googlemail.com>
> On Thursday 16 August 2007 01:39, Satyam Sharma wrote:
> >
> > static inline void wait_for_init_deassert(atomic_t *deassert)
> > {
> > - while (!atomic_read(deassert));
> > + while (!atomic_read(deassert))
> > + cpu_relax();
> > return;
> > }
>
> For less-than-briliant people like me, it's totally non-obvious that
> cpu_relax() is needed for correctness here, not just to make P4 happy.
>
> IOW: "atomic_read" name quite unambiguously means "I will read
> this variable from main memory". Which is not true and creates
> potential for confusion and bugs.
To me, "atomic_read" means a read which is synchronized with other
changes to the variable (using the atomic_XXX functions) in such
a way that I will always only see the "before" or "after"
state of the variable - never an intermediate state while a
modification is happening. It doesn't imply that I have to
see the "after" state immediately after another thread modifies
it.
Perhaps the Linux atomic_XXX functions work like that, or used
to work like that, but it's counter-intuitive to me that "atomic"
should imply a memory read.
Later,
Kenn
^ permalink raw reply
* Re: [PATCH 0/9 Rev3] Implement batching skb API and support in IPoIB
From: jamal @ 2007-08-24 12:36 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David Miller, rick.jones2, krkumar2, gaagaan, general, herbert,
jagana, jeff, johnpol, kaber, mcarlson, mchan, netdev,
peter.p.waskiewicz.jr, rdreier, Robert.Olsson, sri, tgraf, xma
In-Reply-To: <20070823203425.2a32fb3f@freepuppy.rosehill.hemminger.net>
On Thu, 2007-23-08 at 20:34 -0700, Stephen Hemminger wrote:
> A current hot topic of research is reducing the number of ACK's to make TCP
> work better over asymmetric links like 3G.
One other good reason to reduce ACKs to battery powered (3G) terminals
is it reduces the power consumption i.e you have longer battery life.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Denys Vlasenko @ 2007-08-24 12:50 UTC (permalink / raw)
To: Paul Mackerras
Cc: Satyam Sharma, Stefan Richter, 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,
Herbert Xu, Paul E. McKenney
In-Reply-To: <18115.35524.56393.347841@cargo.ozlabs.ibm.com>
On Thursday 16 August 2007 00:22, Paul Mackerras wrote:
> Satyam Sharma writes:
> In the kernel we use atomic variables in precisely those situations
> where a variable is potentially accessed concurrently by multiple
> CPUs, and where each CPU needs to see updates done by other CPUs in a
> timely fashion. That is what they are for. Therefore the compiler
> must not cache values of atomic variables in registers; each
> atomic_read must result in a load and each atomic_set must result in a
> store. Anything else will just lead to subtle bugs.
Amen.
--
vda
^ permalink raw reply
* Re: [PATCH] i386: Fix a couple busy loops in mach_wakecpu.h:wait_for_init_deassert()
From: Satyam Sharma @ 2007-08-24 13:30 UTC (permalink / raw)
To: Denys Vlasenko
Cc: Heiko Carstens, Herbert Xu, Chris Snook, clameter,
Linux Kernel Mailing List, linux-arch, Linus Torvalds, netdev,
Andrew Morton, ak, davem, schwidefsky, wensong, horms, wjiang,
cfriesen, zlynx, rpjday, jesper.juhl, segher, Nick Piggin
In-Reply-To: <200708241259.33659.vda.linux@googlemail.com>
Hi Denys,
On Fri, 24 Aug 2007, Denys Vlasenko wrote:
> On Thursday 16 August 2007 01:39, Satyam Sharma wrote:
> >
> > static inline void wait_for_init_deassert(atomic_t *deassert)
> > {
> > - while (!atomic_read(deassert));
> > + while (!atomic_read(deassert))
> > + cpu_relax();
> > return;
> > }
>
> For less-than-briliant people like me, it's totally non-obvious that
> cpu_relax() is needed for correctness here, not just to make P4 happy.
This thread has been round-and-round with exactly the same discussions
:-) I had proposed few such variants to make a compiler barrier implicit
in atomic_{read,set} myself, but frankly, at least personally speaking
(now that I know better), I'm not so much in favour of implicit barriers
(compiler, memory or both) in atomic_{read,set}.
This might sound like an about-turn if you read my own postings to Nick
Piggin from a week back, but I do agree with most his opinions on the
matter now -- separation of barriers from atomic ops is actually good,
beneficial to certain code that knows what it's doing, explicit usage
of barriers stands out more clearly (most people here who deal with it
do know cpu_relax() is an explicit compiler barrier) compared to an
implicit usage in an atomic_read() or such variant ...
> IOW: "atomic_read" name quite unambiguously means "I will read
> this variable from main memory". Which is not true and creates
> potential for confusion and bugs.
I'd have to disagree here -- atomic ops are all about _atomicity_ of
memory accesses, not _making_ them happen (or visible to other CPUs)
_then and there_ itself. The latter are the job of barriers.
The behaviour (and expectations) are quite comprehensively covered in
atomic_ops.txt -- let alone atomic_{read,set}, even atomic_{inc,dec}
are permitted by archs' implementations to _not_ have any memory
barriers, for that matter. [It is unrelated that on x86 making them
SMP-safe requires the use of the LOCK prefix that also happens to be
an implicit memory barrier.]
An argument was also made about consistency of atomic_{read,set} w.r.t.
the other atomic ops -- but clearly, they are all already consistent!
All of them are atomic :-) The fact that atomic_{read,set} do _not_
require any inline asm or LOCK prefix whereas the others do, has to do
with the fact that unlike all others, atomic_{read,set} are not RMW ops
and hence guaranteed to be atomic just as they are in plain & simple C.
But if people do seem to have a mixed / confused notion of atomicity
and barriers, and if there's consensus, then as I'd said earlier, I
have no issues in going with the consensus (eg. having API variants).
Linus would be more difficult to convince, however, I suspect :-)
Satyam
^ permalink raw reply
* RFC: issues concerning the next NAPI interface
From: Jan-Bernd Themann @ 2007-08-24 13:59 UTC (permalink / raw)
To: netdev
Cc: Christoph Raisch, Jan-Bernd Themann, linux-kernel, linux-ppc,
Marcus Eder, Thomas Klein, Stefan Roscher
Hi,
when I tried to get the eHEA driver working with the new interface,
the following issues came up.
1) The current implementation of netif_rx_schedule, netif_rx_complete
and the net_rx_action have the following problem: netif_rx_schedule
sets the NAPI_STATE_SCHED flag and adds the NAPI instance to the poll_list.
netif_rx_action checks NAPI_STATE_SCHED, if set it will add the device
to the poll_list again (as well). netif_rx_complete clears the NAPI_STATE_SCHED.
If an interrupt handler calls netif_rx_schedule on CPU 2
after netif_rx_complete has been called on CPU 1 (and the poll function
has not returned yet), the NAPI instance will be added twice to the
poll_list (by netif_rx_schedule and net_rx_action). Problems occur when
netif_rx_complete is called twice for the device (BUG() called)
2) If an ethernet chip supports multiple receive queues, the queues are
currently all processed on the CPU where the interrupt comes in. This
is because netif_rx_schedule will always add the rx queue to the CPU's
napi poll_list. The result under heavy presure is that all queues will
gather on the weakest CPU (with highest CPU load) after some time as they
will stay there as long as the entire queue is emptied. On SMP systems
this behaviour is not desired. It should also work well without interrupt
pinning.
It would be nice if it is possible to schedule queues to other CPU's, or
at least to use interrupts to put the queue to another cpu (not nice for
as you never know which one you will hit).
I'm not sure how bad the tradeoff would be.
3) On modern systems the incoming packets are processed very fast. Especially
on SMP systems when we use multiple queues we process only a few packets
per napi poll cycle. So NAPI does not work very well here and the interrupt
rate is still high. What we need would be some sort of timer polling mode
which will schedule a device after a certain amount of time for high load
situations. With high precision timers this could work well. Current
usual timers are too slow. A finer granularity would be needed to keep the
latency down (and queue length moderate).
What do you think?
Thanks,
Jan-Bernd
^ permalink raw reply
* Re: [PATCH] i386: Fix a couple busy loops in mach_wakecpu.h:wait_for_init_deassert()
From: Denys Vlasenko @ 2007-08-24 14:25 UTC (permalink / raw)
To: Kenn Humborg
Cc: Satyam Sharma, Heiko Carstens, Herbert Xu, Chris Snook, clameter,
Linux Kernel Mailing List, linux-arch, Linus Torvalds, netdev,
Andrew Morton, ak, davem, schwidefsky, wensong, horms, wjiang,
cfriesen, zlynx, rpjday, jesper.juhl, segher
In-Reply-To: <NBBBIGEGHIGMPCNKHCECAEPEELAA.kenn@bluetree.ie>
On Friday 24 August 2007 13:12, Kenn Humborg wrote:
> > On Thursday 16 August 2007 01:39, Satyam Sharma wrote:
> > > static inline void wait_for_init_deassert(atomic_t *deassert)
> > > {
> > > - while (!atomic_read(deassert));
> > > + while (!atomic_read(deassert))
> > > + cpu_relax();
> > > return;
> > > }
> >
> > For less-than-briliant people like me, it's totally non-obvious that
> > cpu_relax() is needed for correctness here, not just to make P4 happy.
> >
> > IOW: "atomic_read" name quite unambiguously means "I will read
> > this variable from main memory". Which is not true and creates
> > potential for confusion and bugs.
>
> To me, "atomic_read" means a read which is synchronized with other
> changes to the variable (using the atomic_XXX functions) in such
> a way that I will always only see the "before" or "after"
> state of the variable - never an intermediate state while a
> modification is happening. It doesn't imply that I have to
> see the "after" state immediately after another thread modifies
> it.
So you are ok with compiler propagating n1 to n2 here:
n1 += atomic_read(x);
other_variable++;
n2 += atomic_read(x);
without accessing x second time. What's the point? Any sane coder
will say that explicitly anyway:
tmp = atomic_read(x);
n1 += tmp;
other_variable++;
n2 += tmp;
if only for the sake of code readability. Because first code
is definitely hinting that it reads RAM twice, and it's actively *bad*
for code readability when in fact it's not the case!
Locking, compiler and CPU barriers are complicated enough already,
please don't make them even harder to understand.
--
vda
^ permalink raw reply
* Re: RFC: issues concerning the next NAPI interface
From: akepner @ 2007-08-24 15:37 UTC (permalink / raw)
To: Jan-Bernd Themann
Cc: netdev, Christoph Raisch, Jan-Bernd Themann, linux-kernel,
linux-ppc, Marcus Eder, Thomas Klein, Stefan Roscher
In-Reply-To: <200708241559.17055.ossthema@de.ibm.com>
On Fri, Aug 24, 2007 at 03:59:16PM +0200, Jan-Bernd Themann wrote:
> .......
> 3) On modern systems the incoming packets are processed very fast. Especially
> on SMP systems when we use multiple queues we process only a few packets
> per napi poll cycle. So NAPI does not work very well here and the interrupt
> rate is still high. What we need would be some sort of timer polling mode
> which will schedule a device after a certain amount of time for high load
> situations. With high precision timers this could work well. Current
> usual timers are too slow. A finer granularity would be needed to keep the
> latency down (and queue length moderate).
>
We found the same on ia64-sn systems with tg3 a couple of years
ago. Using simple interrupt coalescing ("don't interrupt until
you've received N packets or M usecs have elapsed") worked
reasonably well in practice. If your h/w supports that (and I'd
guess it does, since it's such a simple thing), you might try
it.
--
Arthur
^ permalink raw reply
* [PATCH 1/1] Dynamically allocate the loopback device
From: dlezcano @ 2007-08-24 15:36 UTC (permalink / raw)
To: davem, shemminger; +Cc: netdev, containers, Daniel Lezcano, Eric W. Biederman
From: Daniel Lezcano <dlezcano@fr.ibm.com>
Doing this makes loopback.c a better example of how to do a
simple network device, and it removes the special case
single static allocation of a struct net_device, hopefully
making maintenance easier.
Applies against net-2.6.24
Tested on i386, x86_64
Compiled on ia64, sparc
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Acked-By: Kirill Korotaev <dev@sw.ru>
Acked-by: Benjamin Thery <benjamin.thery@bull.net>
---
drivers/net/loopback.c | 63 +++++++++++++++++++++++---------------
include/linux/netdevice.h | 2 +-
net/core/dst.c | 8 ++--
net/decnet/dn_dev.c | 4 +-
net/decnet/dn_route.c | 14 ++++----
net/ipv4/devinet.c | 6 ++--
net/ipv4/ipconfig.c | 6 ++--
net/ipv4/ipvs/ip_vs_core.c | 2 +-
net/ipv4/route.c | 18 +++++-----
net/ipv4/xfrm4_policy.c | 2 +-
net/ipv6/addrconf.c | 15 +++++---
net/ipv6/ip6_input.c | 2 +-
net/ipv6/netfilter/ip6t_REJECT.c | 2 +-
net/ipv6/route.c | 15 +++-----
net/ipv6/xfrm6_policy.c | 2 +-
net/xfrm/xfrm_policy.c | 4 +-
16 files changed, 89 insertions(+), 76 deletions(-)
diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 5106c23..3642aff 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -199,44 +199,57 @@ static const struct ethtool_ops loopback_ethtool_ops = {
.get_rx_csum = always_on,
};
-/*
- * The loopback device is special. There is only one instance and
- * it is statically allocated. Don't do this for other devices.
- */
-struct net_device loopback_dev = {
- .name = "lo",
- .get_stats = &get_stats,
- .mtu = (16 * 1024) + 20 + 20 + 12,
- .hard_start_xmit = loopback_xmit,
- .hard_header = eth_header,
- .hard_header_cache = eth_header_cache,
- .header_cache_update = eth_header_cache_update,
- .hard_header_len = ETH_HLEN, /* 14 */
- .addr_len = ETH_ALEN, /* 6 */
- .tx_queue_len = 0,
- .type = ARPHRD_LOOPBACK, /* 0x0001*/
- .rebuild_header = eth_rebuild_header,
- .flags = IFF_LOOPBACK,
- .features = NETIF_F_SG | NETIF_F_FRAGLIST
+static void loopback_setup(struct net_device *dev)
+{
+ dev->get_stats = &get_stats;
+ dev->mtu = (16 * 1024) + 20 + 20 + 12;
+ dev->hard_start_xmit = loopback_xmit;
+ dev->hard_header = eth_header;
+ dev->hard_header_cache = eth_header_cache;
+ dev->header_cache_update = eth_header_cache_update;
+ dev->hard_header_len = ETH_HLEN; /* 14 */
+ dev->addr_len = ETH_ALEN; /* 6 */
+ dev->tx_queue_len = 0;
+ dev->type = ARPHRD_LOOPBACK; /* 0x0001*/
+ dev->rebuild_header = eth_rebuild_header;
+ dev->flags = IFF_LOOPBACK;
+ dev->features = NETIF_F_SG | NETIF_F_FRAGLIST
#ifdef LOOPBACK_TSO
| NETIF_F_TSO
#endif
| NETIF_F_NO_CSUM | NETIF_F_HIGHDMA
- | NETIF_F_LLTX,
- .ethtool_ops = &loopback_ethtool_ops,
-};
+ | NETIF_F_LLTX;
+ dev->ethtool_ops = &loopback_ethtool_ops;
+}
/* Setup and register the loopback device. */
static int __init loopback_init(void)
{
- int err = register_netdev(&loopback_dev);
+ struct net_device *dev;
+ int err;
+
+ err = -ENOMEM;
+ dev = alloc_netdev(0, "lo", loopback_setup);
+ if (!dev)
+ goto out;
+
+ err = register_netdev(dev);
+ if (err)
+ goto out_free_netdev;
+ err = 0;
+ loopback_dev = dev;
+
+out:
if (err)
panic("loopback: Failed to register netdevice: %d\n", err);
-
return err;
+out_free_netdev:
+ free_netdev(dev);
+ goto out;
};
-module_init(loopback_init);
+fs_initcall(loopback_init);
+struct net_device *loopback_dev;
EXPORT_SYMBOL(loopback_dev);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8d12f02..7cd0641 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -680,7 +680,7 @@ struct packet_type {
#include <linux/interrupt.h>
#include <linux/notifier.h>
-extern struct net_device loopback_dev; /* The loopback */
+extern struct net_device *loopback_dev; /* The loopback */
extern struct list_head dev_base_head; /* All devices */
extern rwlock_t dev_base_lock; /* Device list lock */
diff --git a/net/core/dst.c b/net/core/dst.c
index c6a0587..ad8549e 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -236,13 +236,13 @@ static inline void dst_ifdown(struct dst_entry *dst, struct net_device *dev,
if (!unregister) {
dst->input = dst->output = dst_discard;
} else {
- dst->dev = &loopback_dev;
- dev_hold(&loopback_dev);
+ dst->dev = loopback_dev;
+ dev_hold(dst->dev);
dev_put(dev);
if (dst->neighbour && dst->neighbour->dev == dev) {
- dst->neighbour->dev = &loopback_dev;
+ dst->neighbour->dev = loopback_dev;
dev_put(dev);
- dev_hold(&loopback_dev);
+ dev_hold(dst->neighbour->dev);
}
}
}
diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
index fa6604f..9fea83e 100644
--- a/net/decnet/dn_dev.c
+++ b/net/decnet/dn_dev.c
@@ -868,10 +868,10 @@ last_chance:
rv = dn_dev_get_first(dev, addr);
read_unlock(&dev_base_lock);
dev_put(dev);
- if (rv == 0 || dev == &loopback_dev)
+ if (rv == 0 || dev == loopback_dev)
return rv;
}
- dev = &loopback_dev;
+ dev = loopback_dev;
dev_hold(dev);
goto last_chance;
}
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index a4a6209..8c04ebc 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -883,7 +883,7 @@ static int dn_route_output_slow(struct dst_entry **pprt, const struct flowi *old
.scope = RT_SCOPE_UNIVERSE,
} },
.mark = oldflp->mark,
- .iif = loopback_dev.ifindex,
+ .iif = loopback_dev->ifindex,
.oif = oldflp->oif };
struct dn_route *rt = NULL;
struct net_device *dev_out = NULL, *dev;
@@ -900,7 +900,7 @@ static int dn_route_output_slow(struct dst_entry **pprt, const struct flowi *old
"dn_route_output_slow: dst=%04x src=%04x mark=%d"
" iif=%d oif=%d\n", dn_ntohs(oldflp->fld_dst),
dn_ntohs(oldflp->fld_src),
- oldflp->mark, loopback_dev.ifindex, oldflp->oif);
+ oldflp->mark, loopback_dev->ifindex, oldflp->oif);
/* If we have an output interface, verify its a DECnet device */
if (oldflp->oif) {
@@ -953,7 +953,7 @@ source_ok:
err = -EADDRNOTAVAIL;
if (dev_out)
dev_put(dev_out);
- dev_out = &loopback_dev;
+ dev_out = loopback_dev;
dev_hold(dev_out);
if (!fl.fld_dst) {
fl.fld_dst =
@@ -962,7 +962,7 @@ source_ok:
if (!fl.fld_dst)
goto out;
}
- fl.oif = loopback_dev.ifindex;
+ fl.oif = loopback_dev->ifindex;
res.type = RTN_LOCAL;
goto make_route;
}
@@ -1008,7 +1008,7 @@ source_ok:
if (dev_out)
dev_put(dev_out);
if (dn_dev_islocal(neigh->dev, fl.fld_dst)) {
- dev_out = &loopback_dev;
+ dev_out = loopback_dev;
res.type = RTN_LOCAL;
} else {
dev_out = neigh->dev;
@@ -1029,7 +1029,7 @@ source_ok:
/* Possible improvement - check all devices for local addr */
if (dn_dev_islocal(dev_out, fl.fld_dst)) {
dev_put(dev_out);
- dev_out = &loopback_dev;
+ dev_out = loopback_dev;
dev_hold(dev_out);
res.type = RTN_LOCAL;
goto select_source;
@@ -1065,7 +1065,7 @@ select_source:
fl.fld_src = fl.fld_dst;
if (dev_out)
dev_put(dev_out);
- dev_out = &loopback_dev;
+ dev_out = loopback_dev;
dev_hold(dev_out);
fl.oif = dev_out->ifindex;
if (res.fi)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 5b77bda..808f529 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -203,7 +203,7 @@ static void inetdev_destroy(struct in_device *in_dev)
ASSERT_RTNL();
dev = in_dev->dev;
- if (dev == &loopback_dev)
+ if (dev == loopback_dev)
return;
in_dev->dead = 1;
@@ -1058,7 +1058,7 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
in_dev = inetdev_init(dev);
if (!in_dev)
return notifier_from_errno(-ENOMEM);
- if (dev == &loopback_dev) {
+ if (dev == loopback_dev) {
IN_DEV_CONF_SET(in_dev, NOXFRM, 1);
IN_DEV_CONF_SET(in_dev, NOPOLICY, 1);
}
@@ -1074,7 +1074,7 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
case NETDEV_UP:
if (dev->mtu < 68)
break;
- if (dev == &loopback_dev) {
+ if (dev == loopback_dev) {
struct in_ifaddr *ifa;
if ((ifa = inet_alloc_ifa()) != NULL) {
ifa->ifa_local =
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index c5b2470..3ec7690 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -189,11 +189,11 @@ static int __init ic_open_devs(void)
rtnl_lock();
/* bring loopback device up first */
- if (dev_change_flags(&loopback_dev, loopback_dev.flags | IFF_UP) < 0)
- printk(KERN_ERR "IP-Config: Failed to open %s\n", loopback_dev.name);
+ if (dev_change_flags(loopback_dev, loopback_dev->flags | IFF_UP) < 0)
+ printk(KERN_ERR "IP-Config: Failed to open %s\n", loopback_dev->name);
for_each_netdev(dev) {
- if (dev == &loopback_dev)
+ if (dev == loopback_dev)
continue;
if (user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
(!(dev->flags & IFF_LOOPBACK) &&
diff --git a/net/ipv4/ipvs/ip_vs_core.c b/net/ipv4/ipvs/ip_vs_core.c
index f005a2f..7450326 100644
--- a/net/ipv4/ipvs/ip_vs_core.c
+++ b/net/ipv4/ipvs/ip_vs_core.c
@@ -961,7 +961,7 @@ ip_vs_in(unsigned int hooknum, struct sk_buff **pskb,
* ... don't know why 1st test DOES NOT include 2nd (?)
*/
if (unlikely(skb->pkt_type != PACKET_HOST
- || skb->dev == &loopback_dev || skb->sk)) {
+ || skb->dev == loopback_dev || skb->sk)) {
IP_VS_DBG(12, "packet type=%d proto=%d daddr=%d.%d.%d.%d ignored\n",
skb->pkt_type,
ip_hdr(skb)->protocol,
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index c7ca94b..4f13385 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1404,8 +1404,8 @@ static void ipv4_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
{
struct rtable *rt = (struct rtable *) dst;
struct in_device *idev = rt->idev;
- if (dev != &loopback_dev && idev && idev->dev == dev) {
- struct in_device *loopback_idev = in_dev_get(&loopback_dev);
+ if (dev != loopback_dev && idev && idev->dev == dev) {
+ struct in_device *loopback_idev = in_dev_get(loopback_dev);
if (loopback_idev) {
rt->idev = loopback_idev;
in_dev_put(idev);
@@ -1557,7 +1557,7 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
#endif
rth->rt_iif =
rth->fl.iif = dev->ifindex;
- rth->u.dst.dev = &loopback_dev;
+ rth->u.dst.dev = loopback_dev;
dev_hold(rth->u.dst.dev);
rth->idev = in_dev_get(rth->u.dst.dev);
rth->fl.oif = 0;
@@ -1814,7 +1814,7 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
if (res.type == RTN_LOCAL) {
int result;
result = fib_validate_source(saddr, daddr, tos,
- loopback_dev.ifindex,
+ loopback_dev->ifindex,
dev, &spec_dst, &itag);
if (result < 0)
goto martian_source;
@@ -1881,7 +1881,7 @@ local_input:
#endif
rth->rt_iif =
rth->fl.iif = dev->ifindex;
- rth->u.dst.dev = &loopback_dev;
+ rth->u.dst.dev = loopback_dev;
dev_hold(rth->u.dst.dev);
rth->idev = in_dev_get(rth->u.dst.dev);
rth->rt_gateway = daddr;
@@ -2151,7 +2151,7 @@ static int ip_route_output_slow(struct rtable **rp, const struct flowi *oldflp)
RT_SCOPE_UNIVERSE),
} },
.mark = oldflp->mark,
- .iif = loopback_dev.ifindex,
+ .iif = loopback_dev->ifindex,
.oif = oldflp->oif };
struct fib_result res;
unsigned flags = 0;
@@ -2245,9 +2245,9 @@ static int ip_route_output_slow(struct rtable **rp, const struct flowi *oldflp)
fl.fl4_dst = fl.fl4_src = htonl(INADDR_LOOPBACK);
if (dev_out)
dev_put(dev_out);
- dev_out = &loopback_dev;
+ dev_out = loopback_dev;
dev_hold(dev_out);
- fl.oif = loopback_dev.ifindex;
+ fl.oif = loopback_dev->ifindex;
res.type = RTN_LOCAL;
flags |= RTCF_LOCAL;
goto make_route;
@@ -2292,7 +2292,7 @@ static int ip_route_output_slow(struct rtable **rp, const struct flowi *oldflp)
fl.fl4_src = fl.fl4_dst;
if (dev_out)
dev_put(dev_out);
- dev_out = &loopback_dev;
+ dev_out = loopback_dev;
dev_hold(dev_out);
fl.oif = dev_out->ifindex;
if (res.fi)
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 4ff8ed3..29ab3de 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -306,7 +306,7 @@ static void xfrm4_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
xdst = (struct xfrm_dst *)dst;
if (xdst->u.rt.idev->dev == dev) {
- struct in_device *loopback_idev = in_dev_get(&loopback_dev);
+ struct in_device *loopback_idev = in_dev_get(loopback_dev);
BUG_ON(!loopback_idev);
do {
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 91ef3be..d806f89 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2399,7 +2399,7 @@ static int addrconf_ifdown(struct net_device *dev, int how)
ASSERT_RTNL();
- if (dev == &loopback_dev && how == 1)
+ if (dev == loopback_dev && how == 1)
how = 0;
rt6_ifdown(dev);
@@ -4203,16 +4203,19 @@ int __init addrconf_init(void)
* device and it being up should be removed.
*/
rtnl_lock();
- if (!ipv6_add_dev(&loopback_dev))
+ if (!ipv6_add_dev(loopback_dev))
err = -ENOMEM;
rtnl_unlock();
if (err)
return err;
- ip6_null_entry.rt6i_idev = in6_dev_get(&loopback_dev);
+ ip6_null_entry.u.dst.dev = loopback_dev;
+ ip6_null_entry.rt6i_idev = in6_dev_get(loopback_dev);
#ifdef CONFIG_IPV6_MULTIPLE_TABLES
- ip6_prohibit_entry.rt6i_idev = in6_dev_get(&loopback_dev);
- ip6_blk_hole_entry.rt6i_idev = in6_dev_get(&loopback_dev);
+ ip6_prohibit_entry.u.dst.dev = loopback_dev;
+ ip6_prohibit_entry.rt6i_idev = in6_dev_get(loopback_dev);
+ ip6_blk_hole_entry.u.dst.dev = loopback_dev;
+ ip6_blk_hole_entry.rt6i_idev = in6_dev_get(loopback_dev);
#endif
register_netdevice_notifier(&ipv6_dev_notf);
@@ -4267,7 +4270,7 @@ void __exit addrconf_cleanup(void)
continue;
addrconf_ifdown(dev, 1);
}
- addrconf_ifdown(&loopback_dev, 2);
+ addrconf_ifdown(loopback_dev, 2);
/*
* Check hash table.
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 30a5cb1..15d7910 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -86,7 +86,7 @@ int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt
*
* BTW, when we send a packet for our own local address on a
* non-loopback interface (e.g. ethX), it is being delivered
- * via the loopback interface (lo) here; skb->dev = &loopback_dev.
+ * via the loopback interface (lo) here; skb->dev = loopback_dev.
* It, however, should be considered as if it is being
* arrived via the sending interface (ethX), because of the
* nature of scoping architecture. --yoshfuji
diff --git a/net/ipv6/netfilter/ip6t_REJECT.c b/net/ipv6/netfilter/ip6t_REJECT.c
index 2f487cd..5086053 100644
--- a/net/ipv6/netfilter/ip6t_REJECT.c
+++ b/net/ipv6/netfilter/ip6t_REJECT.c
@@ -167,7 +167,7 @@ static inline void
send_unreach(struct sk_buff *skb_in, unsigned char code, unsigned int hooknum)
{
if (hooknum == NF_IP6_LOCAL_OUT && skb_in->dev == NULL)
- skb_in->dev = &loopback_dev;
+ skb_in->dev = loopback_dev;
icmpv6_send(skb_in, ICMPV6_DEST_UNREACH, code, 0, NULL);
}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 55ea80f..3f5c65f 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -137,7 +137,6 @@ struct rt6_info ip6_null_entry = {
.dst = {
.__refcnt = ATOMIC_INIT(1),
.__use = 1,
- .dev = &loopback_dev,
.obsolete = -1,
.error = -ENETUNREACH,
.metrics = { [RTAX_HOPLIMIT - 1] = 255, },
@@ -163,7 +162,6 @@ struct rt6_info ip6_prohibit_entry = {
.dst = {
.__refcnt = ATOMIC_INIT(1),
.__use = 1,
- .dev = &loopback_dev,
.obsolete = -1,
.error = -EACCES,
.metrics = { [RTAX_HOPLIMIT - 1] = 255, },
@@ -183,7 +181,6 @@ struct rt6_info ip6_blk_hole_entry = {
.dst = {
.__refcnt = ATOMIC_INIT(1),
.__use = 1,
- .dev = &loopback_dev,
.obsolete = -1,
.error = -EINVAL,
.metrics = { [RTAX_HOPLIMIT - 1] = 255, },
@@ -223,8 +220,8 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
struct rt6_info *rt = (struct rt6_info *)dst;
struct inet6_dev *idev = rt->rt6i_idev;
- if (dev != &loopback_dev && idev != NULL && idev->dev == dev) {
- struct inet6_dev *loopback_idev = in6_dev_get(&loopback_dev);
+ if (dev != loopback_dev && idev != NULL && idev->dev == dev) {
+ struct inet6_dev *loopback_idev = in6_dev_get(loopback_dev);
if (loopback_idev != NULL) {
rt->rt6i_idev = loopback_idev;
in6_dev_put(idev);
@@ -1187,12 +1184,12 @@ int ip6_route_add(struct fib6_config *cfg)
if ((cfg->fc_flags & RTF_REJECT) ||
(dev && (dev->flags&IFF_LOOPBACK) && !(addr_type&IPV6_ADDR_LOOPBACK))) {
/* hold loopback dev/idev if we haven't done so. */
- if (dev != &loopback_dev) {
+ if (dev != loopback_dev) {
if (dev) {
dev_put(dev);
in6_dev_put(idev);
}
- dev = &loopback_dev;
+ dev = loopback_dev;
dev_hold(dev);
idev = in6_dev_get(dev);
if (!idev) {
@@ -1896,13 +1893,13 @@ struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev,
if (rt == NULL)
return ERR_PTR(-ENOMEM);
- dev_hold(&loopback_dev);
+ dev_hold(loopback_dev);
in6_dev_hold(idev);
rt->u.dst.flags = DST_HOST;
rt->u.dst.input = ip6_input;
rt->u.dst.output = ip6_output;
- rt->rt6i_dev = &loopback_dev;
+ rt->rt6i_dev = loopback_dev;
rt->rt6i_idev = idev;
rt->u.dst.metrics[RTAX_MTU-1] = ipv6_get_mtu(rt->rt6i_dev);
rt->u.dst.metrics[RTAX_ADVMSS-1] = ipv6_advmss(dst_mtu(&rt->u.dst));
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 3ec0c47..cc07216 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -375,7 +375,7 @@ static void xfrm6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
xdst = (struct xfrm_dst *)dst;
if (xdst->u.rt6.rt6i_idev->dev == dev) {
- struct inet6_dev *loopback_idev = in6_dev_get(&loopback_dev);
+ struct inet6_dev *loopback_idev = in6_dev_get(loopback_dev);
BUG_ON(!loopback_idev);
do {
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index c7b2503..ca37eec 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1947,8 +1947,8 @@ static int stale_bundle(struct dst_entry *dst)
void xfrm_dst_ifdown(struct dst_entry *dst, struct net_device *dev)
{
while ((dst = dst->child) && dst->xfrm && dst->dev == dev) {
- dst->dev = &loopback_dev;
- dev_hold(&loopback_dev);
+ dst->dev = loopback_dev;
+ dev_hold(dst->dev);
dev_put(dev);
}
}
--
1.5.2.4
^ permalink raw reply related
* Re: RFC: issues concerning the next NAPI interface
From: Jan-Bernd Themann @ 2007-08-24 15:47 UTC (permalink / raw)
To: akepner
Cc: netdev, Christoph Raisch, Jan-Bernd Themann, linux-kernel,
linux-ppc, Marcus Eder, Thomas Klein, Stefan Roscher
In-Reply-To: <20070824153703.GN5592@sgi.com>
Hi,
On Friday 24 August 2007 17:37, akepner@sgi.com wrote:
> On Fri, Aug 24, 2007 at 03:59:16PM +0200, Jan-Bernd Themann wrote:
> > .......
> > 3) On modern systems the incoming packets are processed very fast. Especially
> > on SMP systems when we use multiple queues we process only a few packets
> > per napi poll cycle. So NAPI does not work very well here and the interrupt
> > rate is still high. What we need would be some sort of timer polling mode
> > which will schedule a device after a certain amount of time for high load
> > situations. With high precision timers this could work well. Current
> > usual timers are too slow. A finer granularity would be needed to keep the
> > latency down (and queue length moderate).
> >
>
> We found the same on ia64-sn systems with tg3 a couple of years
> ago. Using simple interrupt coalescing ("don't interrupt until
> you've received N packets or M usecs have elapsed") worked
> reasonably well in practice. If your h/w supports that (and I'd
> guess it does, since it's such a simple thing), you might try
> it.
>
I don't see how this should work. Our latest machines are fast enough that they
simply empty the queue during the first poll iteration (in most cases).
Even if you wait until X packets have been received, it does not help for
the next poll cycle. The average number of packets we process per poll queue
is low. So a timer would be preferable that periodically polls the
queue, without the need of generating a HW interrupt. This would allow us
to wait until a reasonable amount of packets have been received in the meantime
to keep the poll overhead low. This would also be useful in combination
with LRO.
Regards,
Jan-Bernd
^ permalink raw reply
* Re: [PATCH 14/30] net: Kill some unneeded allocation return value casts in libertas
From: Dan Williams @ 2007-08-24 15:50 UTC (permalink / raw)
To: Jesper Juhl
Cc: Linux Kernel Mailing List, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2ac91f5f0eed967cf1709cfbe476b351e536c299.1187912217.git.jesper.juhl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Fri, 2007-08-24 at 02:03 +0200, Jesper Juhl wrote:
> kmalloc() and friends return void*, no need to cast it.
Applied to libertas-2.6 'for-linville' branch, thanks.
Dan
> Signed-off-by: Jesper Juhl <jesper.juhl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/net/wireless/libertas/debugfs.c | 2 +-
> drivers/net/wireless/libertas/ethtool.c | 3 +--
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/libertas/debugfs.c b/drivers/net/wireless/libertas/debugfs.c
> index 715cbda..6ade63e 100644
> --- a/drivers/net/wireless/libertas/debugfs.c
> +++ b/drivers/net/wireless/libertas/debugfs.c
> @@ -1839,7 +1839,7 @@ static ssize_t wlan_debugfs_write(struct file *f, const char __user *buf,
> char *p2;
> struct debug_data *d = (struct debug_data *)f->private_data;
>
> - pdata = (char *)kmalloc(cnt, GFP_KERNEL);
> + pdata = kmalloc(cnt, GFP_KERNEL);
> if (pdata == NULL)
> return 0;
>
> diff --git a/drivers/net/wireless/libertas/ethtool.c b/drivers/net/wireless/libertas/ethtool.c
> index 96f1974..7dad493 100644
> --- a/drivers/net/wireless/libertas/ethtool.c
> +++ b/drivers/net/wireless/libertas/ethtool.c
> @@ -60,8 +60,7 @@ static int libertas_ethtool_get_eeprom(struct net_device *dev,
>
> // mutex_lock(&priv->mutex);
>
> - adapter->prdeeprom =
> - (char *)kmalloc(eeprom->len+sizeof(regctrl), GFP_KERNEL);
> + adapter->prdeeprom = kmalloc(eeprom->len+sizeof(regctrl), GFP_KERNEL);
> if (!adapter->prdeeprom)
> return -ENOMEM;
> memcpy(adapter->prdeeprom, ®ctrl, sizeof(regctrl));
^ permalink raw reply
* Re: RFC: issues concerning the next NAPI interface
From: Stephen Hemminger @ 2007-08-24 15:52 UTC (permalink / raw)
To: Jan-Bernd Themann
Cc: akepner, netdev, Christoph Raisch, Jan-Bernd Themann,
linux-kernel, linux-ppc, Marcus Eder, Thomas Klein,
Stefan Roscher
In-Reply-To: <200708241747.16592.ossthema@de.ibm.com>
On Fri, 24 Aug 2007 17:47:15 +0200
Jan-Bernd Themann <ossthema@de.ibm.com> wrote:
> Hi,
>
> On Friday 24 August 2007 17:37, akepner@sgi.com wrote:
> > On Fri, Aug 24, 2007 at 03:59:16PM +0200, Jan-Bernd Themann wrote:
> > > .......
> > > 3) On modern systems the incoming packets are processed very fast. Especially
> > > on SMP systems when we use multiple queues we process only a few packets
> > > per napi poll cycle. So NAPI does not work very well here and the interrupt
> > > rate is still high. What we need would be some sort of timer polling mode
> > > which will schedule a device after a certain amount of time for high load
> > > situations. With high precision timers this could work well. Current
> > > usual timers are too slow. A finer granularity would be needed to keep the
> > > latency down (and queue length moderate).
> > >
> >
> > We found the same on ia64-sn systems with tg3 a couple of years
> > ago. Using simple interrupt coalescing ("don't interrupt until
> > you've received N packets or M usecs have elapsed") worked
> > reasonably well in practice. If your h/w supports that (and I'd
> > guess it does, since it's such a simple thing), you might try
> > it.
> >
>
> I don't see how this should work. Our latest machines are fast enough that they
> simply empty the queue during the first poll iteration (in most cases).
> Even if you wait until X packets have been received, it does not help for
> the next poll cycle. The average number of packets we process per poll queue
> is low. So a timer would be preferable that periodically polls the
> queue, without the need of generating a HW interrupt. This would allow us
> to wait until a reasonable amount of packets have been received in the meantime
> to keep the poll overhead low. This would also be useful in combination
> with LRO.
>
You need hardware support for deferred interrupts. Most devices have it (e1000, sky2, tg3)
and it interacts well with NAPI. It is not a generic thing you want done by the stack,
you want the hardware to hold off interrupts until X packets or Y usecs have expired.
The parameters for controlling it are already in ethtool, the issue is finding a good
default set of values for a wide range of applications and architectures. Maybe some
heuristic based on processor speed would be a good starting point. The dynamic irq
moderation stuff is not widely used because it is too hard to get right.
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: [Devel] [PATCH 1/1] Dynamically allocate the loopback device
From: Denis V. Lunev @ 2007-08-24 15:55 UTC (permalink / raw)
To: dlezcano; +Cc: davem, shemminger, containers, netdev
In-Reply-To: <11879698031215-git-send-email-dlezcano@fr.ibm.com>
dlezcano@fr.ibm.com wrote:
> From: Daniel Lezcano <dlezcano@fr.ibm.com>
>
> Doing this makes loopback.c a better example of how to do a
> simple network device, and it removes the special case
> single static allocation of a struct net_device, hopefully
> making maintenance easier.
>
> Applies against net-2.6.24
>
> Tested on i386, x86_64
> Compiled on ia64, sparc
I think that a small note, that initialization order is changed will be
good to record. After this, loopback MUST be allocated before any other
networking subsystem initialization. And this is an important change.
Regards,
Den
^ permalink raw reply
* Re: Problem with implementation of TCP_DEFER_ACCEPT?
From: John Heffner @ 2007-08-24 16:09 UTC (permalink / raw)
To: TJ; +Cc: netdev
In-Reply-To: <1187947887.6976.18.camel@bagoas.tjworld.net>
TJ wrote:
> Right now Juniper are claiming the issue that brought this to the
> surface (the bug linked to in my original post) is a problem with the
> implementation of TCP_DEFER_ACCEPT.
>
> My position so far is that the Juniper DX OS is not following the HTTP
> standard because it doesn't send a request with the connection, and as I
> read the end of section 1.4 of RFC2616, an HTTP connection should be
> accompanied by a request.
>
> Can anyone confirm my interpretation or provide references to firm it
> up, or refute it?
You can think of TCP_DEFER_ACCEPT as an implicit application close()
after a certain timeout, when not receiving a request. All HTTP servers
do this anyway (though I think technically they're supposed to send a
408 Request Timeout error it seems many do not). It's a very valid
question for Juniper as to why their box is failing to fill requests
when its back-end connection has gone away, instead of re-establishing
the connection and filling the request.
-John
^ permalink raw reply
* RE: [PATCH] i386: Fix a couple busy loops in mach_wakecpu.h:wait_for_init_deassert()
From: Luck, Tony @ 2007-08-24 16:19 UTC (permalink / raw)
To: Denys Vlasenko, Satyam Sharma
Cc: Heiko Carstens, Herbert Xu, Chris Snook, clameter,
Linux Kernel Mailing List, linux-arch, Linus Torvalds, netdev,
Andrew Morton, ak, davem, schwidefsky, wensong, horms, wjiang,
cfriesen, zlynx, rpjday, jesper.juhl, segher
In-Reply-To: <200708241259.33659.vda.linux@googlemail.com>
>> static inline void wait_for_init_deassert(atomic_t *deassert)
>> {
>> - while (!atomic_read(deassert));
>> + while (!atomic_read(deassert))
>> + cpu_relax();
>> return;
>> }
>
> For less-than-briliant people like me, it's totally non-obvious that
> cpu_relax() is needed for correctness here, not just to make P4 happy.
Not just P4 ... there are other threaded cpus where it is useful to
let the core know that this is a busy loop so it would be a good thing
to let other threads have priority.
Even on a non-threaded cpu the cpu_relax() could be useful in the
future to hint to the cpu that it could drop into a lower power
hogging state.
But I agree with your main point that the loop without the cpu_relax()
looks like it ought to work because atomic_read() ought to actually
go out and read memory each time around the loop.
-Tony
^ permalink raw reply
* Re: [PATCH 0/9 Rev3] Implement batching skb API and support in IPoIB
From: Rick Jones @ 2007-08-24 16:25 UTC (permalink / raw)
To: Stephen Hemminger
Cc: hadi, David Miller, krkumar2, gaagaan, general, herbert, jagana,
jeff, johnpol, kaber, mcarlson, mchan, netdev,
peter.p.waskiewicz.jr, rdreier, Robert.Olsson, sri, tgraf, xma
In-Reply-To: <20070823203425.2a32fb3f@freepuppy.rosehill.hemminger.net>
>
> A current hot topic of research is reducing the number of ACK's to make TCP
> work better over asymmetric links like 3G.
Oy. People running Solaris and HP-UX have been "researching" ACK reductions
since 1997 if not earlier.
rick jones
^ permalink raw reply
* Re: RFC: issues concerning the next NAPI interface
From: Linas Vepstas @ 2007-08-24 16:45 UTC (permalink / raw)
To: Jan-Bernd Themann
Cc: netdev, Thomas Klein, Jan-Bernd Themann, linux-kernel, linux-ppc,
Christoph Raisch, Marcus Eder, Stefan Roscher
In-Reply-To: <200708241559.17055.ossthema@de.ibm.com>
On Fri, Aug 24, 2007 at 03:59:16PM +0200, Jan-Bernd Themann wrote:
> 3) On modern systems the incoming packets are processed very fast. Especially
> on SMP systems when we use multiple queues we process only a few packets
> per napi poll cycle. So NAPI does not work very well here and the interrupt
> rate is still high.
I saw this too, on a system that is "modern" but not terribly fast, and
only slightly (2-way) smp. (the spidernet)
I experimented wih various solutions, none were terribly exciting. The
thing that killed all of them was a crazy test case that someone sprung on
me: They had written a worst-case network ping-pong app: send one
packet, wait for reply, send one packet, etc.
If I waited (indefinitely) for a second packet to show up, the test case
completely stalled (since no second packet would ever arrive). And if I
introduced a timer to wait for a second packet, then I just increased
the latency in the response to the first packet, and this was noticed,
and folks complained.
In the end, I just let it be, and let the system work as a busy-beaver,
with the high interrupt rate. Is this a wise thing to do? I was
thinking that, if the system is under heavy load, then the interrupt
rate would fall, since (for less pathological network loads) more
packets would queue up before the poll was serviced. But I did not
actually measure the interrupt rate under heavy load ...
--linas
^ permalink raw reply
* Re: [Devel] [PATCH 1/1] Dynamically allocate the loopback device
From: Daniel Lezcano @ 2007-08-24 16:44 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: dlezcano, davem, shemminger, containers, netdev
In-Reply-To: <46CEFF83.3040003@gmail.com>
Denis V. Lunev wrote:
> dlezcano@fr.ibm.com wrote:
>> From: Daniel Lezcano <dlezcano@fr.ibm.com>
>>
>> Doing this makes loopback.c a better example of how to do a
>> simple network device, and it removes the special case
>> single static allocation of a struct net_device, hopefully
>> making maintenance easier.
>>
>> Applies against net-2.6.24
>>
>> Tested on i386, x86_64
>> Compiled on ia64, sparc
>
> I think that a small note, that initialization order is changed will be
> good to record. After this, loopback MUST be allocated before any other
> networking subsystem initialization. And this is an important change.
>
> Regards,
> Den
>
Thanks Denis to point that.
-- Daniel
^ permalink raw reply
* Re: RFC: issues concerning the next NAPI interface
From: David Stevens @ 2007-08-24 16:50 UTC (permalink / raw)
To: Stephen Hemminger
Cc: akepner, linux-kernel, linux-ppc, Marcus Eder, netdev,
netdev-owner, Jan-Bernd Themann, Christoph Raisch, Stefan Roscher,
Jan-Bernd Themann, Thomas Klein
In-Reply-To: <20070824085203.42f4305c@freepuppy.rosehill.hemminger.net>
Stephen Hemminger <shemminger@linux-foundation.org> wrote on 08/24/2007
08:52:03 AM:
>
> You need hardware support for deferred interrupts. Most devices have it
> (e1000, sky2, tg3)
> and it interacts well with NAPI. It is not a generic thing you want done
by the stack,
> you want the hardware to hold off interrupts until X packets or Y usecs
have expired.
For generic hardware that doesn't support it, couldn't you use an
estimater
and adjust the timer dynamicly in software based on sampled values? Switch
to per-packet
interrupts when the receive rate is low...
Actually, that's how I thought NAPI worked before I found out
otherwise (ie,
before I looked :-)).
The hardware-accelerated one is essentially siloing as done by
ancient serial
devices on UNIX systems. If you had a tunable for a target count, and an
estimator
for the time interval, then switch to per-packet when the estimator
exceeds a tunable
max threshold (and also, I suppose, if you near overflowing the ring on
the min
timer granularity), you get almost all of it, right?
Problem is if it increases rapidly, you may drop packets before
you notice
that the ring is full in the current estimated interval.
+-DLS
^ permalink raw reply
* Re: RFC: issues concerning the next NAPI interface
From: Linas Vepstas @ 2007-08-24 16:51 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Jan-Bernd Themann, Thomas Klein, Marcus, Jan-Bernd Themann,
netdev, linux-kernel, Christoph Raisch, linux-ppc, akepner, Eder,
Stefan Roscher
In-Reply-To: <20070824085203.42f4305c@freepuppy.rosehill.hemminger.net>
On Fri, Aug 24, 2007 at 08:52:03AM -0700, Stephen Hemminger wrote:
>
> You need hardware support for deferred interrupts. Most devices have it (e1000, sky2, tg3)
> and it interacts well with NAPI. It is not a generic thing you want done by the stack,
> you want the hardware to hold off interrupts until X packets or Y usecs have expired.
Just to be clear, in the previous email I posted on this thread, I
described a worst-case network ping-pong test case (send a packet, wait
for reply), and found out that a deffered interrupt scheme just damaged
the performance of the test case. Since the folks who came up with the
test case were adamant, I turned off the defferred interrupts.
While defferred interrupts are an "obvious" solution, I decided that
they weren't a good solution. (And I have no other solution to offer).
--linas
^ permalink raw reply
* Re: [PATCH] i386: Fix a couple busy loops in mach_wakecpu.h:wait_for_init_deassert()
From: Christoph Lameter @ 2007-08-24 17:06 UTC (permalink / raw)
To: Satyam Sharma
Cc: Denys Vlasenko, Heiko Carstens, Herbert Xu, Chris Snook,
Linux Kernel Mailing List, linux-arch, Linus Torvalds, netdev,
Andrew Morton, ak, davem, schwidefsky, wensong, horms, wjiang,
cfriesen, zlynx, rpjday, jesper.juhl, segher, Nick Piggin
In-Reply-To: <alpine.LFD.0.999.0708241812110.20322@enigma.security.iitk.ac.in>
On Fri, 24 Aug 2007, Satyam Sharma wrote:
> But if people do seem to have a mixed / confused notion of atomicity
> and barriers, and if there's consensus, then as I'd said earlier, I
> have no issues in going with the consensus (eg. having API variants).
> Linus would be more difficult to convince, however, I suspect :-)
The confusion may be the result of us having barrier semantics in
atomic_read. If we take that out then we may avoid future confusions.
^ permalink raw reply
* Re: RFC: issues concerning the next NAPI interface
From: Rick Jones @ 2007-08-24 17:07 UTC (permalink / raw)
To: Linas Vepstas
Cc: Stephen Hemminger, Jan-Bernd Themann, Thomas Klein, Marcus,
Jan-Bernd Themann, netdev, linux-kernel, Christoph Raisch,
linux-ppc, akepner, Eder, Stefan Roscher
In-Reply-To: <20070824165110.GH4282@austin.ibm.com>
> Just to be clear, in the previous email I posted on this thread, I
> described a worst-case network ping-pong test case (send a packet, wait
> for reply), and found out that a deffered interrupt scheme just damaged
> the performance of the test case. Since the folks who came up with the
> test case were adamant, I turned off the defferred interrupts.
> While defferred interrupts are an "obvious" solution, I decided that
> they weren't a good solution. (And I have no other solution to offer).
Sounds exactly like the default netperf TCP_RR test and any number of other
benchmarks. The "send a request, wait for reply, send next request, etc etc
etc" is a rather common application behaviour afterall.
rick jones
^ permalink raw reply
* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Christoph Lameter @ 2007-08-24 17:15 UTC (permalink / raw)
To: Denys Vlasenko
Cc: Paul Mackerras, Satyam Sharma, Stefan Richter, 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,
Herbert Xu, Paul E. McKenney
In-Reply-To: <200708241350.45061.vda.linux@googlemail.com>
On Fri, 24 Aug 2007, Denys Vlasenko wrote:
> On Thursday 16 August 2007 00:22, Paul Mackerras wrote:
> > Satyam Sharma writes:
> > In the kernel we use atomic variables in precisely those situations
> > where a variable is potentially accessed concurrently by multiple
> > CPUs, and where each CPU needs to see updates done by other CPUs in a
> > timely fashion. That is what they are for. Therefore the compiler
> > must not cache values of atomic variables in registers; each
> > atomic_read must result in a load and each atomic_set must result in a
> > store. Anything else will just lead to subtle bugs.
>
> Amen.
A "timely" fashion? One cannot rely on something like that when coding.
The visibility of updates is insured by barriers and not by some fuzzy
notion of "timeliness".
^ permalink raw reply
* Re: RFC: issues concerning the next NAPI interface
From: James Chapman @ 2007-08-24 17:16 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Jan-Bernd Themann, akepner, netdev, Christoph Raisch,
Jan-Bernd Themann, linux-kernel, linux-ppc, Marcus Eder,
Thomas Klein, Stefan Roscher
In-Reply-To: <20070824085203.42f4305c@freepuppy.rosehill.hemminger.net>
Stephen Hemminger wrote:
> On Fri, 24 Aug 2007 17:47:15 +0200
> Jan-Bernd Themann <ossthema@de.ibm.com> wrote:
>
>> Hi,
>>
>> On Friday 24 August 2007 17:37, akepner@sgi.com wrote:
>>> On Fri, Aug 24, 2007 at 03:59:16PM +0200, Jan-Bernd Themann wrote:
>>>> .......
>>>> 3) On modern systems the incoming packets are processed very fast. Especially
>>>> on SMP systems when we use multiple queues we process only a few packets
>>>> per napi poll cycle. So NAPI does not work very well here and the interrupt
>>>> rate is still high. What we need would be some sort of timer polling mode
>>>> which will schedule a device after a certain amount of time for high load
>>>> situations. With high precision timers this could work well. Current
>>>> usual timers are too slow. A finer granularity would be needed to keep the
>>>> latency down (and queue length moderate).
>>>>
>>> We found the same on ia64-sn systems with tg3 a couple of years
>>> ago. Using simple interrupt coalescing ("don't interrupt until
>>> you've received N packets or M usecs have elapsed") worked
>>> reasonably well in practice. If your h/w supports that (and I'd
>>> guess it does, since it's such a simple thing), you might try
>>> it.
>>>
>> I don't see how this should work. Our latest machines are fast enough that they
>> simply empty the queue during the first poll iteration (in most cases).
>> Even if you wait until X packets have been received, it does not help for
>> the next poll cycle. The average number of packets we process per poll queue
>> is low. So a timer would be preferable that periodically polls the
>> queue, without the need of generating a HW interrupt. This would allow us
>> to wait until a reasonable amount of packets have been received in the meantime
>> to keep the poll overhead low. This would also be useful in combination
>> with LRO.
>>
>
> You need hardware support for deferred interrupts. Most devices have it (e1000, sky2, tg3)
> and it interacts well with NAPI. It is not a generic thing you want done by the stack,
> you want the hardware to hold off interrupts until X packets or Y usecs have expired.
Does hardware interrupt mitigation really interact well with NAPI? In my
experience, holding off interrupts for X packets or Y usecs does more
harm than good; such hardware features are useful only when the OS has
no NAPI-like mechanism.
When tuning NAPI drivers for packets/sec performance (which is a good
indicator of driver performance), I make sure that the driver stays in
NAPI polled mode while it has any rx or tx work to do. If the CPU is
fast enough that all work is always completed on each poll, I have the
driver stay in polled mode until dev->poll() is called N times with no
work being done. This keeps interrupts disabled for reasonable traffic
levels, while minimizing packet processing latency. No need for hardware
interrupt mitigation.
> The parameters for controlling it are already in ethtool, the issue is finding a good
> default set of values for a wide range of applications and architectures. Maybe some
> heuristic based on processor speed would be a good starting point. The dynamic irq
> moderation stuff is not widely used because it is too hard to get right.
I agree. It would be nice to find a way for the typical user to derive
best values for these knobs for his/her particular system. Perhaps a
tool using pktgen and network device phy internal loopback could be
developed?
--
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development
^ permalink raw reply
* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Linus Torvalds @ 2007-08-24 17:19 UTC (permalink / raw)
To: Denys Vlasenko
Cc: Satyam Sharma, Christoph Lameter, Paul E. McKenney, Herbert Xu,
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: <200708241319.05760.vda.linux@googlemail.com>
On Fri, 24 Aug 2007, Denys Vlasenko wrote:
>
> > No, you don't use "x.counter++". But you *do* use
> >
> > if (atomic_read(&x) <= 1)
> >
> > and loading into a register is stupid and pointless, when you could just
> > do it as a regular memory-operand to the cmp instruction.
>
> It doesn't mean that (volatile int*) cast is bad, it means that current gcc
> is bad (or "not good enough"). IOW: instead of avoiding volatile cast,
> it's better to fix the compiler.
I would agree that fixing the compiler in this case would be a good thing,
even quite regardless of any "atomic_read()" discussion.
I just have a strong suspicion that "volatile" performance is so low down
the list of any C compiler persons interest, that it's never going to
happen. And quite frankly, I cannot blame the gcc guys for it.
That's especially as "volatile" really isn't a very good feature of the C
language, and is likely to get *less* interesting rather than more (as
user space starts to be more and more threaded, "volatile" gets less and
less useful.
[ Ie, currently, I think you can validly use "volatile" in a "sigatomic_t"
kind of way, where there is a single thread, but with asynchronous
events. In that kind of situation, I think it's probably useful. But
once you get multiple threads, it gets pointless.
Sure: you could use "volatile" together with something like Dekker's or
Peterson's algorithm that doesn't depend on cache coherency (that's
basically what the C "volatile" keyword approximates: not atomic
accesses, but *uncached* accesses! But let's face it, that's way past
insane. ]
So I wouldn't expect "volatile" to ever really generate better code. It
might happen as a side effect of other improvements (eg, I might hope that
the SSA work would eventually lead to gcc having a much better defined
model of valid optimizations, and maybe better code generation for
volatile accesses fall out cleanly out of that), but in the end, it's such
an ugly special case in C, and so seldom used, that I wouldn't depend on
it.
> Linus, in all honesty gcc has many more cases of suboptimal code,
> case of "volatile" is just one of many.
Well, the thing is, quite often, many of those "suboptimal code"
generations fall into two distinct classes:
- complex C code. I can't really blame the compiler too much for this.
Some things are *hard* to optimize, and for various scalability
reasons, you often end up having limits in the compiler where it
doesn't even _try_ doing certain optimizations if you have excessive
complexity.
- bad register allocation. Register allocation really is hard, and
sometimes gcc just does the "obviously wrong" thing, and you end up
having totally unnecessary spills.
> Off the top of my head:
Yes, "unsigned long long" with x86 has always generated atrocious code. In
fact, I would say that historically it was really *really* bad. These
days, gcc actually does a pretty good job, but I'm not surprised that it's
still quite possible to find cases where it did some optimization (in this
case, apparently noticing that "shift by >= 32 bits" causes the low
register to be pointless) and then missed *another* optimization (better
register use) because that optimization had been done *before* the first
optimization was done.
That's a *classic* example of compiler code generation issues, and quite
frankly, I think that's very different from the issue of "volatile".
Quite frankly, I'd like there to be more competition in the open source
compiler game, and that might cause some upheavals, but on the whole, gcc
actually does a pretty damn good job.
Linus
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox