Netdev List
 help / color / mirror / Atom feed
* Re: SO_REUSEPORT - can it be done in kernel?
From: Thomas Graf @ 2011-03-01 14:30 UTC (permalink / raw)
  To: Eric Dumazet, Herbert Xu, David Miller, rick.jones2, therbert,
	wsommerfeld
In-Reply-To: <20110301142235.GA10761@canuck.infradead.org>

On Tue, Mar 01, 2011 at 09:22:35AM -0500, Thomas Graf wrote:
> On Tue, Mar 01, 2011 at 03:06:59PM +0100, Eric Dumazet wrote:
> > Would be nice to cpu affine named to _not_ run on CPU11, just to
> > specialize it for TX completions and have softirq time percentage and
> > "perf top -C 11 " results

CPU 1 isolated as well (named running with mask 0,2-10)

----------------------------------------------------------------------------------------------------------------------
   PerfTop:     580 irqs/sec  kernel:100.0%  exact:  0.0% [1000Hz cpu-clock-msecs],  (all, CPU: 1)
----------------------------------------------------------------------------------------------------------------------

             samples  pcnt function                    DSO
             _______ _____ ___________________________ ___________________________________________________________

              283.00  9.2% get_rx_page_info            /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
              256.00  8.4% _raw_spin_unlock_irqrestore /lib/modules/2.6.38-rc5+/build/vmlinux                     
              190.00  6.2% be_poll_rx                  /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
              182.00  5.9% get_page_from_freelist      /lib/modules/2.6.38-rc5+/build/vmlinux                     
              157.00  5.1% intel_idle                  /lib/modules/2.6.38-rc5+/build/vmlinux                     
              143.00  4.7% __do_softirq                /lib/modules/2.6.38-rc5+/build/vmlinux                     
              133.00  4.3% sock_queue_rcv_skb          /lib/modules/2.6.38-rc5+/build/vmlinux                     
              133.00  4.3% __udp4_lib_lookup           /lib/modules/2.6.38-rc5+/build/vmlinux                     
              131.00  4.3% sk_run_filter               /lib/modules/2.6.38-rc5+/build/vmlinux                     
              114.00  3.7% getnstimeofday              /lib/modules/2.6.38-rc5+/build/vmlinux                     
              112.00  3.7% __alloc_skb                 /lib/modules/2.6.38-rc5+/build/vmlinux                     
              103.00  3.4% read_tsc                    /lib/modules/2.6.38-rc5+/build/vmlinux                     
              100.00  3.3% __netif_receive_skb         /lib/modules/2.6.38-rc5+/build/vmlinux                     
               95.00  3.1% udp_queue_rcv_skb           /lib/modules/2.6.38-rc5+/build/vmlinux                     
               82.00  2.7% sock_def_readable           /lib/modules/2.6.38-rc5+/build/vmlinux                     
               79.00  2.6% kmem_cache_alloc_node_trace /lib/modules/2.6.38-rc5+/build/vmlinux                     
               72.00  2.3% _raw_spin_lock              /lib/modules/2.6.38-rc5+/build/vmlinux                     
               67.00  2.2% __phys_addr                 /lib/modules/2.6.38-rc5+/build/vmlinux                     
               63.00  2.1% is_swiotlb_buffer           /lib/modules/2.6.38-rc5+/build/vmlinux                     
               51.00  1.7% __udp4_lib_rcv              /lib/modules/2.6.38-rc5+/build/vmlinux                     
               48.00  1.6% memcpy                      /lib/modules/2.6.38-rc5+/build/vmlinux                     
               47.00  1.5% ip_rcv                      /lib/modules/2.6.38-rc5+/build/vmlinux                     
               46.00  1.5% kmem_cache_alloc_node       /lib/modules/2.6.38-rc5+/build/vmlinux                     
               44.00  1.4% dma_issue_pending_all       /lib/modules/2.6.38-rc5+/build/vmlinux                     
               40.00  1.3% ip_route_input_common       /lib/modules/2.6.38-rc5+/build/vmlinux                     
               36.00  1.2% __alloc_pages_nodemask      /lib/modules/2.6.38-rc5+/build/vmlinux                     
               33.00  1.1% be_post_rx_frags            /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
               24.00  0.8% alloc_pages_current         /lib/modules/2.6.38-rc5+/build/vmlinux                     
               21.00  0.7% packet_rcv                  /lib/modules/2.6.38-rc5+/build/vmlinux                     
               20.00  0.7% local_bh_enable             /lib/modules/2.6.38-rc5+/build/vmlinux                     
               17.00  0.6% consume_skb                 /lib/modules/2.6.38-rc5+/build/vmlinux                     
               16.00  0.5% next_zones_zonelist         /lib/modules/2.6.38-rc5+/build/vmlinux                     
               14.00  0.5% selinux_socket_sock_rcv_skb /lib/modules/2.6.38-rc5+/build/vmlinux                     
               13.00  0.4% ip_local_deliver            /lib/modules/2.6.38-rc5+/build/vmlinux                     
               11.00  0.4% sk_filter                   /lib/modules/2.6.38-rc5+/build/vmlinux                     
               10.00  0.3% get_rps_cpu                 /lib/modules/2.6.38-rc5+/build/vmlinux                     
                9.00  0.3% native_read_tsc             /lib/modules/2.6.38-rc5+/build/vmlinux                     
                8.00  0.3% local_bh_disable            /lib/modules/2.6.38-rc5+/build/vmlinux                     
                8.00  0.3% eth_type_trans              /lib/modules/2.6.38-rc5+/build/vmlinux                     
                8.00  0.3% napi_complete               /lib/modules/2.6.38-rc5+/build/vmlinux                     
                7.00  0.2% netif_receive_skb           /lib/modules/2.6.38-rc5+/build/vmlinux                     
                7.00  0.2% dso__find_symbol            /usr/bin/perf                                              
                7.00  0.2% __kmalloc_node_track_caller /lib/modules/2.6.38-rc5+/build/vmlinux             

^ permalink raw reply

* Re: Bug inkvm_set_irq
From: Jean-Philippe Menil @ 2011-03-01 14:39 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, kvm, virtualization
In-Reply-To: <20110301070333.GA5972@redhat.com>

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

