linux-kernel - Re: [PATCH rdma-next] RDMA/rdmavt: Decouple QP and SGE lists allocations

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]

Date:   Wed, 19 May 2021 07:56:32 -0400
From:   Dennis Dalessandro <dennis.dalessandro@...nelisnetworks.com>
To:     Leon Romanovsky <leon@...nel.org>, Jason Gunthorpe <jgg@...dia.com>
Cc:     "Marciniszyn, Mike" <mike.marciniszyn@...nelisnetworks.com>,
        Doug Ledford <dledford@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>
Subject: Re: [PATCH rdma-next] RDMA/rdmavt: Decouple QP and SGE lists
 allocations

On 5/19/21 3:50 AM, Leon Romanovsky wrote:
> On Fri, May 14, 2021 at 12:02:37PM -0300, Jason Gunthorpe wrote:
>> On Fri, May 14, 2021 at 03:00:37PM +0000, Marciniszyn, Mike wrote:
>>>> The core stuff in ib_qp is not performance sensitive and has no obvious node
>>>> affinity since it relates primarily to simple control stuff.
>>>>
>>>
>>> The current rvt_qp "inherits" from ib_qp, so the fields in the
>>> "control" stuff are performance critical especially for receive
>>> processing and have historically live in the same allocation.
>>
>> This is why I said "core stuff in ib_qp" if drivers are adding
>> performance stuff to their own structs then that is the driver's
>> responsibility to handle.
> 
> Can I learn from this response that node aware allocation is not needed,
> and this patch can go as is.

It's pretty clearly a NAK from us. The code was purposely written this 
way, it was done years ago (from day 1 basically), changing it now is 
concerning for performance.

Perhaps the code can be enhanced to move more stuff into the driver's 
own structs as Jason points out, but that should happen first. For now I 
still don't understand why the core can't optionally make the allocation 
per node.

-Denny