On 10/13/21 2:01 PM, Dmitry Kozlyuk wrote:
> Mempool is a generic allocator that is not necessarily used for device
> IO operations and its memory for DMA. Add MEMPOOL_F_NON_IO flag to mark
> such mempools automatically if their objects are not contiguous
> or IOVA are not available. Components can inspect this flag
> in order to optimize their memory management.
> Discussion: https://mails.dpdk.org/archives/dev/2021-August/216654.html
>
> Signed-off-by: Dmitry Kozlyuk <dkozl...@nvidia.com>
> Acked-by: Matan Azrad <ma...@nvidia.com>
See review notes below. With review notes processed:
Reviewed-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
[snip]
> diff --git a/doc/guides/rel_notes/release_21_11.rst
> b/doc/guides/rel_notes/release_21_11.rst
> index f643a61f44..74e0e6f495 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -226,6 +226,9 @@ API Changes
> the crypto/security operation. This field will be used to communicate
> events such as soft expiry with IPsec in lookaside mode.
>
> +* mempool: Added ``MEMPOOL_F_NON_IO`` flag to give a hint to DPDK components
> + that objects from this pool will not be used for device IO (e.g. DMA).
> +
>
> ABI Changes
> -----------
> diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
> index 51c0ba2931..2204f140b3 100644
> --- a/lib/mempool/rte_mempool.c
> +++ b/lib/mempool/rte_mempool.c
> @@ -371,6 +371,8 @@ rte_mempool_populate_iova(struct rte_mempool *mp, char
> *vaddr,
>
> STAILQ_INSERT_TAIL(&mp->mem_list, memhdr, next);
> mp->nb_mem_chunks++;
> + if (iova == RTE_BAD_IOVA)
> + mp->flags |= MEMPOOL_F_NON_IO;
As I understand rte_mempool_populate_iova() may be called
few times for one mempool. The flag must be set if all
invocations are done with RTE_BAD_IOVA. So, it should be
set by default and just removed when iova != RTE_BAD_IOVA
happens.
Yes, it is a corner case. May be it makes sense to
cover it by unit test as well.
>
> /* Report the mempool as ready only when fully populated. */
> if (mp->populated_size >= mp->size)
> diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
> index 663123042f..029b62a650 100644
> --- a/lib/mempool/rte_mempool.h
> +++ b/lib/mempool/rte_mempool.h
> @@ -262,6 +262,8 @@ struct rte_mempool {
> #define MEMPOOL_F_SC_GET 0x0008 /**< Default get is
> "single-consumer".*/
> #define MEMPOOL_F_POOL_CREATED 0x0010 /**< Internal: pool is created. */
> #define MEMPOOL_F_NO_IOVA_CONTIG 0x0020 /**< Don't need IOVA contiguous
> objs. */
> +#define MEMPOOL_F_NON_IO 0x0040
> + /**< Internal: pool is not usable for device IO (DMA). */
Please, put the documentation before the define.
/** Internal: pool is not usable for device IO (DMA). */
#define MEMPOOL_F_NON_IO 0x0040
>
> /**
> * @internal When debug is enabled, store some statistics.
> @@ -991,6 +993,9 @@ typedef void (rte_mempool_ctor_t)(struct rte_mempool *,
> void *);
> * "single-consumer". Otherwise, it is "multi-consumers".
> * - MEMPOOL_F_NO_IOVA_CONTIG: If set, allocated objects won't
> * necessarily be contiguous in IO memory.
> + * - MEMPOOL_F_NON_IO: If set, the mempool is considered to be
> + * never used for device IO, i.e. for DMA operations.
> + * It's a hint to other components and does not affect the mempool
> behavior.
I tend to say that it should not be here if the flag is
internal.
> * @return
> * The pointer to the new allocated mempool, on success. NULL on error
> * with rte_errno set appropriately. Possible rte_errno values include:
>