Le 01/03/2011 08:03, Michael S. Tsirkin a écrit :
> On Mon, Feb 28, 2011 at 11:34:16PM +0100, Jean-Philippe Menil wrote:
>> Hi,
>>
>> here is another trace with kvm.ko compiled with debug flags.
>>
>> the bug:
>> [12099.503414] BUG: unable to handle kernel paging request at
>> 000000000b6635e9
>> [12099.503462] IP: [<ffffffffa03ee877>] kvm_set_irq+0x37/0x140 [kvm]
>> [12099.503521] PGD 45d8d2067 PUD 45d58e067 PMD 0
>> [12099.503560] Oops: 0000 [#1] SMP
>> [12099.503591] last sysfs file:
>> /sys/devices/system/cpu/cpu11/cache/index2/shared_cpu_map
>> [12099.503641] CPU 0
>> [12099.503648] Modules linked in: netconsole configfs vhost_net
>> macvtap macvlan tun veth powernow_k8 mperf cpufreq_userspace
>> cpufreq_stats cpufreq_powersave cpufreq_ondemand freq_table
>> cpufreq_conservative fuse xt_physdev ip6t_LOG ip6table_filter
>> ip6_tables ipt_LOG xt_multiport xt_limit xt_tcpudp xt_state
>> iptable_filter ip_tables x_tables nf_conntrack_tftp nf_conntrack_ftp
>> nf_conntrack_ipv4 nf_defrag_ipv4 8021q bridge stp ext2 mbcache
>> dm_round_robin dm_multipath nf_conntrack_ipv6 nf_conntrack
>> nf_defrag_ipv6 kvm_amd kvm ipv6 snd_pcm snd_timer snd soundcore
>> snd_page_alloc shpchp pci_hotplug tpm_tis i2c_nforce2 tpm i2c_core
>> pcspkr evdev psmouse joydev tpm_bios processor ghes dcdbas hed
>> button serio_raw thermal_sys xfs exportfs dm_mod sg sr_mod cdrom
>> usbhid hid usb_storage ses sd_mod enclosure megaraid_sas ohci_hcd
>> lpfc scsi_transport_fc bnx2 scsi_tgt scsi_mod ehci_hcd [last
>> unloaded: scsi_wait_scan]
>> [12099.504277]
>> [12099.504302] Pid: 1742, comm: kworker/0:2 Not tainted
>> 2.6.37.2-dsiun-110105+ #2 Dell Inc. PowerEdge M605/0K543T
>> [12099.504373] RIP: 0010:[<ffffffffa03ee877>]  [<ffffffffa03ee877>]
>> kvm_set_irq+0x37/0x140 [kvm]
>> [12099.504444] RSP: 0018:ffff88045e013d00  EFLAGS: 00010246
>> [12099.504474] RAX: 000000000b6634c1 RBX: 0000000000000018 RCX:
>> 0000000000000001
>> [12099.504508] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
>> ffff880419b600c0
>> [12099.504541] RBP: ffff88045e013dd0 R08: ffff88045e012000 R09:
>> 0000000000000000
>> [12099.504575] R10: 0000000000000000 R11: 00000000ffffffff R12:
>> ffff880419b600c0
>> [12099.504609] R13: ffff880419b600c0 R14: ffffffffa03efaa0 R15:
>> 0000000000000001
>> [12099.504643] FS:  00007f3abaa05710(0000) GS:ffff88007f800000(0000)
>> knlGS:0000000000000000
>> [12099.504693] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>> [12099.504724] CR2: 000000000b6635e9 CR3: 000000045e2bc000 CR4:
>> 00000000000006f0
>> [12099.504757] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
>> 0000000000000000
>> [12099.504791] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
>> 0000000000000400
>> [12099.504825] Process kworker/0:2 (pid: 1742, threadinfo
>> ffkvm_set_irqff88045e012000, task ffff88045ffb0d60)
>> [12099.504874] Stack:
>> [12099.504897]  00000000000119c0 00000000000119c0 00000000000119c0
>> ffff88045ffb0d60
>> [12099.504953]  ffff88045ffb1010 ffff88045e013fd8 ffff88045ffb1018
>> ffff88045e012010
>> [12099.505009]  00000000000119c0 ffff88045e013fd8 00000000000119c0
>> 00000000000119c0
>> [12099.505065] Call Trace:
>> [12099.505099]  [<ffffffff813818ce>] ? common_interrupt+0xe/0x13
>> [12099.505145]  [<ffffffffa03efaa0>] ? irqfd_inject+0x0/0x50 [kvm]
>> [12099.505145]  [<ffffffffa03efaca>] irqfd_inject+0x2a/0x50 [kvm]
>> [12099.505145]  [<ffffffff8106b7bb>] process_one_work+0x11b/0x450
>> [12099.505145]  [<ffffffff8106bf37>] worker_thread+0x157/0x410
>> [12099.505145]  [<ffffffff8103a569>] ? __wake_up_common+0x59/0x90
>> [12099.505145]  [<ffffffff8106bde0>] ? worker_thread+0x0/0x410
>> [12099.505145]  [<ffffffff8106f996>] kthread+0x96/0xa0
>> [12099.505145]  [<ffffffff81003c64>] kernel_thread_helper+0x4/0x10
>> [12099.505145]  [<ffffffff8106f900>] ? kthread+0x0/0xa0
>> [12099.505145]  [<ffffffff81003c60>] ? kernel_thread_helper+0x0/0x10
>> [12099.505145] Code: 55 49 89 fd 41 54 53 89 d3 48 81 ec a8 00 00 00
>> 8b 15 a6 75 03 00 89 b5 3c ff ff ff 85 d2 0f 85 d5 00 00 00 49 8b 85
>> 58 24 00 00<3b>  98 28 01 00 00 73 61 89 db 48 8b 84 d8 30 01 00 00
>> 48 85 c0
>> [12099.505145] RIP  [<ffffffffa03ee877>] kvm_set_irq+0x37/0x140 [kvm]
>> [12099.505145]  RSP<ffff88045e013d00>
>> [12099.505145] CR2: 000000000b6635e9
>>
>>
>> markup_oops result:
>>
>> root@ayrshire:~# cat bug.txt | perl markup_oops.pl -m
>> /lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko
>> /boot/vmlinuz-2.6.37.2-dsiun-110105+
>> vmaoffset = 18446744072103034880 ffffffffa03ee841:	48 89 e5   	mov
>> %rsp,%rbp
>>   ffffffffa03ee844:	41 57                	push   %r15
>>   ffffffffa03ee846:	41 89 cf             	mov    %ecx,%r15d  |  %r15
>> =>  1  %ecx = 1
>>   ffffffffa03ee849:	41 56                	push   %r14        |  %r14
>> =>  ffffffffa03efaa0
>>   ffffffffa03ee84b:	41 55                	push   %r13
>>   ffffffffa03ee84d:	49 89 fd             	mov    %rdi,%r13   |  %edi
>> = ffff880419b600c0  %r13 =>  ffff880419b600c0
>>   ffffffffa03ee850:	41 54                	push   %r12        |  %r12
>> =>  ffff880419b600c0
>>   ffffffffa03ee852:	53                   	push   %rbx
>>   ffffffffa03ee853:	89 d3                	mov    %edx,%ebx   |  %ebx =>  18
>>   ffffffffa03ee855:	48 81 ec a8 00 00 00 	sub    $0xa8,%rsp
>>   ffffffffa03ee85c:	8b 15 00 00 00 00    	mov    0x0(%rip),%edx
>> # ffffffffa03ee862<kvm_set_irq+0x22>
>>   ffffffffa03ee862:	89 b5 3c ff ff ff    	mov    %esi,-0xc4(%rbp) |
>> %esi = 0
>>   ffffffffa03ee868:	85 d2                	test   %edx,%edx   |  %edx =>  0
>>   ffffffffa03ee86a:	0f 85 d5 00 00 00    	jne    ffffffffa03ee945
>> <kvm_set_irq+0x105>
>>   ffffffffa03ee870:	49 8b 85 58 24 00 00 	mov    0x2458(%r13),%rax |
>> %eax =>  b6634c1  %r13 = ffff880419b600c0
>> *ffffffffa03ee877:	3b 98 28 01 00 00    	cmp    0x128(%rax),%ebx |
>> %eax = b6634c1  %ebx = 18<--- faulting instruction
>>   ffffffffa03ee87d:	73 61                	jae    ffffffffa03ee8e0
>> <kvm_set_irq+0xa0>
>>   ffffffffa03ee87f:	89 db                	mov    %ebx,%ebx
>>   ffffffffa03ee881:	48 8b 84 d8 30 01 00 	mov    0x130(%rax,%rbx,8),%rax
>>   ffffffffa03ee888:	00
>>   ffffffffa03ee889:	48 85 c0             	test   %rax,%rax
>>   ffffffffa03ee88c:	74 52                	je     ffffffffa03ee8e0
>> <kvm_set_irq+0xa0>
>>   ffffffffa03ee88e:	48 8d 95 40 ff ff ff 	lea    -0xc0(%rbp),%rdx
>>   ffffffffa03ee895:	31 db                	xor    %ebx,%ebx
>>   ffffffffa03ee897:	48 8b 08             	mov    (%rax),%rcx
>>   ffffffffa03ee89a:	83 c3 01             	add    $0x1,%ebx
>>   ffffffffa03ee89d:	0f 18 09             	prefetcht0 (%rcx)
>>   ffffffffa03ee8a0:	48 8b 48 e0          	mov    -0x20(%rax),%rcx
>>   ffffffffa03ee8a4:	48 89 0a             	mov    %rcx,(%rdx)
>>   ffffffffa03ee8a7:	48 8b 48 e8          	mov    -0x18(%rax),%rcx
>>   ffffffffa03ee8ab:	48 89 4a 08          	mov    %rcx,0x8(%rdx)
>>   ffffffffa03ee8af:	48 8b 48 f0          	mov    -0x10(%rax),%rcx
>>   ffffffffa03ee8b3:	48 89 4a 10          	mov    %rcx,0x10(%rdx)
>>   ffffffffa03ee8b7:	48 8b 48 f8          	mov    -0x8(%rax),%rcx
>>   ffffffffa03ee8bb:	48 89 4a 18          	mov    %rcx,0x18(%rdx)
>>   ffffffffa03ee8bf:	48 8b 08             	mov    (%rax),%rcx
>>
>> The relvant part of objdump for kvm_set_irq:
>> root@ayrshire:~# objdump -ldS
>> /lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko>
>> dump.txt
>>
>> 0000000000006840<kvm_set_irq>:
>> kvm_set_irq():
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:148
>>      6840:       55                      push   %rbp
>>      6841:       48 89 e5                mov    %rsp,%rbp
>>      6844:       41 57                   push   %r15
>>      6846:       41 89 cf                mov    %ecx,%r15d
>>      6849:       41 56                   push   %r14
>>      684b:       41 55                   push   %r13
>>      684d:       49 89 fd                mov    %rdi,%r13
>>      6850:       41 54                   push   %r12
>>      6852:       53                      push   %rbx
>>      6853:       89 d3                   mov    %edx,%ebx
>>      6855:       48 81 ec a8 00 00 00    sub    $0xa8,%rsp
>> trace_kvm_set_irq():
>> /usr/src/GIT/linux-2.6-stable/include/trace/events/kvm.h:10
>>      685c:       8b 15 00 00 00 00       mov    0x0(%rip),%edx
>> # 6862<kvm_set_irq+0x22>
>> kvm_set_irq():
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:148
>>      6862:       89 b5 3c ff ff ff       mov    %esi,-0xc4(%rbp)
>> trace_kvm_set_irq():
>> /usr/src/GIT/linux-2.6-stable/include/trace/events/kvm.h:10
>>      6868:       85 d2                   test   %edx,%edx
>>      686a:       0f 85 d5 00 00 00       jne    6945<kvm_set_irq+0x105>
>> kvm_set_irq():
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:161
>>      6870:       49 8b 85 58 24 00 00    mov    0x2458(%r13),%rax
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
>>      6877:       3b 98 28 01 00 00       cmp    0x128(%rax),%ebx
>>      687d:       73 61                   jae    68e0<kvm_set_irq+0xa0>
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:163
>>      687f:       89 db                   mov    %ebx,%ebx
>>      6881:       48 8b 84 d8 30 01 00    mov    0x130(%rax,%rbx,8),%rax
>>      6888:       00
>>      6889:       48 85 c0                test   %rax,%rax
>>      688c:       74 52                   je     68e0<kvm_set_irq+0xa0>
>>      688e:       48 8d 95 40 ff ff ff    lea    -0xc0(%rbp),%rdx
>>      6895:       31 db                   xor    %ebx,%ebx
>>      6897:       48 8b 08                mov    (%rax),%rcx
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:164
>>      689a:       83 c3 01                add    $0x1,%ebx
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:163
>>      689d:       0f 18 09                prefetcht0 (%rcx)
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:164
>>      68a0:       48 8b 48 e0             mov    -0x20(%rax),%rcx
>>      68a4:       48 89 0a                mov    %rcx,(%rdx)
>>      68a7:       48 8b 48 e8             mov    -0x18(%rax),%rcx
>>      68ab:       48 89 4a 08             mov    %rcx,0x8(%rdx)
>>      68af:       48 8b 48 f0             mov    -0x10(%rax),%rcx
>>      68b3:       48 89 4a 10             mov    %rcx,0x10(%rdx)
>>      68b7:       48 8b 48 f8             mov    -0x8(%rax),%rcx
>>      68bb:       48 89 4a 18             mov    %rcx,0x18(%rdx)
>>      68bf:       48 8b 08                mov    (%rax),%rcx
>>      68c2:       48 89 4a 20             mov    %rcx,0x20(%rdx)
>>      68c6:       48 8b 48 08             mov    0x8(%rax),%rcx
>>      68ca:       48 89 4a 28             mov    %rcx,0x28(%rdx)
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:163
>>      68ce:       48 8b 00                mov    (%rax),%rax
>>      68d1:       48 83 c2 30             add    $0x30,%rdx
>>      68d5:       48 85 c0                test   %rax,%rax
>>      68d8:       75 bd                   jne    6897<kvm_set_irq+0x57>
>>      68da:       eb 06                   jmp    68e2<kvm_set_irq+0xa2>
>>      68dc:       0f 1f 40 00             nopl   0x0(%rax)
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
>>      68e0:       31 db                   xor    %ebx,%ebx
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:169
>>      68e2:       4c 8d b5 40 ff ff ff    lea    -0xc0(%rbp),%r14
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
>>      68e9:       41 bc ff ff ff ff       mov    $0xffffffff,%r12d
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:167
>>      68ef:       85 db                   test   %ebx,%ebx
>>      68f1:       74 3d                   je     6930<kvm_set_irq+0xf0>
>>      68f3:       83 eb 01                sub    $0x1,%ebx
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:169
>>      68f6:       44 89 f9                mov    %r15d,%ecx
>>      68f9:       8b 95 3c ff ff ff       mov    -0xc4(%rbp),%edx
>>      68ff:       48 63 c3                movslq %ebx,%rax
>>      6902:       4c 89 ee                mov    %r13,%rsi
>>      6905:       48 8d 04 40             lea    (%rax,%rax,2),%rax
>>      6909:       48 c1 e0 04             shl    $0x4,%rax
>>      690d:       49 8d 3c 06             lea    (%r14,%rax,1),%rdi
>>      6911:       ff 94 05 48 ff ff ff    callq  *-0xb8(%rbp,%rax,1)
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:170
>>      6918:       85 c0                   test   %eax,%eax
>>      691a:       78 d3                   js     68ef<kvm_set_irq+0xaf>
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:173
>>      691c:       45 85 e4                test   %r12d,%r12d
>>      691f:       ba 00 00 00 00          mov    $0x0,%edx
>>      6924:       44 0f 48 e2             cmovs  %edx,%r12d
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:167
>>      6928:       85 db                   test   %ebx,%ebx
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:173
>>      692a:       46 8d 24 20             lea    (%rax,%r12,1),%r12d
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:167
>>      692e:       75 c3                   jne    68f3<kvm_set_irq+0xb3>
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:177
>>      6930:       48 81 c4 a8 00 00 00    add    $0xa8,%rsp
>>      6937:       44 89 e0                mov    %r12d,%eax
>>      693a:       5b                      pop    %rbx
>>      693b:       41 5c                   pop    %r12
>>      693d:       41 5d                   pop    %r13
>>      693f:       41 5e                   pop    %r14
>>      6941:       41 5f                   pop    %r15
>>      6943:       c9                      leaveq
>>      6944:       c3                      retq
>> trace_kvm_set_irq():
>> /usr/src/GIT/linux-2.6-stable/include/trace/events/kvm.h:10
>>      6945:       4c 8b 25 00 00 00 00    mov    0x0(%rip),%r12
>> # 694c<kvm_set_irq+0x10c>
>>      694c:       4d 85 e4                test   %r12,%r12
>>      694f:       0f 84 1b ff ff ff       je     6870<kvm_set_irq+0x30>
>>      6955:       49 8b 04 24             mov    (%r12),%rax
>>      6959:       49 8b 7c 24 08          mov    0x8(%r12),%rdi
>>      695e:       49 83 c4 10             add    $0x10,%r12
>>      6962:       8b 8d 3c ff ff ff       mov    -0xc4(%rbp),%ecx
>>      6968:       44 89 fa                mov    %r15d,%edx
>>      696b:       89 de                   mov    %ebx,%esi
>>      696d:       ff d0                   callq  *%rax
>>      696f:       49 8b 04 24             mov    (%r12),%rax
>>      6973:       48 85 c0                test   %rax,%rax
>>      6976:       75 e1                   jne    6959<kvm_set_irq+0x119>
>>      6978:       e9 f3 fe ff ff          jmpq   6870<kvm_set_irq+0x30>
>> kvm_set_irq():
>>      697d:       0f 1f 00                nopl   (%rax)
>>
>> So, if i've read correctly, the offset is 0x6877 ?
>>
>> root@ayrshire:~# addr2line -e
>> /lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko
>> 0x6877
>> /usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
>>
>>
>> Is it the correct way to analyse this?
>>
>> Regards.
>
> Yes.  So we have:
>
>          irq_rt = rcu_dereference(kvm->irq_routing);
>
>>   ffffffffa03ee870:	49 8b 85 58 24 00 00 	mov    0x2458(%r13),%rax |
>> %eax =>  b6634c1  %r13 = ffff880419b600c0
>
>          if (irq<  irq_rt->nr_rt_entries)
>
>> *ffffffffa03ee877:	3b 98 28 01 00 00    	cmp    0x128(%rax),%ebx |
>> %eax = b6634c1  %ebx = 18<--- faulting instruction
>
> The problem then is that while the kvm pointer is
> ffff880419b600c0 which looks sane,
> the value we read from kvm->irq_routing is b6634c1 which
> does not make sense. When we dereference that, kaboom.
>
> Is the kvm pointer wrong or the memory corrupted?
> Try printing the kvm pointer during
> initialization, e.g. in kvm_vm_ioctl_create_vcpu,
> then and compare to markup_oops.
>
>
Hi,

so this time the bug is:

[17882.612303] BUG: unable to handle kernel paging request at 
0000000000002458
[17882.612342] IP: [<ffffffffa03898a0>] kvm_set_irq+0x30/0x140 [kvm]

markup_oops give me this:

root@ayrshire:~# cat bug-0103.txt | perl markup_oops.pl -m 
/lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko 
/boot/vmlinuz-2.6.37.2-dsiun-110105+
vmaoffset = 18446744072102621184 ffffffffa0389871:	48 89 e5 
   	mov    %rsp,%rbp
  ffffffffa0389874:	41 57                	push   %r15
  ffffffffa0389876:	41 89 cf             	mov    %ecx,%r15d  |  %r15 => 
1  %ecx = 1
  ffffffffa0389879:	41 56                	push   %r14        |  %r14 => 
ffffffffa038aad0
  ffffffffa038987b:	41 55                	push   %r13
  ffffffffa038987d:	49 89 fd             	mov    %rdi,%r13   |  %edi = 0 
  %r13 => 0
  ffffffffa0389880:	41 54                	push   %r12        |  %r12 => 0
  ffffffffa0389882:	53                   	push   %rbx
  ffffffffa0389883:	89 d3                	mov    %edx,%ebx   |  %ebx => 1a
  ffffffffa0389885:	48 81 ec a8 00 00 00 	sub    $0xa8,%rsp
  ffffffffa038988c:	8b 15 00 00 00 00    	mov    0x0(%rip),%edx        # 
ffffffffa0389892 <kvm_set_irq+0x22>
  ffffffffa0389892:	89 b5 3c ff ff ff    	mov    %esi,-0xc4(%rbp) | 
%esi = 0
  ffffffffa0389898:	85 d2                	test   %edx,%edx   |  %edx => 0
  ffffffffa038989a:	0f 85 d5 00 00 00    	jne    ffffffffa0389975 
<kvm_set_irq+0x105>
*ffffffffa03898a0:	49 8b 85 58 24 00 00 	mov    0x2458(%r13),%rax | 
%eax = 0  %r13 = 0 <--- faulting instruction
  ffffffffa03898a7:	3b 98 28 01 00 00    	cmp    0x128(%rax),%ebx
  ffffffffa03898ad:	73 61                	jae    ffffffffa0389910 
<kvm_set_irq+0xa0>
  ffffffffa03898af:	89 db                	mov    %ebx,%ebx
  ffffffffa03898b1:	48 8b 84 d8 30 01 00 	mov    0x130(%rax,%rbx,8),%rax
  ffffffffa03898b8:	00
  ffffffffa03898b9:	48 85 c0             	test   %rax,%rax
  ffffffffa03898bc:	74 52                	je     ffffffffa0389910 
<kvm_set_irq+0xa0>
  ffffffffa03898be:	48 8d 95 40 ff ff ff 	lea    -0xc0(%rbp),%rdx
  ffffffffa03898c5:	31 db                	xor    %ebx,%ebx
  ffffffffa03898c7:	48 8b 08             	mov    (%rax),%rcx
  ffffffffa03898ca:	83 c3 01             	add    $0x1,%ebx
  ffffffffa03898cd:	0f 18 09             	prefetcht0 (%rcx)
  ffffffffa03898d0:	48 8b 48 e0          	mov    -0x20(%rax),%rcx
  ffffffffa03898d4:	48 89 0a             	mov    %rcx,(%rdx)
  ffffffffa03898d7:	48 8b 48 e8          	mov    -0x18(%rax),%rcx
  ffffffffa03898db:	48 89 4a 08          	mov    %rcx,0x8(%rdx)
  ffffffffa03898df:	48 8b 48 f0          	mov    -0x10(%rax),%rcx
  ffffffffa03898e3:	48 89 4a 10          	mov    %rcx,0x10(%rdx)
  ffffffffa03898e7:	48 8b 48 f8          	mov    -0x8(%rax),%rcx
  ffffffffa03898eb:	48 89 4a 18          	mov    %rcx,0x18(%rdx)

wich correspond to offset 68a0 (from objdump):

kvm_set_irq():
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:161
     68a0:       49 8b 85 58 24 00 00    mov    0x2458(%r13),%rax
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:162
     68a7:       3b 98 28 01 00 00       cmp    0x128(%rax),%ebx

root@ayrshire:~# addr2line -e 
/lib/modules/2.6.37.2-dsiun-110105+/kernel/arch/x86/kvm/kvm.ko 0x68a0
/usr/src/GIT/linux-2.6-stable/arch/x86/kvm/../../../virt/kvm/irq_comm.c:161

So here kvm->irq_routing is null.

How can it be?

Regards.

[-- Attachment #2: jean-philippe_menil.vcf --]
[-- Type: text/x-vcard, Size: 361 bytes --]

begin:vcard
fn:Jean-Philippe Menil
n:Menil;Jean-Philippe
org;quoted-printable:Universit=C3=A9 de Nantes;IRTS - DSI
adr;quoted-printable:;;2 rue de la Houssini=C3=A8re;Nantes;Loire-Atlantique;44382;France
email;internet:jean-philippe.menil@univ-nantes.fr
title;quoted-printable:Administrateur R=C3=A9seau
url:http://www.cri.univ-nantes.fr
version:2.1
end:vcard


^ permalink raw reply

* Re: [PATCH]drivers:isdn:istream.c Fix typo pice to piece
From: Jiri Kosina @ 2011-03-01 14:47 UTC (permalink / raw)
  To: Justin P. Mattock; +Cc: mac, isdn, netdev
In-Reply-To: <alpine.LNX.2.00.1103011521160.32580@pobox.suse.cz>

On Tue, 1 Mar 2011, Jiri Kosina wrote:

> > The below patch changes a typo "pice" to "piece"
> > 
> > Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
> > 
> > ---
> >  drivers/isdn/hardware/eicon/istream.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/isdn/hardware/eicon/istream.c b/drivers/isdn/hardware/eicon/istream.c
> > index 18f8798..7bd5baa 100644
> > --- a/drivers/isdn/hardware/eicon/istream.c
> > +++ b/drivers/isdn/hardware/eicon/istream.c
> > @@ -62,7 +62,7 @@ void diva_xdi_provide_istream_info (ADAPTER* a,
> >    stream interface.
> >    If synchronous service was requested, then function
> >    does return amount of data written to stream.
> > -  'final' does indicate that pice of data to be written is
> > +  'final' does indicate that piece of data to be written is
> >    final part of frame (necessary only by structured datatransfer)
> >    return  0 if zero lengh packet was written
> >    return -1 if stream is full
> 
> Applied.

Bah, I had broken local clone of linux-next, I see it's already there.

Sorry for the noise.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

^ permalink raw reply

* Re: SO_REUSEPORT - can it be done in kernel?
From: Eric Dumazet @ 2011-03-01 14:52 UTC (permalink / raw)
  To: Thomas Graf
  Cc: Herbert Xu, David Miller, rick.jones2, therbert, wsommerfeld,
	daniel.baluta, netdev
In-Reply-To: <20110301143050.GB10761@canuck.infradead.org>

Le mardi 01 mars 2011 à 09:30 -0500, Thomas Graf a écrit :
> On Tue, Mar 01, 2011 at 09:22:35AM -0500, Thomas Graf wrote:
> > On Tue, Mar 01, 2011 at 03:06:59PM +0100, Eric Dumazet wrote:
> > > Would be nice to cpu affine named to _not_ run on CPU11, just to
> > > specialize it for TX completions and have softirq time percentage and
> > > "perf top -C 11 " results
> 
> CPU 1 isolated as well (named running with mask 0,2-10)
> 
> ----------------------------------------------------------------------------------------------------------------------
>    PerfTop:     580 irqs/sec  kernel:100.0%  exact:  0.0% [1000Hz cpu-clock-msecs],  (all, CPU: 1)
> ----------------------------------------------------------------------------------------------------------------------
> 
>              samples  pcnt function                    DSO
>              _______ _____ ___________________________ ___________________________________________________________
> 
>               283.00  9.2% get_rx_page_info            /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
>               256.00  8.4% _raw_spin_unlock_irqrestore /lib/modules/2.6.38-rc5+/build/vmlinux                     
>               190.00  6.2% be_poll_rx                  /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
>               182.00  5.9% get_page_from_freelist      /lib/modules/2.6.38-rc5+/build/vmlinux                     
>               157.00  5.1% intel_idle                  /lib/modules/2.6.38-rc5+/build/vmlinux                     
>               143.00  4.7% __do_softirq                /lib/modules/2.6.38-rc5+/build/vmlinux                     
>               133.00  4.3% sock_queue_rcv_skb          /lib/modules/2.6.38-rc5+/build/vmlinux                     
>               133.00  4.3% __udp4_lib_lookup           /lib/modules/2.6.38-rc5+/build/vmlinux                     
>               131.00  4.3% sk_run_filter               /lib/modules/2.6.38-rc5+/build/vmlinux   

sk_run_filter ? Do you have a packet filter running ?



^ permalink raw reply

* source route ignored in favor of local interface
From: Joe Buehler @ 2011-03-01 14:57 UTC (permalink / raw)
  To: netdev

I have a LINUX box talking on many different networks at the same time.  Since
IP addresses on the networks can overlap (they are completely different
networks) we use source routing and NAT to get packets going in and out of the
right interfaces.

Everything works great, with one exception.  If I try to talk to a remote host
that happens to have the same IP address as one of my interfaces, the kernel
routes the packet to the local interface.

It looks to me as though the problem is that the source routes are lower
priority than the local interfaces.  As soon as the kernel sees a destination
address that matches a local interface it routes to the local interface and pays
no attention to the source route.

I consider this a bug.  Is there any way to change this behavior?

The kernel involved is 2.6.27.7, with patches from Cavium for support of their
hardware.

Joe Buehler



^ permalink raw reply

* Re: SO_REUSEPORT - can it be done in kernel?
From: Thomas Graf @ 2011-03-01 15:07 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Herbert Xu, David Miller, rick.jones2, therbert, wsommerfeld,
	daniel.baluta, netdev
In-Reply-To: <1298991160.3284.108.camel@edumazet-laptop>

On Tue, Mar 01, 2011 at 03:52:40PM +0100, Eric Dumazet wrote:
> Le mardi 01 mars 2011 à 09:30 -0500, Thomas Graf a écrit :
> > On Tue, Mar 01, 2011 at 09:22:35AM -0500, Thomas Graf wrote:
> > > On Tue, Mar 01, 2011 at 03:06:59PM +0100, Eric Dumazet wrote:
> > > > Would be nice to cpu affine named to _not_ run on CPU11, just to
> > > > specialize it for TX completions and have softirq time percentage and
> > > > "perf top -C 11 " results
> > 
> > CPU 1 isolated as well (named running with mask 0,2-10)
> > 
> > ----------------------------------------------------------------------------------------------------------------------
> >    PerfTop:     580 irqs/sec  kernel:100.0%  exact:  0.0% [1000Hz cpu-clock-msecs],  (all, CPU: 1)
> > ----------------------------------------------------------------------------------------------------------------------
> > 
> >              samples  pcnt function                    DSO
> >              _______ _____ ___________________________ ___________________________________________________________
> > 
> >               283.00  9.2% get_rx_page_info            /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
> >               256.00  8.4% _raw_spin_unlock_irqrestore /lib/modules/2.6.38-rc5+/build/vmlinux                     
> >               190.00  6.2% be_poll_rx                  /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
> >               182.00  5.9% get_page_from_freelist      /lib/modules/2.6.38-rc5+/build/vmlinux                     
> >               157.00  5.1% intel_idle                  /lib/modules/2.6.38-rc5+/build/vmlinux                     
> >               143.00  4.7% __do_softirq                /lib/modules/2.6.38-rc5+/build/vmlinux                     
> >               133.00  4.3% sock_queue_rcv_skb          /lib/modules/2.6.38-rc5+/build/vmlinux                     
> >               133.00  4.3% __udp4_lib_lookup           /lib/modules/2.6.38-rc5+/build/vmlinux                     
> >               131.00  4.3% sk_run_filter               /lib/modules/2.6.38-rc5+/build/vmlinux   
> 
> sk_run_filter ? Do you have a packet filter running ?

dhclient was running. With dhclient killed:

----------------------------------------------------------------------------------------------------------------------
   PerfTop:     726 irqs/sec  kernel:99.9%  exact:  0.0% [1000Hz cpu-clock-msecs],  (all, CPU: 1)
----------------------------------------------------------------------------------------------------------------------

             samples  pcnt function                    DSO
             _______ _____ ___________________________ ___________________________________________________________

              472.00 10.6% get_rx_page_info            /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
              419.00  9.4% _raw_spin_unlock_irqrestore /lib/modules/2.6.38-rc5+/build/vmlinux                     
              280.00  6.3% be_poll_rx                  /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
              259.00  5.8% get_page_from_freelist      /lib/modules/2.6.38-rc5+/build/vmlinux                     
              248.00  5.6% __do_softirq                /lib/modules/2.6.38-rc5+/build/vmlinux                     
              238.00  5.4% intel_idle                  /lib/modules/2.6.38-rc5+/build/vmlinux                     
              204.00  4.6% sock_queue_rcv_skb          /lib/modules/2.6.38-rc5+/build/vmlinux                     
              189.00  4.3% __udp4_lib_lookup           /lib/modules/2.6.38-rc5+/build/vmlinux                     
              178.00  4.0% getnstimeofday              /lib/modules/2.6.38-rc5+/build/vmlinux                     
              169.00  3.8% __alloc_skb                 /lib/modules/2.6.38-rc5+/build/vmlinux                     
              144.00  3.2% read_tsc                    /lib/modules/2.6.38-rc5+/build/vmlinux                     
              143.00  3.2% sock_def_readable           /lib/modules/2.6.38-rc5+/build/vmlinux                     
              138.00  3.1% udp_queue_rcv_skb           /lib/modules/2.6.38-rc5+/build/vmlinux                     
              115.00  2.6% kmem_cache_alloc_node_trace /lib/modules/2.6.38-rc5+/build/vmlinux                     
              114.00  2.6% __netif_receive_skb         /lib/modules/2.6.38-rc5+/build/vmlinux                     
              109.00  2.5% _raw_spin_lock              /lib/modules/2.6.38-rc5+/build/vmlinux                     
              100.00  2.3% is_swiotlb_buffer           /lib/modules/2.6.38-rc5+/build/vmlinux                     
               90.00  2.0% __phys_addr                 /lib/modules/2.6.38-rc5+/build/vmlinux                     
               82.00  1.8% __udp4_lib_rcv              /lib/modules/2.6.38-rc5+/build/vmlinux                     
               80.00  1.8% kmem_cache_alloc_node       /lib/modules/2.6.38-rc5+/build/vmlinux                     
               73.00  1.6% ip_route_input_common       /lib/modules/2.6.38-rc5+/build/vmlinux                     
               60.00  1.4% memcpy                      /lib/modules/2.6.38-rc5+/build/vmlinux                     
               59.00  1.3% dma_issue_pending_all       /lib/modules/2.6.38-rc5+/build/vmlinux                     
               58.00  1.3% ip_rcv                      /lib/modules/2.6.38-rc5+/build/vmlinux                     
               57.00  1.3% be_post_rx_frags            /lib/modules/2.6.38-rc5+/kernel/drivers/net/benet/be2net.ko
               49.00  1.1% __alloc_pages_nodemask      /lib/modules/2.6.38-rc5+/build/vmlinux                     
               45.00  1.0% alloc_pages_current         /lib/modules/2.6.38-rc5+/build/vmlinux                     
               27.00  0.6% get_rps_cpu                 /lib/modules/2.6.38-rc5+/build/vmlinux                     
               23.00  0.5% napi_complete               /lib/modules/2.6.38-rc5+/build/vmlinux                     
               22.00  0.5% ip_local_deliver            /lib/modules/2.6.38-rc5+/build/vmlinux                     
               18.00  0.4% selinux_socket_sock_rcv_skb /lib/modules/2.6.38-rc5+/build/vmlinux                     
               17.00  0.4% native_read_tsc             /lib/modules/2.6.38-rc5+/build/vmlinux                     
               16.00  0.4% local_bh_enable             /lib/modules/2.6.38-rc5+/build/vmlinux                     
               16.00  0.4% next_zones_zonelist         /lib/modules/2.6.38-rc5+/build/vmlinux                     
               14.00  0.3% sk_filter                   /lib/modules/2.6.38-rc5+/build/vmlinux                     
               13.00  0.3% eth_type_trans              /lib/modules/2.6.38-rc5+/build/vmlinux                     
               10.00  0.2% __kmalloc_node_track_caller /lib/modules/2.6.38-rc5+/build/vmlinux                     
               10.00  0.2% _raw_spin_lock_irqsave      /lib/modules/2.6.38-rc5+/build/vmlinux                     
                9.00  0.2% raw_local_deliver           /lib/modules/2.6.38-rc5+/build/vmlinux                     
                8.00  0.2% __udp_queue_rcv_skb         /lib/modules/2.6.38-rc5+/build/vmlinux                     
                8.00  0.2% netif_receive_skb           /lib/modules/2.6.38-rc5+/build/vmlinux                     
                8.00  0.2% ip_queue_rcv_skb            /lib/modules/2.6.38-rc5+/build/vmlinux                     
                7.00  0.2% net_rx_action               /lib/modules/2.6.38-rc5+/build/vmlinux                     
                6.00  0.1% swiotlb_map_page            /lib/modules/2.6.38-rc5+/build/vmlinux                     
                6.00  0.1% __sk_mem_schedule           /lib/modules/2.6.38-rc5+/build/vmlinux                     
                6.00  0.1% dso__find_symbol            /usr/bin/perf                                              
                6.00  0.1% __netdev_alloc_skb          /lib/modules/2.6.38-rc5+/build/vmlinux              

^ permalink raw reply

* Re: [PATCH 2/4] slub,rcu: don't assume the size of struct rcu_head
From: Christoph Lameter @ 2011-03-01 15:11 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Lai Jiangshan, Ingo Molnar, Paul E. McKenney, Eric Dumazet,
	David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
In-Reply-To: <AANLkTimXy2Yaj+NTDMNTWuLqHHfKZJhVDpeXj3CfMvBf@mail.gmail.com>

On Tue, 1 Mar 2011, Pekka Enberg wrote:

> The SLAB and SLUB patches are fine by me if there are going to be real
> users for this. Christoph, Paul?

The solution is a bit overkill. It would be much simpler to add a union to
struct page that has lru and the rcu in there similar things can be done
for SLAB and the network layer. A similar issue already exists for the
spinlock in struct page. Lets follow the existing way of handling this.

Struct page may be larger for debugging purposes already because of the
need for extended spinlock data.

^ permalink raw reply

* Re: [patch net-next-2.6] bonding: remove skb_share_check in handle_frame
From: Changli Gao @ 2011-03-01 15:12 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, fubar, eric.dumazet, nicolas.2p.debian, andy
In-Reply-To: <20110301092907.GG2855@psychotron.redhat.com>

On Tue, Mar 1, 2011 at 5:29 PM, Jiri Pirko <jpirko@redhat.com> wrote:
> Unapplicable, sorry (wrong branch :(). Here's corrected patch:
>
> Subject: [PATCH net-next-2.6 v2] bonding: remove skb_share_check in handle_frame
>
> No need to do share check here.
>

I don't think so. Although you avoid netif_rx(), you can't avoid
ptype_all handlers. In fact, all the RX handlers should has this
check(), if they may modify the skb.


-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* [PATCH 1/2 net-next][v2] bonding: fix incorrect transmit queue offset
From: Andy Gospodarek @ 2011-03-01 15:31 UTC (permalink / raw)
  To: Phil Oester; +Cc: David Miller, bhutchings, andy, netdev, fubar
In-Reply-To: <20110225225636.GA18792@linuxace.com>

On Fri, Feb 25, 2011 at 02:56:36PM -0800, Phil Oester wrote:
> On Wed, Feb 23, 2011 at 03:54:51PM -0800, David Miller wrote:
> > From: Ben Hutchings <bhutchings@solarflare.com>
> > Date: Wed, 23 Feb 2011 23:37:49 +0000
> > 
> > > On Wed, 2011-02-23 at 15:13 -0800, David Miller wrote:
> > >> From: Phil Oester <kernel@linuxace.com>
> > >> Date: Wed, 23 Feb 2011 15:08:44 -0800
> > >> 
> > >> > On Wed, Feb 23, 2011 at 02:42:49PM -0500, Andy Gospodarek wrote:
> > >> >> +     while (txq >= dev->real_num_tx_queues) {
> > >> >> +             /* let the user know if we do not have enough tx queues */
> > >> >> +             if (net_ratelimit())
> > >> >> +                     pr_warning("%s selects invalid tx queue %d.  Consider"
> > >> >> +                                " setting module option tx_queues > %d.",
> > >> >> +                                dev->name, txq, dev->real_num_tx_queues);
> > >> >> +             txq -= dev->real_num_tx_queues;
> > >> >> +     }
> > >> > 
> > >> > Think this would be better as a WARN_ONCE, as otherwise syslog will still
> > >> > get flooded with this - even when ratelimited.  See get_rps_cpu in 
> > >> > net/core/dev.c as an example.o
> > >> 
> > >> Agreed.
> > > 
> > > This shouldn't WARN at all.  It is perfectly valid (though non-optimal)
> > > to have different numbers of queues on two different multiqueue devices.
> > 
> > That's also a good point.
> 
> The patch works as expected.  Do we have any agreement on a final version?
> 

Thanks for the testing, Phil.

I'm in favor of this patch as it does alert the admin that bonding may
not have enough default queues, but it is not as verbose (backtrace et
al) and likely to create bug reports as a message from WARN_ON.

Signed-off-by: Andy Gospodarek <andy@greyhouse.net>

---
 drivers/net/bonding/bond_main.c |   26 +++++++++++++++++++-------
 1 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 584f97b..acc05d6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4643,15 +4643,27 @@ out:
 	return res;
 }
 
+/*
+ * This helper function exists to help dev_pick_tx get the correct
+ * destination queue.  Using a helper function skips the a call to
+ * skb_tx_hash and will put the skbs in the queue we expect on their
+ * way down to the bonding driver.
+ */
 static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
 {
-	/*
-	 * This helper function exists to help dev_pick_tx get the correct
-	 * destination queue.  Using a helper function skips the a call to
-	 * skb_tx_hash and will put the skbs in the queue we expect on their
-	 * way down to the bonding driver.
-	 */
-	return skb->queue_mapping;
+	u16 txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
+
+	if (txq >= dev->real_num_tx_queues) {
+		/* let the user know if we do not have enough tx queues */
+		if (net_ratelimit())
+			pr_warning("%s selects invalid tx queue %d.  Consider"
+				   " setting module option tx_queues > %d.",
+				   dev->name, txq, dev->real_num_tx_queues);
+		do
+			txq -= dev->real_num_tx_queues;
+		while (txq >= dev->real_num_tx_queues);
+	}
+	return txq;
 }
 
 static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev)

^ permalink raw reply related

* Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure
From: Stuart Menefy @ 2011-03-01 15:20 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Peppe CAVALLARO, linux-sh@vger.kernel.org, netdev@vger.kernel.org,
	John Stultz, Thomas Gleixner, linux-kernel@vger.kernel.org
In-Reply-To: <201102241820.55873.arnd@arndb.de>

Hi Arnd

Thanks for the comments.

On 24/02/11 17:20, Arnd Bergmann wrote:
> On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
>> From: Stuart Menefy <stuart.menefy@st.com>
>>
>> Many devices targeted at the embedded market provide a number of
>> generic timers which are capable of generating interrupts at a
>> requested rate. These can then be used in the implementation of drivers
>> for other peripherals which require a timer interrupt, without having
>> to provide an additional timer as part of that peripheral.
>>
>> A code provides a simple abstraction layer which allows a timer to be
>> registered, and for a driver to request a timer.
>>
>> Currently this doesn't provide any of the additional information, such
>> as precision or position in clock framework which might be required
>> for a fully featured driver.
> 
> This code should probably be discussed on a more broader
> platform than the netdev and linux-sh mailing lists,
> as the scope is neither sh nor network specific.
> 
> You should at least add linux-kernel@vger.kernel.org, possibly
> also linux-arch@vger.kernel.org.
> 
> Further, John and Thomas are responsible for the timekeeping
> infrastructure, and they are probably interested in this
> as well.
> 
> Why is this code useful to you? In the scenarios I've seen, the
> board can always assign a timer to a specific device in a fixed
> way that can be describe in a board file or device tree.

What we were trying to do was separate the code which actually manipulates
the timer hardware from the code which wants that timer service. In this
case it was a network device driver which is used on multiple SoC devices,
while the timer hardware tends to differ from device to device.

The other user of this code which we have is an OProfile driver, which
with this change can now be independent of the hardware it is running on,
while the previous version manipulated the timer hardware directly.

> Also, what is the difference between this and clkdev?

clkdev can be used to find a struct clk, which is fine if you just want to
read the time. In this instance we want to get interrupts from the timer
hardware, which isn't supported by the clk infrastructure.

If anything this duplicates clockevents. The main reason for not using
clockevents was that nobody else does! Currently clockevents are
used strictly for time keeping within the kernel, and most architectures
only register those which are intended to be used for this purpose.
We felt a bit nervous about adding code to register all the device's timers
as clockevents, and having the network device driver pick up one of those
for its own use.

>> Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
>> Hacked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> ---
>>  drivers/clocksource/Makefile       |    1 +
>>  drivers/clocksource/generictimer.c |   60 ++++++++++++++++++++++++++++++++++++
>>  include/linux/generictimer.h       |   41 ++++++++++++++++++++++++
>>  3 files changed, 102 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/clocksource/generictimer.c
>>  create mode 100644 include/linux/generictimer.h
> 
> I don't think it fits that well into the drivers/clocksource directory,
> because you don't actually register a struct clock_event_device or
> struct clocksource.

True. The intent was that this would be a third interface onto timer
hardware, alongside clocksources and clockevents.

> I don't know if this could also be merged with the clocksource infrastructure,
> but your code currently doesn't do that.

It would probably be clockevent rather than clocksource, but it could be if
people felt that was a better way to go.

>> +struct generic_timer *generic_timer_claim(void (*handler) (void *), void *data)
>> +{
>> +	struct generic_timer *gt = NULL;
>> +
>> +	if (!handler) {
>> +		pr_err("%s: invalid handler\n", __func__);
>> +		return NULL;
>> +	}
>> +
>> +	mutex_lock(&gt_mutex);
>> +	if (!list_empty(&gt_list)) {
>> +		struct list_head *list = gt_list.next;
>> +		list_del(list);
>> +		gt = container_of(list, struct generic_timer, list);
>> +	}
>> +	mutex_unlock(&gt_mutex);
>> +
>> +	if (!gt)
>> +		return NULL;
>> +
>> +	/* Prepare the new handler */
>> +	gt->priv_handler = handler;
>> +	gt->data = data;
>> +
>> +	return gt;
>> +}
> 
> This does not seem very generic. You put timers into the list and take
> them out again, but don't have any way to deal with timers that match
> specific purposes. It obviously works for your specific use case where
> you register exactly one timer, and use that in exactly one driver.
> 
> If more drivers were converted to generic_timer, which is obviously
> the intention, then you might have a situation with very different
> timers (fixed/variable tick, high/low frequencies, accurate/inaccurate),
> or you might have fewer timers than users.

Agreed, this was a first 'keep it simple' pass, maybe its too simple.

^ permalink raw reply

* [PATCH net-next-2.6] benet: use GFP_KERNEL allocations when possible
From: Eric Dumazet @ 2011-03-01 15:48 UTC (permalink / raw)
  To: Ajit Khaparde; +Cc: netdev

Extend be_alloc_pages() with a gfp parameter, so that we use GFP_KERNEL
allocations instead of GFP_ATOMIC when not running in softirq context.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/benet/be_main.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 0bdccb1..ef66dc6 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -1169,20 +1169,20 @@ static inline void be_rx_compl_reset(struct be_eth_rx_compl *rxcp)
 	rxcp->dw[offsetof(struct amap_eth_rx_compl, valid) / 32] = 0;
 }
 
-static inline struct page *be_alloc_pages(u32 size)
+static inline struct page *be_alloc_pages(u32 size, gfp_t gfp)
 {
-	gfp_t alloc_flags = GFP_ATOMIC;
 	u32 order = get_order(size);
+
 	if (order > 0)
-		alloc_flags |= __GFP_COMP;
-	return  alloc_pages(alloc_flags, order);
+		gfp |= __GFP_COMP;
+	return  alloc_pages(gfp, order);
 }
 
 /*
  * Allocate a page, split it to fragments of size rx_frag_size and post as
  * receive buffers to BE
  */
-static void be_post_rx_frags(struct be_rx_obj *rxo)
+static void be_post_rx_frags(struct be_rx_obj *rxo, gfp_t gfp)
 {
 	struct be_adapter *adapter = rxo->adapter;
 	struct be_rx_page_info *page_info_tbl = rxo->page_info_tbl;
@@ -1196,7 +1196,7 @@ static void be_post_rx_frags(struct be_rx_obj *rxo)
 	page_info = &rxo->page_info_tbl[rxq->head];
 	for (posted = 0; posted < MAX_RX_POST && !page_info->page; posted++) {
 		if (!pagep) {
-			pagep = be_alloc_pages(adapter->big_page_size);
+			pagep = be_alloc_pages(adapter->big_page_size, gfp);
 			if (unlikely(!pagep)) {
 				rxo->stats.rx_post_fail++;
 				break;
@@ -1753,7 +1753,7 @@ static int be_poll_rx(struct napi_struct *napi, int budget)
 
 	/* Refill the queue */
 	if (atomic_read(&rxo->q.used) < RX_FRAGS_REFILL_WM)
-		be_post_rx_frags(rxo);
+		be_post_rx_frags(rxo, GFP_ATOMIC);
 
 	/* All consumed */
 	if (work_done < budget) {
@@ -1890,7 +1890,7 @@ static void be_worker(struct work_struct *work)
 
 		if (rxo->rx_post_starved) {
 			rxo->rx_post_starved = false;
-			be_post_rx_frags(rxo);
+			be_post_rx_frags(rxo, GFP_KERNEL);
 		}
 	}
 	if (!adapter->ue_detected && !lancer_chip(adapter))
@@ -2138,7 +2138,7 @@ static int be_open(struct net_device *netdev)
 	u16 link_speed;
 
 	for_all_rx_queues(adapter, rxo, i) {
-		be_post_rx_frags(rxo);
+		be_post_rx_frags(rxo, GFP_KERNEL);
 		napi_enable(&rxo->rx_eq.napi);
 	}
 	napi_enable(&tx_eq->napi);



^ permalink raw reply related

* Re: [PATCH 2/3] [RFC] Changes for MQ virtio-net
From: Krishna Kumar2 @ 2011-03-01 16:02 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: anthony, arnd, avi, davem, eric.dumazet, horms, kvm, netdev,
	rusty
In-Reply-To: <20110228094320.GB28006@redhat.com>

"Michael S. Tsirkin" <mst@redhat.com> wrote on 02/28/2011 03:13:20 PM:

Thank you once again for your feedback on both these patches.
I will send the qemu patch tomorrow. I will also send the next
version incorporating these suggestions once we finalize some
minor points.

> Overall looks good.
> The numtxqs meaning the number of rx queues needs some cleanup.
> init/cleanup routines need more symmetry.
> Error handling on setup also seems slightly buggy or at least
asymmetrical.
> Finally, this will use up a large number of MSI vectors,
> while TX interrupts mostly stay unused.
>
> Some comments below.
>
> > +/* Maximum number of individual RX/TX queues supported */
> > +#define VIRTIO_MAX_TXQS		 		 16
> > +
>
> This also does not seem to belong in the header.

Both virtio-net and vhost need some check to make sure very
high values are not passed by userspace. Is this not required?

> > +#define VIRTIO_NET_F_NUMTXQS		 21		 /* Device supports multiple
TX queue */
>
> VIRTIO_NET_F_MULTIQUEUE ?

Yes, that's a better name.

> > @@ -34,6 +38,8 @@ struct virtio_net_config {
> >  		 __u8 mac[6];
> >  		 /* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
> >  		 __u16 status;
> > +		 /* number of RX/TX queues */
> > +		 __u16 numtxqs;
>
> The interface here is a bit ugly:
> - this is really both # of tx and rx queues but called numtxqs
> - there's a hardcoded max value
> - 0 is assumed to be same as 1
> - assumptions above are undocumented.
>
> One way to address this could be num_queue_pairs, and something like
> 		 /* The actual number of TX and RX queues is num_queue_pairs +
1 each. */
> 		 __u16 num_queue_pairs;
> (and tweak code to match).
>
> Alternatively, have separate registers for the number of tx and rx
queues.

OK, so virtio_net_config has num_queue_pairs, and this gets converted to
numtxqs in virtnet_info?

> > +struct virtnet_info {
> > +		 struct send_queue **sq;
> > +		 struct receive_queue **rq;
> > +
> > +		 /* read-mostly variables */
> > +		 int numtxqs ____cacheline_aligned_in_smp;
>
> Why do you think this alignment is a win?

Actually this code was from the earlier patchset (MQ TX only) where
the layout was different. Now rq and sq are allocated as follows:
	vi->sq = kzalloc(numtxqs * sizeof(*vi->sq), GFP_KERNEL);
	for (i = 0; i < numtxqs; i++) {
		vi->sq[i] = kzalloc(sizeof(*vi->sq[i]), GFP_KERNEL);
Since the two pointers becomes read-only during use, there is no cache
line dirty'ing.  I will remove this directive.

> > +/*
> > + * Note for 'qnum' below:
> > + *		 first 'numtxqs' vqs are RX, next 'numtxqs' vqs are TX.
> > + */
>
> Another option to consider is to have them RX,TX,RX,TX:
> this way vq->queue_index / 2 gives you the
> queue pair number, no need to read numtxqs. On the other hand, it makes
the
> #RX==#TX assumption even more entrenched.

OK. I was following how many drivers were allocating RX and TX's
together - eg ixgbe_adapter has tx_ring and rx_ring arrays; bnx2
has rx_buf_ring and tx_buf_ring arrays, etc. Also, vhost has some
code that processes tx first before rx (e.g. vhost_net_stop/flush),
so this approach seemed helpful. I am OK either way, what do you
suggest?

> > +		 err = vi->vdev->config->find_vqs(vi->vdev, totalvqs, vqs,
callbacks,
> > +		 		 		 		 		  (const char
**)names);
> > +		 if (err)
> > +		 		 goto free_params;
> > +
>
> This would use up quite a lot of vectors. However,
> tx interrupt is, in fact, slow path. So, assuming we don't have
> enough vectors to use per vq, I think it's a good idea to
> support reducing MSI vector usage by mapping all TX VQs to the same
vector
> and separate vectors for RX.
> The hypervisor actually allows this, but we don't have an API at the
virtio
> level to pass that info to virtio pci ATM.
> Any idea what a good API to use would be?

Yes, it is a waste to have these vectors for tx ints. I initially
thought of adding a flag to virtio_device to pass to vp_find_vqs,
but it won't work, so a new API is needed. I can work with you on
this in the background if you like.

> > +		 for (i = 0; i < numtxqs; i++) {
> > +		 		 vi->rq[i]->rvq = vqs[i];
> > +		 		 vi->sq[i]->svq = vqs[i + numtxqs];
>
> This logic is spread all over. We need some kind of macro to
> get queue number of vq number and back.

Will add this.

> > +		 if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)) {
> > +		 		 vi->cvq = vqs[i + numtxqs];
> > +
> > +		 		 if (virtio_has_feature(vi->vdev,
VIRTIO_NET_F_CTRL_VLAN))
> > +		 		 		 vi->dev->features |=
NETIF_F_HW_VLAN_FILTER;
>
> This bit does not seem to belong in initialize_vqs.

I will move it back to probe.

> > +		 err = virtio_config_val(vdev, VIRTIO_NET_F_NUMTXQS,
> > +		 		 		 		 offsetof(struct
virtio_net_config, numtxqs),
> > +		 		 		 		 &numtxqs);
> > +
> > +		 /* We need atleast one txq */
> > +		 if (err || !numtxqs)
> > +		 		 numtxqs = 1;
>
> err is okay, but should we just fail on illegal values?
> Or change the semantics:
>	n = 0;
>	err = virtio_config_val(vdev, VIRTIO_NET_F_NUMTXQS,
>				offsetof(struct virtio_net_config, numtxqs),
>				&n);
>	numtxq = n + 1;

Will this be better:
	int num_queue_pairs = 2;
	int numtxqs;

	err = virtio_config_val(vdev, VIRTIO_NET_F_MULTIQUEUE,
				offsetof(struct virtio_net_config,
					 num_queue_pairs), &num_queue_pairs);
	<ignore error, if any>
	numtxqs = num_queue_pairs / 2;

> > +		 if (numtxqs > VIRTIO_MAX_TXQS)
> > +		 		 return -EINVAL;
>
> Do we strictly need this?
> I think we should just use whatever hardware has,
> or alternatively somehow ignore the unused queues
> (easy for tx, not sure about rx).

vq's are matched between qemu, virtio-net and vhost. Isn't some check
required that userspace has not passed a bad value?

> > +		 		 if (vi->rq[i]->num == 0) {
> > +		 		 		 err = -ENOMEM;
> > +		 		 		 goto free_recv_bufs;
> > +		 		 }
> >  		 }
> If this fails for vq > 0, you have to detach bufs.

Right, will fix this.

> >  free_vqs:
> > +		 for (i = 0; i < numtxqs; i++)
> > +		 		 cancel_delayed_work_sync(&vi->rq[i]->refill);
> >  		 vdev->config->del_vqs(vdev);
> > -free:
> > +		 free_rq_sq(vi);
>
> If we have a wrapper to init all vqs, pls add a wrapper to clean up
> all vqs as well.

Will add that.

> > +		 for (i = 0; i < vi->numtxqs; i++) {
> > +		 		 struct virtqueue *rvq = vi->rq[i]->rvq;
> > +
> > +		 		 while (1) {
> > +		 		 		 buf = virtqueue_detach_unused_buf
(rvq);
> > +		 		 		 if (!buf)
> > +		 		 		 		 break;
> > +		 		 		 if (vi->mergeable_rx_bufs || vi->
big_packets)
> > +		 		 		 		 give_pages(vi->rq[i],
buf);
> > +		 		 		 else
> > +		 		 		 		 dev_kfree_skb(buf);
> > +		 		 		 --vi->rq[i]->num;
> > +		 		 }
> > +		 		 BUG_ON(vi->rq[i]->num != 0);
> >  		 }
> > -		 BUG_ON(vi->num != 0);
> > +
> > +		 free_rq_sq(vi);
>
>
> This looks wrong here. This function should detach
> and free all bufs, not internal malloc stuff.

That is being done by free_receive_buf after free_unused_bufs()
returns. I hope this addresses your point.

> I think we should have free_unused_bufs that handles
> a single queue, and call it in a loop.

OK, so define free_unused_bufs() as:

static void free_unused_bufs(struct virtnet_info *vi, struct virtqueue
*svq,
			     struct virtqueue *rvq)
{
	/* Use svq and rvq with the remaining code unchanged */
}

Thanks,

- KK


^ permalink raw reply

* Re: [PATCH 3/3] [RFC] Changes for MQ vhost
From: Krishna Kumar2 @ 2011-03-01 16:04 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: anthony, arnd, avi, davem, eric.dumazet, horms, kvm, netdev,
	rusty
In-Reply-To: <20110228100423.GC28006@redhat.com>

"Michael S. Tsirkin" <mst@redhat.com> wrote on 02/28/2011 03:34:23 PM:

> > The number of vhost threads is <= #txqs.  Threads handle more
> > than one txq when #txqs is more than MAX_VHOST_THREADS (4).
>
> It is this sharing that prevents us from just reusing multiple vhost
> descriptors?

Sorry, I didn't understand this question.

> 4 seems a bit arbitrary - do you have an explanation
> on why this is a good number?

I was not sure what is the best way - a sysctl parameter? Or should the
maximum depend on number of host cpus? But that results in too many
threads, e.g. if I have 16 cpus and 16 txqs.

> > +		 struct task_struct *worker; /* worker for this vq */
> > +		 spinlock_t *work_lock;		 /* points to a dev->work_lock[] entry
*/
> > +		 struct list_head *work_list;		 /* points to a dev->work_list[]
entry */
> > +		 int qnum;		 /* 0 for RX, 1 -> n-1 for TX */
>
> Is this right?

Will fix this.

> > @@ -122,12 +128,33 @@ struct vhost_dev {
> >  		 int nvqs;
> >  		 struct file *log_file;
> >  		 struct eventfd_ctx *log_ctx;
> > -		 spinlock_t work_lock;
> > -		 struct list_head work_list;
> > -		 struct task_struct *worker;
> > +		 spinlock_t *work_lock[MAX_VHOST_THREADS];
> > +		 struct list_head *work_list[MAX_VHOST_THREADS];
>
> This looks a bit strange. Won't sticking everything in a single
> array of structures rather than multiple arrays be better for cache
> utilization?

Correct. In that context, which is better:
	struct {
		spinlock_t *work_lock;
		struct list_head *work_list;
	} work[MAX_VHOST_THREADS];
or, to make sure work_lock/work_list is cache-aligned:
	struct work_lock_list {
		spinlock_t work_lock;
		struct list_head work_list;
	} ____cacheline_aligned_in_smp;
and define:
	struct vhost_dev {
		...
		struct work_lock_list work[MAX_VHOST_THREADS];
	};
Second method uses a little more space but each vhost needs only
one (read-only) cache line. I tested with this and can confirm it
aligns each element on a cache-line. BW improved slightly (upto
3%), remote SD improves by upto -4% or so.

> > +static inline int get_nvhosts(int nvqs)
>
> nvhosts -> nthreads?

Yes.

> > +static inline int vhost_get_thread_index(int index, int numtxqs, int
nvhosts)
> > +{
> > +		 return (index % numtxqs) % nvhosts;
> > +}
> > +
>
> As the only caller passes MAX_VHOST_THREADS,
> just use that?

Yes, nice catch.

> >  struct vhost_net {
> >  		 struct vhost_dev dev;
> > -		 struct vhost_virtqueue vqs[VHOST_NET_VQ_MAX];
> > -		 struct vhost_poll poll[VHOST_NET_VQ_MAX];
> > +		 struct vhost_virtqueue *vqs;
> > +		 struct vhost_poll *poll;
> > +		 struct socket **socks;
> >  		 /* Tells us whether we are polling a socket for TX.
> >  		  * We only do this when socket buffer fills up.
> >  		  * Protected by tx vq lock. */
> > -		 enum vhost_net_poll_state tx_poll_state;
> > +		 enum vhost_net_poll_state *tx_poll_state;
>
> another array?

Yes... I am also allocating twice the space than what is required
to make it's usage simple. Please let me know what you feel about
this.

Thanks,

- KK


^ permalink raw reply

* Re: SO_REUSEPORT - can it be done in kernel?
From: Eric Dumazet @ 2011-03-01 16:31 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Thomas Graf, David Miller, rick.jones2, therbert, wsommerfeld,
	daniel.baluta, netdev
In-Reply-To: <20110301131823.GB8028@gondor.apana.org.au>

Le mardi 01 mars 2011 à 21:18 +0800, Herbert Xu a écrit :
> On Tue, Mar 01, 2011 at 02:03:29PM +0100, Eric Dumazet wrote:
> >
> > I believe its now done properly (in net-next-2.6) with commit
> > 4f57c087de9b46182 (net: implement mechanism for HW based QOS)
> 
> Nope, that has nothing to do with this.

Right, I was thinking of commit 1d24eb4815d1e0e8 (xps: Transmit Packet
Steering)

Now you say all this stuff should be replaced by "use this cpu number
nly", just because you have a multi threaded process sending UDP frames
trough one socket...

This wont work for tcp streams, you could imagine a multi-threaded
application using a shared tcp socket as well. Too many OOO packets.




^ permalink raw reply

* Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure
From: Arnd Bergmann @ 2011-03-01 16:43 UTC (permalink / raw)
  To: Stuart Menefy
  Cc: Peppe CAVALLARO, linux-sh@vger.kernel.org, netdev@vger.kernel.org,
	John Stultz, Thomas Gleixner, linux-kernel@vger.kernel.org,
	Russell King - ARM Linux
In-Reply-To: <4D6D0EA3.9000504@st.com>

On Tuesday 01 March 2011, Stuart Menefy wrote:
> On 24/02/11 17:20, Arnd Bergmann wrote:
>
> > Why is this code useful to you? In the scenarios I've seen, the
> > board can always assign a timer to a specific device in a fixed
> > way that can be describe in a board file or device tree.
> 
> What we were trying to do was separate the code which actually manipulates
> the timer hardware from the code which wants that timer service. In this
> case it was a network device driver which is used on multiple SoC devices,
> while the timer hardware tends to differ from device to device.

Right. It certainly makes sense to have an well-defined interface between
the user and the provider of a timer interrupt.

> The other user of this code which we have is an OProfile driver, which
> with this change can now be independent of the hardware it is running on,
> while the previous version manipulated the timer hardware directly.

Ok.

> > Also, what is the difference between this and clkdev?
> 
> clkdev can be used to find a struct clk, which is fine if you just want to
> read the time. In this instance we want to get interrupts from the timer
> hardware, which isn't supported by the clk infrastructure.

(adding Russell to Cc)

Is this something that could sensibly be added to clk/clkdev?

> If anything this duplicates clockevents. The main reason for not using
> clockevents was that nobody else does! Currently clockevents are
> used strictly for time keeping within the kernel, and most architectures
> only register those which are intended to be used for this purpose.
> We felt a bit nervous about adding code to register all the device's timers
> as clockevents, and having the network device driver pick up one of those
> for its own use.

I see. Using a clock_event_device for anything but the system timer tick
is currently not supported, so it certainly would not be straightforward.

I think you need a bit of both, clkdev and clockevent. I think the
options you have are:

1. copy the clkdev code to make it possible to associate a device with
a periodic timer.
2. extend the clkdev/clk code to handle periodic interrupts and reuse
the infrastructure there.
3. extend the clockevent code to make it possible for regular device
drivers to use a clockevent source.

I have no idea which makes the most sense (or if there are other ideas).
Maybe Russell, Thomas or John can comment.

	Arnd

^ permalink raw reply

* Re: SO_REUSEPORT - can it be done in kernel?
From: Eric Dumazet @ 2011-03-01 16:43 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David Miller, rick.jones2, therbert, wsommerfeld, daniel.baluta,
	netdev
In-Reply-To: <20110301123519.GA7453@gondor.apana.org.au>

Le mardi 01 mars 2011 à 20:35 +0800, Herbert Xu a écrit :
> On Mon, Feb 28, 2011 at 07:36:59PM +0800, Herbert Xu wrote:
> > Here are the patches I used.  Please don't them yet as I intend
> > to clean them up quite a bit.
> 
> OK here is the version ready for merging (please retest them
> though as I have changed things substantially).
> 
> The main change is that the legacy UDP code path is now gone
> so we use the same UDP header generation whether corking is on
> or off.
> 
> I will add IPv6 support in a later patch set.
> 
> Thanks!

For the whole patchset :

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Tests were fine on my dev machine.

Thanks



^ permalink raw reply

* Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure
From: Thomas Gleixner @ 2011-03-01 16:48 UTC (permalink / raw)
  To: Stuart Menefy
  Cc: Arnd Bergmann, Peppe CAVALLARO, linux-sh@vger.kernel.org,
	netdev@vger.kernel.org, John Stultz, linux-kernel@vger.kernel.org
In-Reply-To: <4D6D0EA3.9000504@st.com>

On Tue, 1 Mar 2011, Stuart Menefy wrote:
> On 24/02/11 17:20, Arnd Bergmann wrote:
> > On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
> >> From: Stuart Menefy <stuart.menefy@st.com>
> >>
> >> Many devices targeted at the embedded market provide a number of
> >> generic timers which are capable of generating interrupts at a
> >> requested rate. These can then be used in the implementation of drivers
> >> for other peripherals which require a timer interrupt, without having
> >> to provide an additional timer as part of that peripheral.

Why can't you just use an hrtimer and be done with it? Just because
there is some extra hardware in the chip?

> If anything this duplicates clockevents. The main reason for not using
> clockevents was that nobody else does! Currently clockevents are
> used strictly for time keeping within the kernel, and most architectures
> only register those which are intended to be used for this purpose.
> We felt a bit nervous about adding code to register all the device's timers
> as clockevents, and having the network device driver pick up one of those
> for its own use.

We had this discussion before and there was never an real outcome as
it turned out that hrtimers provide enough abstraction for this kind
of problems.
 
> True. The intent was that this would be a third interface onto timer
> hardware, alongside clocksources and clockevents.
> 
> > I don't know if this could also be merged with the clocksource infrastructure,
> > but your code currently doesn't do that.
> 
> It would probably be clockevent rather than clocksource, but it could be if
> people felt that was a better way to go.

If we get some reasonable explanation why an extra timer interrupt is
solving a specific problem better than hrtimers we can do that, but
that needs more thought than picking the first available clockevent
from a list. If we come to the conclusion, that we want/need this kind
of functionality then I really prefer not to create yet another piece
of infrastructure which deals with timer devices.

Thanks,

	tglx

^ permalink raw reply

* Re: [Lxc-users] Bad checksums and lost packets with macvlan on dummy
From: Patrick McHardy @ 2011-03-01 16:51 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: Eric Dumazet, Andrian Nord, lxc-users, Linux Netdev List
In-Reply-To: <4D6CF4A8.6000205@free.fr>

On 01.03.2011 14:29, Daniel Lezcano wrote:
> On 02/28/2011 08:45 AM, Eric Dumazet wrote:
>>> In the normal case, dummy0 is supposed to drop the packets. But with
>>> macvlan these packets are broadcasted to the other macvlan ports, so no
>>> checksum is computed when the packets are transmitted between macvlan1
>>> and macvlan2.
>> So where frames get bad checksums ?
>>
>> In this "bridge" mode, I suspect the broadcast is done _before_ sending
>> frame to dummy, so maybe macvlan should not inherit from lowerdev in
>> this particular case ?
> 
> Hi Eric,
> 
> yes, you are right, the packets are sent before.
> 
> In the 'macvlan_queue_xmit', the code checks the dev is in 'bridge'
> mode. If so, it looks if there is a destination port for the packet and
> then calls the 'forward' callback which is 'dev_forward_skb'.
> 
> I was able to reproduce the same problem with qemu and an emulated
> 'e1000' card instead of dummy0. The packets are dropped too.
> 
> Patrick, do you have any suggestions to fix this ?

Since the frames are only looped back locally, I suppose the easiest
fix would be to mark them with CHECKSUM_UNNECESSARY. Alternatively
we need to complete the checksum manually, similar to what
dev_hard_start_xmit() does.

^ permalink raw reply

* Networking debug - kernel tools
From: Daniel Baluta @ 2011-03-01 16:55 UTC (permalink / raw)
  To: netdev

Hello all,

Having the right tools and knowledge about how to use them
can make our life easier in finding bugs / improving performance.

Would you care to share with us what tools do you use to
analyze networking subsystem's performance on high load
machines?

Pointers to documentation are highly welcomed :).

thanks,
Daniel.

^ permalink raw reply

* [PATCH 1/2] tlan: Remove changelog
From: Joe Perches @ 2011-03-01 16:56 UTC (permalink / raw)
  To: Samuel Chessman; +Cc: Sakari Ailus, netdev, linux-kernel

As it isn't necessary nor really useful any longer.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/tlan.c |  145 ----------------------------------------------------
 1 files changed, 0 insertions(+), 145 deletions(-)

diff --git a/drivers/net/tlan.c b/drivers/net/tlan.c
index e48a808..7721e6c 100644
--- a/drivers/net/tlan.c
+++ b/drivers/net/tlan.c
@@ -25,151 +25,6 @@
  *		Microchip Technology, 24C01A/02A/04A Data Sheet
  *			available in PDF format from www.microchip.com
  *
- * Change History
- *
- *	Tigran Aivazian <tigran@sco.com>:	TLan_PciProbe() now uses
- *						new PCI BIOS interface.
- *	Alan Cox	<alan@lxorguk.ukuu.org.uk>:
- *						Fixed the out of memory
- *						handling.
- *
- *	Torben Mathiasen <torben.mathiasen@compaq.com> New Maintainer!
- *
- *	v1.1 Dec 20, 1999    - Removed linux version checking
- *			       Patch from Tigran Aivazian.
- *			     - v1.1 includes Alan's SMP updates.
- *			     - We still have problems on SMP though,
- *			       but I'm looking into that.
- *
- *	v1.2 Jan 02, 2000    - Hopefully fixed the SMP deadlock.
- *			     - Removed dependency of HZ being 100.
- *			     - We now allow higher priority timers to
- *			       overwrite timers like TLAN_TIMER_ACTIVITY
- *			       Patch from John Cagle <john.cagle@compaq.com>.
- *			     - Fixed a few compiler warnings.
- *
- *	v1.3 Feb 04, 2000    - Fixed the remaining HZ issues.
- *			     - Removed call to pci_present().
- *			     - Removed SA_INTERRUPT flag from irq handler.
- *			     - Added __init and __initdata to reduce resisdent
- *			       code size.
- *			     - Driver now uses module_init/module_exit.
- *			     - Rewrote init_module and tlan_probe to
- *			       share a lot more code. We now use tlan_probe
- *			       with builtin and module driver.
- *			     - Driver ported to new net API.
- *			     - tlan.txt has been reworked to reflect current
- *			       driver (almost)
- *			     - Other minor stuff
- *
- *	v1.4 Feb 10, 2000    - Updated with more changes required after Dave's
- *			       network cleanup in 2.3.43pre7 (Tigran & myself)
- *			     - Minor stuff.
- *
- *	v1.5 March 22, 2000  - Fixed another timer bug that would hang the
- *			       driver if no cable/link were present.
- *			     - Cosmetic changes.
- *			     - TODO: Port completely to new PCI/DMA API
- *				     Auto-Neg fallback.
- *
- *	v1.6 April 04, 2000  - Fixed driver support for kernel-parameters.
- *			       Haven't tested it though, as the kernel support
- *			       is currently broken (2.3.99p4p3).
- *			     - Updated tlan.txt accordingly.
- *			     - Adjusted minimum/maximum frame length.
- *			     - There is now a TLAN website up at
- *			       http://hp.sourceforge.net/
- *
- *	v1.7 April 07, 2000  - Started to implement custom ioctls. Driver now
- *			       reports PHY information when used with Donald
- *			       Beckers userspace MII diagnostics utility.
- *
- *	v1.8 April 23, 2000  - Fixed support for forced speed/duplex settings.
- *			     - Added link information to Auto-Neg and forced
- *			       modes. When NIC operates with auto-neg the driver
- *			       will report Link speed & duplex modes as well as
- *			       link partner abilities. When forced link is used,
- *			       the driver will report status of the established
- *			       link.
- *			       Please read tlan.txt for additional information.
- *			     - Removed call to check_region(), and used
- *			       return value of request_region() instead.
- *
- *	v1.8a May 28, 2000   - Minor updates.
- *
- *	v1.9 July 25, 2000   - Fixed a few remaining Full-Duplex issues.
- *			     - Updated with timer fixes from Andrew Morton.
- *			     - Fixed module race in TLan_Open.
- *			     - Added routine to monitor PHY status.
- *			     - Added activity led support for Proliant devices.
- *
- *	v1.10 Aug 30, 2000   - Added support for EISA based tlan controllers
- *			       like the Compaq NetFlex3/E.
- *			     - Rewrote tlan_probe to better handle multiple
- *			       bus probes. Probing and device setup is now
- *			       done through TLan_Probe and TLan_init_one. Actual
- *			       hardware probe is done with kernel API and
- *			       TLan_EisaProbe.
- *			     - Adjusted debug information for probing.
- *			     - Fixed bug that would cause general debug
- *			       information to be printed after driver removal.
- *			     - Added transmit timeout handling.
- *			     - Fixed OOM return values in tlan_probe.
- *			     - Fixed possible mem leak in tlan_exit
- *			       (now tlan_remove_one).
- *			     - Fixed timer bug in TLan_phyMonitor.
- *			     - This driver version is alpha quality, please
- *			       send me any bug issues you may encounter.
- *
- *	v1.11 Aug 31, 2000   - Do not try to register irq 0 if no irq line was
- *			       set for EISA cards.
- *			     - Added support for NetFlex3/E with nibble-rate
- *			       10Base-T PHY. This is untestet as I haven't got
- *			       one of these cards.
- *			     - Fixed timer being added twice.
- *			     - Disabled PhyMonitoring by default as this is
- *			       work in progress. Define MONITOR to enable it.
- *			     - Now we don't display link info with PHYs that
- *			       doesn't support it (level1).
- *			     - Incresed tx_timeout beacuse of auto-neg.
- *			     - Adjusted timers for forced speeds.
- *
- *	v1.12 Oct 12, 2000   - Minor fixes (memleak, init, etc.)
- *
- *	v1.13 Nov 28, 2000   - Stop flooding console with auto-neg issues
- *			       when link can't be established.
- *			     - Added the bbuf option as a kernel parameter.
- *			     - Fixed ioaddr probe bug.
- *			     - Fixed stupid deadlock with MII interrupts.
- *			     - Added support for speed/duplex selection with
- *			       multiple nics.
- *			     - Added partly fix for TX Channel lockup with
- *			       TLAN v1.0 silicon. This needs to be investigated
- *			       further.
- *
- *	v1.14 Dec 16, 2000   - Added support for servicing multiple frames per.
- *			       interrupt. Thanks goes to
- *			       Adam Keys <adam@ti.com>
- *			       Denis Beaudoin <dbeaudoin@ti.com>
- *			       for providing the patch.
- *			     - Fixed auto-neg output when using multiple
- *			       adapters.
- *			     - Converted to use new taskq interface.
- *
- *	v1.14a Jan 6, 2001   - Minor adjustments (spinlocks, etc.)
- *
- *	Samuel Chessman <chessman@tux.org> New Maintainer!
- *
- *	v1.15 Apr 4, 2002    - Correct operation when aui=1 to be
- *			       10T half duplex no loopback
- *			       Thanks to Gunnar Eikman
- *
- *	Sakari Ailus <sakari.ailus@iki.fi>:
- *
- *	v1.15a Dec 15 2008   - Remove bbuf support, it doesn't work anyway.
- *	v1.16  Jan 6  2011   - Make checkpatch.pl happy.
- *	v1.17  Jan 6  2011   - Add suspend/resume support.
- *
  ******************************************************************************/
 
 #include <linux/module.h>
-- 
1.7.4.2.g597a6.dirty

^ permalink raw reply related

* [PATCH 2/2] tlan: Use pr_fmt, pr_<level> and netdev_<level>
From: Joe Perches @ 2011-03-01 16:56 UTC (permalink / raw)
  To: Samuel Chessman; +Cc: Sakari Ailus, netdev, linux-kernel
In-Reply-To: <901ce76108a5ab7327159b57a1b8c9283673532b.1298998514.git.joe@perches.com>

Neatening and standardization to the current logging mechanisms.
Miscellaneous speen/speed typo correction.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/tlan.c |  164 ++++++++++++++++++++++++---------------------------
 1 files changed, 77 insertions(+), 87 deletions(-)

diff --git a/drivers/net/tlan.c b/drivers/net/tlan.c
index 7721e6c..ace6404 100644
--- a/drivers/net/tlan.c
+++ b/drivers/net/tlan.c
@@ -27,6 +27,8 @@
  *
  ******************************************************************************/
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/ioport.h>
@@ -59,7 +61,7 @@ module_param_array(speed, int, NULL, 0);
 MODULE_PARM_DESC(aui, "ThunderLAN use AUI port(s) (0-1)");
 MODULE_PARM_DESC(duplex,
 		 "ThunderLAN duplex setting(s) (0-default, 1-half, 2-full)");
-MODULE_PARM_DESC(speed, "ThunderLAN port speen setting(s) (0,10,100)");
+MODULE_PARM_DESC(speed, "ThunderLAN port speed setting(s) (0,10,100)");
 
 MODULE_AUTHOR("Maintainer: Samuel Chessman <chessman@tux.org>");
 MODULE_DESCRIPTION("Driver for TI ThunderLAN based ethernet PCI adapters");
@@ -397,7 +399,7 @@ static int __init tlan_probe(void)
 {
 	int rc = -ENODEV;
 
-	printk(KERN_INFO "%s", tlan_banner);
+	pr_info("%s", tlan_banner);
 
 	TLAN_DBG(TLAN_DEBUG_PROBE, "Starting PCI Probe....\n");
 
@@ -406,16 +408,16 @@ static int __init tlan_probe(void)
 	rc = pci_register_driver(&tlan_driver);
 
 	if (rc != 0) {
-		printk(KERN_ERR "TLAN: Could not register pci driver.\n");
+		pr_err("Could not register pci driver\n");
 		goto err_out_pci_free;
 	}
 
 	TLAN_DBG(TLAN_DEBUG_PROBE, "Starting EISA Probe....\n");
 	tlan_eisa_probe();
 
-	printk(KERN_INFO "TLAN: %d device%s installed, PCI: %d  EISA: %d\n",
-	       tlan_devices_installed, tlan_devices_installed == 1 ? "" : "s",
-	       tlan_have_pci, tlan_have_eisa);
+	pr_info("%d device%s installed, PCI: %d  EISA: %d\n",
+		tlan_devices_installed, tlan_devices_installed == 1 ? "" : "s",
+		tlan_have_pci, tlan_have_eisa);
 
 	if (tlan_devices_installed == 0) {
 		rc = -ENODEV;
@@ -474,7 +476,7 @@ static int __devinit tlan_probe1(struct pci_dev *pdev,
 
 		rc = pci_request_regions(pdev, tlan_signature);
 		if (rc) {
-			printk(KERN_ERR "TLAN: Could not reserve IO regions\n");
+			pr_err("Could not reserve IO regions\n");
 			goto err_out;
 		}
 	}
@@ -482,7 +484,7 @@ static int __devinit tlan_probe1(struct pci_dev *pdev,
 
 	dev = alloc_etherdev(sizeof(struct tlan_priv));
 	if (dev == NULL) {
-		printk(KERN_ERR "TLAN: Could not allocate memory for device.\n");
+		pr_err("Could not allocate memory for device\n");
 		rc = -ENOMEM;
 		goto err_out_regions;
 	}
@@ -501,8 +503,7 @@ static int __devinit tlan_probe1(struct pci_dev *pdev,
 
 		rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
 		if (rc) {
-			printk(KERN_ERR
-			       "TLAN: No suitable PCI mapping available.\n");
+			pr_err("No suitable PCI mapping available\n");
 			goto err_out_free_dev;
 		}
 
@@ -516,7 +517,7 @@ static int __devinit tlan_probe1(struct pci_dev *pdev,
 			}
 		}
 		if (!pci_io_base) {
-			printk(KERN_ERR "TLAN: No IO mappings available\n");
+			pr_err("No IO mappings available\n");
 			rc = -EIO;
 			goto err_out_free_dev;
 		}
@@ -572,13 +573,13 @@ static int __devinit tlan_probe1(struct pci_dev *pdev,
 
 	rc = tlan_init(dev);
 	if (rc) {
-		printk(KERN_ERR "TLAN: Could not set up device.\n");
+		pr_err("Could not set up device\n");
 		goto err_out_free_dev;
 	}
 
 	rc = register_netdev(dev);
 	if (rc) {
-		printk(KERN_ERR "TLAN: Could not register device.\n");
+		pr_err("Could not register device\n");
 		goto err_out_uninit;
 	}
 
@@ -595,12 +596,11 @@ static int __devinit tlan_probe1(struct pci_dev *pdev,
 		tlan_have_eisa++;
 	}
 
-	printk(KERN_INFO "TLAN: %s irq=%2d, io=%04x, %s, Rev. %d\n",
-	       dev->name,
-	       (int) dev->irq,
-	       (int) dev->base_addr,
-	       priv->adapter->device_label,
-	       priv->adapter_rev);
+	netdev_info(dev, "irq=%2d, io=%04x, %s, Rev. %d\n",
+		    (int)dev->irq,
+		    (int)dev->base_addr,
+		    priv->adapter->device_label,
+		    priv->adapter_rev);
 	return 0;
 
 err_out_uninit:
@@ -716,7 +716,7 @@ static void  __init tlan_eisa_probe(void)
 		}
 
 		if (debug == 0x10)
-			printk(KERN_INFO "Found one\n");
+			pr_info("Found one\n");
 
 
 		/* Get irq from board */
@@ -745,12 +745,12 @@ static void  __init tlan_eisa_probe(void)
 
 out:
 		if (debug == 0x10)
-			printk(KERN_INFO "None found\n");
+			pr_info("None found\n");
 		continue;
 
 out2:
 		if (debug == 0x10)
-			printk(KERN_INFO "Card found but it is not enabled, skipping\n");
+			pr_info("Card found but it is not enabled, skipping\n");
 		continue;
 
 	}
@@ -818,8 +818,7 @@ static int tlan_init(struct net_device *dev)
 	priv->dma_size = dma_size;
 
 	if (priv->dma_storage == NULL) {
-		printk(KERN_ERR
-		       "TLAN:  Could not allocate lists and buffers for %s.\n",
+		pr_err("Could not allocate lists and buffers for %s\n",
 		       dev->name);
 		return -ENOMEM;
 	}
@@ -837,9 +836,8 @@ static int tlan_init(struct net_device *dev)
 					 (u8) priv->adapter->addr_ofs + i,
 					 (u8 *) &dev->dev_addr[i]);
 	if (err) {
-		printk(KERN_ERR "TLAN: %s: Error reading MAC from eeprom: %d\n",
-		       dev->name,
-		       err);
+		pr_err("%s: Error reading MAC from eeprom: %d\n",
+		       dev->name, err);
 	}
 	dev->addr_len = 6;
 
@@ -883,8 +881,8 @@ static int tlan_open(struct net_device *dev)
 			  dev->name, dev);
 
 	if (err) {
-		pr_err("TLAN:  Cannot open %s because IRQ %d is already in use.\n",
-		       dev->name, dev->irq);
+		netdev_err(dev, "Cannot open because IRQ %d is already in use\n",
+			   dev->irq);
 		return err;
 	}
 
@@ -1367,8 +1365,8 @@ static u32 tlan_handle_tx_eof(struct net_device *dev, u16 host_int)
 	}
 
 	if (!ack)
-		printk(KERN_INFO
-		       "TLAN: Received interrupt for uncompleted TX frame.\n");
+		netdev_info(dev,
+			    "Received interrupt for uncompleted TX frame\n");
 
 	if (eoc) {
 		TLAN_DBG(TLAN_DEBUG_TX,
@@ -1522,8 +1520,8 @@ drop_and_reuse:
 	}
 
 	if (!ack)
-		printk(KERN_INFO
-		       "TLAN: Received interrupt for uncompleted RX frame.\n");
+		netdev_info(dev,
+			    "Received interrupt for uncompleted RX frame\n");
 
 
 	if (eoc) {
@@ -1579,7 +1577,7 @@ drop_and_reuse:
 
 static u32 tlan_handle_dummy(struct net_device *dev, u16 host_int)
 {
-	pr_info("TLAN:  Test interrupt on %s.\n", dev->name);
+	netdev_info(dev, "Test interrupt\n");
 	return 1;
 
 }
@@ -1673,7 +1671,7 @@ static u32 tlan_handle_status_check(struct net_device *dev, u16 host_int)
 	if (host_int & TLAN_HI_IV_MASK) {
 		netif_stop_queue(dev);
 		error = inl(dev->base_addr + TLAN_CH_PARM);
-		pr_info("TLAN:  %s: Adaptor Error = 0x%x\n", dev->name, error);
+		netdev_info(dev, "Adaptor Error = 0x%x\n", error);
 		tlan_read_and_clear_stats(dev, TLAN_RECORD);
 		outl(TLAN_HC_AD_RST, dev->base_addr + TLAN_HOST_CMD);
 
@@ -1914,7 +1912,7 @@ static void tlan_reset_lists(struct net_device *dev)
 		list->buffer[0].count = TLAN_MAX_FRAME_SIZE | TLAN_LAST_BUFFER;
 		skb = netdev_alloc_skb_ip_align(dev, TLAN_MAX_FRAME_SIZE + 5);
 		if (!skb) {
-			pr_err("TLAN: out of memory for received data.\n");
+			netdev_err(dev, "Out of memory for received data\n");
 			break;
 		}
 
@@ -1998,13 +1996,13 @@ static void tlan_print_dio(u16 io_base)
 	u32 data0, data1;
 	int	i;
 
-	pr_info("TLAN:   Contents of internal registers for io base 0x%04hx.\n",
-	       io_base);
-	pr_info("TLAN:      Off.  +0	 +4\n");
+	pr_info("Contents of internal registers for io base 0x%04hx\n",
+		io_base);
+	pr_info("Off.  +0        +4\n");
 	for (i = 0; i < 0x4C; i += 8) {
 		data0 = tlan_dio_read32(io_base, i);
 		data1 = tlan_dio_read32(io_base, i + 0x4);
-		pr_info("TLAN:      0x%02x  0x%08x 0x%08x\n", i, data0, data1);
+		pr_info("0x%02x  0x%08x 0x%08x\n", i, data0, data1);
 	}
 
 }
@@ -2033,14 +2031,14 @@ static void tlan_print_list(struct tlan_list *list, char *type, int num)
 {
 	int i;
 
-	pr_info("TLAN:   %s List %d at %p\n", type, num, list);
-	pr_info("TLAN:      Forward    = 0x%08x\n",  list->forward);
-	pr_info("TLAN:      CSTAT      = 0x%04hx\n", list->c_stat);
-	pr_info("TLAN:      Frame Size = 0x%04hx\n", list->frame_size);
+	pr_info("%s List %d at %p\n", type, num, list);
+	pr_info("   Forward    = 0x%08x\n",  list->forward);
+	pr_info("   CSTAT      = 0x%04hx\n", list->c_stat);
+	pr_info("   Frame Size = 0x%04hx\n", list->frame_size);
 	/* for (i = 0; i < 10; i++) { */
 	for (i = 0; i < 2; i++) {
-		pr_info("TLAN:      Buffer[%d].count, addr = 0x%08x, 0x%08x\n",
-		       i, list->buffer[i].count, list->buffer[i].address);
+		pr_info("   Buffer[%d].count, addr = 0x%08x, 0x%08x\n",
+			i, list->buffer[i].count, list->buffer[i].address);
 	}
 
 }
@@ -2255,7 +2253,7 @@ tlan_finish_reset(struct net_device *dev)
 	if ((priv->adapter->flags & TLAN_ADAPTER_UNMANAGED_PHY) ||
 	    (priv->aui)) {
 		status = MII_GS_LINK;
-		pr_info("TLAN:  %s: Link forced.\n", dev->name);
+		netdev_info(dev, "Link forced\n");
 	} else {
 		tlan_mii_read_reg(dev, phy, MII_GEN_STS, &status);
 		udelay(1000);
@@ -2267,24 +2265,21 @@ tlan_finish_reset(struct net_device *dev)
 			tlan_mii_read_reg(dev, phy, MII_AN_LPA, &partner);
 			tlan_mii_read_reg(dev, phy, TLAN_TLPHY_PAR, &tlphy_par);
 
-			pr_info("TLAN: %s: Link active with ", dev->name);
-			if (!(tlphy_par & TLAN_PHY_AN_EN_STAT)) {
-				pr_info("forced 10%sMbps %s-Duplex\n",
-					tlphy_par & TLAN_PHY_SPEED_100
-					? "" : "0",
-					tlphy_par & TLAN_PHY_DUPLEX_FULL
-					? "Full" : "Half");
-			} else {
-				pr_info("Autonegotiation enabled, at 10%sMbps %s-Duplex\n",
-					tlphy_par & TLAN_PHY_SPEED_100
-					? "" : "0",
-					tlphy_par & TLAN_PHY_DUPLEX_FULL
-					? "Full" : "half");
-				pr_info("TLAN: Partner capability: ");
-				for (i = 5; i <= 10; i++)
-					if (partner & (1<<i))
-						printk("%s", media[i-5]);
-				printk("\n");
+			netdev_info(dev,
+				    "Link active with %s %uMbps %s-Duplex\n",
+				    !(tlphy_par & TLAN_PHY_AN_EN_STAT)
+				    ? "forced" : "Autonegotiation enabled,",
+				    tlphy_par & TLAN_PHY_SPEED_100
+				    ? 100 : 10,
+				    tlphy_par & TLAN_PHY_DUPLEX_FULL
+				    ? "Full" : "Half");
+
+			if (tlphy_par & TLAN_PHY_AN_EN_STAT) {
+				netdev_info(dev, "Partner capability:");
+				for (i = 5; i < 10; i++)
+					if (partner & (1 << i))
+						pr_cont(" %s", media[i-5]);
+				pr_cont("\n");
 			}
 
 			tlan_dio_write8(dev->base_addr, TLAN_LED_REG,
@@ -2296,7 +2291,7 @@ tlan_finish_reset(struct net_device *dev)
 			tlan_set_timer(dev, (10*HZ), TLAN_TIMER_LINK_BEAT);
 #endif
 		} else if (status & MII_GS_LINK)  {
-			pr_info("TLAN: %s: Link active\n", dev->name);
+			netdev_info(dev, "Link active\n");
 			tlan_dio_write8(dev->base_addr, TLAN_LED_REG,
 					TLAN_LED_LINK);
 		}
@@ -2322,8 +2317,7 @@ tlan_finish_reset(struct net_device *dev)
 		outl(TLAN_HC_GO | TLAN_HC_RT, dev->base_addr + TLAN_HOST_CMD);
 		netif_carrier_on(dev);
 	} else {
-		pr_info("TLAN: %s: Link inactive, will retry in 10 secs...\n",
-		       dev->name);
+		netdev_info(dev, "Link inactive, will retry in 10 secs...\n");
 		tlan_set_timer(dev, (10*HZ), TLAN_TIMER_FINISH_RESET);
 		return;
 	}
@@ -2407,23 +2401,20 @@ static void tlan_phy_print(struct net_device *dev)
 	phy = priv->phy[priv->phy_num];
 
 	if (priv->adapter->flags & TLAN_ADAPTER_UNMANAGED_PHY) {
-		pr_info("TLAN:   Device %s, Unmanaged PHY.\n", dev->name);
+		netdev_info(dev, "Unmanaged PHY\n");
 	} else if (phy <= TLAN_PHY_MAX_ADDR) {
-		pr_info("TLAN:   Device %s, PHY 0x%02x.\n", dev->name, phy);
-		pr_info("TLAN:      Off.  +0     +1     +2     +3\n");
+		netdev_info(dev, "PHY 0x%02x\n", phy);
+		pr_info("   Off.  +0     +1     +2     +3\n");
 		for (i = 0; i < 0x20; i += 4) {
-			pr_info("TLAN:      0x%02x", i);
 			tlan_mii_read_reg(dev, phy, i, &data0);
-			printk(" 0x%04hx", data0);
 			tlan_mii_read_reg(dev, phy, i + 1, &data1);
-			printk(" 0x%04hx", data1);
 			tlan_mii_read_reg(dev, phy, i + 2, &data2);
-			printk(" 0x%04hx", data2);
 			tlan_mii_read_reg(dev, phy, i + 3, &data3);
-			printk(" 0x%04hx\n", data3);
+			pr_info("   0x%02x 0x%04hx 0x%04hx 0x%04hx 0x%04hx\n",
+				i, data0, data1, data2, data3);
 		}
 	} else {
-		pr_info("TLAN:   Device %s, Invalid PHY.\n", dev->name);
+		netdev_info(dev, "Invalid PHY\n");
 	}
 
 }
@@ -2490,7 +2481,7 @@ static void tlan_phy_detect(struct net_device *dev)
 	else if (priv->phy[0] != TLAN_PHY_NONE)
 		priv->phy_num = 0;
 	else
-		pr_info("TLAN:  Cannot initialize device, no PHY was found!\n");
+		netdev_info(dev, "Cannot initialize device, no PHY was found!\n");
 
 }
 
@@ -2618,8 +2609,7 @@ static void tlan_phy_start_link(struct net_device *dev)
 			 * but the card need additional time to start AN.
 			 * .5 sec should be plenty extra.
 			 */
-			pr_info("TLAN: %s: Starting autonegotiation.\n",
-				dev->name);
+			netdev_info(dev, "Starting autonegotiation\n");
 			tlan_set_timer(dev, (2*HZ), TLAN_TIMER_PHY_FINISH_AN);
 			return;
 		}
@@ -2682,16 +2672,16 @@ static void tlan_phy_finish_auto_neg(struct net_device *dev)
 		 * more time.  Perhaps we should fail after a while.
 		 */
 		if (!priv->neg_be_verbose++) {
-			pr_info("TLAN:  Giving autonegotiation more time.\n");
-			pr_info("TLAN:  Please check that your adapter has\n");
-			pr_info("TLAN:  been properly connected to a HUB or Switch.\n");
-			pr_info("TLAN:  Trying to establish link in the background...\n");
+			pr_info("Giving autonegotiation more time.\n");
+			pr_info("Please check that your adapter has\n");
+			pr_info("been properly connected to a HUB or Switch.\n");
+			pr_info("Trying to establish link in the background...\n");
 		}
 		tlan_set_timer(dev, (8*HZ), TLAN_TIMER_PHY_FINISH_AN);
 		return;
 	}
 
-	pr_info("TLAN: %s: Autonegotiation complete.\n", dev->name);
+	netdev_info(dev, "Autonegotiation complete\n");
 	tlan_mii_read_reg(dev, phy, MII_AN_ADV, &an_adv);
 	tlan_mii_read_reg(dev, phy, MII_AN_LPA, &an_lpa);
 	mode = an_adv & an_lpa & 0x03E0;
@@ -2716,11 +2706,11 @@ static void tlan_phy_finish_auto_neg(struct net_device *dev)
 		    (an_adv & an_lpa & 0x0040)) {
 			tlan_mii_write_reg(dev, phy, MII_GEN_CTL,
 					   MII_GC_AUTOENB | MII_GC_DUPLEX);
-			pr_info("TLAN:  Starting internal PHY with FULL-DUPLEX\n");
+			netdev_info(dev, "Starting internal PHY with FULL-DUPLEX\n");
 		} else {
 			tlan_mii_write_reg(dev, phy, MII_GEN_CTL,
 					   MII_GC_AUTOENB);
-			pr_info("TLAN:  Starting internal PHY with HALF-DUPLEX\n");
+			netdev_info(dev, "Starting internal PHY with HALF-DUPLEX\n");
 		}
 	}
 
-- 
1.7.4.2.g597a6.dirty

^ permalink raw reply related

* [PATCH 1/5] tg3: use <linux/io.h> and <linux/uaccess.h> instead <asm/io.h> and <asm/uaccess.h>
From: Javier Martinez Canillas @ 2011-03-01 17:04 UTC (permalink / raw)
  To: Matt Carlson
  Cc: Michael Chan, Grant Likely, netdev, kernelnewbies,
	Javier Martinez Canillas

It is proper style to include linux/foo.h instead asm/foo.h if both exist

Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
 drivers/net/tg3.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 6be4185..4410d73 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -48,9 +48,9 @@
 #include <net/ip.h>
 
 #include <asm/system.h>
-#include <asm/io.h>
+#include <linux/io.h>
 #include <asm/byteorder.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 
 #ifdef CONFIG_SPARC
 #include <asm/idprom.h>
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 1/5] tg3: use usleep_range not msleep for small sleeps
From: Javier Martinez Canillas @ 2011-03-01 17:04 UTC (permalink / raw)
  To: Matt Carlson
  Cc: Michael Chan, Grant Likely, netdev, kernelnewbies,
	Javier Martinez Canillas
In-Reply-To: <1298999069-12740-1-git-send-email-martinez.javier@gmail.com>


Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
 drivers/net/tg3.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 6be4185..3310c7a 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -2410,7 +2410,7 @@ static int tg3_nvram_read_using_eeprom(struct tg3 *tp,
 
 		if (tmp & EEPROM_ADDR_COMPLETE)
 			break;
-		msleep(1);
+		usleep_range(1000, 2000);
 	}
 	if (!(tmp & EEPROM_ADDR_COMPLETE))
 		return -EBUSY;
@@ -2688,7 +2688,7 @@ static int tg3_power_down_prepare(struct tg3 *tp)
 			tg3_read_mem(tp, NIC_SRAM_FW_ASF_STATUS_MBOX, &val);
 			if (val == ~NIC_SRAM_FIRMWARE_MBOX_MAGIC1)
 				break;
-			msleep(1);
+			usleep_range(1000, 2000);
 		}
 	}
 	if (tp->tg3_flags & TG3_FLAG_WOL_CAP)
@@ -8901,7 +8901,7 @@ static int tg3_test_interrupt(struct tg3 *tp)
 			break;
 		}
 
-		msleep(10);
+		usleep_range(10000, 20000);
 	}
 
 	tg3_disable_ints(tp);
