netdev - Re: rtnl_lock() question

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: <867cf373f204715aec3b2e04ef9f65454cf25a2e.camel@mellanox.com>
Date:   Wed, 4 Sep 2019 23:23:43 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "jonathan.lemon@...il.com" <jonathan.lemon@...il.com>,
        "eric.dumazet@...il.com" <eric.dumazet@...il.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: rtnl_lock() question

On Wed, 2019-09-04 at 09:38 -0700, Jonathan Lemon wrote:
> On 4 Sep 2019, at 0:39, Eric Dumazet wrote:
> 
> > On 9/3/19 11:55 PM, Jonathan Lemon wrote:
> > > How appropriate is it to hold the rtnl_lock() across a sleepable
> > > memory allocation?  On one hand it's just a mutex, but it would
> > > seem like it could block quite a few things.
> > > 
> > 
> > Sure, all GFP_KERNEL allocations can sleep for quite a while.
> > 
> > On the other hand, we may want to delay stuff if memory is under 
> > pressure,
> > or complex operations like NEWLINK would fail.
> > 
> > RTNL is mostly taken for control path operations, we prefer them to
> > be
> > mostly reliable, otherwise admins job would be a nightmare.
> > 
> > In some cases, it is relatively easy to pre-allocate memory before 
> > rtnl is taken,
> > but that will only take care of some selected paths.
> 
> The particular code path that I'm looking at is
> mlx5e_tx_timeout_work().
> 
> This is called on TX timeout, and mlx5 wants to move an entire
> channel
> and all the supporting structures elsewhere.  Under the rtnl_lock(),
> it
> calls kvzmalloc() in order to grab a large chunk of contig memory,
> which
> ends up stalling the system.
> 
> I suspect these large allocation should really be done outside the
> lock.

I am afraid that is impossible, at least not for all allocations

some allocations require parameters that should remain valid and
constant across the whole reconfiguration procedure such
params.num_channels, so they must be done inside the lock. 

other allocations are buried deep inside mlx5 that by doing pre
allocations is going to require a lot of refactoring. 

One idea is to use some sort of mem cache specifically for mlx5
reconfiguration that is cheaper to call than raw kvzalloc ? but
different objects  in the mlx5 reconfiguration path requires differnt
memory types, numa affinity etc.. which might make the cache harder to
satisfy all requirements.