DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>,
	dev@dpdk.org, tiwei.bie@intel.com, zhihong.wang@intel.com
Subject: Re: [dpdk-dev] [PATCH 1/4] eal: add new API to register contiguous external memory
Date: Thu, 23 Jan 2020 13:06:14 +0000	[thread overview]
Message-ID: <0583a91b-1250-8df7-ece5-3b9693c5e587@intel.com> (raw)
In-Reply-To: <20191213141322.32730-2-maxime.coquelin@redhat.com>

On 13-Dec-19 2:13 PM, Maxime Coquelin wrote:
> This new API allows to pass a file descriptor while
> registering external and contiguous memory in the IOVA space.
> 
> This is required for using Virtio-user PMD with application
> using external memory for the mbuf's buffers, like Seastar
> or VPP.
> 
> FD is only attached to the segments if single_file_segment
> option is enabled.
>  > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Hi Maxime,

We've discussed this privately already, but i'll send feedback anyway 
just so that it's visible.

> ---
>   lib/librte_eal/common/eal_common_memory.c  | 75 +++++++++++++++++++++-
>   lib/librte_eal/common/include/rte_memory.h | 46 +++++++++++++
>   lib/librte_eal/common/malloc_heap.c        | 17 ++++-
>   lib/librte_eal/common/malloc_heap.h        |  2 +-
>   lib/librte_eal/common/rte_malloc.c         |  2 +-
>   lib/librte_eal/rte_eal_version.map         |  3 +
>   6 files changed, 141 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
> index 4a9cc1f19a..7a4b371828 100644
> --- a/lib/librte_eal/common/eal_common_memory.c
> +++ b/lib/librte_eal/common/eal_common_memory.c
> @@ -772,6 +772,79 @@ rte_memseg_get_fd_offset(const struct rte_memseg *ms, size_t *offset)
>   	return ret;
>   }
>   
> +int
> +rte_extmem_register_contig(void *va_addr, size_t len, rte_iova_t iova_addr,
> +		size_t page_sz, int fd)
> +{

I don't see why do you have to link IOVA-contiguousness with setting an 
fd - a chunk of memory backed by an fd does not necessarily have to be 
IOVA-contiguous.

In fact, registering IOVA-contiguous memory is already possible - you 
just specify the IOVA table such that each successive IOVA is contiguous 
to the preceding one.

So, IMO, the 'fd' part should be decoupled from registering.

Also, we support two modes of operation (fd per segment and fd per 
segment list), it would be nice to have both.

For example, consider the following:

// set fd for an entire segment list, addressed by VA and len.
// must match a memory chunk that was registered earlier
rte_extmem_set_seg_fd(..., int fd);

// set per-segment fd's for the segment list, addressed by VA and len.
// must match a memory chunk that was registered earlier, and length of
// fd array must add up to length of the chunk
rte_extmem_set_seg_fd_list(..., int *fd, int len);

This would essentially accomplish what you're trying to do by just 
adding the missing piece required.

Question: do we want to be able to set segment fd's for externally 
allocated by internally managed memory? E.g. something that was added 
via malloc_heap_add_memory rather than extmem_register? I would guess 
so, so maybe it shouldn't be under rte_extmem namespace.

-- 
Thanks,
Anatoly

  reply	other threads:[~2020-01-23 13:06 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-13 14:13 [dpdk-dev] [PATCH 0/4] Add external contiguous memory support to Virtio-user Maxime Coquelin
2019-12-13 14:13 ` [dpdk-dev] [PATCH 1/4] eal: add new API to register contiguous external memory Maxime Coquelin
2020-01-23 13:06   ` Burakov, Anatoly [this message]
2019-12-13 14:13 ` [dpdk-dev] [PATCH 2/4] eal: allow getting memory segment FD with " Maxime Coquelin
2019-12-13 14:13 ` [dpdk-dev] [PATCH 3/4] bus/vdev: add DMA mapping supprt Maxime Coquelin
2019-12-13 14:13 ` [dpdk-dev] [PATCH 4/4] net/virtio: add DMA mapping callback to virtio-user Maxime Coquelin
2020-01-08 16:11 ` [dpdk-dev] [PATCH 0/4] Add external contiguous memory support to Virtio-user Maxime Coquelin
2023-08-25  9:55 ` David Marchand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0583a91b-1250-8df7-ece5-3b9693c5e587@intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=dev@dpdk.org \
    --cc=maxime.coquelin@redhat.com \
    --cc=tiwei.bie@intel.com \
    --cc=zhihong.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).