@@ -11863,7 +11863,7 @@ static void __devinit tg3_nvram_init(struct tg3 *tp)
 	      (EEPROM_DEFAULT_CLOCK_PERIOD <<
 	       EEPROM_ADDR_CLKPERD_SHIFT)));
 
-	msleep(1);
+	usleep_range(1000, 2000);
 
 	/* Enable seeprom accesses. */
 	tw32_f(GRC_LOCAL_CTRL,
@@ -11956,7 +11956,7 @@ static int tg3_nvram_write_block_using_eeprom(struct tg3 *tp,
 
 			if (val & EEPROM_ADDR_COMPLETE)
 				break;
-			msleep(1);
+			usleep_range(1000, 2000);
 		}
 		if (!(val & EEPROM_ADDR_COMPLETE)) {
 			rc = -EBUSY;
@@ -12263,7 +12263,7 @@ static void __devinit tg3_get_eeprom_hw_cfg(struct tg3 *tp)
 	pci_read_config_word(tp->pdev, tp->pm_cap + PCI_PM_CTRL, &pmcsr);
 	pmcsr &= ~PCI_PM_CTRL_STATE_MASK;
 	pci_write_config_word(tp->pdev, tp->pm_cap + PCI_PM_CTRL, pmcsr);
-	msleep(1);
+	usleep_range(1000, 2000);
 
 	/* Make sure register accesses (indirect or otherwise)
 	 * will function correctly.
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 3/5] tg3: Enclose macro with complex values in parenthesis
From: Javier Martinez Canillas @ 2011-03-01 17:04 UTC (permalink / raw)
  To: Matt Carlson
  Cc: Michael Chan, Grant Likely, netdev, kernelnewbies,
	Javier Martinez Canillas
In-Reply-To: <1298999069-12740-1-git-send-email-martinez.javier@gmail.com>

Macros with complex values should be enclosed in parenthesis.
Change accordingly.

Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
 drivers/net/tg3.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 6dc7a8a..c67eb19 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -513,16 +513,16 @@ static void tg3_write32_mbox_5906(struct tg3 *tp, u32 off, u32 val)
 	writel(val, tp->regs + off + GRCMBOX_BASE);
 }
 
-#define tw32_mailbox(reg, val)		tp->write32_mbox(tp, reg, val)
+#define tw32_mailbox(reg, val)		(tp->write32_mbox(tp, reg, val))
 #define tw32_mailbox_f(reg, val)	tw32_mailbox_flush(tp, (reg), (val))
-#define tw32_rx_mbox(reg, val)		tp->write32_rx_mbox(tp, reg, val)
-#define tw32_tx_mbox(reg, val)		tp->write32_tx_mbox(tp, reg, val)
-#define tr32_mailbox(reg)		tp->read32_mbox(tp, reg)
+#define tw32_rx_mbox(reg, val)		(tp->write32_rx_mbox(tp, reg, val))
+#define tw32_tx_mbox(reg, val)		(tp->write32_tx_mbox(tp, reg, val))
+#define tr32_mailbox(reg)		(tp->read32_mbox(tp, reg))
 
-#define tw32(reg, val)			tp->write32(tp, reg, val)
+#define tw32(reg, val)			(tp->write32(tp, reg, val))
 #define tw32_f(reg, val)		_tw32_flush(tp, (reg), (val), 0)
 #define tw32_wait_f(reg, val, us)	_tw32_flush(tp, (reg), (val), (us))
-#define tr32(reg)			tp->read32(tp, reg)
+#define tr32(reg)			(tp->read32(tp, reg))
 
 static void tg3_write_mem(struct tg3 *tp, u32 off, u32 val)
 {
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 4/5] tg3: Don't use IRQF_SAMPLE_RANDOM
From: Javier Martinez Canillas @ 2011-03-01 17:04 UTC (permalink / raw)
  To: Matt Carlson
  Cc: Michael Chan, Grant Likely, netdev, kernelnewbies,
	Javier Martinez Canillas
In-Reply-To: <1298999069-12740-1-git-send-email-martinez.javier@gmail.com>

This flag is scheduled for removal so we shouldn't used it.

Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
 drivers/net/tg3.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index c67eb19..58c6049 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -8844,12 +8844,11 @@ static int tg3_request_irq(struct tg3 *tp, int irq_num)
 		fn = tg3_msi;
 		if (tp->tg3_flags2 & TG3_FLG2_1SHOT_MSI)
 			fn = tg3_msi_1shot;
-		flags = IRQF_SAMPLE_RANDOM;
 	} else {
 		fn = tg3_interrupt;
 		if (tp->tg3_flags & TG3_FLAG_TAGGED_STATUS)
 			fn = tg3_interrupt_tagged;
-		flags = IRQF_SHARED | IRQF_SAMPLE_RANDOM;
+		flags = IRQF_SHARED;
 	}
 
 	return request_irq(tnapi->irq_vec, fn, flags, name, tnapi);
@@ -8880,7 +8879,7 @@ static int tg3_test_interrupt(struct tg3 *tp)
 	}
 
 	err = request_irq(tnapi->irq_vec, tg3_test_isr,
-			  IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, tnapi);
+			  IRQF_SHARED, dev->name, tnapi);
 	if (err)
 		return err;
 
-- 
1.7.2.3


^ permalink raw reply related


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