netdev - Re: [PATCH] mlx5: Remove call to ida_pre

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: <20180316013007.GA1875@bombadil.infradead.org>
Date:   Thu, 15 Mar 2018 18:30:07 -0700
From:   Matthew Wilcox <willy@...radead.org>
To:     Saeed Mahameed <saeedm@...lanox.com>
Cc:     Matan Barak <matanb@...lanox.com>,
        Maor Gottlieb <maorg@...lanox.com>,
        "leon@...nel.org" <leon@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>
Subject: Re: [PATCH] mlx5: Remove call to ida_pre_get

On Thu, Mar 15, 2018 at 11:58:07PM +0000, Saeed Mahameed wrote:
> On Wed, 2018-03-14 at 19:57 -0700, Matthew Wilcox wrote:
> > From: Matthew Wilcox <mawilcox@...rosoft.com>
> > 
> > The mlx5 driver calls ida_pre_get() in a loop for no readily apparent
> > reason.  The driver uses ida_simple_get() which will call
> > ida_pre_get()
> > by itself and there's no need to use ida_pre_get() unless using
> > ida_get_new().
> > 
> 
> Hi Matthew,
> 
> Is this is causing any issues ? or just a simple cleanup ?

I'm removing the API.  At the end of this cleanup, there will be no more
preallocation; instead we will rely on the slab allocator not sucking.

> Adding Maor, the author of this change,
> 
> I believe the idea is to speed up insert_fte (which calls
> ida_simple_get) since insert_fte runs under the FTE write semaphore,
> in this case if ida_pre_get was successful before taking the semaphore
> for all the FTE nodes in the loop, this will be a huge win for
> ida_simple_get which will immediately return success without even
> trying to allocate.

I think that's misguided.  The IDA allocator is only going to allocate
memory once in every 1024 allocations.  Also, it does try to allocate,
even if there are preallocated nodes.  So you're just wasting time,
unfortunately.