On 03/15/2018 03:00 PM, Burakov, Anatoly wrote:
On 15-Mar-18 11:49 AM, Andrew Rybchenko wrote:
On 03/15/2018 12:48 PM, Burakov, Anatoly wrote:
On 14-Mar-18 5:24 PM, Andrew Rybchenko wrote:
On 03/14/2018 07:53 PM, Burakov, Anatoly wrote:
On 14-Mar-18 4:12 PM, Andrew Rybchenko wrote:
On 03/14/2018 05:40 PM, Burakov, Anatoly wrote:
On 10-Mar-18 3:39 PM, Andrew Rybchenko wrote:
The callback was introduced to let generic code to know octeontx
mempool driver requirements to use single physically contiguous
memory chunk to store all objects and align object address to
total object size. Now these requirements are met using a new
callbacks to calculate required memory chunk size and to populate
objects using provided memory chunk.
These capability flags are not used anywhere else.
Restricting capabilities to flags is not generic and likely to
be insufficient to describe mempool driver features. If required
in the future, API which returns structured information may be
added.
Signed-off-by: Andrew Rybchenko <arybche...@solarflare.com>
---
Just a general comment - it is not enough to describe minimum
memchunk requirements. With memory hotplug patchset that's
hopefully getting merged in 18.05, memzones will no longer be
guaranteed to be IOVA-contiguous. So, if a driver requires its
mempool to not only be populated from a single memzone, but a
single *physically contiguous* memzone, going by only callbacks
will not do, because whether or not something should be a single
memzone says nothing about whether this memzone has to also be
IOVA-contiguous.
So i believe this needs to stay in one form or another.
(also it would be nice to have a flag that a user could pass to
mempool_create that would force memzone reservation be
IOVA-contiguous, but that's a topic for another conversation.
prime user for this would be KNI.)
I think that min_chunk_size should be treated as IOVA-contiguous.
Why? It's perfectly reasonable to e.g. implement a software
mempool driver that would perform some optimizations due to all
objects being in the same VA-contiguous memzone, yet not be
dependent on underlying physical memory layout. These are two
separate concerns IMO.
It looks like there is some misunderstanding here or I simply don't
understand your point.
Above I mean that driver should be able to advertise its
requirements on IOVA-contiguous regions.
If driver do not care about physical memory layout, no problem.
Please correct me if i'm wrong, but my understanding was that you
wanted to use min_chunk as a way to express minimum requirements for
IOVA-contiguous memory. If i understood you correctly, i don't think
that's the way to go because there could be valid use cases where a
mempool driver would like to advertise min_chunk_size to be equal to
its total size (i.e. allocate everything in a single memzone), yet
not require that memzone to be IOVA-contiguous. I think these are
two different concerns, and one does not, and should not imply the
other.
Aha, you're saying that virtual-contiguous and IOVA-contiguous
requirements are different things that it could be usecases where
virtual contiguous is important but IOVA-contiguos is not required.
It is perfectly fine.
As I understand IOVA-contiguous (physical) typically means
virtual-contiguous as well. Requirements to have everything virtually
contiguous and some blocks physically contiguous are unlikely. So, it
may be reduced to either virtual or physical contiguous. If mempool
does not care about physical contiguous at all,
MEMPOOL_F_NO_PHYS_CONTIG flag should be used and min_chunk_size
should mean virtual contiguous requirements. If mempool requires
physical contiguous objects, there is *no* MEMPOOL_F_NO_PHYS_CONTIG
flag and min_chunk_size means physical contiguous requirements.
Fair point. I think we're in agreement now :) This will need to be
documented then.
OK, I'll do. I don't mind to rebase mine patch series on top of yours,
but I'd like to do it a bit later when yours is closer to final version
or even applied - it has really many prerequisites (pre-series) which
should be collected first. It is really major changes.
