* [PATCH] openvswitch: fix linking without CONFIG_NF_CONNTRACK_LABELS
From: Arnd Bergmann @ 2018-11-02 15:36 UTC (permalink / raw)
To: Pravin B Shelar, David S. Miller
Cc: Arnd Bergmann, Pablo Neira Ayuso, Florian Westphal,
Flavio Leitner, Gao Feng, Thierry Du Tre, Yi-Hung Wei, Ed Swierk,
Julia Lawall, netdev, dev, linux-kernel
When CONFIG_CC_OPTIMIZE_FOR_DEBUGGING is enabled, the compiler
fails to optimize out a dead code path, which leads to a link failure:
net/openvswitch/conntrack.o: In function `ovs_ct_set_labels':
conntrack.c:(.text+0x2e60): undefined reference to `nf_connlabels_replace'
In this configuration, we can take a shortcut, and completely
remove the contrack label code. This may also help the regular
optimization.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/openvswitch/conntrack.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 6bec37ab4472..a4660c48ff01 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1203,7 +1203,8 @@ static int ovs_ct_commit(struct net *net, struct sw_flow_key *key,
&info->labels.mask);
if (err)
return err;
- } else if (labels_nonzero(&info->labels.mask)) {
+ } else if (IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS) &&
+ labels_nonzero(&info->labels.mask)) {
err = ovs_ct_set_labels(ct, key, &info->labels.value,
&info->labels.mask);
if (err)
--
2.18.0
^ permalink raw reply related
* WARNING in kmem_cache_create_usercopy
From: syzbot @ 2018-11-02 15:37 UTC (permalink / raw)
To: asmadeus, davem, ericvh, linux-kernel, lucho, netdev,
syzkaller-bugs, v9fs-developer
Hello,
syzbot found the following crash on:
HEAD commit: 4794a36bf08d Add linux-next specific files for 20180928
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=124f814e400000
kernel config: https://syzkaller.appspot.com/x/.config?x=b0ba9bb377f8ae91
dashboard link: https://syzkaller.appspot.com/bug?extid=0c1d61e4db7db94102ca
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1511532a400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1701f831400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+0c1d61e4db7db94102ca@syzkaller.appspotmail.com
WARNING: CPU: 0 PID: 6065 at mm/slab_common.c:473
kmem_cache_create_usercopy+0xad/0x240 mm/slab_common.c:473
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 6065 Comm: syz-executor140 Not tainted
4.19.0-rc5-next-20180928+ #84
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1d3/0x2c4 lib/dump_stack.c:113
panic+0x238/0x4e7 kernel/panic.c:184
__warn.cold.8+0x163/0x1ba kernel/panic.c:536
report_bug+0x254/0x2d0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:178 [inline]
do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
do_invalid_op+0x36/0x40 arch/x86/kernel/traps.c:290
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:969
RIP: 0010:kmem_cache_create_usercopy+0xad/0x240 mm/slab_common.c:473
Code: 44 89 f0 25 00 60 de 04 45 85 ed 89 45 cc 75 0b 8b 45 d0 85 c0 0f 85
8e 01 00 00 44 39 eb 72 0a 89 d8 44 29 e8 3b 45 d0 73 7e <0f> 0b c7 45 d0
00 00 00 00 4c 8b 45 10 44 89 fa 89 de 4c 89 e7 8b
RSP: 0018:ffff8801bc23f5d0 EFLAGS: 00010213
RAX: 0000000000000000 RBX: 0000000000000008 RCX: 0000000000000006
RDX: 0000000000000000 RSI: 0000000000000020 RDI: ffffffff88b04b20
RBP: ffff8801bc23f608 R08: fffffbfff1283a2d R09: fffffbfff1283a2c
R10: ffff8801bc23f5c0 R11: ffffffff8941d167 R12: ffffffff88b04b20
R13: 00000000fffffffd R14: 0000000000000000 R15: 0000000000000000
p9_client_create+0xa58/0x1769 net/9p/client.c:1054
v9fs_session_init+0x217/0x1bb0 fs/9p/v9fs.c:421
v9fs_mount+0x7c/0x8f0 fs/9p/vfs_super.c:135
legacy_get_tree+0x131/0x460 fs/fs_context.c:718
vfs_get_tree+0x1cb/0x5c0 fs/super.c:1795
do_new_mount fs/namespace.c:2648 [inline]
do_mount+0x70c/0x1d90 fs/namespace.c:2974
ksys_mount+0x12d/0x140 fs/namespace.c:3190
__do_sys_mount fs/namespace.c:3204 [inline]
__se_sys_mount fs/namespace.c:3201 [inline]
__x64_sys_mount+0xbe/0x150 fs/namespace.c:3201
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x440189
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffdd30ec3c8 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 0000000000440189
RDX: 00000000200008c0 RSI: 0000000020000000 RDI: 0000000000000000
RBP: 00000000006ca018 R08: 0000000020000a80 R09: 00000000004002c8
R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000401a10
R13: 0000000000401aa0 R14: 0000000000000000 R15: 0000000000000000
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* general protection fault in xfrmi_lookup
From: syzbot @ 2018-11-02 15:40 UTC (permalink / raw)
To: davem, herbert, linux-kernel, netdev, steffen.klassert,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: e2a322a0c8ce Merge branch 'net-smc-userspace-breakage-fixes'
git tree: net
console output: https://syzkaller.appspot.com/x/log.txt?x=12087ce9400000
kernel config: https://syzkaller.appspot.com/x/.config?x=c0af03fe452b65fb
dashboard link: https://syzkaller.appspot.com/bug?extid=ef5639e6d1435df3a1f1
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+ef5639e6d1435df3a1f1@syzkaller.appspotmail.com
RBP: 0000000020000000 R08: 00000000000000f0 R09: 0000000000000000
R10: 0000000000000064 R11: 0000000000000293 R12: 00007f25cc19f6d4
R13: 00000000004c48a4 R14: 00000000004d7b90 R15: 0000000000000004
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 32654 Comm: syz-executor4 Not tainted 4.19.0-rc6+ #134
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:__read_once_size include/linux/compiler.h:188 [inline]
RIP: 0010:net_generic include/net/netns/generic.h:45 [inline]
RIP: 0010:xfrmi_lookup.isra.18+0x11f/0x6c0 net/xfrm/xfrm_interface.c:61
Code: 57 af d3 fa 45 84 ed 0f 84 4c 04 00 00 e8 79 ae d3 fa 48 8d bb 08 1b
00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f
85 7b 05 00 00 4c 8d 6d 98 48 8b 9b 08 1b 00 00 48
RSP: 0018:ffff880181996878 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000800 RCX: ffffc90009ecc000
RDX: 0000000000000461 RSI: ffffffff86ab2717 RDI: 0000000000002308
RBP: ffff880181996998 R08: ffff88019690c700 R09: 0000000000000000
R10: fffffbfff128759a R11: 0000000000000000 R12: 0000000000000036
R13: 0000000000000000 R14: ffff8801d596dd68 R15: ffffea00000000d0
FS: 00007f25cc19f700(0000) GS:ffff8801daf00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000001cc821000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
xfrmi_rcv_cb+0xef/0x9d0 net/xfrm/xfrm_interface.c:256
xfrm6_rcv_cb+0x220/0x400 net/ipv6/xfrm6_protocol.c:59
xfrm_rcv_cb net/xfrm/xfrm_input.c:108 [inline]
xfrm_input+0x8aa/0x3190 net/xfrm/xfrm_input.c:496
xfrm6_rcv_spi net/ipv6/xfrm6_input.c:31 [inline]
xfrm6_rcv_tnl+0x168/0x1d0 net/ipv6/xfrm6_input.c:74
xfrm6_rcv+0x17/0x20 net/ipv6/xfrm6_input.c:81
xfrm6_esp_rcv+0x1a5/0x3a0 net/ipv6/xfrm6_protocol.c:74
ip6_input_finish+0x3fc/0x1aa0 net/ipv6/ip6_input.c:383
NF_HOOK include/linux/netfilter.h:289 [inline]
ip6_input+0xe9/0x600 net/ipv6/ip6_input.c:426
ip6_mc_input+0x48a/0xd20 net/ipv6/ip6_input.c:503
dst_input include/net/dst.h:450 [inline]
ip6_rcv_finish+0x17a/0x330 net/ipv6/ip6_input.c:76
NF_HOOK include/linux/netfilter.h:289 [inline]
ipv6_rcv+0x120/0x640 net/ipv6/ip6_input.c:271
__netif_receive_skb_one_core+0x14d/0x200 net/core/dev.c:4891
__netif_receive_skb+0x2c/0x1e0 net/core/dev.c:5001
netif_receive_skb_internal+0x12c/0x620 net/core/dev.c:5104
napi_frags_finish net/core/dev.c:5642 [inline]
napi_gro_frags+0x75a/0xc90 net/core/dev.c:5715
tun_get_user+0x3189/0x4250 drivers/net/tun.c:1923
tun_chr_write_iter+0xb9/0x154 drivers/net/tun.c:1968
call_write_iter include/linux/fs.h:1808 [inline]
do_iter_readv_writev+0x8b0/0xa80 fs/read_write.c:680
do_iter_write+0x185/0x5f0 fs/read_write.c:959
vfs_writev+0x1f1/0x360 fs/read_write.c:1004
do_writev+0x11a/0x310 fs/read_write.c:1039
__do_sys_writev fs/read_write.c:1112 [inline]
__se_sys_writev fs/read_write.c:1109 [inline]
__x64_sys_writev+0x75/0xb0 fs/read_write.c:1109
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457431
Code: 75 14 b8 14 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 54 b5 fb ff c3 48
83 ec 08 e8 1a 2d 00 00 48 89 04 24 b8 14 00 00 00 0f 05 <48> 8b 3c 24 48
89 c2 e8 63 2d 00 00 48 89 d0 48 83 c4 08 48 3d 01
RSP: 002b:00007f25cc19eba0 EFLAGS: 00000293 ORIG_RAX: 0000000000000014
RAX: ffffffffffffffda RBX: 000000000000004a RCX: 0000000000457431
RDX: 0000000000000001 RSI: 00007f25cc19ebf0 RDI: 00000000000000f0
RBP: 0000000020000000 R08: 00000000000000f0 R09: 0000000000000000
R10: 0000000000000064 R11: 0000000000000293 R12: 00007f25cc19f6d4
R13: 00000000004c48a4 R14: 00000000004d7b90 R15: 0000000000000004
Modules linked in:
---[ end trace 803f6dba779493b5 ]---
RIP: 0010:__read_once_size include/linux/compiler.h:188 [inline]
RIP: 0010:net_generic include/net/netns/generic.h:45 [inline]
RIP: 0010:xfrmi_lookup.isra.18+0x11f/0x6c0 net/xfrm/xfrm_interface.c:61
Code: 57 af d3 fa 45 84 ed 0f 84 4c 04 00 00 e8 79 ae d3 fa 48 8d bb 08 1b
00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f
85 7b 05 00 00 4c 8d 6d 98 48 8b 9b 08 1b 00 00 48
RSP: 0018:ffff880181996878 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000800 RCX: ffffc90009ecc000
RDX: 0000000000000461 RSI: ffffffff86ab2717 RDI: 0000000000002308
RBP: ffff880181996998 R08: ffff88019690c700 R09: 0000000000000000
R10: fffffbfff128759a R11: 0000000000000000 R12: 0000000000000036
R13: 0000000000000000 R14: ffff8801d596dd68 R15: ffffea00000000d0
FS: 00007f25cc19f700(0000) GS:ffff8801daf00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000001cc821000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* Re: [PATCH v3 1/2] kretprobe: produce sane stack traces
From: Josh Poimboeuf @ 2018-11-02 15:43 UTC (permalink / raw)
To: Steven Rostedt
Cc: Aleksa Sarai, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Masami Hiramatsu, Jonathan Corbet,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, Shuah Khan,
Alexei Starovoitov, Daniel Borkmann, Brendan Gregg,
Christian Brauner, Aleksa Sarai, netdev, linux-doc
In-Reply-To: <20181102091658.1bc979a4@gandalf.local.home>
On Fri, Nov 02, 2018 at 09:16:58AM -0400, Steven Rostedt wrote:
> On Fri, 2 Nov 2018 17:59:32 +1100
> Aleksa Sarai <cyphar@cyphar.com> wrote:
>
> > As an aside, I just tested with the frame unwinder and it isn't thrown
> > off-course by kretprobe_trampoline (though obviously the stack is still
> > wrong). So I think we just need to hook into the ORC unwinder to get it
> > to continue skipping up the stack, as well as add the rewriting code for
> > the stack traces (for all unwinders I guess -- though ideally we should
>
> I agree that this is the right solution.
Sounds good to me.
However, it would be *really* nice if function graph and kretprobes
shared the same infrastructure, like they do for function entry.
There's a lot of duplicated effort there.
> > do this without having to add the same code to every architecture).
>
> True, and there's an art to consolidating the code between
> architectures.
>
> I'm currently looking at function graph and seeing if I can consolidate
> it too. And I'm also trying to get multiple uses to hook into its
> infrastructure. I think I finally figured out a way to do so.
>
> The reason it is difficult, is that you need to maintain state between
> the entry of a function and the exit for each task and callback that is
> registered. Hence, it's a 3x tuple (function stack, task, callbacks).
> And this must be maintained with preemption. A task may sleep for
> minutes, and the state needs to be retained.
>
> The only state that must be retained is the function stack with the
> task, because if that gets out of sync, the system crashes. But the
> callback state can be removed.
>
> Here's what is there now:
>
> When something is registered with the function graph tracer, every
> task gets a shadowed stack. A hook is added to fork to add shadow
> stacks to new tasks. Once a shadow stack is added to a task, that
> shadow stack is never removed until the task exits.
>
> When the function is entered, the real return code is stored in the
> shadow stack and the trampoline address is put in its place.
>
> On return, the trampoline is called, and it will pop off the return
> code from the shadow stack and return to that.
>
> The issue with multiple users, is that different users may want to
> trace different functions. On entry, the user could say it doesn't want
> to trace the current function, and the return part must not be called
> on exit. Keeping track of which user needs the return called is the
> tricky part.
>
> Here's what I plan on implementing:
>
> Along with a shadow stack, I was going to add a 4096 byte (one page)
> array that holds 64 8 byte masks to every task as well. This will allow
> 64 simultaneous users (which is rather extreme). If we need to support
> more, we could allocate another page for all tasks. The 8 byte mask
> will represent each depth (allowing to do this for 64 function call
> stack depth, which should also be enough).
>
> Each user will be assigned one of the masks. Each bit in the mask
> represents the depth of the shadow stack. When a function is called,
> each user registered with the function graph tracer will get called
> (if they asked to be called for this function, via the ftrace_ops
> hashes) and if they want to trace the function, then the bit is set in
> the mask for that stack depth.
>
> When the function exits the function and we pop off the return code
> from the shadow stack, we then look at all the bits set for the
> corresponding users, and call their return callbacks, and ignore
> anything that is not set.
>
>
> When a user is unregistered, it the corresponding bits that represent
> it are cleared, and it the return callback will not be called. But the
> tasks being traced will still have their shadow stack to allow it to
> get back to normal.
>
> I'll hopefully have a prototype ready by plumbers.
Why do we need multiple users? It would be a lot simpler if we could
just enforce a single user per fgraphed/kretprobed function (and return
-EBUSY if it's already being traced/probed).
> And this too will require each architecture to probably change. As a
> side project to this, I'm going to try to consolidate the function
> graph code among all the architectures as well. Not an easy task.
Do you mean implementing HAVE_FUNCTION_GRAPH_RET_ADDR_PTR for all the
arches? If so, I think have an old crusty patch which attempted to
that. I could try to dig it up if you're interested.
--
Josh
^ permalink raw reply
* [PATCH 0/1] vhost: parallel virtqueue handling
From: Vitaly Mayatskikh @ 2018-11-02 16:07 UTC (permalink / raw)
To: Michael S . Tsirkin
Cc: Jason Wang, kvm, virtualization, netdev, linux-kernel,
Vitaly Mayatskikh
Hi,
I stumbled across poor performance of virtio-blk while working on a
high-performance network storage protocol. Moving virtio-blk's host
side to kernel did increase single queue IOPS, but multiqueue disk
still was not scaling well. It turned out that vhost handles events
from all virtio queues in one helper thread, and that's pretty much a
big serialization point.
The following patch enables events handling in per-queue thread and
increases IO concurrency, see IOPS numbers:
# num-queues
# bare metal
# virtio-blk
# vhost-blk
1 171k 148k 195k
2 328k 249k 349k
3 479k 179k 501k
4 622k 143k 620k
5 755k 136k 737k
6 887k 131k 830k
7 1004k 126k 926k
8 1099k 117k 1001k
9 1194k 115k 1055k
10 1278k 109k 1130k
11 1345k 110k 1119k
12 1411k 104k 1201k
13 1466k 106k 1260k
14 1517k 103k 1296k
15 1552k 102k 1322k
16 1480k 101k 1346k
Vitaly Mayatskikh (1):
vhost: add per-vq worker thread
drivers/vhost/vhost.c | 123 +++++++++++++++++++++++++++++++-----------
drivers/vhost/vhost.h | 11 +++-
2 files changed, 100 insertions(+), 34 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH] net/mlx5e: fix high stack usage
From: Jason Gunthorpe @ 2018-11-02 16:08 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Saeed Mahameed, Leon Romanovsky, David S. Miller, Tariq Toukan,
Eran Ben Elisha, Boris Pismenny, Ilya Lesokhin, Moshe Shemesh,
Kamal Heib, netdev, linux-rdma, linux-kernel
In-Reply-To: <20181102153316.1492515-1-arnd@arndb.de>
On Fri, Nov 02, 2018 at 04:33:03PM +0100, Arnd Bergmann wrote:
> A patch that looks harmless causes the stack usage of the mlx5e_grp_sw_update_stats()
> function to drastically increase with x86 gcc-4.9 and higher (tested up to 8.1):
>
> drivers/net/ethernet/mellanox/mlx5/core/en_stats.c: In function ‘mlx5e_grp_sw_update_stats’:
> drivers/net/ethernet/mellanox/mlx5/core/en_stats.c:216:1: warning: the frame size of 1276 bytes is larger than 500 bytes [-Wframe-larger-than=]
Why is the stack size so big here? The mlx5e_sw_stats is < 500 bytes
and all the other on-stack stuff looks pretty small?
> By splitting out the loop body into a non-inlined function, the stack size goes
> back down to under 500 bytes.
Does this actually reduce the stack consumed or does this just suppress
the warning?
Jason
^ permalink raw reply
* Re: [PATCH v3 1/2] kretprobe: produce sane stack traces
From: Steven Rostedt @ 2018-11-02 16:13 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Aleksa Sarai, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Masami Hiramatsu, Jonathan Corbet,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, Shuah Khan,
Alexei Starovoitov, Daniel Borkmann, Brendan Gregg,
Christian Brauner, Aleksa Sarai, netdev, linux-doc
In-Reply-To: <20181102154325.bt6xoysl4xdl33wd@treble>
On Fri, 2 Nov 2018 10:43:26 -0500
Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > I'll hopefully have a prototype ready by plumbers.
>
> Why do we need multiple users? It would be a lot simpler if we could
> just enforce a single user per fgraphed/kretprobed function (and return
> -EBUSY if it's already being traced/probed).
Because that means if function graph tracer is active, then you can't
do a kretprobe, and vice versa. I'd really like to have it working for
multiple users, then we could trace different graph functions and store
them in different buffers. It would also allow for perf to use function
graph tracer too.
>
> > And this too will require each architecture to probably change. As a
> > side project to this, I'm going to try to consolidate the function
> > graph code among all the architectures as well. Not an easy task.
>
> Do you mean implementing HAVE_FUNCTION_GRAPH_RET_ADDR_PTR for all the
> arches? If so, I think have an old crusty patch which attempted to
> that. I could try to dig it up if you're interested.
>
I'd like to have that, but it still requires some work. But I'd just
the truly architecture dependent code be in the architecture (basically
the asm code), and have the ability to move most of the duplicate code
out of the archs.
-- Steve
^ permalink raw reply
* Re: [PULL] vhost: cleanups and fixes
From: Linus Torvalds @ 2018-11-02 16:14 UTC (permalink / raw)
To: mst
Cc: mark.rutland, Kees Cook, kvm, virtualization, netdev,
Linux Kernel Mailing List, Andrew Morton, bijan.mottahedeh,
gedwards, joe, lenaic, liang.z.li, mhocko, mhocko, stefanha,
wei.w.wang
In-Reply-To: <20181102083018-mutt-send-email-mst@kernel.org>
On Fri, Nov 2, 2018 at 6:04 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> I've tried making access_ok mask the parameter it gets.
PLEASE don't do this.
Just use "copy_to/from_user()".
We have had lots of bugs because code bitrots.
And no, the access_ok() checks aren't expensive, not even in a loop.
They *used* to be somewhat expensive compared to the access, but that
simply isn't true any more. The real expense in copy_to_user and
friends are in the user access bit setting (STAC and CLAC on x86),
which easily an order of magnitude more expensive than access_ok().
So just get rid of the double-underscore version. It's basically
always a mis-optimization due to entirely historical reasons. I can
pretty much guarantee that it's not visible in profiles.
Linus
^ permalink raw reply
* [PATCH] ath10k: avoid -Wmaybe-uninitialized warning
From: Arnd Bergmann @ 2018-11-02 16:17 UTC (permalink / raw)
To: Kalle Valo, David S. Miller
Cc: Arnd Bergmann, Rakesh Pillai, Anilkumar Kolli, Balaji Pothunoori,
Yingying Tang, Sriram R, ath10k, linux-wireless, netdev,
linux-kernel
In some configurations the inlining in gcc is suboptimal, causing
a false-positive warning:
drivers/net/wireless/ath/ath10k/mac.c: In function 'ath10k_mac_init_rd':
drivers/net/wireless/ath/ath10k/mac.c:8374:39: error: 'rd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
ar->ath_common.regulatory.current_rd = rd;
If we initialize the output of ath10k_mac_get_wrdd_regulatory()
before returning, this problem goes away.
Fixes: 209b2a68de76 ("ath10k: add platform regulatory domain support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/wireless/ath/ath10k/mac.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index a1c2801ded10..0d5fde28ee44 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -8321,6 +8321,8 @@ static int ath10k_mac_get_wrdd_regulatory(struct ath10k *ar, u16 *rd)
u32 alpha2_code;
char alpha2[3];
+ *rd = ar->hw_eeprom_rd;
+
root_handle = ACPI_HANDLE(&pdev->dev);
if (!root_handle)
return -EOPNOTSUPP;
@@ -8365,11 +8367,9 @@ static int ath10k_mac_init_rd(struct ath10k *ar)
u16 rd;
ret = ath10k_mac_get_wrdd_regulatory(ar, &rd);
- if (ret) {
+ if (ret)
ath10k_dbg(ar, ATH10K_DBG_BOOT,
"fallback to eeprom programmed regulatory settings\n");
- rd = ar->hw_eeprom_rd;
- }
ar->ath_common.regulatory.current_rd = rd;
return 0;
--
2.18.0
^ permalink raw reply related
* Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed
From: Andrey Ryabinin @ 2018-11-02 16:19 UTC (permalink / raw)
To: Peter Zijlstra, Trond Myklebust
Cc: mark.rutland@arm.com, linux-kernel@vger.kernel.org,
ralf@linux-mips.org, jlayton@kernel.org,
linuxppc-dev@lists.ozlabs.org, bfields@fieldses.org,
linux-mips@linux-mips.org, linux@roeck-us.net,
linux-nfs@vger.kernel.org, akpm@linux-foundation.org,
will.deacon@arm.com, boqun.feng@gmail.com, paul.burton@mips.com,
anna.schumaker@netapp.com, jhogan@kernel.org, "netdev@vger.ke
In-Reply-To: <20181101163212.GF3159@hirez.programming.kicks-ass.net>
On 11/01/2018 07:32 PM, Peter Zijlstra wrote:
> On Thu, Nov 01, 2018 at 03:22:15PM +0000, Trond Myklebust wrote:
>> On Thu, 2018-11-01 at 15:59 +0100, Peter Zijlstra wrote:
>>> On Thu, Nov 01, 2018 at 01:18:46PM +0000, Mark Rutland wrote:
>
>>>>> My one question (and the reason why I went with cmpxchg() in the
>>>>> first place) would be about the overflow behaviour for
>>>>> atomic_fetch_inc() and friends. I believe those functions should
>>>>> be OK on x86, so that when we overflow the counter, it behaves
>>>>> like an unsigned value and wraps back around. Is that the case
>>>>> for all architectures?
>>>>>
>>>>> i.e. are atomic_t/atomic64_t always guaranteed to behave like
>>>>> u32/u64 on increment?
>>>>>
>>>>> I could not find any documentation that explicitly stated that
>>>>> they should.
>>>>
>>>> Peter, Will, I understand that the atomic_t/atomic64_t ops are
>>>> required to wrap per 2's-complement. IIUC the refcount code relies
>>>> on this.
>>>>
>>>> Can you confirm?
>>>
>>> There is quite a bit of core code that hard assumes 2s-complement.
>>> Not only for atomics but for any signed integer type. Also see the
>>> kernel using -fno-strict-overflow which implies -fwrapv, which
>>> defines signed overflow to behave like 2s-complement (and rids us of
>>> that particular UB).
>>
>> Fair enough, but there have also been bugfixes to explicitly fix unsafe
>> C standards assumptions for signed integers. See, for instance commit
>> 5a581b367b5d "jiffies: Avoid undefined behavior from signed overflow"
>> from Paul McKenney.
>
> Yes, I feel Paul has been to too many C/C++ committee meetings and got
> properly paranoid. Which isn't always a bad thing :-)
>
> But for us using -fno-strict-overflow which actually defines signed
> overflow, I myself am really not worried. I'm also not sure if KASAN has
> been taught about this, or if it will still (incorrectly) warn about UB
> for signed types.
>
UBSAN warns about signed overflows despite -fno-strict-overflow if gcc version is < 8.
I have learned recently that UBSAN in GCC 8 ignores signed overflows if -fno-strict-overflow of fwrapv is used.
$ cat signed_overflow.c
#include <stdio.h>
__attribute__((noinline))
int foo(int a, int b)
{
return a+b;
}
int main(void)
{
int a = 0x7fffffff;
int b = 2;
printf("%d\n", foo(a,b));
return 0;
}
$ gcc-8.2.0 -fsanitize=signed-integer-overflow signed_overflow.c && ./a.out
signed_overflow.c:6:10: runtime error: signed integer overflow: 2147483647 + 2 cannot be represented in type 'int'
-2147483647
$ gcc-8.2.0 -fno-strict-overflow -fsanitize=signed-integer-overflow signed_overflow.c && ./a.out
-2147483647
$ gcc-7.3.0 -fsanitize=signed-integer-overflow signed_overflow.c && ./a.out
signed_overflow.c:6:10: runtime error: signed integer overflow: 2147483647 + 2 cannot be represented in type 'int'
-2147483647
$ gcc-7.3.0 -fno-strict-overflow -fsanitize=signed-integer-overflow signed_overflow.c && ./a.out
signed_overflow.c:6:10: runtime error: signed integer overflow: 2147483647 + 2 cannot be represented in type 'int'
-2147483647
clang behaves the same way as GCC 8:
$ clang -fsanitize=signed-integer-overflow signed_overflow.c && ./a.out
signed_overflow.c:6:10: runtime error: signed integer overflow: 2147483647 + 2 cannot be represented in type 'int'
-2147483647
$ clang -fno-strict-overflow -fsanitize=signed-integer-overflow signed_overflow.c && ./a.out
-2147483647
We can always just drop -fsanitize=signed-integer-overflow if it considered too noisy.
Although it did catch some real bugs.
^ permalink raw reply
* Re: [PATCH] net/mlx5e: fix high stack usage
From: Arnd Bergmann @ 2018-11-02 16:23 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Saeed Mahameed, Leon Romanovsky, David S. Miller, Tariq Toukan,
Eran Ben Elisha, Boris Pismenny, Ilya Lesokhin, Moshe Shemesh,
Kamal Heib, netdev, linux-rdma, linux-kernel
In-Reply-To: <20181102160858.GA17096@ziepe.ca>
On 11/2/18, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Fri, Nov 02, 2018 at 04:33:03PM +0100, Arnd Bergmann wrote:
>> A patch that looks harmless causes the stack usage of the
>> mlx5e_grp_sw_update_stats()
>> function to drastically increase with x86 gcc-4.9 and higher (tested up to
>> 8.1):
>>
>> drivers/net/ethernet/mellanox/mlx5/core/en_stats.c: In function
>> ‘mlx5e_grp_sw_update_stats’:
>> drivers/net/ethernet/mellanox/mlx5/core/en_stats.c:216:1: warning: the
>> frame size of 1276 bytes is larger than 500 bytes [-Wframe-larger-than=]
>
> Why is the stack size so big here? The mlx5e_sw_stats is < 500 bytes
> and all the other on-stack stuff looks pretty small?
I am not entirely sure, but my analysis indicates that gcc tries loop unrolling
or some other optimization that leads to two copies on the stack.
>> By splitting out the loop body into a non-inlined function, the stack size
>> goes
>> back down to under 500 bytes.
>
> Does this actually reduce the stack consumed or does this just suppress
> the warning?
It definitely reduces the total stack usage, the separate functions just
had the expected stack usage that was a few hundred bytes combined.
Arnd
^ permalink raw reply
* Re: [PATCH] ath10k: avoid -Wmaybe-uninitialized warning
From: Brian Norris @ 2018-11-02 16:34 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Kalle Valo, davem, pillair, akolli, bpothuno, yintang, srirrama,
ath10k, linux-wireless, netdev, Linux Kernel
In-Reply-To: <20181102161833.2956376-1-arnd@arndb.de>
Hi,
On Fri, Nov 2, 2018 at 9:19 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> In some configurations the inlining in gcc is suboptimal, causing
> a false-positive warning:
>
> drivers/net/wireless/ath/ath10k/mac.c: In function 'ath10k_mac_init_rd':
> drivers/net/wireless/ath/ath10k/mac.c:8374:39: error: 'rd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> ar->ath_common.regulatory.current_rd = rd;
>
> If we initialize the output of ath10k_mac_get_wrdd_regulatory()
> before returning, this problem goes away.
>
> Fixes: 209b2a68de76 ("ath10k: add platform regulatory domain support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/wireless/ath/ath10k/mac.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index a1c2801ded10..0d5fde28ee44 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -8321,6 +8321,8 @@ static int ath10k_mac_get_wrdd_regulatory(struct ath10k *ar, u16 *rd)
> u32 alpha2_code;
> char alpha2[3];
>
> + *rd = ar->hw_eeprom_rd;
> +
Maybe it's just me, but it seems kinda weird for this function to
assign a (valid) value to its "output" and still potentially return an
error.
If you really need to work around this compiler bug, maybe just put
the eeprom assignment back in ath10k_mac_init_rd()? I'll leave it up
to Kalle as to whether he wants to work around the compiler at all :)
Oh wait, one more thing: this is actually an invalid refactoring. See
how this function assigns '*rd' later in error cases. Today, we still
treat those as errors and clobber those with the eeprom value, but
now, you're making the fallback case continue to use the erroneous
value (0xffff). You need to make that use a local variable and avoid
clobbering *rd, if you want this to be correct.
> root_handle = ACPI_HANDLE(&pdev->dev);
Side note: your patch made me notice -- this code is *not* only used
on PCI devices, yet it utilizes the 'pci_dev' structure. Fortunately,
it only does this to needless convert it right back to a bare
'device', and then the ACPI code safely handles non-ACPI devices
(should result in -EOPNOTSUPP below), but this is awfully strange
code.
Anyway, I'm just thinking out loud. I'll probably patch the pci_dev out myself.
Brian
> if (!root_handle)
> return -EOPNOTSUPP;
> @@ -8365,11 +8367,9 @@ static int ath10k_mac_init_rd(struct ath10k *ar)
> u16 rd;
>
> ret = ath10k_mac_get_wrdd_regulatory(ar, &rd);
> - if (ret) {
> + if (ret)
> ath10k_dbg(ar, ATH10K_DBG_BOOT,
> "fallback to eeprom programmed regulatory settings\n");
> - rd = ar->hw_eeprom_rd;
> - }
>
> ar->ath_common.regulatory.current_rd = rd;
> return 0;
> --
> 2.18.0
>
^ permalink raw reply
* Re: [PULL] vhost: cleanups and fixes
From: Michael S. Tsirkin @ 2018-11-02 16:59 UTC (permalink / raw)
To: Linus Torvalds
Cc: mark.rutland, Kees Cook, kvm, virtualization, netdev,
Linux Kernel Mailing List, Andrew Morton, bijan.mottahedeh,
gedwards, joe, lenaic, liang.z.li, mhocko, mhocko, stefanha,
wei.w.wang, Jason Wang
In-Reply-To: <CAHk-=wh_bQK5zs+CwQ5eyodq4sQT0eOPp60qzvVL2_EtgETP-g@mail.gmail.com>
On Fri, Nov 02, 2018 at 09:14:51AM -0700, Linus Torvalds wrote:
> On Fri, Nov 2, 2018 at 6:04 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > I've tried making access_ok mask the parameter it gets.
>
> PLEASE don't do this.
Okay.
> Just use "copy_to/from_user()".
Just for completeness I'd like to point out for vhost the copies are
done from the kernel thread. So yes we can switch to copy_to/from_user
but for e.g. 32-bit userspace running on top of a 64 bit kernel it is
IIUC not sufficient - we must *also* do access_ok checks on control path
when addresses are passed to the kernel and when current points to the
correct task struct.
> We have had lots of bugs because code bitrots.
Yes, I wish we did not need these access_ok checks and could just rely
on copy_to/from_user.
> And no, the access_ok() checks aren't expensive, not even in a loop.
> They *used* to be somewhat expensive compared to the access, but that
> simply isn't true any more. The real expense in copy_to_user and
> friends are in the user access bit setting (STAC and CLAC on x86),
> which easily an order of magnitude more expensive than access_ok().
>
> So just get rid of the double-underscore version. It's basically
> always a mis-optimization due to entirely historical reasons. I can
> pretty much guarantee that it's not visible in profiles.
>
> Linus
OK. So maybe we should focus on switching to user_access_begin/end +
unsafe_get_user/unsafe_put_user in a loop which does seem to be
measureable. That moves the barrier out of the loop, which seems to be
consistent with what you would expect.
--
MST
^ permalink raw reply
* Re: [PATCH] net/mlx5e: fix high stack usage
From: Jason Gunthorpe @ 2018-11-02 17:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Saeed Mahameed, Leon Romanovsky, David S. Miller, Tariq Toukan,
Eran Ben Elisha, Boris Pismenny, Ilya Lesokhin, Moshe Shemesh,
Kamal Heib, netdev, linux-rdma, linux-kernel
In-Reply-To: <CAK8P3a2pvkfR1A6dHAMm8ETd13E1LSxXrEsQHSOFYWbr-7R2Sw@mail.gmail.com>
On Fri, Nov 02, 2018 at 05:23:26PM +0100, Arnd Bergmann wrote:
> On 11/2/18, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > On Fri, Nov 02, 2018 at 04:33:03PM +0100, Arnd Bergmann wrote:
> >> A patch that looks harmless causes the stack usage of the
> >> mlx5e_grp_sw_update_stats()
> >> function to drastically increase with x86 gcc-4.9 and higher (tested up to
> >> 8.1):
> >>
> >> drivers/net/ethernet/mellanox/mlx5/core/en_stats.c: In function
> >> ‘mlx5e_grp_sw_update_stats’:
> >> drivers/net/ethernet/mellanox/mlx5/core/en_stats.c:216:1: warning: the
> >> frame size of 1276 bytes is larger than 500 bytes [-Wframe-larger-than=]
> >
> > Why is the stack size so big here? The mlx5e_sw_stats is < 500 bytes
> > and all the other on-stack stuff looks pretty small?
>
> I am not entirely sure, but my analysis indicates that gcc tries loop unrolling
> or some other optimization that leads to two copies on the stack.
Wow how strange
Did you consier adding a
__attribute__((optimize("no-unroll-loops")))
type macro?
Jason
^ permalink raw reply
* Re: [PULL] vhost: cleanups and fixes
From: Linus Torvalds @ 2018-11-02 17:10 UTC (permalink / raw)
To: mst
Cc: mark.rutland, lenaic, mhocko, Kees Cook, kvm, netdev, liang.z.li,
Linux Kernel Mailing List, virtualization, stefanha, joe,
Andrew Morton, mhocko, bijan.mottahedeh
In-Reply-To: <20181102122937-mutt-send-email-mst@kernel.org>
On Fri, Nov 2, 2018 at 9:59 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> Just for completeness I'd like to point out for vhost the copies are
> done from the kernel thread. So yes we can switch to copy_to/from_user
> but for e.g. 32-bit userspace running on top of a 64 bit kernel it is
> IIUC not sufficient - we must *also* do access_ok checks on control path
> when addresses are passed to the kernel and when current points to the
> correct task struct.
Don't you take over the VM with "use_mm()" when you do the copies? So
yes, it's a kernel thread, but it has a user VM, and though that
should have the user limits.
No?
Linus
^ permalink raw reply
* Re: [PULL] vhost: cleanups and fixes
From: Linus Torvalds @ 2018-11-02 17:15 UTC (permalink / raw)
To: mst, Al Viro
Cc: mark.rutland, Kees Cook, kvm, virtualization, netdev,
Linux Kernel Mailing List, Andrew Morton, bijan.mottahedeh,
gedwards, joe, lenaic, liang.z.li, mhocko, mhocko, stefanha,
wei.w.wang, jasowang
In-Reply-To: <CAHk-=wibzqo7C+mS+BgZxRbgdWe2w5F39EhuFUhZUxvotoGLuA@mail.gmail.com>
On Fri, Nov 2, 2018 at 10:10 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Don't you take over the VM with "use_mm()" when you do the copies? So
> yes, it's a kernel thread, but it has a user VM, and though that
> should have the user limits.
Oooh. *Just* as I sent this, I realized that "use_mm()" doesn't update
the thread addr_limit.
That actually looks like a bug to me - although one that you've
apparently been aware of and worked around.
Wouldn't it be nicer to just make "use_mm()" do
set_fs(USER_DS);
instead? And undo it on unuse_mm()?
And, in fact, maybe we should default kernel threads to have a zero
address limit, so that they can't do any user accesses at all without
doing this?
Adding Al to the cc, because I think he's been looking at set_fs() in general.
Linus
^ permalink raw reply
* Re: [PULL] vhost: cleanups and fixes
From: Michael S. Tsirkin @ 2018-11-02 17:21 UTC (permalink / raw)
To: Linus Torvalds
Cc: mark.rutland, Kees Cook, kvm, virtualization, netdev,
Linux Kernel Mailing List, Andrew Morton, bijan.mottahedeh,
gedwards, joe, lenaic, liang.z.li, mhocko, mhocko, stefanha,
wei.w.wang, jasowang
In-Reply-To: <CAHk-=wibzqo7C+mS+BgZxRbgdWe2w5F39EhuFUhZUxvotoGLuA@mail.gmail.com>
On Fri, Nov 02, 2018 at 10:10:45AM -0700, Linus Torvalds wrote:
> On Fri, Nov 2, 2018 at 9:59 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > Just for completeness I'd like to point out for vhost the copies are
> > done from the kernel thread. So yes we can switch to copy_to/from_user
> > but for e.g. 32-bit userspace running on top of a 64 bit kernel it is
> > IIUC not sufficient - we must *also* do access_ok checks on control path
> > when addresses are passed to the kernel and when current points to the
> > correct task struct.
>
> Don't you take over the VM with "use_mm()" when you do the copies?
Yes we do.
> So
> yes, it's a kernel thread, but it has a user VM, and though that
> should have the user limits.
>
> No?
>
> Linus
Here's what I meant: we have
#define access_ok(type, addr, size) \
({ \
WARN_ON_IN_IRQ(); \
likely(!__range_not_ok(addr, size, user_addr_max())); \
})
and
#define user_addr_max() (current->thread.addr_limit.seg)
it seems that it depends on current not on the active mm.
get_user and friends are similar:
ENTRY(__get_user_1)
mov PER_CPU_VAR(current_task), %_ASM_DX
cmp TASK_addr_limit(%_ASM_DX),%_ASM_AX
jae bad_get_user
sbb %_ASM_DX, %_ASM_DX /* array_index_mask_nospec() */
and %_ASM_DX, %_ASM_AX
ASM_STAC
1: movzbl (%_ASM_AX),%edx
xor %eax,%eax
ASM_CLAC
ret
ENDPROC(__get_user_1)
EXPORT_SYMBOL(__get_user_1)
--
MST
^ permalink raw reply
* Re: [PULL] vhost: cleanups and fixes
From: Michael S. Tsirkin @ 2018-11-02 18:12 UTC (permalink / raw)
To: Linus Torvalds
Cc: mark.rutland, Kees Cook, kvm, virtualization, netdev,
Linux Kernel Mailing List, Andrew Morton, bijan.mottahedeh,
gedwards, joe, lenaic, liang.z.li, mhocko, mhocko, stefanha,
wei.w.wang, jasowang
In-Reply-To: <CAHk-=wjAvx_rWKct5RtqRrVEgNxrFF=1qQwkz+AegDvd4yXW=g@mail.gmail.com>
On Fri, Nov 02, 2018 at 11:02:35AM -0700, Linus Torvalds wrote:
> On Fri, Nov 2, 2018 at 10:21 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > it seems that it depends on current not on the active mm.
>
> Yes, see my other suggestion to just fix "use_mm()" to update the address range.
>
> Would you mind testing that?
Sure, I'll test it.
> Because that would seem to be the *much* less error-prone model..
>
> Linus
I agree, it's always been bothering me.
--
MST
^ permalink raw reply
* Re: [RFC bpf-next] libbpf: increase rlimit before trying to create BPF maps
From: Daniel Borkmann @ 2018-11-02 9:08 UTC (permalink / raw)
To: Quentin Monnet, Alexei Starovoitov; +Cc: netdev, oss-drivers
In-Reply-To: <bb9118cc-daa2-8e73-96e4-79fed2a1de0a@netronome.com>
On 11/01/2018 06:18 PM, Quentin Monnet wrote:
> 2018-10-30 15:23 UTC+0000 ~ Quentin Monnet <quentin.monnet@netronome.com>
>> The limit for memory locked in the kernel by a process is usually set to
>> 64 bytes by default. This can be an issue when creating large BPF maps.
>> A workaround is to raise this limit for the current process before
>> trying to create a new BPF map. Changing the hard limit requires the
>> CAP_SYS_RESOURCE and can usually only be done by root user (but then
>> only root can create BPF maps).
>
> Sorry, the parenthesis is not correct: non-root users can in fact create
> BPF maps as well. If a non-root user calls the function to create a map,
> setrlimit() will fail silently (but set errno), and the program will
> simply go on with its rlimit unchanged.
>
>> As far as I know there is not API to get the current amount of memory
>> locked for a user, therefore we cannot raise the limit only when
>> required. One solution, used by bcc, is to try to create the map, and on
>> getting a EPERM error, raising the limit to infinity before giving
>> another try. Another approach, used in iproute, is to raise the limit in
>> all cases, before trying to create the map.
>>
>> Here we do the same as in iproute2: the rlimit is raised to infinity
>> before trying to load the map.
>>
>> I send this patch as a RFC to see if people would prefer the bcc
>> approach instead, or the rlimit change to be in bpftool rather than in
>> libbpf.
I'd avoid doing something like this in a generic library; it's basically an
ugly hack for the kind of accounting we're doing and only shows that while
this was "good enough" to start off with in the early days, we should be
doing something better today if every application raises it to inf anyway
then it's broken. :) It just shows that this missed its purpose. Similarly
to the jit_limit discussion on rlimit, perhaps we should be considering
switching to something else entirely from kernel side. Could be something
like memcg but this definitely needs some more evaluation first. (Meanwhile
I'd not change the lib but callers instead and once we have something better
in place we remove this type of "raising to inf" from the tree ...)
>> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
>> ---
>> tools/lib/bpf/bpf.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>> index 03f9bcc4ef50..456a5a7b112c 100644
>> --- a/tools/lib/bpf/bpf.c
>> +++ b/tools/lib/bpf/bpf.c
>> @@ -26,6 +26,8 @@
>> #include <unistd.h>
>> #include <asm/unistd.h>
>> #include <linux/bpf.h>
>> +#include <sys/resource.h>
>> +#include <sys/types.h>
>> #include "bpf.h"
>> #include "libbpf.h"
>> #include <errno.h>
>> @@ -68,8 +70,11 @@ static inline int sys_bpf(enum bpf_cmd cmd, union bpf_attr *attr,
>> int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr)
>> {
>> __u32 name_len = create_attr->name ? strlen(create_attr->name) : 0;
>> + struct rlimit rinf = { RLIM_INFINITY, RLIM_INFINITY };
>> union bpf_attr attr;
>>
>> + setrlimit(RLIMIT_MEMLOCK, &rinf);
>> +
>> memset(&attr, '\0', sizeof(attr));
>>
>> attr.map_type = create_attr->map_type;
>>
>
^ permalink raw reply
* [PATCH] net: core: netpoll: Enable netconsole IPv6 link local address
From: Matwey V. Kornilov @ 2018-11-02 18:19 UTC (permalink / raw)
To: David S. Miller
Cc: matwey.kornilov, Eric Dumazet, Ingo Molnar, Thomas Gleixner,
Frederic Weisbecker, Debabrata Banerjee, Dave Jones, netdev,
linux-kernel, Matwey V. Kornilov
There is no reason to discard using source link local address when
remote netconsole IPv6 address is set to be link local one.
The patch allows administrators to use IPv6 netconsole without
explicitly configuring source address:
netconsole=@/,@fe80::5054:ff:fe2f:6012/
Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
---
net/core/netpoll.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 5da9552b186b..2b9fdbc43205 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -717,7 +717,8 @@ int netpoll_setup(struct netpoll *np)
read_lock_bh(&idev->lock);
list_for_each_entry(ifp, &idev->addr_list, if_list) {
- if (ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL)
+ if (!!(ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL) !=
+ !!(ipv6_addr_type(&np->remote_ip.in6) & IPV6_ADDR_LINKLOCAL))
continue;
np->local_ip.in6 = ifp->addr;
err = 0;
--
2.16.4
^ permalink raw reply related
* Re: [PULL] vhost: cleanups and fixes
From: Al Viro @ 2018-11-02 19:01 UTC (permalink / raw)
To: Linus Torvalds
Cc: mst, mark.rutland, Kees Cook, kvm, virtualization, netdev,
Linux Kernel Mailing List, Andrew Morton, bijan.mottahedeh,
gedwards, joe, lenaic, liang.z.li, mhocko, mhocko, stefanha,
wei.w.wang, jasowang
In-Reply-To: <CAHk-=wj5ZnqmpDuXBCcND8nMNitNyRf_4KQSzSUfQvX2-wOYsg@mail.gmail.com>
On Fri, Nov 02, 2018 at 10:15:56AM -0700, Linus Torvalds wrote:
> On Fri, Nov 2, 2018 at 10:10 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Don't you take over the VM with "use_mm()" when you do the copies? So
> > yes, it's a kernel thread, but it has a user VM, and though that
> > should have the user limits.
>
> Oooh. *Just* as I sent this, I realized that "use_mm()" doesn't update
> the thread addr_limit.
>
> That actually looks like a bug to me - although one that you've
> apparently been aware of and worked around.
>
> Wouldn't it be nicer to just make "use_mm()" do
>
> set_fs(USER_DS);
>
> instead? And undo it on unuse_mm()?
>
> And, in fact, maybe we should default kernel threads to have a zero
> address limit, so that they can't do any user accesses at all without
> doing this?
Try it and watch it fail to set initramfs up, let alone exec the init...
> Adding Al to the cc, because I think he's been looking at set_fs() in general.
It would be the right thing (with return to KERNEL_DS), but I'm not certain
if GPU users will survive - these two
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h:157: use_mm(mmptr); \
drivers/gpu/drm/i915/gvt/kvmgt.c:1799: use_mm(kvm->mm);
I don't understand the call chains there (especially for the first one) well
enough to tell.
^ permalink raw reply
* Re: [PATCH bpf 3/3] bpf: show main program address in bpf_prog_info->jited_ksyms
From: Daniel Borkmann @ 2018-11-02 9:57 UTC (permalink / raw)
To: Song Liu, netdev; +Cc: kernel-team, ast, sandipan
In-Reply-To: <20181101070058.2760251-4-songliubraving@fb.com>
On 11/01/2018 08:00 AM, Song Liu wrote:
> Currently, when there is not subprog (prog->aux->func_cnt == 0),
> bpf_prog_info does not return any jited_ksyms. This patch adds
> main program address (prog->bpf_func) to jited_ksyms.
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
> kernel/bpf/syscall.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 34a9eef5992c..7293b17ca62a 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2158,7 +2158,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
> }
>
> ulen = info.nr_jited_ksyms;
> - info.nr_jited_ksyms = prog->aux->func_cnt;
> + info.nr_jited_ksyms = prog->aux->func_cnt ? : 1;
> if (info.nr_jited_ksyms && ulen) {
> if (bpf_dump_raw_ok()) {
> u64 __user *user_ksyms;
> @@ -2170,9 +2170,17 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
> */
> ulen = min_t(u32, info.nr_jited_ksyms, ulen);
> user_ksyms = u64_to_user_ptr(info.jited_ksyms);
> - for (i = 0; i < ulen; i++) {
> - ksym_addr = (ulong) prog->aux->func[i]->bpf_func;
> - if (put_user((u64) ksym_addr, &user_ksyms[i]))
> + if (prog->aux->func_cnt) {
> + for (i = 0; i < ulen; i++) {
> + ksym_addr = (ulong)
Small nit: can we change ksym_addr, the above and below cast to kernel-style
'unsigned long' while at it?
> + prog->aux->func[i]->bpf_func;
> + if (put_user((u64) ksym_addr,
> + &user_ksyms[i]))
> + return -EFAULT;
> + }
> + } else {
> + ksym_addr = (ulong) prog->bpf_func;
> + if (put_user((u64) ksym_addr, &user_ksyms[0]))
> return -EFAULT;
If we do this here, I think we should also update nr_jited_func_lens to copy
prog->jited_len to user space to be consistent with this change here. In case
of multi-func, the latter copies the len of the main program, and the lens of
the subprogs. Given we push the address for it to user space, we should then
also push the main prog len if it's only main prog there so this case doesn't
need any special handling by user space.
> }
> } else {
>
^ permalink raw reply
* Re: [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
From: Stefano Brivio @ 2018-11-02 9:58 UTC (permalink / raw)
To: David Ahern; +Cc: Yoann P., Stephen Hemminger, netdev
In-Reply-To: <adb1ade4-e284-73fc-ff77-c1b1b2b4c3f1@gmail.com>
On Wed, 31 Oct 2018 20:48:05 -0600
David Ahern <dsahern@gmail.com> wrote:
> On 10/30/18 11:34 AM, Stefano Brivio wrote:
> >
> > - the current column abstraction is rather lightweight, things are
> > already buffered in the defined column order so we don't have to jump
> > back and forth in the buffer while rendering. Doing that needs some
> > extra care to avoid a performance hit, but it's probably doable, I
> > can put that on my to-do list
>
> The ss command is always a pain; it's much better in newer releases but
> I am always having to adjust terminal width.
Ouch. Do you have some examples?
--
Stefano
^ permalink raw reply
* Re: [PATCH iproute2 net-next 0/3] ss: Allow selection of columns to be displayed
From: Stefano Brivio @ 2018-11-02 9:58 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: David Ahern, Yoann P., Stephen Hemminger, netdev
In-Reply-To: <20181101140623.4d6211a0@cakuba.netronome.com>
On Thu, 1 Nov 2018 14:06:23 -0700
Jakub Kicinski <kubakici@wp.pl> wrote:
> JSONification would probably be quite an undertaking for ss :(
Probably not too much, and we would skip buffering for JSON as we don't
need to print columns, so we don't have to care about buffering and
rendering performance too much.
All it takes is to extend a bit the out() function and pass the right
arguments to it -- it looks way easier than implementing format
strings, even though I'd like the latter more.
--
Stefano
^ permalink raw reply
* Re: Ethernet on my CycloneV broke since 4.9.124
From: Clément Péron @ 2018-11-02 10:02 UTC (permalink / raw)
To: Dinh Nguyen; +Cc: netdev
In-Reply-To: <d1029588-e181-b804-d2f7-cc9990f6bf70@kernel.org>
Hi Dinh,
On Wed, 31 Oct 2018 at 23:02, Dinh Nguyen <dinguyen@kernel.org> wrote:
>
> Hi Clement,
>
> On 10/31/2018 10:36 AM, Clément Péron wrote:
> > Hi Dinh,
> >
> > On Wed, 31 Oct 2018 at 15:42, Dinh Nguyen <dinguyen@kernel.org> wrote:
> >>
> >> Hi Clement,
> >>
> >> On 10/31/2018 08:01 AM, Clément Péron wrote:
> >>> Hi,
> >>>
> >>> The patch "net: stmmac: socfpga: add additional ocp reset line for
> >>> Stratix10" introduce in 4.9.124 broke the ethernet on my CycloneV
> >>> board.
> >>>
> >>> When I boot i have this issue :
> >>>
> >>> socfpga-dwmac ff702000.ethernet: error getting reset control of ocp -2
> >>> socfpga-dwmac: probe of ff702000.ethernet failed with error -2
> >>>
> >>> Reverting the commit : 6f37f7b62baa6a71d7f3f298acb64de51275e724 fix the issue.
> >>>
> >>
> >> Are you sure? I just booted v4.9.124 and did not see any errors. The
> >> error should not appear because the commit is using
> >> devm_reset_control_get_optional().
> >
> > I'm booting on 4.9.130 actually, Agree with you that
> > devm_reset_control_get_optional should not failed but checking other
> > usage of this helper
> > https://elixir.bootlin.com/linux/v4.9.135/source/drivers/i2c/busses/i2c-mv64xxx.c#L824
> > https://elixir.bootlin.com/linux/v4.9.135/source/drivers/crypto/sunxi-ss/sun4i-ss-core.c#L259
> > Show that they don't check for errors except for PROBE_DEFER
> >
>
> I made a mistake, I was booting linux-next. I am seeing the error with
> v4.9.124. It's due to this commit not getting backported:
>
> "bb475230b8e59a reset: make optional functions really optional"
>
> I have backported the patch and is available here if you like to take a
> look:
Thanks, works fine on my board too.
Regards,
Clement
>
> https://git.kernel.org/pub/scm/linux/kernel/git/dinguyen/linux.git/log/?h=v4.9.124_optional_reset
>
> Dinh
^ 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