lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1485836511.20550.21.camel@perches.com>
Date:   Mon, 30 Jan 2017 20:21:51 -0800
From:   Joe Perches <joe@...ches.com>
To:     Alexei Starovoitov <ast@...com>,
        "David S . Miller" <davem@...emloft.net>
Cc:     Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org
Subject: Re: [PATCH v2 net-next] net: ethtool: convert large order kmalloc
 allocations to vzalloc

On Mon, 2017-01-30 at 19:41 -0800, Alexei Starovoitov wrote:
> On 1/30/17 7:28 PM, Joe Perches wrote:
> > On Mon, 2017-01-30 at 18:25 -0800, Alexei Starovoitov wrote:
> > > under memory pressure 'ethtool -S' command may warn:
> > > [ 2374.385195] ethtool: page allocation failure: order:4, mode:0x242c0c0
> > > [ 2374.405573] CPU: 12 PID: 40211 Comm: ethtool Not tainted
> > > [ 2374.423071] Call Trace:
> > > [ 2374.423076]  [<ffffffff8148cb29>] dump_stack+0x4d/0x64
> > > [ 2374.423080]  [<ffffffff811667cb>] warn_alloc_failed+0xeb/0x150
> > > [ 2374.423082]  [<ffffffff81169cd3>] ? __alloc_pages_direct_compact+0x43/0xf0
> > > [ 2374.423084]  [<ffffffff8116a25c>] __alloc_pages_nodemask+0x4dc/0xbf0
> > > [ 2374.423091]  [<ffffffffa0023dc2>] ? cmd_exec+0x722/0xcd0 [mlx5_core]
> > > [ 2374.423095]  [<ffffffff811b3dcc>] alloc_pages_current+0x8c/0x110
> > > [ 2374.423097]  [<ffffffff81168859>] alloc_kmem_pages+0x19/0x90
> > > [ 2374.423099]  [<ffffffff81186e5e>] kmalloc_order_trace+0x2e/0xe0
> > > [ 2374.423101]  [<ffffffff811c0084>] __kmalloc+0x204/0x220
> > > [ 2374.423105]  [<ffffffff816c269e>] dev_ethtool+0xe4e/0x1f80
> > > [ 2374.423106]  [<ffffffff816b967e>] ? dev_get_by_name_rcu+0x5e/0x80
> > > [ 2374.423108]  [<ffffffff816d6926>] dev_ioctl+0x156/0x560
> > > [ 2374.423111]  [<ffffffff811d4c68>] ? mem_cgroup_commit_charge+0x78/0x3c0
> > > [ 2374.423117]  [<ffffffff8169d542>] sock_do_ioctl+0x42/0x50
> > > [ 2374.423119]  [<ffffffff8169d9c3>] sock_ioctl+0x1b3/0x250
> > > [ 2374.423121]  [<ffffffff811f0f42>] do_vfs_ioctl+0x92/0x580
> > > [ 2374.423123]  [<ffffffff8100222b>] ? do_audit_syscall_entry+0x4b/0x70
> > > [ 2374.423124]  [<ffffffff8100287c>] ? syscall_trace_enter_phase1+0xfc/0x120
> > > [ 2374.423126]  [<ffffffff811f14a9>] SyS_ioctl+0x79/0x90
> > > [ 2374.423127]  [<ffffffff81002bb0>] do_syscall_64+0x50/0xa0
> > > [ 2374.423129]  [<ffffffff817e19bc>] entry_SYSCALL64_slow_path+0x25/0x25
> > > 
> > > ~1160 mlx5 counters ~= order 4 allocation which is unlikely to succeed
> > > under memory pressure. Convert them to vzalloc() as ethtool_get_regs() does.
> > > Also take care of drivers without counters similar to
> > > commit 67ae7cf1eeda ("ethtool: Allow zero-length register dumps again")
> > > and reduce warn_on to warn_on_once.
> > 
> > I think this is generally not a good idea as
> > most uses already fit fine in a kcalloc.
> 
> most nics have large numbers of counters that don't fit into one page.
> that's already pushing mm.

I think that's untrue.

Some nics have large numbers of counters, especially
those with multiple tx and rx queues.

A typical nic has a few dozen.

> especially in this case control plane apps call 'ethtool -S'
> periodically.
> 
> > Maybe use Michal Hocko's kvmalloc changes.
> > https://lkml.org/lkml/2017/1/30/120
> 
> v1 discussion here
> http://patchwork.ozlabs.org/patch/721122/
> as I mentioned there long term I don't mind using kvmalloc,
> but the issue has to be fixed now. Either via vzalloc or nowarn+noretry.
> My stress testing with memory hog shows that kmalloc fails
> quite often, thankfully user space daemon is ready for failures,
> whereas vzalloc approach works all the time and no extra headaches
> for user space.

There is a much lower pool available for vmalloc than
kmalloc and kmalloc should be preferred, but hey, fix
it first, then maybe fix it better later after the
kvmalloc stuff actually exists in the tree.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