* Re: Bug inkvm_set_irq
From: Michael S. Tsirkin @ 2011-03-01 7:03 UTC (permalink / raw)
To: Jean-Philippe Menil; +Cc: netdev, kvm, virtualization
In-Reply-To: <4D6C22E8.2020007@univ-nantes.fr>
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.
--
MST
^ permalink raw reply
* Re: txqueuelen has wrong units; should be time
From: David Miller @ 2011-03-01 7:25 UTC (permalink / raw)
To: acahalan
Cc: johnwheffner, linville, eric.dumazet, jussi.kivilinna, swmike,
linux-kernel, netdev
In-Reply-To: <AANLkTi=eDQh9Kvr9N_dTC5Cnbcfb+DbO-6tLxkZiEp9v@mail.gmail.com>
From: Albert Cahalan <acahalan@gmail.com>
Date: Tue, 1 Mar 2011 01:54:09 -0500
> In case it makes you feel any better, consider two cases
> where synchronous feedback is already impossible.
> One is when you're routing packets that merely pass through.
> The other is when some other box is doing that to you.
> Either way, packets go bye-bye and nobody tells TCP.
I consider ECN quite synchronous, and routers will set ECN bits to
propagate congestion information when they do or are about to drop
packets.
^ permalink raw reply
* Re: txqueuelen has wrong units; should be time
From: Eric Dumazet @ 2011-03-01 7:26 UTC (permalink / raw)
To: Albert Cahalan
Cc: David Miller, johnwheffner, linville, jussi.kivilinna, swmike,
linux-kernel, netdev
In-Reply-To: <AANLkTi=eDQh9Kvr9N_dTC5Cnbcfb+DbO-6tLxkZiEp9v@mail.gmail.com>
Le mardi 01 mars 2011 à 01:54 -0500, Albert Cahalan a écrit :
> On Mon, Feb 28, 2011 at 11:18 PM, David Miller <davem@davemloft.net> wrote:
> > From: Albert Cahalan <acahalan@gmail.com>
> > Date: Mon, 28 Feb 2011 23:11:13 -0500
> >
> >> It sounds like you need a callback or similar, so that TCP can be
> >> informed later that the drop has occurred.
> >
> > By that point we could have already sent an entire RTT's worth
> > of data, or more.
> >
> > It needs to be synchronous, otherwise performance suffers.
>
> Ouch. OTOH, the current situation: performance suffers.
>
> In case it makes you feel any better, consider two cases
> where synchronous feedback is already impossible.
> One is when you're routing packets that merely pass through.
> The other is when some other box is doing that to you.
> Either way, packets go bye-bye and nobody tells TCP.
So in a hurry we decide to drop packets blindly because kernel took the
cpu to perform an urgent task ?
Bufferbloat is a configuration/tuning problem, not a "everything must be
redone" problem. We add new qdiscs (CHOKe, SFB, QFQ, ...) and let admins
do their job. Problem is most admins are unaware of the problems, and
only buy more bandwidth.
And no, there is no "generic" solution, unless you have a lab with two
machines back to back (private link) and a known workload.
We might need some changes (including new APIs).
ECN is a forward step. Blindly dropping packets before ever sending them
is a step backward.
We should allow some trafic spikes, or many applications will stop
working. Unless all applications are fixed, we are stuck.
Only if the queue stay loaded a long time (yet another parameter) we can
try to drop packets.
^ permalink raw reply
* Re: [PATCH] net: add Faraday FTMAC100 10/100 Ethernet driver
From: Eric Dumazet @ 2011-03-01 7:27 UTC (permalink / raw)
To: Po-Yu Chuang
Cc: netdev, linux-kernel, bhutchings, joe, dilinger, mirqus, davem,
Po-Yu Chuang
In-Reply-To: <1298962129-1605-1-git-send-email-ratbert.chuang@gmail.com>
Le mardi 01 mars 2011 à 14:48 +0800, Po-Yu Chuang a écrit :
> From: Po-Yu Chuang <ratbert@faraday-tech.com>
>
> FTMAC100 Ethernet Media Access Controller supports 10/100 Mbps and
> MII. This driver has been working on some ARM/NDS32 SoC's including
> Faraday A320 and Andes AG101.
>
> Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
> ---
> v2:
> always use NAPI
> do not use our own net_device_stats structure
> don't set trans_start and last_rx
> stats.rx_packets and stats.rx_bytes include dropped packets
> add missed netif_napi_del()
> initialize spinlocks in probe function
> remove rx_lock and hw_lock
> use netdev_[err/info/dbg] instead of dev_* ones
> use netdev_alloc_skb_ip_align()
> remove ftmac100_get_stats()
> use is_valid_ether_addr() instead of is_zero_ether_addr()
> add const to ftmac100_ethtool_ops and ftmac100_netdev_ops
> use net_ratelimit() instead of printk_ratelimit()
> no explicit inline
> use %pM to print MAC address
> add comment before wmb
> use napi poll() to handle all interrupts
>
> v3:
> undo "stats.rx_packets and stats.rx_bytes include dropped packets"
> ftmac100_mdio_read() returns 0 if error
> fix comment typos
> use pr_fmt and pr_info
> define INT_MASK_ALL_ENABLED
> define MACCR_ENABLE_ALL
> do not count length error many times
> use bool/true/false
> use cpu_to_le32/le32_to_cpu to access descriptors
> indent style fix
>
> v4:
> should not access skb after netif_receive_skb()
> use resource_size()
> better way to use cpu_to_le32/le32_to_cpu
> use spin_lock() for tx_lock
> combine all netdev_info() together in ftmac100_poll()
>
> v5:
> use dev_kfree_skb() in ftmac100_tx_complete_packet()
> cpu_to_le32/le32_to_cpu usage fix
> drop GFP_DMA
>
> v6:
> cpu_to_le32/le32_to_cpu usage fix
> remove "tx queue full" message
> reduce critical section protected by tx_lock
> add check of MAX_PKT_SIZE and RX_BUF_SIZE
> add __exit to ftmac100_remove()
> simplify ftmac100_rx_packet()
> zero copy - use skb_fill_page_desc() and __pskb_pull_tail().
> pull more data to skb head to include tcp/ip header
>
> v7:
> allocate 128 bytes skb and pull 64 bytes only
>
> drivers/net/Kconfig | 9 +
> drivers/net/Makefile | 1 +
> drivers/net/ftmac100.c | 1196 ++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/net/ftmac100.h | 180 ++++++++
> 4 files changed, 1386 insertions(+), 0 deletions(-)
> create mode 100644 drivers/net/ftmac100.c
> create mode 100644 drivers/net/ftmac100.h
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH] f_phonet: avoid pskb_pull(), fix OOPS with CONFIG_HIGHMEM
From: Rémi Denis-Courmont @ 2011-03-01 7:34 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20110228.123844.70195701.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Monday 28 February 2011 22:38:44 ext David Miller, you wrote:
> From: Rémi Denis-Courmont <remi.denis-courmont-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> Date: Wed, 23 Feb 2011 14:51:33 +0200
>
> > This is similar to what we already do in cdc-phonet.c in the same
> > situation. pskb_pull() refuses to work with HIGHMEM, even if it is
> > known that the socket buffer is entirely in "low" memory.
> >
> > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
>
> Applied, thanks.
Hmm, this is already somewhere in linux-usb, from what I understood (I suppose
git-merge does not care much).
--
Rémi Denis-Courmont
http://www.remlab.net/
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] f_phonet: avoid pskb_pull(), fix OOPS with CONFIG_HIGHMEM
From: David Miller @ 2011-03-01 7:37 UTC (permalink / raw)
To: remi.denis-courmont-xNZwKgViW5gAvxtiuMwx3w
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <201103010934.30730.remi.denis-courmont-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
From: "Rémi Denis-Courmont" <remi.denis-courmont-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
Date: Tue, 1 Mar 2011 09:34:30 +0200
> On Monday 28 February 2011 22:38:44 ext David Miller, you wrote:
>> From: Rémi Denis-Courmont <remi.denis-courmont-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
>> Date: Wed, 23 Feb 2011 14:51:33 +0200
>>
>> > This is similar to what we already do in cdc-phonet.c in the same
>> > situation. pskb_pull() refuses to work with HIGHMEM, even if it is
>> > known that the socket buffer is entirely in "low" memory.
>> >
>> > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont-xNZwKgViW5g@public.gmane.orgm>
>>
>> Applied, thanks.
>
> Hmm, this is already somewhere in linux-usb, from what I understood (I suppose
> git-merge does not care much).
Yeah, it'll work itself out without any problems.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [question] bond_set_slave_inactive_flags
From: Jiri Pirko @ 2011-03-01 7:43 UTC (permalink / raw)
To: fubar; +Cc: netdev
Jay,
Looking at function bond_set_slave_inactive_flags:
static inline void bond_set_slave_inactive_flags(struct slave *slave)
{
struct bonding *bond = netdev_priv(slave->dev->master);
if (!bond_is_lb(bond))
slave->state = BOND_STATE_BACKUP;
if (!bond->params.all_slaves_active)
slave->dev->priv_flags |= IFF_SLAVE_INACTIVE;
if (slave_do_arp_validate(bond, slave))
slave->dev->priv_flags |= IFF_SLAVE_NEEDARP;
}
Why BOND_STATE_BACKUP is set only for non-lb bonds?
This was introduced by 8f903c708fcc2b579ebf16542bf6109bad593a1d but
I do not see why.
Thanks for reply in advance.
Jirka
^ permalink raw reply
* Re: [PATCH] cxgb{3,4}*: improve Kconfig dependencies
From: Jan Beulich @ 2011-03-01 7:46 UTC (permalink / raw)
To: Dimitris Michailidis; +Cc: linux-scsi, netdev
In-Reply-To: <1298950455-4497-1-git-send-email-dm@chelsio.com>
>>> On 01.03.11 at 04:34, Dimitris Michailidis <dm@chelsio.com> wrote:
> - Remove the dependency of cxgb4 and cxgb4vf on INET. cxgb3 really
> depends on INET, keep it but add it directly to the driver's Kconfig
> entry.
> - Make the iSCSI drivers cxgb3i and cxgb4i available in the SCSI menu
> without requiring any options in the net driver menu to be enabled
> first. Add needed selects so the iSCSI drivers can build their
> corresponding net drivers.
> - Remove CHELSIO_T*_DEPENDS.
>
> Signed-off-by: Dimitris Michailidis <dm@chelsio.com>
> ---
> drivers/net/Kconfig | 21 +++------------------
> drivers/scsi/cxgbi/cxgb3i/Kconfig | 4 +++-
> drivers/scsi/cxgbi/cxgb4i/Kconfig | 4 +++-
> 3 files changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index f4b3927..6e09d5f 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -2595,14 +2595,9 @@ config CHELSIO_T1_1G
> Enables support for Chelsio's gigabit Ethernet PCI cards. If you
> are using only 10G cards say 'N' here.
>
> -config CHELSIO_T3_DEPENDS
> - tristate
> - depends on PCI && INET
> - default y
> -
> config CHELSIO_T3
> tristate "Chelsio Communications T3 10Gb Ethernet support"
> - depends on CHELSIO_T3_DEPENDS
> + depends on PCI && INET
Forgot to remove INET here?
Besides that,
Acked-by: Jan Beulich <jbeulich@novell.com>
> select FW_LOADER
> select MDIO
> help
> @@ -2620,14 +2615,9 @@ config CHELSIO_T3
> To compile this driver as a module, choose M here: the module
> will be called cxgb3.
>
> -config CHELSIO_T4_DEPENDS
> - tristate
> - depends on PCI && INET
> - default y
> -
> config CHELSIO_T4
> tristate "Chelsio Communications T4 Ethernet support"
> - depends on CHELSIO_T4_DEPENDS
> + depends on PCI
> select FW_LOADER
> select MDIO
> help
> @@ -2645,14 +2635,9 @@ config CHELSIO_T4
> To compile this driver as a module choose M here; the module
> will be called cxgb4.
>
> -config CHELSIO_T4VF_DEPENDS
> - tristate
> - depends on PCI && INET
> - default y
> -
> config CHELSIO_T4VF
> tristate "Chelsio Communications T4 Virtual Function Ethernet support"
> - depends on CHELSIO_T4VF_DEPENDS
> + depends on PCI
> help
> This driver supports Chelsio T4-based gigabit and 10Gb Ethernet
> adapters with PCI-E SR-IOV Virtual Functions.
> diff --git a/drivers/scsi/cxgbi/cxgb3i/Kconfig
> b/drivers/scsi/cxgbi/cxgb3i/Kconfig
> index 5cf4e98..11dff23 100644
> --- a/drivers/scsi/cxgbi/cxgb3i/Kconfig
> +++ b/drivers/scsi/cxgbi/cxgb3i/Kconfig
> @@ -1,6 +1,8 @@
> config SCSI_CXGB3_ISCSI
> tristate "Chelsio T3 iSCSI support"
> - depends on CHELSIO_T3_DEPENDS
> + depends on PCI && INET
> + select NETDEVICES
> + select NETDEV_10000
> select CHELSIO_T3
> select SCSI_ISCSI_ATTRS
> ---help---
> diff --git a/drivers/scsi/cxgbi/cxgb4i/Kconfig
> b/drivers/scsi/cxgbi/cxgb4i/Kconfig
> index bb94b39..d5302c2 100644
> --- a/drivers/scsi/cxgbi/cxgb4i/Kconfig
> +++ b/drivers/scsi/cxgbi/cxgb4i/Kconfig
> @@ -1,6 +1,8 @@
> config SCSI_CXGB4_ISCSI
> tristate "Chelsio T4 iSCSI support"
> - depends on CHELSIO_T4_DEPENDS
> + depends on PCI && INET
> + select NETDEVICES
> + select NETDEV_10000
> select CHELSIO_T4
> select SCSI_ISCSI_ATTRS
> ---help---
^ permalink raw reply
* [PATCH 0/4] rcu: don't assume the size of struct rcu_head
From: Lai Jiangshan @ 2011-03-01 8:03 UTC (permalink / raw)
To: Ingo Molnar
Cc: Paul E. McKenney, Christoph Lameter, Pekka Enberg, Eric Dumazet,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
We always keep the struct rcu_head very small, but we may change it in future
or under some CONFIGs.
There are some other systems may assume the size of struct rcu_head as 2 * sizeof(long).
These assumptions obstruct us to add debug information or priority information
to struct rcu_head. It is time to fix them.
It is glad that I just find 3 places which need to be fixed. These 4 patches
are just cleanup patches when the size of struct rcu_head == 2 * sizeof(long).
NO overhead added and NO behavior changed.
Even when the size of struct rcu_head becomes larger, only slub is changed a little.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH 1/4] slub: automatically reserve bytes at the end of slab
From: Lai Jiangshan @ 2011-03-01 8:03 UTC (permalink / raw)
To: Ingo Molnar
Cc: Paul E. McKenney, Christoph Lameter, Pekka Enberg, Eric Dumazet,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
There is no "struct" for slub's slab, it shares with struct page.
But struct page is very small, it is insufficient when we need
to add some metadata for slab.
So we add a field "reserved" to struct kmem_cache, when a slab
is allocated, kmem_cache->reserved bytes are automatically reserved
at the end of the slab for slab's metadata.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
include/linux/slub_def.h | 1 +
mm/slub.c | 40 +++++++++++++++++++++++-----------------
2 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index 8b6e8ae..ae0093c 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -83,6 +83,7 @@ struct kmem_cache {
void (*ctor)(void *);
int inuse; /* Offset to metadata */
int align; /* Alignment */
+ int reserved; /* Reserved bytes at the end of slabs */
unsigned long min_partial;
const char *name; /* Name (only for display!) */
struct list_head list; /* List of slab caches */
diff --git a/mm/slub.c b/mm/slub.c
index e15aa7f..ad545b2 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -281,11 +281,16 @@ static inline int slab_index(void *p, struct kmem_cache *s, void *addr)
return (p - addr) / s->size;
}
+static inline int order_objects(int order, unsigned long size, int reserved)
+{
+ return ((PAGE_SIZE << order) - reserved) / size;
+}
+
static inline struct kmem_cache_order_objects oo_make(int order,
- unsigned long size)
+ unsigned long size, int reserved)
{
struct kmem_cache_order_objects x = {
- (order << OO_SHIFT) + (PAGE_SIZE << order) / size
+ (order << OO_SHIFT) + order_objects(order, size, reserved)
};
return x;
@@ -617,7 +622,7 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
return 1;
start = page_address(page);
- length = (PAGE_SIZE << compound_order(page));
+ length = (PAGE_SIZE << compound_order(page)) - s->reserved;
end = start + length;
remainder = length % s->size;
if (!remainder)
@@ -698,7 +703,7 @@ static int check_slab(struct kmem_cache *s, struct page *page)
return 0;
}
- maxobj = (PAGE_SIZE << compound_order(page)) / s->size;
+ maxobj = order_objects(compound_order(page), s->size, s->reserved);
if (page->objects > maxobj) {
slab_err(s, page, "objects %u > max %u",
s->name, page->objects, maxobj);
@@ -748,7 +753,7 @@ static int on_freelist(struct kmem_cache *s, struct page *page, void *search)
nr++;
}
- max_objects = (PAGE_SIZE << compound_order(page)) / s->size;
+ max_objects = order_objects(compound_order(page), s->size, s->reserved);
if (max_objects > MAX_OBJS_PER_PAGE)
max_objects = MAX_OBJS_PER_PAGE;
@@ -1988,13 +1993,13 @@ static int slub_nomerge;
* the smallest order which will fit the object.
*/
static inline int slab_order(int size, int min_objects,
- int max_order, int fract_leftover)
+ int max_order, int fract_leftover, int reserved)
{
int order;
int rem;
int min_order = slub_min_order;
- if ((PAGE_SIZE << min_order) / size > MAX_OBJS_PER_PAGE)
+ if (order_objects(min_order, size, reserved) > MAX_OBJS_PER_PAGE)
return get_order(size * MAX_OBJS_PER_PAGE) - 1;
for (order = max(min_order,
@@ -2003,10 +2008,10 @@ static inline int slab_order(int size, int min_objects,
unsigned long slab_size = PAGE_SIZE << order;
- if (slab_size < min_objects * size)
+ if (slab_size < min_objects * size + reserved)
continue;
- rem = slab_size % size;
+ rem = (slab_size - reserved) % size;
if (rem <= slab_size / fract_leftover)
break;
@@ -2016,7 +2021,7 @@ static inline int slab_order(int size, int min_objects,
return order;
}
-static inline int calculate_order(int size)
+static inline int calculate_order(int size, int reserved)
{
int order;
int min_objects;
@@ -2034,14 +2039,14 @@ static inline int calculate_order(int size)
min_objects = slub_min_objects;
if (!min_objects)
min_objects = 4 * (fls(nr_cpu_ids) + 1);
- max_objects = (PAGE_SIZE << slub_max_order)/size;
+ max_objects = order_objects(slub_max_order, size, reserved);
min_objects = min(min_objects, max_objects);
while (min_objects > 1) {
fraction = 16;
while (fraction >= 4) {
order = slab_order(size, min_objects,
- slub_max_order, fraction);
+ slub_max_order, fraction, reserved);
if (order <= slub_max_order)
return order;
fraction /= 2;
@@ -2053,14 +2058,14 @@ static inline int calculate_order(int size)
* We were unable to place multiple objects in a slab. Now
* lets see if we can place a single object there.
*/
- order = slab_order(size, 1, slub_max_order, 1);
+ order = slab_order(size, 1, slub_max_order, 1, reserved);
if (order <= slub_max_order)
return order;
/*
* Doh this slab cannot be placed using slub_max_order.
*/
- order = slab_order(size, 1, MAX_ORDER, 1);
+ order = slab_order(size, 1, MAX_ORDER, 1, reserved);
if (order < MAX_ORDER)
return order;
return -ENOSYS;
@@ -2311,7 +2316,7 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order)
if (forced_order >= 0)
order = forced_order;
else
- order = calculate_order(size);
+ order = calculate_order(size, s->reserved);
if (order < 0)
return 0;
@@ -2329,8 +2334,8 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order)
/*
* Determine the number of objects per slab
*/
- s->oo = oo_make(order, size);
- s->min = oo_make(get_order(size), size);
+ s->oo = oo_make(order, size, s->reserved);
+ s->min = oo_make(get_order(size), size, s->reserved);
if (oo_objects(s->oo) > oo_objects(s->max))
s->max = s->oo;
@@ -2349,6 +2354,7 @@ static int kmem_cache_open(struct kmem_cache *s,
s->objsize = size;
s->align = align;
s->flags = kmem_cache_flags(size, flags, name, ctor);
+ s->reserved = 0;
if (!calculate_sizes(s, -1))
goto error;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH 2/4] slub,rcu: don't assume the size of struct rcu_head
From: Lai Jiangshan @ 2011-03-01 8:03 UTC (permalink / raw)
To: Ingo Molnar
Cc: Paul E. McKenney, Christoph Lameter, Pekka Enberg, Eric Dumazet,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
The size of struct rcu_head may be changed. When it becomes larger,
it will pollute the page array.
We reserve some some bytes for struct rcu_head when a slab
is allocated in this situation.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
slub.c | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index ad545b2..ceb135a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1254,21 +1254,38 @@ static void __free_slab(struct kmem_cache *s, struct page *page)
__free_pages(page, order);
}
+#define need_reserve_slab_rcu \
+ (sizeof(((struct page *)NULL)->lru) < sizeof(struct rcu_head))
+
static void rcu_free_slab(struct rcu_head *h)
{
struct page *page;
- page = container_of((struct list_head *)h, struct page, lru);
+ if (need_reserve_slab_rcu)
+ page = virt_to_head_page(h);
+ else
+ page = container_of((struct list_head *)h, struct page, lru);
+
__free_slab(page->slab, page);
}
static void free_slab(struct kmem_cache *s, struct page *page)
{
if (unlikely(s->flags & SLAB_DESTROY_BY_RCU)) {
- /*
- * RCU free overloads the RCU head over the LRU
- */
- struct rcu_head *head = (void *)&page->lru;
+ struct rcu_head *head;
+
+ if (need_reserve_slab_rcu) {
+ int order = compound_order(page);
+ int offset = (PAGE_SIZE << order) - s->reserved;
+
+ BUG_ON(s->reserved != sizeof(*head));
+ head = page_address(page) + offset;
+ } else {
+ /*
+ * RCU free overloads the RCU head over the LRU
+ */
+ head = (void *)&page->lru;
+ }
call_rcu(head, rcu_free_slab);
} else
@@ -2356,6 +2373,9 @@ static int kmem_cache_open(struct kmem_cache *s,
s->flags = kmem_cache_flags(size, flags, name, ctor);
s->reserved = 0;
+ if (need_reserve_slab_rcu && (s->flags & SLAB_DESTROY_BY_RCU))
+ s->reserved = sizeof(struct rcu_head);
+
if (!calculate_sizes(s, -1))
goto error;
if (disable_higher_order_debug) {
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH 3/4] slab,rcu: don't assume the size of struct rcu_head
From: Lai Jiangshan @ 2011-03-01 8:03 UTC (permalink / raw)
To: Ingo Molnar
Cc: Paul E. McKenney, Christoph Lameter, Pekka Enberg, Eric Dumazet,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
The size of struct rcu_head may be changed. When it becomes larger,
it may pollute the data after struct slab.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/mm/slab.c b/mm/slab.c
index 37961d1..52cf0b4 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -191,22 +191,6 @@ typedef unsigned int kmem_bufctl_t;
#define SLAB_LIMIT (((kmem_bufctl_t)(~0U))-3)
/*
- * struct slab
- *
- * Manages the objs in a slab. Placed either at the beginning of mem allocated
- * for a slab, or allocated from an general cache.
- * Slabs are chained into three list: fully used, partial, fully free slabs.
- */
-struct slab {
- struct list_head list;
- unsigned long colouroff;
- void *s_mem; /* including colour offset */
- unsigned int inuse; /* num of objs active in slab */
- kmem_bufctl_t free;
- unsigned short nodeid;
-};
-
-/*
* struct slab_rcu
*
* slab_destroy on a SLAB_DESTROY_BY_RCU cache uses this structure to
@@ -219,8 +203,6 @@ struct slab {
*
* rcu_read_lock before reading the address, then rcu_read_unlock after
* taking the spinlock within the structure expected at that address.
- *
- * We assume struct slab_rcu can overlay struct slab when destroying.
*/
struct slab_rcu {
struct rcu_head head;
@@ -229,6 +211,27 @@ struct slab_rcu {
};
/*
+ * struct slab
+ *
+ * Manages the objs in a slab. Placed either at the beginning of mem allocated
+ * for a slab, or allocated from an general cache.
+ * Slabs are chained into three list: fully used, partial, fully free slabs.
+ */
+struct slab {
+ union {
+ struct {
+ struct list_head list;
+ unsigned long colouroff;
+ void *s_mem; /* including colour offset */
+ unsigned int inuse; /* num of objs active in slab */
+ kmem_bufctl_t free;
+ unsigned short nodeid;
+ };
+ struct slab_rcu __slab_cover_slab_rcu;
+ };
+};
+
+/*
* struct array_cache
*
* Purpose:
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH 4/4] net,rcu: don't assume the size of struct rcu_head
From: Lai Jiangshan @ 2011-03-01 8:03 UTC (permalink / raw)
To: Ingo Molnar
Cc: Paul E. McKenney, Christoph Lameter, Pekka Enberg, Eric Dumazet,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
struct dst_entry assumes the size of struct rcu_head as 2 * sizeof(long)
and manually adds pads for aligning for "__refcnt".
When the size of struct rcu_head is changed, these manual padding
is wrong. Use __attribute__((aligned (64))) instead.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/include/net/dst.h b/include/net/dst.h
index 93b0310..4ef6c4a 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -62,8 +62,6 @@ struct dst_entry {
struct hh_cache *hh;
#ifdef CONFIG_XFRM
struct xfrm_state *xfrm;
-#else
- void *__pad1;
#endif
int (*input)(struct sk_buff*);
int (*output)(struct sk_buff*);
@@ -74,23 +72,18 @@ struct dst_entry {
#ifdef CONFIG_NET_CLS_ROUTE
__u32 tclassid;
-#else
- __u32 __pad2;
#endif
/*
* Align __refcnt to a 64 bytes alignment
* (L1_CACHE_SIZE would be too much)
- */
-#ifdef CONFIG_64BIT
- long __pad_to_align_refcnt[1];
-#endif
- /*
+ *
* __refcnt wants to be on a different cache line from
* input/output/ops or performance tanks badly
*/
- atomic_t __refcnt; /* client references */
+ atomic_t __refcnt /* client references */
+ __attribute__((aligned (64)));
int __use;
unsigned long lastuse;
union {
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: [PATCH 4/4] net,rcu: don't assume the size of struct rcu_head
From: David Miller @ 2011-03-01 8:16 UTC (permalink / raw)
To: laijs
Cc: mingo, paulmck, cl, penberg, eric.dumazet, mpm, linux-mm,
linux-kernel, netdev
In-Reply-To: <4D6CA860.3020409@cn.fujitsu.com>
From: Lai Jiangshan <laijs@cn.fujitsu.com>
Date: Tue, 01 Mar 2011 16:03:44 +0800
>
> struct dst_entry assumes the size of struct rcu_head as 2 * sizeof(long)
> and manually adds pads for aligning for "__refcnt".
>
> When the size of struct rcu_head is changed, these manual padding
> is wrong. Use __attribute__((aligned (64))) instead.
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
We don't want to use the align if it's going to waste lots of space.
Instead we want to rearrange the structure so that the alignment comes
more cheaply.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 4/4] net,rcu: don't assume the size of struct rcu_head
From: Eric Dumazet @ 2011-03-01 8:20 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Ingo Molnar, Paul E. McKenney, Christoph Lameter, Pekka Enberg,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
In-Reply-To: <4D6CA860.3020409@cn.fujitsu.com>
Le mardi 01 mars 2011 à 16:03 +0800, Lai Jiangshan a écrit :
> struct dst_entry assumes the size of struct rcu_head as 2 * sizeof(long)
> and manually adds pads for aligning for "__refcnt".
>
> When the size of struct rcu_head is changed, these manual padding
> is wrong. Use __attribute__((aligned (64))) instead.
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> diff --git a/include/net/dst.h b/include/net/dst.h
> index 93b0310..4ef6c4a 100644
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -62,8 +62,6 @@ struct dst_entry {
> struct hh_cache *hh;
> #ifdef CONFIG_XFRM
> struct xfrm_state *xfrm;
> -#else
> - void *__pad1;
> #endif
> int (*input)(struct sk_buff*);
> int (*output)(struct sk_buff*);
> @@ -74,23 +72,18 @@ struct dst_entry {
>
> #ifdef CONFIG_NET_CLS_ROUTE
> __u32 tclassid;
> -#else
> - __u32 __pad2;
> #endif
>
>
> /*
> * Align __refcnt to a 64 bytes alignment
> * (L1_CACHE_SIZE would be too much)
> - */
> -#ifdef CONFIG_64BIT
> - long __pad_to_align_refcnt[1];
> -#endif
> - /*
> + *
> * __refcnt wants to be on a different cache line from
> * input/output/ops or performance tanks badly
> */
> - atomic_t __refcnt; /* client references */
> + atomic_t __refcnt /* client references */
> + __attribute__((aligned (64)));
> int __use;
> unsigned long lastuse;
> union {
If struct rcu_head is bigger, this is for debugging purposes, so we dont
care about performance, and can avoid wasting ~64 bytes.
Some machines still have about 2.000.000 active dst entries : the
convoluted checks we added in include/net/dst.h are here to make sure we
dont have huge holes in the dst structure.
(This might change when/if IP route cache is gone)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 4/4] net,rcu: don't assume the size of struct rcu_head
From: Eric Dumazet @ 2011-03-01 8:20 UTC (permalink / raw)
To: David Miller
Cc: laijs, mingo, paulmck, cl, penberg, mpm, linux-mm, linux-kernel,
netdev
In-Reply-To: <20110301.001638.104075130.davem@davemloft.net>
Le mardi 01 mars 2011 à 00:16 -0800, David Miller a écrit :
> From: Lai Jiangshan <laijs@cn.fujitsu.com>
> Date: Tue, 01 Mar 2011 16:03:44 +0800
>
> >
> > struct dst_entry assumes the size of struct rcu_head as 2 * sizeof(long)
> > and manually adds pads for aligning for "__refcnt".
> >
> > When the size of struct rcu_head is changed, these manual padding
> > is wrong. Use __attribute__((aligned (64))) instead.
> >
> > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>
> We don't want to use the align if it's going to waste lots of space.
>
> Instead we want to rearrange the structure so that the alignment comes
> more cheaply.
Oh well, I should have read your answer before sending mine :)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: net: allow handlers to be processed for orig_dev
From: Nicolas de Pesloüan @ 2011-03-01 8:24 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <20110301062631.GC2855@psychotron.redhat.com>
Le 01/03/2011 07:26, Jiri Pirko a écrit :
> This was there before, I forgot about this. Allows deliveries to
> ptype_base handlers registered for orig_dev. I presume this is still
> desired.
Yes, it is. Sorry for missing it in my review.
> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
> ---
> net/core/dev.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 30440e7..9f66de9 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3208,7 +3208,8 @@ ncls:
> list_for_each_entry_rcu(ptype,
> &ptype_base[ntohs(type)& PTYPE_HASH_MASK], list) {
> if (ptype->type == type&&
> - (ptype->dev == null_or_dev || ptype->dev == skb->dev)) {
> + (ptype->dev == null_or_dev || ptype->dev == skb->dev ||
> + ptype->dev == orig_dev)) {
> if (pt_prev)
> ret = deliver_skb(skb, pt_prev, orig_dev);
> pt_prev = ptype;
^ permalink raw reply
* ICMP reply uses wrong source address as destinatio
From: Anders Nilsson Plymoth @ 2011-03-01 8:49 UTC (permalink / raw)
To: netdev
Dear linux kernel enthusiasts,
I came upon an issue where ICMP reply packets were issued towards the
IP address of the receiving interface, rather than the source IP
address.
Looking at the kernel code, I saw that this is caused by the following
line in net/ipv4/icmp.c function icmp_reply:
daddr = ipc.addr = rt->rt_src;
For most cases the original line of code is ok, but in some situations
doesn't arrive to the kernel from the network device, but through some
other mechanism such as a userspace application. In these cases the
receiving device in the skb appears to be the loopback interface, not
a physical device. icmp_reply will thus issue the reply to the
loopback IP address, rather than the source IP address as it should.
While googling to see if this issue have been submitted, I found this
two posts that address the same problem:
h**p://www.mail-archive.com/linux-kernel@vger.kernel.org/msg209746.html
h**p://www.mail-archive.com/linux-kernel@vger.kernel.org/msg208272.html
Some of the questions there are easy to answer; such as this doesn't
affect DNAT, and if source address is not set then you can' reply
anyway.
As to the statement:
"... which IP address should be used as the source
1. the destination address of the packet that generated the message
or.
2. the IP address that the machine would use by default if the machine
were to generate a new connection to the destination."
These may be relevant questions, but the ICMP RFC clearly states the
answer is 1. 2. may seem relevant to multi-homing, but its not the
role of the ICMP reply to resolve multi-homing issues.
The following code will correct the issue.
{
struct iphdr *ip = ip_hdr(skb);
daddr = ipc.addr = ip->saddr;
}
The only functions that use icmp_reply are icmp_echo and
icmp_timestamp, and this change do not modify their behavior. After
extensive testing, in regular setups and DNATed situations, I can
verify this change works as intended.
Thanks,
Anders
^ permalink raw reply
* [PATCH 4/4 V2] net,rcu: don't assume the size of struct rcu_head
From: Lai Jiangshan @ 2011-03-01 8:53 UTC (permalink / raw)
To: David Miller
Cc: mingo, paulmck, cl, penberg, eric.dumazet, mpm, linux-mm,
linux-kernel, netdev
In-Reply-To: <20110301.001638.104075130.davem@davemloft.net>
On 03/01/2011 04:16 PM, David Miller wrote:
> From: Lai Jiangshan <laijs@cn.fujitsu.com>
> Date: Tue, 01 Mar 2011 16:03:44 +0800
>
>>
>> struct dst_entry assumes the size of struct rcu_head as 2 * sizeof(long)
>> and manually adds pads for aligning for "__refcnt".
>>
>> When the size of struct rcu_head is changed, these manual padding
>> is wrong. Use __attribute__((aligned (64))) instead.
>>
>> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>
> We don't want to use the align if it's going to waste lots of space.
>
> Instead we want to rearrange the structure so that the alignment comes
> more cheaply.
Subject: [PATCH 4/4 V2] net,rcu: don't assume the size of struct rcu_head
struct dst_entry assumes the size of struct rcu_head as 2 * sizeof(long)
and manually adds pads for aligning for "__refcnt".
When the size of struct rcu_head is changed, these manual padding
are hardly suit for the changes. So we rearrange the structure,
and move the seldom access rcu_head to the end of the structure.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/include/net/dst.h b/include/net/dst.h
index 93b0310..d8c5296 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -37,7 +37,6 @@
struct sk_buff;
struct dst_entry {
- struct rcu_head rcu_head;
struct dst_entry *child;
struct net_device *dev;
short error;
@@ -78,6 +77,13 @@ struct dst_entry {
__u32 __pad2;
#endif
+ unsigned long lastuse;
+ union {
+ struct dst_entry *next;
+ struct rtable __rcu *rt_next;
+ struct rt6_info *rt6_next;
+ struct dn_route __rcu *dn_next;
+ };
/*
* Align __refcnt to a 64 bytes alignment
@@ -92,13 +98,7 @@ struct dst_entry {
*/
atomic_t __refcnt; /* client references */
int __use;
- unsigned long lastuse;
- union {
- struct dst_entry *next;
- struct rtable __rcu *rt_next;
- struct rt6_info *rt6_next;
- struct dn_route __rcu *dn_next;
- };
+ struct rcu_head rcu_head;
};
#ifdef __KERNEL__
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: [PATCH] cxgb{3,4}*: improve Kconfig dependencies
From: Dimitris Michailidis @ 2011-03-01 9:10 UTC (permalink / raw)
To: Jan Beulich; +Cc: linux-scsi, netdev
In-Reply-To: <4D6CB26802000078000343BD@vpn.id2.novell.com>
Jan Beulich wrote:
>>>> On 01.03.11 at 04:34, Dimitris Michailidis <dm@chelsio.com> wrote:
>> - Remove the dependency of cxgb4 and cxgb4vf on INET. cxgb3 really
>> depends on INET, keep it but add it directly to the driver's Kconfig
>> entry.
>> - Make the iSCSI drivers cxgb3i and cxgb4i available in the SCSI menu
>> without requiring any options in the net driver menu to be enabled
>> first. Add needed selects so the iSCSI drivers can build their
>> corresponding net drivers.
>> - Remove CHELSIO_T*_DEPENDS.
>>
>> Signed-off-by: Dimitris Michailidis <dm@chelsio.com>
>> ---
>> drivers/net/Kconfig | 21 +++------------------
>> drivers/scsi/cxgbi/cxgb3i/Kconfig | 4 +++-
>> drivers/scsi/cxgbi/cxgb4i/Kconfig | 4 +++-
>> 3 files changed, 9 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>> index f4b3927..6e09d5f 100644
>> --- a/drivers/net/Kconfig
>> +++ b/drivers/net/Kconfig
>> @@ -2595,14 +2595,9 @@ config CHELSIO_T1_1G
>> Enables support for Chelsio's gigabit Ethernet PCI cards. If you
>> are using only 10G cards say 'N' here.
>>
>> -config CHELSIO_T3_DEPENDS
>> - tristate
>> - depends on PCI && INET
>> - default y
>> -
>> config CHELSIO_T3
>> tristate "Chelsio Communications T3 10Gb Ethernet support"
>> - depends on CHELSIO_T3_DEPENDS
>> + depends on PCI && INET
>
> Forgot to remove INET here?
Didn't forget it, this driver has a real dependency on INET (ARP more
specifically). I mentioned it also in the commit message.
^ permalink raw reply
* Re: [PATCH 4/4 V2] net,rcu: don't assume the size of struct rcu_head
From: Eric Dumazet @ 2011-03-01 9:20 UTC (permalink / raw)
To: Lai Jiangshan
Cc: David Miller, mingo, paulmck, cl, penberg, mpm, linux-mm,
linux-kernel, netdev
In-Reply-To: <4D6CB414.8050107@cn.fujitsu.com>
Le mardi 01 mars 2011 à 16:53 +0800, Lai Jiangshan a écrit :
> On 03/01/2011 04:16 PM, David Miller wrote:
> > From: Lai Jiangshan <laijs@cn.fujitsu.com>
> > Date: Tue, 01 Mar 2011 16:03:44 +0800
> >
> >>
> >> struct dst_entry assumes the size of struct rcu_head as 2 * sizeof(long)
> >> and manually adds pads for aligning for "__refcnt".
> >>
> >> When the size of struct rcu_head is changed, these manual padding
> >> is wrong. Use __attribute__((aligned (64))) instead.
> >>
> >> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> >
> > We don't want to use the align if it's going to waste lots of space.
> >
> > Instead we want to rearrange the structure so that the alignment comes
> > more cheaply.
>
> Subject: [PATCH 4/4 V2] net,rcu: don't assume the size of struct rcu_head
>
> struct dst_entry assumes the size of struct rcu_head as 2 * sizeof(long)
> and manually adds pads for aligning for "__refcnt".
>
> When the size of struct rcu_head is changed, these manual padding
> are hardly suit for the changes. So we rearrange the structure,
> and move the seldom access rcu_head to the end of the structure.
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
>
> diff --git a/include/net/dst.h b/include/net/dst.h
> index 93b0310..d8c5296 100644
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -37,7 +37,6 @@
> struct sk_buff;
>
> struct dst_entry {
> - struct rcu_head rcu_head;
> struct dst_entry *child;
> struct net_device *dev;
> short error;
> @@ -78,6 +77,13 @@ struct dst_entry {
> __u32 __pad2;
> #endif
>
> + unsigned long lastuse;
> + union {
> + struct dst_entry *next;
> + struct rtable __rcu *rt_next;
> + struct rt6_info *rt6_next;
> + struct dn_route __rcu *dn_next;
> + };
>
> /*
> * Align __refcnt to a 64 bytes alignment
> @@ -92,13 +98,7 @@ struct dst_entry {
> */
> atomic_t __refcnt; /* client references */
> int __use;
> - unsigned long lastuse;
> - union {
> - struct dst_entry *next;
> - struct rtable __rcu *rt_next;
> - struct rt6_info *rt6_next;
> - struct dn_route __rcu *dn_next;
> - };
> + struct rcu_head rcu_head;
> };
>
> #ifdef __KERNEL__
Nope...
"lastuse" and "next" must be in this place, or this introduce false
sharing we wanted to avoid in the past.
I suggest you leave this code as is, we will address the problem when
rcu_head changes (assuming we can test a CONFIG_RCU_HEAD_DEBUG or
something)
First part of "struct dst_entry" is mostly read, while part beginning
after refcnt is often written.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [patch net-next-2.6] bonding: remove skb_share_check in handle_frame
From: Jiri Pirko @ 2011-03-01 9:29 UTC (permalink / raw)
To: netdev; +Cc: davem, fubar, eric.dumazet, nicolas.2p.debian, andy
In-Reply-To: <20110301062250.GB2855@psychotron.redhat.com>
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.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/bonding/bond_main.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 584f97b..367ea60 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1498,9 +1498,6 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
struct net_device *slave_dev;
struct net_device *bond_dev;
- skb = skb_share_check(skb, GFP_ATOMIC);
- if (unlikely(!skb))
- return NULL;
slave_dev = skb->dev;
bond_dev = ACCESS_ONCE(slave_dev->master);
if (unlikely(!bond_dev))
--
1.7.3.4
^ permalink raw reply related
* Re: [GIT/PATCH v3] xen network backend driver
From: Ian Campbell @ 2011-03-01 9:31 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev@vger.kernel.org, xen-devel, Jeremy Fitzhardinge,
Herbert Xu, Konrad Rzeszutek Wilk, Francois Romieu
In-Reply-To: <1298919198.2569.14.camel@bwh-desktop>
On Mon, 2011-02-28 at 18:53 +0000, Ben Hutchings wrote:
>
> > --- /dev/null
> > +++ b/drivers/net/xen-netback/common.h
> [...]
> > + /* Statistics */
> > + int rx_gso_checksum_fixup;
>
> This should be defined as unsigned long (ideally it would be u64, but
> that can't be updated atomically on 32-bit systems).
Thanks, I'll address all your comments in netback shortly but first I
guess xen-netfront also needs this:
Ian.
8<-----------------------------
>From d04fc6794249e26a5e5ba5fabf1456bb0e0309d2 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Tue, 1 Mar 2011 09:29:45 +0000
Subject: [PATCH] xen: netfront: ethtool stats fields should be unsigned long
Fixup the rx_gso_checksum_fixup field added in e0ce4af920eb to be
unsigned long as suggested by Ben Hutchings in
<1298919198.2569.14.camel@bwh-desktop>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/xen-netfront.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index a6ab973..df45323 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -122,7 +122,7 @@ struct netfront_info {
struct mmu_update rx_mmu[NET_RX_RING_SIZE];
/* Statistics */
- int rx_gso_checksum_fixup;
+ unsigned long rx_gso_checksum_fixup;
};
struct netfront_rx_info {
--
1.5.6.5
^ permalink raw reply related
* Re: [PATCH] cxgb{3,4}*: improve Kconfig dependencies
From: Jan Beulich @ 2011-03-01 9:33 UTC (permalink / raw)
To: Dimitris Michailidis; +Cc: linux-scsi, netdev
In-Reply-To: <4D6CB7F2.9020902@chelsio.com>
>>> On 01.03.11 at 10:10, Dimitris Michailidis <dm@chelsio.com> wrote:
> Jan Beulich wrote:
>>>>> On 01.03.11 at 04:34, Dimitris Michailidis <dm@chelsio.com> wrote:
>>> - Remove the dependency of cxgb4 and cxgb4vf on INET. cxgb3 really
>>> depends on INET, keep it but add it directly to the driver's Kconfig
>>> entry.
>>> - Make the iSCSI drivers cxgb3i and cxgb4i available in the SCSI menu
>>> without requiring any options in the net driver menu to be enabled
>>> first. Add needed selects so the iSCSI drivers can build their
>>> corresponding net drivers.
>>> - Remove CHELSIO_T*_DEPENDS.
>>>
>>> Signed-off-by: Dimitris Michailidis <dm@chelsio.com>
>>> ---
>>> drivers/net/Kconfig | 21 +++------------------
>>> drivers/scsi/cxgbi/cxgb3i/Kconfig | 4 +++-
>>> drivers/scsi/cxgbi/cxgb4i/Kconfig | 4 +++-
>>> 3 files changed, 9 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>>> index f4b3927..6e09d5f 100644
>>> --- a/drivers/net/Kconfig
>>> +++ b/drivers/net/Kconfig
>>> @@ -2595,14 +2595,9 @@ config CHELSIO_T1_1G
>>> Enables support for Chelsio's gigabit Ethernet PCI cards. If you
>>> are using only 10G cards say 'N' here.
>>>
>>> -config CHELSIO_T3_DEPENDS
>>> - tristate
>>> - depends on PCI && INET
>>> - default y
>>> -
>>> config CHELSIO_T3
>>> tristate "Chelsio Communications T3 10Gb Ethernet support"
>>> - depends on CHELSIO_T3_DEPENDS
>>> + depends on PCI && INET
>>
>> Forgot to remove INET here?
>
> Didn't forget it, this driver has a real dependency on INET (ARP more
> specifically). I mentioned it also in the commit message.
Oh, sorry, must have read this too quickly.
Jan
^ permalink raw reply
* Re: [GIT/PATCH v3] xen network backend driver
From: Ian Campbell @ 2011-03-01 10:06 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev@vger.kernel.org, xen-devel, Jeremy Fitzhardinge,
Herbert Xu, Konrad Rzeszutek Wilk, Francois Romieu
In-Reply-To: <1298919198.2569.14.camel@bwh-desktop>
On Mon, 2011-02-28 at 18:53 +0000, Ben Hutchings wrote:
> On Mon, 2011-02-28 at 17:27 +0000, Ian Campbell wrote:
> This should be defined as unsigned long (ideally it would be u64, but
> that can't be updated atomically on 32-bit systems).
[...]
> Don't update last_rx; it's only needed on slave devices of a bond, and
> the bonding driver takes care of it now.
I made these two changes.
> [...]
> > +static int xenvif_change_mtu(struct net_device *dev, int mtu)
> > +{
> > + struct xenvif *vif = netdev_priv(dev);
> > + int max = vif->can_sg ? 65535 - ETH_HLEN : ETH_DATA_LEN;
> > + if (mtu > max)
> > + return -EINVAL;
> > + dev->mtu = mtu;
> > + return 0;
> > +}
> [...]
>
> Since any VLAN tag must be inserted inline, shouldn't the MTU limit be
> 65535 - VLAN_ETH_HLEN?
In that case shouldn't the other case also be ETH_FRAME_LEN -
VLAN_ETH_HLEN?
I'm not sure what is customary wrt MTU vs VLAN (or other) overheads
under Linux, do we take the hit of the overhead for every device
regardless of VLAN being configured or not or do we expect that people
will configure the MTU as necessary when they configure a VLAN?
Netback itself will cope fine with either MTU, it's the external
connectivity which will actually matter. e.g. the usual configuration
would be (where vifX.Y represents potentially multiple netback devices):
eth0 <-> eth0.VLAN <-> br0.VLAN <=> vifX.Y
So ultimately it will be the eth0 hardware/driver which matters.
There is a comment in net/8021q/vlan.c which says
/* need 4 bytes for extra VLAN header info,
* hope the underlying device can handle it.
*/
and propagates the underlying device's MTU unmodified so it seems that
the norm is to leave the MTU at maximum assuming no VLAN overhead and
defer any required tweaking to the admin?
Alternatively you might have the VLAN on the eth0 device inside the
guest (e.g. netback<->netfront acts like a trunk link) in which case
basically the same argument applies?
I don't really mind either way so I'm happy to follow whatever the
convention is.
Ian.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox