Re: [PATCH v2 10/10] nvmet: Optionally use PCI P2P memory

Logan Gunthorpe
2018-03-02T17:10:31Z

On 02/03/18 09:18 AM, Jason Gunthorpe wrote:

> This allocator is already seems not useful for the P2P target memory
> on a Mellanox NIC due to the way it has a special allocation flow
> (windowing) and special usage requirements..
>
> Nor can it be usefull for the doorbell memory in the NIC.

No one says every P2P use has to use P2P memory. Doorbells are obviously
not P2P memory. But it's the p2mem interface that's important and the
interface absolutely does not belong in the NVMe driver. Once you have
the P2P memory interface you need an allocator behind it and the obvious
place is in the P2P code to handle the common case where you're just
mapping a BAR. We don't need to implement a genalloc in every driver
that has P2P memory attached with it. If some hardware wants to expose
memory that requires complicated allocation it's up to them to solve
that problem but that isn't enough justification, to me, to push common
code into every driver.

> Both of these are existing use cases for P2P with out of tree patches..

And we have out of tree code that uses the generic allocator part of
p2pmem.

> The allocator seems to only be able to solve the CMB problem, and I
> think due to that it is better to put this allocator in the NVMe
> driver and not the core code.. At least until we find a 2nd user that
> needs the same allocation scheme...

See the first P2PMEM RFC. We used Chelsio's NIC instead of the CMB with
a very similar allocation scheme. We'd still be enabling that NIC in the
same way if we didn't run into hardware issues with it. A simple BAR
with memory behind it is always going to be the most common case.

Logan