* Re: [PATCH] e1000: Dump the eeprom when a user encounters a bad checksum
From: Kok, Auke @ 2007-12-15 20:35 UTC (permalink / raw)
To: Joe Perches
Cc: Auke Kok, jeff, netdev, davem, john.ronciak, jesse.brandeburg,
Randy Dunlap
In-Reply-To: <1197682306.4861.72.camel@localhost>
Joe Perches wrote:
> On Fri, 2007-12-14 at 15:35 -0800, Auke Kok wrote:
>> + printk(KERN_ERR "/*********************/\n");
>> + printk(KERN_ERR "Current EEPROM: 0x%04x\nCalculated : 0x%04x\n",
>> + csum_old, csum_new);
>
> Multiline printks need a KERN_<level> after every newline. Perhaps:
>
> printk(KERN_ERR "Current EEPROM: 0x%04x\n"
> KERN_ERR "Calculated : 0x%04x\n",
> csum_old, csum_new);
>
>> + printk(KERN_ERR "Offset Values\n");
>> + printk(KERN_ERR "====== ======\n");
>> + for (i = 0; i < eeprom.len; i += 16)
>> + printk(KERN_ERR "0x%04x "
>> + "%02x %02x %02x %02x %02x %02x %02x %02x "
>> + "%02x %02x %02x %02x %02x %02x %02x %02x\n",
>> + i, data[i], data[i + 1], data[i + 2], data[i + 3],
>> + data[i + 4], data[i + 5], data[i + 6], data[i + 7],
>> + data[i + 8], data[i + 9], data[i + 10], data[i + 11],
>> + data[i + 12], data[i + 13], data[i + 14], data[i + 15]);
>
> print_hex_dump(KERN_ERR, "", DUMP_PREFIX_OFFSET, 16, 1, data,
> eeprom.len, true);
>
>> + printk(KERN_ERR "Include this output when contacting your support "
>> + "provider.\n\nThis is not a software error! Something bad "
>> + "happened to your hardware or\nEEPROM image. Ignoring this "
>> + "problem could result in further problems,\npossibly loss "
>> + "of data, corruption or system hangs!\n\n");
>> + printk(KERN_ERR "The MAC Address will be reset to 00:00:00:00:00:00, "
>> + "which is invalid\nand requires you to set the proper MAC "
>> + "address manually before continuing\nto enable this network "
>> + "device.\n\n");
>> + printk(KERN_ERR "Please inspect the EEPROM dump and report the issue "
>> + "to your hardware vendor\nor Intel Customer Support: "
>> + "linux-nics@intel.com\n");
>
> multiline printks...
>
given these and Randy's comments I'll respin this and resubmit. thanks for the
comments!
Auke
^ permalink raw reply
* Re: Regression: Wireshark sees no packets in 2.6.24-rc3
From: Johannes Berg @ 2007-12-15 19:57 UTC (permalink / raw)
To: Ray Lee; +Cc: Gabriel C, Rafael J. Wysocki, netdev, LKML
In-Reply-To: <2c0942db0712150016r149d285bs8f4e1c444e71afea@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 693 bytes --]
On Sat, 2007-12-15 at 00:16 -0800, Ray Lee wrote:
> On Dec 14, 2007 11:09 PM, Ray Lee <ray@madrabbit.org> wrote:
> > On Dec 14, 2007 6:41 PM, Gabriel C <nix.or.die@googlemail.com> wrote:
> > Correct, absolutely no traffic. So if it works for you, then either
> > it's something that got fixed between -rc3 and -rc5, or something odd
> > when I did a make oldconfig, I suppose. (Or because I'm on an x86-64
> > kernel?) Regardless, -rc5 is currently building, and I'll try it in
> > the morning.
>
> -rc5 works great. Really don't know what's different between my -rc3
> and -rc5 builds.
I have an -rc3+wireless bits which also works great wrt.
tcpdump/wireshark.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] [UDP6]: Counter increment on BH mode
From: Ingo Molnar @ 2007-12-15 18:43 UTC (permalink / raw)
To: Herbert Xu
Cc: Alexey Kuznetsov, Wang Chen, Gerrit Renker, davem, andi, netdev,
Linux Kernel Mailing List, Christoph Lameter
In-Reply-To: <20071215135851.GA29063@gondor.apana.org.au>
* Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Ob Tue, Dec 04, 2007 at 12:17:23AM +1100, Herbert Xu wrote:
> >
> > Never mind, we already have that in local_t and as Alexey correctly
> > points out, USER is still going to be the expensive variant with the
> > preempt_disable (well until BH gets threaded). So how about this patch?
>
> I didn't hear any objections so here is the patch again.
>
> [SNMP]: Fix SNMP counters with PREEMPT
>
> The SNMP macros use raw_smp_processor_id() in process context which is
> illegal because the process may be preempted and then migrated to
> another CPU.
nit: please use 'invalid' instead of 'illegal'.
> This patch makes it use get_cpu/put_cpu to disable preemption.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> - (per_cpu_ptr(mib[1], raw_smp_processor_id())->mibs[field]++)
> + do { \
> + per_cpu_ptr(mib[1], get_cpu())->mibs[field]++; \
> + put_cpu(); \
> + } while (0)
> - (per_cpu_ptr(mib[1], raw_smp_processor_id())->mibs[field] += addend)
> + do { \
> + per_cpu_ptr(mib[1], get_cpu())->mibs[field] += addend; \
> + put_cpu(); \
> + } while (0)
we could perhaps introduce stat_smp_processor_id(), which signals that
the CPU id is used for statistical purposes and does not have to be
exact? In any case, your patch looks good too.
Ingo
^ permalink raw reply
* Re: 2.6.24-rc5-mm1 -- inconsistent {in-softirq-W} -> {softirq-on-R} usage.
From: Jarek Poplawski @ 2007-12-15 18:42 UTC (permalink / raw)
To: Andrew Morton
Cc: Miles Lane, linux-kernel, netdev, Ingo Molnar, Peter Zijlstra
In-Reply-To: <20071215031341.66dc0d05.akpm@linux-foundation.org>
Andrew Morton wrote, On 12/15/2007 12:13 PM:
> On Fri, 14 Dec 2007 22:58:24 -0500 "Miles Lane" <miles.lane@gmail.com> wrote:
...
>> I applied the patch and then tried my test again. This time my system
>> locked up.
>> Perhaps I should open a new thread for this, since the problem looks
>> pretty different.
>>
>> Dec 14 21:32:55 feargod kernel: process `cat' is using deprecated
>> sysctl (syscall) net.ipv6.neigh.default.retrans_time; Use
>> net.ipv6.neigh.default.retran
>> s_time_ms instead.
>> Dec 14 21:32:55 feargod kernel:
>> Dec 14 21:32:55 feargod kernel: =====================================
>> Dec 14 21:32:55 feargod kernel: [ BUG: bad unlock balance detected! ]
>> Dec 14 21:32:55 feargod kernel: -------------------------------------
>> Dec 14 21:32:55 feargod kernel: cat/6180 is trying to release lock
>> (kkkkkkk�����H3��) at:
>> Dec 14 21:32:55 feargod kernel: [packet_seq_stop+0xe/0x10]
>> packet_seq_stop+0xe/0x10
>> Dec 14 21:32:55 feargod kernel: but there are no more locks to release!
>> Dec 14 21:32:55 feargod kernel:
>> Dec 14 21:32:55 feargod kernel: other info that might help us debug this:
>> Dec 14 21:32:55 feargod kernel: 2 locks held by cat/6180:
>> Dec 14 21:32:55 feargod kernel: #0: (&p->lock){--..}, at:
>> [crypto_algapi:seq_read+0x25/0x191c1] seq_read+0x25/0x26f
>> Dec 14 21:32:55 feargod kernel: #1:
>> (&net->packet.sklist_lock){-.--}, at: [packet_seq_start+0x14/0x4d]
>> packet_seq_start+0x14/0x4d
Miles, I didn't check this yet, but since there were some considerable
changes, including locking, you could try with reverting some of these
last 3 patches to net/packet by Denis:
http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.25.git;a=history;f=net/packet/af_packet.c;h=485af5691d64270a02322925a6ccfad9d02b7f78;hb=HEAD
Regards,
Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH 3/3] [UDP6]: Counter increment on BH mode
From: Eric Dumazet @ 2007-12-15 17:03 UTC (permalink / raw)
To: Herbert Xu
Cc: Alexey Kuznetsov, Wang Chen, Gerrit Renker, davem, andi, netdev,
Linux Kernel Mailing List, Christoph Lameter, Ingo Molnar
In-Reply-To: <20071215135851.GA29063@gondor.apana.org.au>
Herbert Xu a écrit :
> Ob Tue, Dec 04, 2007 at 12:17:23AM +1100, Herbert Xu wrote:
>> Never mind, we already have that in local_t and as Alexey correctly
>> points out, USER is still going to be the expensive variant with the
>> preempt_disable (well until BH gets threaded). So how about this patch?
>
> I didn't hear any objections so here is the patch again.
>
> [SNMP]: Fix SNMP counters with PREEMPT
>
> The SNMP macros use raw_smp_processor_id() in process context
> which is illegal because the process may be preempted and then
> migrated to another CPU.
>
> This patch makes it use get_cpu/put_cpu to disable preemption.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> Cheers,
#define SNMP_INC_STATS_USER(mib, field) \
- (per_cpu_ptr(mib[1], raw_smp_processor_id())->mibs[field]++)
+ do { \
+ per_cpu_ptr(mib[1], get_cpu())->mibs[field]++; \
+ put_cpu(); \
+ } while (0)
#define SNMP_INC_STATS(mib, field) \
(per_cpu_ptr(mib[!in_softirq()], raw_smp_processor_id())->mibs[field]++)
How come you change SNMP_INC_STATS_USER() but not SNMP_INC_STATS() ?
^ permalink raw reply
* Re: 2.6.24-rc5-mm1 -- inconsistent {in-softirq-W} -> {softirq-on-R} usage.
From: Jarek Poplawski @ 2007-12-15 17:06 UTC (permalink / raw)
To: Andrew Morton
Cc: Miles Lane, linux-kernel, netdev, Ingo Molnar, Peter Zijlstra
In-Reply-To: <20071215031341.66dc0d05.akpm@linux-foundation.org>
Andrew Morton wrote, On 12/15/2007 12:13 PM:
> On Fri, 14 Dec 2007 22:58:24 -0500 "Miles Lane" <miles.lane@gmail.com> wrote:
>
>> On Dec 14, 2007 6:36 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
>>> On Fri, 14 Dec 2007 17:13:21 -0500
>>> "Miles Lane" <miles.lane@gmail.com> wrote:
...
>>>> Pid: 6944, comm: cat Not tainted (2.6.24-rc5-mm1 #26)
...
>>>> note: cat[6944] exited with preempt_count 4
...
>> Dec 14 21:32:55 feargod kernel: cat/6180 is trying to release lock
...
> Can you suggest a way in which others can reproduce this?
Buy a cat?
Jarek P.
^ permalink raw reply
* Re: [PATCH 2/4] [CORE]: datagram: mem_scheudle functions
From: Herbert Xu @ 2007-12-15 15:32 UTC (permalink / raw)
To: Hideo AOKI
Cc: David Miller, netdev, Takahiro Yasui, Masami Hiramatsu,
Satoshi Oshima, billfink, Andi Kleen, Evgeniy Polyakov,
Stephen Hemminger, yoshfuji, Yumiko Sugita
In-Reply-To: <476362D8.4070807@redhat.com>
On Sat, Dec 15, 2007 at 12:15:04AM -0500, Hideo AOKI wrote:
> This patch includes changes in network core sub system for memory
> accounting.
>
> Memory scheduling, charging, uncharging and reclaiming functions are
> added. These functions use sk_forward_alloc to store socket local
> accounting. They also need to use lock to keep consistency of
> sk_forward_alloc and memory_allocated. They currently support only
> datagram protocols.
Thanks for the patch. I think it's generally on the right track
but there's still a few issues with the implementation.
> + spin_lock_irqsave(&sk->sk_lock.slock, flags);
Please use bh_lock_sock since this must never be used from an
IRQ handler.
> +static inline void sk_mem_reclaim(struct sock *sk)
> +{
> + if (sk->sk_type == SOCK_DGRAM)
> + sk_datagram_mem_reclaim(sk);
> +}
Please get rid of these wrappers since we should only get here
for datagram protocols.
> +static inline int sk_account_wmem_charge(struct sock *sk, int size)
> +{
> + unsigned long flags;
> +
> + /* account if protocol supports memory accounting. */
> + if (!sk->sk_prot->memory_allocated || sk->sk_type != SOCK_DGRAM)
> + return 1;
> +
> + spin_lock_irqsave(&sk->sk_lock.slock, flags);
> + if (sk_datagram_wmem_schedule(sk, size)) {
> + sk->sk_forward_alloc -= size;
> + spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
> + return 1;
> + }
> + spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
> + return 0;
> +}
This is probably too big to inline.
> +static inline int sk_account_rmem_charge(struct sock *sk, int size)
> +{
> + unsigned long flags;
> +
> + /* account if protocol supports memory accounting. */
> + if (!sk->sk_prot->memory_allocated || sk->sk_type != SOCK_DGRAM)
> + return 1;
> +
> + spin_lock_irqsave(&sk->sk_lock.slock, flags);
> + if (sk_datagram_rmem_schedule(sk, size)) {
> + sk->sk_forward_alloc -= size;
> + spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
> + return 1;
> + }
> + spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
> + return 0;
> +}
Why are we duplicating the rmem/wmem versions when they're identical?
> -int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> +int sock_queue_rcv_skb_with_owner(struct sock *sk, struct sk_buff *skb,
> + void set_owner_r(struct sk_buff *nskb,
> + struct sock* nsk))
Just make a new function for this rather than playing with function
pointers.
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: [Bugme-new] [Bug 9543] New: RTNL: assertion failed at net/ipv6/addrconf.c (2164)/RTNL: assertion failed at net/ipv4/devinet.c (1055)
From: Andy Gospodarek @ 2007-12-15 15:09 UTC (permalink / raw)
To: Herbert Xu
Cc: Krzysztof Oledzki, Jay Vosburgh, Andrew Morton, bugme-daemon,
shemminger, davem, netdev
In-Reply-To: <20071215041052.GA25324@gondor.apana.org.au>
On Dec 14, 2007 11:10 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Fri, Dec 14, 2007 at 05:47:22PM -0500, Andy Gospodarek wrote:
> >
> > I'm guessing if we go back to using a write-lock for bond->lock this
> > will go back to working again, but I'm not totally convinced since there
> > are plenty of places where we used a read-lock with it.
>
> Sorry I forgot to cc you earlier Andy.
>
> But to fix this you need make sure that all read locks on bond->lock
> in process context disable BH. This is because at least one write
> lock can be taken from BH context.
I agree with you completely, Herbet, which is why I was surprised that
my first apparently did not resolve the issue. I felt it should
have....
> You don't need to turn the read locks into write locks however.
>
> This is also something that we can undo once the set_multicast
> interface has been fixed to not take the tx lock.
>
> 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 2/4] datagram: mem_scheudle functions
From: Herbert Xu @ 2007-12-15 14:45 UTC (permalink / raw)
To: Hideo AOKI
Cc: netdev, David Miller, Satoshi Oshima, Bill Fink, Andi Kleen,
Evgeniy Polyakov, Stephen Hemminger, yoshfuji, Yumiko Sugita
In-Reply-To: <47549B05.5090507@redhat.com>
On Mon, Dec 03, 2007 at 07:10:45PM -0500, Hideo AOKI wrote:
>
> Because we have to call wmem_schedule function in ip_append_data()
> which is used by several protocols both stream and datagram.
> I just thought adding the sk_wmem_schedule() was only way to call
> proper function from ip_append_data().
ip_append_data can't possibly work for stream protocols. So
which stream protocol is calling it?
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 3/3] [UDP6]: Counter increment on BH mode
From: Herbert Xu @ 2007-12-15 13:58 UTC (permalink / raw)
To: Alexey Kuznetsov
Cc: Wang Chen, Gerrit Renker, davem, andi, netdev,
Linux Kernel Mailing List, Christoph Lameter, Ingo Molnar
In-Reply-To: <20071203131723.GA30312@gondor.apana.org.au>
Ob Tue, Dec 04, 2007 at 12:17:23AM +1100, Herbert Xu wrote:
>
> Never mind, we already have that in local_t and as Alexey correctly
> points out, USER is still going to be the expensive variant with the
> preempt_disable (well until BH gets threaded). So how about this patch?
I didn't hear any objections so here is the patch again.
[SNMP]: Fix SNMP counters with PREEMPT
The SNMP macros use raw_smp_processor_id() in process context
which is illegal because the process may be preempted and then
migrated to another CPU.
This patch makes it use get_cpu/put_cpu to disable preemption.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
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
--
diff --git a/include/net/snmp.h b/include/net/snmp.h
index ea206bf..9444b54 100644
--- a/include/net/snmp.h
+++ b/include/net/snmp.h
@@ -23,6 +23,7 @@
#include <linux/cache.h>
#include <linux/snmp.h>
+#include <linux/smp.h>
/*
* Mibs are stored in array of unsigned long.
@@ -137,7 +138,10 @@ struct linux_mib {
#define SNMP_INC_STATS_OFFSET_BH(mib, field, offset) \
(per_cpu_ptr(mib[0], raw_smp_processor_id())->mibs[field + (offset)]++)
#define SNMP_INC_STATS_USER(mib, field) \
- (per_cpu_ptr(mib[1], raw_smp_processor_id())->mibs[field]++)
+ do { \
+ per_cpu_ptr(mib[1], get_cpu())->mibs[field]++; \
+ put_cpu(); \
+ } while (0)
#define SNMP_INC_STATS(mib, field) \
(per_cpu_ptr(mib[!in_softirq()], raw_smp_processor_id())->mibs[field]++)
#define SNMP_DEC_STATS(mib, field) \
@@ -145,6 +149,9 @@ struct linux_mib {
#define SNMP_ADD_STATS_BH(mib, field, addend) \
(per_cpu_ptr(mib[0], raw_smp_processor_id())->mibs[field] += addend)
#define SNMP_ADD_STATS_USER(mib, field, addend) \
- (per_cpu_ptr(mib[1], raw_smp_processor_id())->mibs[field] += addend)
+ do { \
+ per_cpu_ptr(mib[1], get_cpu())->mibs[field] += addend; \
+ put_cpu(); \
+ } while (0)
#endif
^ permalink raw reply related
* Re: Reproducible data corruption with sendfile+vsftp - splice regression?
From: Holger Hoffstaette @ 2007-12-15 13:43 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev
In-Reply-To: <20071206184426.GA32599@electric-eye.fr.zoreil.com>
On Thu, 06 Dec 2007 19:44:26 +0100, Francois Romieu wrote:
> Holger Hoffstaette <holger@wizards.de> : [...]
>> Maybe turning off sendfile or NAPI just lead to random success - so far
>> it really looks like tso on the r8169 is the common cause.
>
> TSO on the r8169 is the magic switch but the regression makes imvho more
> sense from a VM pov:
>
> - the corrupted file has the same size as the expected file
> - the corrupted file exhibits holes which come as a multiple of 4096 bytes
> (8*4k, 2 places, there may be more)
> - the r8169 driver does not know what a page is
> - the 8169 hardware has a small 8192 bytes Tx buffer
>
> It would be nice if someone could do a sendfile + vsftp test with TSO on a
> different hardware. While I could not reproduce the corruption when simply
> downloading a file that I had copied on the server with scp, it triggered
> almost immediately after I copied it locally and tried to download the
> copy.
I tested 2.6.24-rc5 on my T60 (Intel e1000 built with NAPI) and installed
vsftp/apache with sendfile and enabled all offload options incl. TSO.
Repeated downloads of >500 MB with ftp or wget over the NIC onto ram- or
physical disk gives no corruption whatsoever. Speed of download to ramdisk
is a nice continuous 125 MB/sec.
Looks like the r8169 or the driver after all..
thanks
Holger
^ permalink raw reply
* Re: [patch 2/4] net: use mutex_is_locked() for ASSERT_RTNL()
From: Herbert Xu @ 2007-12-15 13:10 UTC (permalink / raw)
To: Andrew Morton; +Cc: David Miller, netdev, Eric W. Biederman
In-Reply-To: <20071215024810.20b8a5ae.akpm@linux-foundation.org>
On Sat, Dec 15, 2007 at 02:48:10AM -0800, Andrew Morton wrote:
>
> When Eric said
>
> > Way way deep in mutex debugging on the slowpath there is a unreadable
> > and incomprehensible WARN_ON in muxtex_trylock that will trigger if
> > you have 10 tons of debugging turned on, and you are in,
> > interrupt context, and you manage to hit the slow path. I think that
> > is a pretty unlikely scenario.
>
> I think he's still right. That's if the warning which he managed to find
> even still exists.
Well at the very start he said:
: When mutex debugging is on taking a mutex complains if we are not
: allowed to sleep. At that point we have called netif_tx_lock_bh
: so we are clearly not allowed to sleep. Arguably this is not a
: problem for mutex_trylock.
This is what threw me off as it implied that we were warning about
ASSERT_RTNL calls in any atomic context, including those with just
soft IRQs disabled, which is an important distinction for the
networking subsystem.
Had he said that it was just IRQ handlers rather than soft IRQs
then we wouldn't be having this discussion now :)
> I think the change which Eric proposed is a good one: it converts
> ASSERT_RTNL() from an atomic rmw which dirties a cacheline which will
> sometimes be owned by a different CPU into a plain old read. It's going to
> make ASSERT_RTNL() heaps cheaper.
I agree. Although paths using ASSERT_RTNL shouldn't be performance
critical since the RTNL should only be held for control operations
as opposed to data transport.
> Now as a separate issue we (ie: you) need to work out what _other_ things
> you want ASSERT_RTNL to check apart from "rtnl must be held".
Since we have now established that ASSERT_RTNL never actually
warned about usage on paths with BH off, I think Eric's original
patch is fine as it is and I owe him an apology.
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
* [PATCH] sky2: check pci_register_driver() error
From: Akinobu Mita @ 2007-12-15 11:58 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
Check pci_register_driver() error and clean up debugfs entries
if error happened.
Cc: Stephen Hemminger <shemminger@linux-foundation.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/net/sky2.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
Index: 2.6-git/drivers/net/sky2.c
===================================================================
--- 2.6-git.orig/drivers/net/sky2.c
+++ 2.6-git/drivers/net/sky2.c
@@ -4426,8 +4426,15 @@ static struct pci_driver sky2_driver = {
static int __init sky2_init_module(void)
{
+ int err;
+
sky2_debug_init();
- return pci_register_driver(&sky2_driver);
+ err = pci_register_driver(&sky2_driver);
+ if (err) {
+ sky2_debug_cleanup();
+ return err;
+ }
+ return 0;
}
static void __exit sky2_cleanup_module(void)
^ permalink raw reply
* Re: 2.6.24-rc5-mm1 -- inconsistent {in-softirq-W} -> {softirq-on-R} usage.
From: Andrew Morton @ 2007-12-15 11:13 UTC (permalink / raw)
To: Miles Lane; +Cc: linux-kernel, netdev, Ingo Molnar, Peter Zijlstra
In-Reply-To: <a44ae5cd0712141958o10d9a320h3b8173019ac6b0eb@mail.gmail.com>
On Fri, 14 Dec 2007 22:58:24 -0500 "Miles Lane" <miles.lane@gmail.com> wrote:
> On Dec 14, 2007 6:36 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Fri, 14 Dec 2007 17:13:21 -0500
> > "Miles Lane" <miles.lane@gmail.com> wrote:
> >
> >
> > > Sorry Andrew, I don't know who to forward this problem to.
> > >
> > > I tried running: find /proc | xargs cat
> > > and got this:
> > >
> > > =================================
> > > [ INFO: inconsistent lock state ]
> > > 2.6.24-rc5-mm1 #26
> > > ---------------------------------
> > > inconsistent {in-softirq-W} -> {softirq-on-R} usage.
> > > cat/6944 [HC0[0]:SC0[0]:HE1:SE1] takes:
> > > BUG: unable to handle kernel paging request at virtual address 0f1eff0b
> > > printing ip: c01fe64d *pde = 00000000
> > > Oops: 0000 [#1] PREEMPT SMP
> > > last sysfs file: /sys/block/sda/sda3/stat
> > > Modules linked in: aes_generic i915 drm rfcomm l2cap bluetooth
> > > cpufreq_stats cpufreq_conservative cpufreq_performance sbs sbshc
> > > dm_crypt sbp2 parport_pc lp parport pcmcia arc4 ecb crypto_blkcipher
> > > cryptomgr crypto_algapi tifm_7xx1 tifm_core yenta_socket
> > > rsrc_nonstatic pcmcia_core iwl3945 iTCO_wdt iTCO_vendor_support
> > > watchdog_core watchdog_dev snd_hda_intel mac80211 snd_pcm_oss
> > > snd_mixer_oss cfg80211 snd_pcm sky2 snd_seq_dummy snd_seq_oss
> > > snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq snd_timer
> > > snd_seq_device snd soundcore snd_page_alloc shpchp pci_hotplug
> > > firewire_ohci firewire_core crc_itu_t ata_generic piix ide_core
> > >
> > > Pid: 6944, comm: cat Not tainted (2.6.24-rc5-mm1 #26)
> > > EIP: 0060:[<c01fe64d>] EFLAGS: 00210097 CPU: 0
> > > EIP is at strnlen+0x9/0x1c
> > > EAX: 0f1eff0b EBX: 0f1eff0b ECX: 0f1eff0b EDX: fffffffe
> > > ESI: c05b74f6 EDI: d6267d94 EBP: d6267cc8 ESP: d6267cc8
> > > DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> > > Process cat (pid: 6944, ti=d6267000 task=d5a09000 task.ti=d6267000)
> > > Stack: d6267cfc c01fdd22 00000400 c05b74f4 00000001 c05b78f4 00000000 ffffffff
> > > ffffffff c048f503 00000400 d5a09000 00000002 d6267d0c c01fdf41 d6267d94
> > > db68c04a d6267d74 c012ae81 d6267d94 00000028 c05b89f7 00200046 00000000
> > > Call Trace:
> > > [<c0108eb2>] show_trace_log_lvl+0x12/0x25
> > > [<c0108f4f>] show_stack_log_lvl+0x8a/0x95
> > > [<c0108fe4>] show_registers+0x8a/0x1bd
> > > [<c010922f>] die+0x118/0x1dc
> > > [<c03cf706>] do_page_fault+0x5a4/0x681
> > > [<c03cdd72>] error_code+0x72/0x78
> > > [<c01fdd22>] vsnprintf+0x277/0x40e
> > > [<c01fdf41>] vscnprintf+0xe/0x1d
> > > [<c012ae81>] vprintk+0xcb/0x2f3
> > > [<c012b0be>] printk+0x15/0x17
> > > [<c0145e55>] print_lock_name+0x4e/0xa2
> > > [<c0146099>] print_lock+0xe/0x3a
> > > [<c01464cf>] print_usage_bug+0xbc/0x117
> > > [<c0146fb6>] mark_lock+0x2e7/0x3fe
> > > [<c0147b9a>] __lock_acquire+0x498/0xbf4
> > > [<c014836c>] lock_acquire+0x76/0x9d
> > > [<c03cd6d2>] _read_lock+0x23/0x32
> > > [<c03491ae>] sock_i_ino+0x14/0x30
> > > [<c03c88ed>] packet_seq_show+0x22/0x75
> > > [<c019b41a>] seq_read+0x19d/0x26f
> > > [<c01b0ded>] proc_reg_read+0x60/0x74
> > > [<c01854aa>] vfs_read+0x8a/0x106
> > > [<c01858a8>] sys_read+0x3b/0x60
> > > [<c0107cea>] sysenter_past_esp+0x6b/0xc1
> > > =======================
> > > Code: 01 00 00 00 4f 89 fa 5f 89 d0 5d c3 55 85 c9 89 e5 57 89 c7 89
> > > d0 74 05 f2 ae 75 01 4f 89 f8 5f 5d c3 55 89 c1 89 e5 89 c8 eb 06 <80>
> > > 38 00 74 07 40 4a 83 fa ff 75 f4 29 c8 5d c3 90 90 90 55 83
> > > EIP: [<c01fe64d>] strnlen+0x9/0x1c SS:ESP 0068:d6267cc8
> > > note: cat[6944] exited with preempt_count 4
> >
> > I'd say you hit a networking locking bug and then when trying to report
> > that bug, lockdep crashed.
> >
> > The networking bug looks to be around sock_i_ino()'s taking of
> > sk_callback_lock with softirq's enabled. Perhaps this will fix it.
> >
> > diff -puN net/core/sock.c~a net/core/sock.c
> > --- a/net/core/sock.c~a
> > +++ a/net/core/sock.c
> > @@ -1115,9 +1115,9 @@ int sock_i_uid(struct sock *sk)
> > {
> > int uid;
> >
> > - read_lock(&sk->sk_callback_lock);
> > + read_lock_bh(&sk->sk_callback_lock);
> > uid = sk->sk_socket ? SOCK_INODE(sk->sk_socket)->i_uid : 0;
> > - read_unlock(&sk->sk_callback_lock);
> > + read_unlock_bh(&sk->sk_callback_lock);
> > return uid;
> > }
> >
> > @@ -1125,9 +1125,9 @@ unsigned long sock_i_ino(struct sock *sk
> > {
> > unsigned long ino;
> >
> > - read_lock(&sk->sk_callback_lock);
> > + read_lock_bh(&sk->sk_callback_lock);
> > ino = sk->sk_socket ? SOCK_INODE(sk->sk_socket)->i_ino : 0;
> > - read_unlock(&sk->sk_callback_lock);
> > + read_unlock_bh(&sk->sk_callback_lock);
> > return ino;
> > }
> >
> > _
> >
> >
>
> I applied the patch and then tried my test again. This time my system
> locked up.
> Perhaps I should open a new thread for this, since the problem looks
> pretty different.
>
> Dec 14 21:32:55 feargod kernel: process `cat' is using deprecated
> sysctl (syscall) net.ipv6.neigh.default.retrans_time; Use
> net.ipv6.neigh.default.retran
> s_time_ms instead.
> Dec 14 21:32:55 feargod kernel:
> Dec 14 21:32:55 feargod kernel: =====================================
> Dec 14 21:32:55 feargod kernel: [ BUG: bad unlock balance detected! ]
> Dec 14 21:32:55 feargod kernel: -------------------------------------
> Dec 14 21:32:55 feargod kernel: cat/6180 is trying to release lock
> (kkkkkkk�����H3��) at:
> Dec 14 21:32:55 feargod kernel: [packet_seq_stop+0xe/0x10]
> packet_seq_stop+0xe/0x10
> Dec 14 21:32:55 feargod kernel: but there are no more locks to release!
> Dec 14 21:32:55 feargod kernel:
> Dec 14 21:32:55 feargod kernel: other info that might help us debug this:
> Dec 14 21:32:55 feargod kernel: 2 locks held by cat/6180:
> Dec 14 21:32:55 feargod kernel: #0: (&p->lock){--..}, at:
> [crypto_algapi:seq_read+0x25/0x191c1] seq_read+0x25/0x26f
> Dec 14 21:32:55 feargod kernel: #1:
> (&net->packet.sklist_lock){-.--}, at: [packet_seq_start+0x14/0x4d]
> packet_seq_start+0x14/0x4d
Bit of a mess. We don't know whether lockdep is broken or if networking is
just feeding it garbage. Or both.
My suspicion is that you've hit bad breakage in networking and lockdep just
isn't sufficiently robust to handle what it's being given.
Can you suggest a way in which others can reproduce this?
What happens if you run the same test with CONFIG_LOCKDEP=n?
^ permalink raw reply
* Re: [NETFILTER] xt_hashlimit : speedups hash_dst()
From: Eric Dumazet @ 2007-12-15 11:04 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: Patrick McHardy, netfilter-devel, netdev
In-Reply-To: <4763AF65.4070200@gmail.com>
Jarek Poplawski a écrit :
> Eric Dumazet wrote, On 12/14/2007 10:37 PM:
>
>> Jarek Poplawski a écrit :
>>> Eric Dumazet wrote, On 12/14/2007 12:09 PM:
>>> ...
>>>
>>>> + /*
>>>> + * Instead of returning hash % ht->cfg.size (implying a divide)
>>>> + * we return the high 32 bits of the (hash * ht->cfg.size) that will
>>>> + * give results between [0 and cfg.size-1] and same hash distribution,
>>>> + * but using a multiply, less expensive than a divide
>>>> + */
>>>> + return ((u64)hash * ht->cfg.size) >> 32;
>>> Are we sure of the same hash distribution? Probably I miss something,
>>> but: if this 'hash' is well distributed on 32 bits, and ht->cfg.size
>>> is smaller than 32 bits, e.g. 256 (8 bits), then this multiplication
>>> moves to the higher 32 of u64 only max. 8 bits of the most significant
>>> byte, and the other three bytes are never used, while division is
>>> always affected by all four bytes...
>> Not sure what you are saying... but if size=256, then, yes, we want a final
>> result between 0 and 255, so three bytes are nul.
>
> Eric, it would be nice to acknowledge David's suggestion that this hash
> size is always power of two here, because otherwise at least your words
> about the same hash distribution according to the "%" variant could be
> wrong (but I don't say the final result would be wrong). Maybe I mix
> up these sizes, but it seems this could be set by a user, and I didn't
> find anything about this power of two necessity?
>
size is not a power of two here.
I prefer to let admins chose their size, since it makes attacker life more
difficult :)
For example, I can tell you I have a server, were size is between 2.000.000
and 3.500.000, I dont want to be forced to use 2097152
A multiply is cheap, at least on current hardware.
-
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch 2/4] net: use mutex_is_locked() for ASSERT_RTNL()
From: Andrew Morton @ 2007-12-15 10:48 UTC (permalink / raw)
To: Herbert Xu; +Cc: David Miller, netdev
In-Reply-To: <20071215061021.GA26247@gondor.apana.org.au>
On Sat, 15 Dec 2007 14:10:21 +0800 Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Fri, Dec 14, 2007 at 09:44:18PM -0800, Andrew Morton wrote:
> >
> > That sounds like a bug in mutex_trylock() to me.
>
> I was relying on
>
> http://kerneltrap.org/mailarchive/linux-netdev/2007/9/28/325129
>
> which seems to be a bogus claim now that I actually look at the
> source code. So in that case I'm OK with your patch as long as
> it warns about hard IRQ usage.
When Eric said
> Way way deep in mutex debugging on the slowpath there is a unreadable
> and incomprehensible WARN_ON in muxtex_trylock that will trigger if
> you have 10 tons of debugging turned on, and you are in,
> interrupt context, and you manage to hit the slow path. I think that
> is a pretty unlikely scenario.
I think he's still right. That's if the warning which he managed to find
even still exists.
I think the change which Eric proposed is a good one: it converts
ASSERT_RTNL() from an atomic rmw which dirties a cacheline which will
sometimes be owned by a different CPU into a plain old read. It's going to
make ASSERT_RTNL() heaps cheaper.
<looks at mutex_is_locked(), rofls at "static inline fastcall", fixes it>
Now as a separate issue we (ie: you) need to work out what _other_ things
you want ASSERT_RTNL to check apart from "rtnl must be held".
If you want to check that no locks are held (which I think is a bit weird,
but whatever) then add might_sleep().
If you want to check that we're not in interrupt context or whatever, then
add the checks and be happy. might_sleep() will of course check for
in_interrupt(), in_irq(), etc so if you go with a might_sleep() then
nothing else needs to be added.
While doing this I'd also suggest that the thing should be uninlined -
it'll probably generate less text and it'll give considerably more
flexibility for adding new debug fetures. Ones which might be controlled at
compile time or runtime. ie:
void __assert_rtnl(const char *file, int line);
#define ASSERT_RTNL() __assert_rtnl(__FILE__, __LINE__)
^ permalink raw reply
* Re: [NETFILTER] xt_hashlimit : speedups hash_dst()
From: Jarek Poplawski @ 2007-12-15 10:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Patrick McHardy, netfilter-devel, netdev
In-Reply-To: <4762F78B.80302@cosmosbay.com>
Eric Dumazet wrote, On 12/14/2007 10:37 PM:
> Jarek Poplawski a écrit :
>> Eric Dumazet wrote, On 12/14/2007 12:09 PM:
>> ...
>>
>>> + /*
>>> + * Instead of returning hash % ht->cfg.size (implying a divide)
>>> + * we return the high 32 bits of the (hash * ht->cfg.size) that will
>>> + * give results between [0 and cfg.size-1] and same hash distribution,
>>> + * but using a multiply, less expensive than a divide
>>> + */
>>> + return ((u64)hash * ht->cfg.size) >> 32;
>> Are we sure of the same hash distribution? Probably I miss something,
>> but: if this 'hash' is well distributed on 32 bits, and ht->cfg.size
>> is smaller than 32 bits, e.g. 256 (8 bits), then this multiplication
>> moves to the higher 32 of u64 only max. 8 bits of the most significant
>> byte, and the other three bytes are never used, while division is
>> always affected by all four bytes...
>
> Not sure what you are saying... but if size=256, then, yes, we want a final
> result between 0 and 255, so three bytes are nul.
Eric, it would be nice to acknowledge David's suggestion that this hash
size is always power of two here, because otherwise at least your words
about the same hash distribution according to the "%" variant could be
wrong (but I don't say the final result would be wrong). Maybe I mix
up these sizes, but it seems this could be set by a user, and I didn't
find anything about this power of two necessity?
Regards,
Jarek P.
-
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Reproducible data corruption with sendfile+vsftp - splice regression?
From: Holger Hoffstaette @ 2007-12-15 10:24 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
In-Reply-To: <pan.2007.12.13.02.19.40.200510@wizards.de>
On Thu, 13 Dec 2007 03:19:43 +0100, Holger Hoffstaette wrote:
> I have now gone back to enable TSO since vsftp with sendfile really seems
> to be the only app that causes this. I have simply set it to
> use_sendfile=NO and no corruption occurs at all; the machine is stable and
> fast.
In the good tradition of proving myself wrong I can reliably create
corrupted files by wget-ting from apache (with sendfile enabled) as
well, so no more TSO after all. No TSO, no corruption.
The same also happens on a different machine with a r8169 (same model).
Tickless kernel makes no difference either. Shot in the dark, but hey..
Holger
^ permalink raw reply
* [PATCH take2] [AX25] circular locking with AX25 connection timeout
From: Jarek Poplawski @ 2007-12-15 10:18 UTC (permalink / raw)
To: Bernard Pidoux; +Cc: David Miller, Ralf Baechle DL5RB, Linux Netdev List
In-Reply-To: <476303C8.2060507@ccr.jussieu.fr>
On Fri, Dec 14, 2007 at 11:29:28PM +0100, Bernard Pidoux wrote:
> Hi,
>
> This patch cancels a circular locking conflict that appeared with a timeout
> of an AX25 connection.
>
> signed off by Jarek Poplawski
One spurious space less here, and maybe a few more words to the changelog.
Regards,
Jarek P.
---------------->
Subject: [PATCH] [NET] AX25: locking dependencies fix in ax25_disconnect
Bernard Pidoux reported these lockdep warnings:
[ INFO: possible irq lock inversion dependency detected ]
2.6.23.1 #1
---------------------------------------------------------
fpac/4933 just changed the state of lock:
(slock-AF_AX25){--..}, at: [<d8be3312>] ax25_disconnect+0x46/0xaf
[ax25]
but this lock was taken by another, soft-irq-safe lock in the past:
(ax25_list_lock){-+..}
and interrupts could create inverse lock ordering between them.
[...]
[ INFO: inconsistent lock state ]
2.6.23.1 #1
---------------------------------
inconsistent {in-softirq-W} -> {softirq-on-W} usage.
ax25_call/4005 [HC0[0]:SC0[0]:HE1:SE1] takes:
(slock-AF_AX25){-+..}, at: [<d8b79312>] ax25_disconnect+0x46/0xaf [ax25]
[...]
This means slock-AF_AX25 could be taken both from softirq and process
context with softirqs enabled, so it's endangered itself, but also makes
ax25_list_lock vulnerable. It was not 100% verified if the real lockup
can happen, but this fix isn't very costly and looks safe anyway.
(It was tested by Bernard with 2.6.23.9 and 2.6.24-rc5 kernels.)
Reported_by: Bernard Pidoux <pidoux@ccr.jussieu.fr>
Tested_by: Bernard Pidoux <pidoux@ccr.jussieu.fr>
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
---
diff -Nurp linux-2.6.24-rc2-/net/ax25/ax25_subr.c linux-2.6.24-rc2+/net/ax25/ax25_subr.c
--- linux-2.6.24-rc2-/net/ax25/ax25_subr.c 2007-10-09 22:31:38.000000000 +0200
+++ linux-2.6.24-rc2+/net/ax25/ax25_subr.c 2007-11-28 11:51:12.000000000 +0100
@@ -279,6 +279,7 @@ void ax25_disconnect(ax25_cb *ax25, int
ax25_link_failed(ax25, reason);
if (ax25->sk != NULL) {
+ local_bh_disable();
bh_lock_sock(ax25->sk);
ax25->sk->sk_state = TCP_CLOSE;
ax25->sk->sk_err = reason;
@@ -288,5 +289,6 @@ void ax25_disconnect(ax25_cb *ax25, int
sock_set_flag(ax25->sk, SOCK_DEAD);
}
bh_unlock_sock(ax25->sk);
+ local_bh_enable();
}
}
^ permalink raw reply
* Re: [PATCH 03/29] mm: slb: add knowledge of reserve pages
From: Daniel Phillips @ 2007-12-15 10:10 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
trond.myklebust
In-Reply-To: <200712141451.09500.phillips@phunq.net>
On Friday 14 December 2007 14:51, I wrote:
> On Friday 14 December 2007 07:39, Peter Zijlstra wrote:
> Note that false sharing of slab pages is still possible between two
> unrelated writeout processes, both of which obey rules for their own
> writeout path, but the pinned combination does not. This still
> leaves a hole through which a deadlock may slip.
Actually, no it doesn't. It in fact does not matter how many unrelated
writeout processes, block devices, whatevers share a slab cache.
Sufficient reserve pages must have been made available (in a perfect
work, by adding extra pages to the memalloc reserve on driver
initialization, in the real world just by having a big memalloc
reserve) to populate the slab up to the sum of the required objects for
all memalloc users sharing the cache.
So I think this slab technique of yours is fundamentally sound, that is
to say, adding a new per-slab flag to keep unbounded numbers of slab
objects with unbounded lifetimes from mixing with the bounded number of
slab objects with bounded lifetimes.
Ponder. OK, here is another issue. Suppose a driver expands the
memalloc reserve by the X number of pages it needs on initialization,
and shrinks it by the same amount on removal, as is right and proper.
The problem is, less than the number of slab pages that got pulled into
slab on behalf of the removed driver may be freed (or made freeable)
back to the global reserve, due to page sharing with an unrelated user.
In theory, the global reserve could be completely depleted by this slab
fragmentation.
OK, that is like the case that I mistakenly raised in the previous mail,
though far less likely to occur, because driver removals are relatively
rare and so would be a fragmentation case so severe as to cause global
reserve depletion.
Even so, if this possibility bothers anybody, it is fairly easy to plug
the hole: associate each slab with a given memalloc user instead of
just having one bit to classify users. So unrelated memalloc users
would never share a slab, no false sharing, everybody happy. The cost:
a new pointer field per slab and a few additional lines of code.
Regards,
Daniel
^ permalink raw reply
* SACK scoreboard (Was: Re: [RFC PATCH net-2.6.25 uncompilable] [TCP]: Avoid breaking GSOed skbs when SACKed one-by-one)
From: Ilpo Järvinen @ 2007-12-15 9:51 UTC (permalink / raw)
To: David Miller; +Cc: lachlan.andrew, Netdev, quetchen
In-Reply-To: <20071211.043239.224938181.davem@davemloft.net>
On Tue, 11 Dec 2007, David Miller wrote:
> Interesting approach, but I think there is limited value to this
> (arguably) complex form.
>
> The core issue is that the data and the SACK state are maintained in
> the same datastructure. The complexity in all the state management
> and fixups in your patch is purely because of this.
Yeah, had just too much time while waiting for person who never
arrived... :-) It would have covered the typical case quite well
tough, for sure it was very intrusive.
> If we maintain SACK scoreboard information seperately, outside of
> the SKB, then there are only two changes to make:
>
> 1) Every access to TCP_SKB_CB() SACK scoreboard is adjusted to
> new data structure.
Not sure if it is going to all that straightforward because you seem to
ignore in this simple description all details of performing the linking
between the structures, which becomes tricky when we add those
retransmission adjustments.
> 2) Retransmit is adjusted so that it can retransmit an SKB
> constructed as a portion of an existing SKB. Since TSO
> implies SG, this can be handled with simple offset and
> length arguments and suitable creation of a clone referencing
> the pages in the SG vector that contain the desired data.
...I.e., how to count retrans_out origins in such case because the
presented sack_ref knows nothing about them nor do we have anything but
one bit in ->sacked per skb. We need the origins when retransmission get
sacked/acked to reduce retrans_out correctly. To solve this, I think the
sack_ref must have ->sacked as well and the structure should store the
R-bits there as well, and may L-bits too though their defination is
more obvious because it's mostly a complement of sacked (except for small
part near fackets_out and timedout marking that probably makes bookkeeping
of L still necessary).
The pcount boundaries are no longer that well defined after we break skb
boundaries during retransmitting, so doing this makes fackets_out
calculation even more trickier to track, unless whole pcount is replaced
by byte based fackets_out :-/, which is very trivial to determine from
seqnos only (TCP_NODELAYers might get unhappy though if we do that in a
straightforward way...).
This would clearly be a good step from one perspective. Nowadays GSO'ed
skbs may get fragmented when mss_now changes, for two or more consecutive
non-SACKed skbs that means one or more extra (small) packets when
retransmitting.
> I would envision this SACK state thing to reference into the
> retransmit queue SKB's somehow. Each SACK block could perhaps
> look something like:
>
> struct sack_ref {
> struct sk_buff *start_skb;
> struct sk_buff *end_skb;
> unsigned int start_off;
> unsigned int len;
> };
...I assume that these are fast searchable? And skbs as well (because the
linking of start/end_skb at minimum is a costly operation otherwise)?
--
i.
^ permalink raw reply
* Re: Regression: Wireshark sees no packets in 2.6.24-rc3
From: Ray Lee @ 2007-12-15 8:16 UTC (permalink / raw)
To: Gabriel C; +Cc: Rafael J. Wysocki, netdev, LKML
In-Reply-To: <2c0942db0712142309v15f79068m8c033180d41b9be@mail.gmail.com>
On Dec 14, 2007 11:09 PM, Ray Lee <ray@madrabbit.org> wrote:
> On Dec 14, 2007 6:41 PM, Gabriel C <nix.or.die@googlemail.com> wrote:
> Correct, absolutely no traffic. So if it works for you, then either
> it's something that got fixed between -rc3 and -rc5, or something odd
> when I did a make oldconfig, I suppose. (Or because I'm on an x86-64
> kernel?) Regardless, -rc5 is currently building, and I'll try it in
> the morning.
-rc5 works great. Really don't know what's different between my -rc3
and -rc5 builds. The diff of .config between the two doesn't show
anything obvious, so perhaps it was something fixed in the interim.
I've gone ahead and closed the bugzilla entry, btw. Thanks, and sorry
for the false (or tardy) alarm.
Ray
^ permalink raw reply
* Re: 2.6.24-rc5-mm1 -- inconsistent {in-softirq-W} -> {softirq-on-R} usage.
From: Herbert Xu @ 2007-12-15 7:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: miles.lane, linux-kernel, netdev, mingo, a.p.zijlstra
In-Reply-To: <20071214153633.5d8f609a.akpm@linux-foundation.org>
Andrew Morton <akpm@linux-foundation.org> wrote:
>
> I'd say you hit a networking locking bug and then when trying to report
> that bug, lockdep crashed.
>
> The networking bug looks to be around sock_i_ino()'s taking of
> sk_callback_lock with softirq's enabled. Perhaps this will fix it.
>
> diff -puN net/core/sock.c~a net/core/sock.c
> --- a/net/core/sock.c~a
> +++ a/net/core/sock.c
> @@ -1115,9 +1115,9 @@ int sock_i_uid(struct sock *sk)
> {
> int uid;
>
> - read_lock(&sk->sk_callback_lock);
> + read_lock_bh(&sk->sk_callback_lock);
> uid = sk->sk_socket ? SOCK_INODE(sk->sk_socket)->i_uid : 0;
> - read_unlock(&sk->sk_callback_lock);
> + read_unlock_bh(&sk->sk_callback_lock);
The callback lock is never taken for writing in BH context so
this shouldn't be needed.
Let's fix the lockdep checker first and then decide what's broken
in networking, OK?
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: Regression: Wireshark sees no packets in 2.6.24-rc3
From: Ray Lee @ 2007-12-15 7:09 UTC (permalink / raw)
To: Gabriel C; +Cc: Rafael J. Wysocki, netdev, LKML
In-Reply-To: <47633ECC.3020804@googlemail.com>
On Dec 14, 2007 6:41 PM, Gabriel C <nix.or.die@googlemail.com> wrote:
> Rafael J. Wysocki wrote:
> > On Friday, 14 of December 2007, Ray Lee wrote:
> >> tshark -i eth0, eth1, lo are all empty. Works under 2.6.23.0 just
> >> fine. A quick scan of the log between 2.6.24-rc3 and current tip
> >> (-rc5) doesn't show any obvious fixes, but then again, what do I know.
> >> I'll check current tip on the weekend when I'll have the luxury to
> >> have my main system down long enough for a test. Right now I'm kinda
> >> up against a deadline, but didn't want to leave it unreported. Should
> >> be easy for someone else to confirm or deny whether current tip has
> >> the problem.
> >
> > FYI, I have created a bugzilla entry for this issue at:
> > http://bugzilla.kernel.org/show_bug.cgi?id=9568
>
> Hmm what do you mean by empty ? it does not capturing anything on that interface ?
Correct, absolutely no traffic. So if it works for you, then either
it's something that got fixed between -rc3 and -rc5, or something odd
when I did a make oldconfig, I suppose. (Or because I'm on an x86-64
kernel?) Regardless, -rc5 is currently building, and I'll try it in
the morning.
> I do run -rc5-git with wireshark-0.99.6 and tshark -i eth0 or lo works here.
Excellent. Thank you for checking!
Rafael: I'll update the bugzilla as warranted after testing.
Ray
^ permalink raw reply
* Re: [patch 2/4] net: use mutex_is_locked() for ASSERT_RTNL()
From: Herbert Xu @ 2007-12-15 6:10 UTC (permalink / raw)
To: Andrew Morton; +Cc: David Miller, netdev
In-Reply-To: <20071214214418.0ecd5e67.akpm@linux-foundation.org>
On Fri, Dec 14, 2007 at 09:44:18PM -0800, Andrew Morton wrote:
>
> That sounds like a bug in mutex_trylock() to me.
I was relying on
http://kerneltrap.org/mailarchive/linux-netdev/2007/9/28/325129
which seems to be a bogus claim now that I actually look at the
source code. So in that case I'm OK with your patch as long as
it warns about hard IRQ usage.
Thanks,
--
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
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