DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Aarnav JP <ajp@marvell.com>, <dev@dpdk.org>,
	Bruce Richardson <bruce.richardson@intel.com>
Cc: <jerinj@marvell.com>, <ndabilpuram@marvell.com>,
	<rbhansali@marvell.com>,  <stable@dpdk.org>
Subject: Re: [PATCH] mem: fix fbarray placement colliding with PCI BARs
Date: Fri, 23 Oct 2026 11:48:18 +0200	[thread overview]
Message-ID: <666ccda0-f613-4df1-b97a-ae470736f419@intel.com> (raw)
In-Reply-To: <20261023060523.978899-1-ajp@marvell.com>

On 10/23/2026 8:05 AM, Aarnav JP wrote:
> Since the VA space of all memseg lists is reserved in one go, it is
> reserved before the memseg lists are created. The fbarray of each list
> is allocated from the same VA allocator, so the first fbarray now ends
> up right after the end of the memseg lists.
> 
> On Linux, the PCI bus maps device BARs in the primary process starting
> at the end of the memseg lists (pci_find_max_end_va()), using it as an
> address hint. This address is now taken by the fbarray, so the hint is
> not honored and the kernel places the first BAR elsewhere in the mmap
> area, among the shared libraries and thread stacks.
> 
> With VFIO, secondary processes map BARs at the addresses chosen by the
> primary process, using MAP_FIXED, which replaces any existing mapping.
> As the mmap area layout is randomized for each process (ASLR), the BAR
> can overlap the secondary process' own libraries or stacks, which are
> then replaced. This intermittently crashes secondary processes during
> EAL init, for example dpdk-proc-info. On arm64 with 64K pages, this
> was seen in about one in ten secondary process starts.
> 
> Create the memseg lists before reserving the VA space, so that, as
> before that change, nothing is mapped right after the memseg lists and
> the BARs can be placed there again. The VA space is still reserved in
> one go.
> 
> Also destroy the memseg lists created so far if a later step fails,
> since their fbarrays now exist before the VA space is reserved.
> 
> Fixes: 3368bc8600e8 ("mem: allocate all VA space in one go")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Aarnav JP <ajp@marvell.com>
> ---
>   lib/eal/common/eal_common_dynmem.c | 46 +++++++++++++++++++++---------
>   1 file changed, 33 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/eal/common/eal_common_dynmem.c b/lib/eal/common/eal_common_dynmem.c
> index 868d274dc0..800f1c63c6 100644
> --- a/lib/eal/common/eal_common_dynmem.c
> +++ b/lib/eal/common/eal_common_dynmem.c
> @@ -29,7 +29,7 @@ eal_dynmem_memseg_lists_init(void)
>   		size_t mem_sz;
>   		size_t va_offset;
>   	} memtypes[RTE_MAX_MEMSEG_LISTS] = {0};
> -	int i, hpi_idx, msl_idx, ret = -1; /* fail unless told to succeed */
> +	int i, hpi_idx, msl_idx, n_msl = 0, ret = -1; /* fail unless told to succeed */
>   	struct rte_memseg_list *msl;
>   	size_t mem_va_len, mem_va_page_sz;
>   	unsigned int n_memtypes, cur_type;
> @@ -132,6 +132,31 @@ eal_dynmem_memseg_lists_init(void)
>   		goto out;
>   	}
>   
> +	/*
> +	 * Create the segment lists (and their fbarrays) before reserving the
> +	 * VA space, so that nothing is mapped right after the VA space. PCI
> +	 * BAR mapping uses the end of the VA space as its address hint.
> +	 */
> +	msl_idx = 0;
> +	for (cur_type = 0; cur_type < n_memtypes; cur_type++) {
> +		struct memtype *type = &memtypes[cur_type];
> +
> +		/* skip page sizes with zero memory limit */
> +		if (type->n_segs == 0)
> +			continue;
> +
> +		EAL_LOG(DEBUG, "Creating segment list: n_segs:%u socket_id:%i hugepage_sz:%" PRIu64,
> +			type->n_segs, type->socket_id, type->page_sz);
> +
> +		msl = &mcfg->memsegs[msl_idx];
> +
> +		if (eal_memseg_list_init(msl, type->page_sz, type->n_segs,
> +				type->socket_id, msl_idx, true))
> +			goto out;
> +		msl_idx++;
> +		n_msl = msl_idx;
> +	}
> +
>   	mem_va_addr = eal_get_virtual_area(NULL, &mem_va_len,
>   			mem_va_page_sz, 0, 0);
>   	if (mem_va_addr == NULL) {
> @@ -139,28 +164,17 @@ eal_dynmem_memseg_lists_init(void)
>   		goto out;
>   	}
>   
> -	/* go through all mem types and create segment lists */
> +	/* go through all mem types and assign VA space to segment lists */
>   	msl_idx = 0;
>   	for (cur_type = 0; cur_type < n_memtypes; cur_type++) {
>   		struct memtype *type = &memtypes[cur_type];
> -		uint64_t pagesz;
> -		int socket_id;
>   
>   		/* skip page sizes with zero memory limit */
>   		if (type->n_segs == 0)
>   			continue;
>   
> -		pagesz = type->page_sz;
> -		socket_id = type->socket_id;
> -
> -		EAL_LOG(DEBUG, "Creating segment list: n_segs:%u socket_id:%i hugepage_sz:%" PRIu64,
> -			type->n_segs, socket_id, pagesz);
> -
>   		msl = &mcfg->memsegs[msl_idx];
>   
> -		if (eal_memseg_list_init(msl, pagesz, type->n_segs, socket_id, msl_idx, true))
> -			goto out;
> -
>   		if (eal_memseg_list_assign(msl,
>   				RTE_PTR_ADD(mem_va_addr, type->va_offset))) {
>   			EAL_LOG(ERR, "Cannot assign VA space for memseg list");
> @@ -172,6 +186,12 @@ eal_dynmem_memseg_lists_init(void)
>   	ret = 0;
>   out:
>   	if (ret != 0) {
> +		/* destroy the segment lists created so far */
> +		for (i = 0; i < n_msl; i++) {
> +			msl = &mcfg->memsegs[i];
> +			rte_fbarray_destroy(&msl->memseg_arr);
> +			memset(msl, 0, sizeof(*msl));

One nit: I'd prefer a struct assignment (`*msl = (struct ...){0}`) 
rather than a memset. AI will probably complain about unchecked destroy 
return value, but I think that if destroy fails in this manner we've got 
bigger problems and there's nothing we can do here.

With the above change,

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

> +		}
>   		if (mem_va_addr != NULL)
>   			eal_mem_free(mem_va_addr, mem_va_len);
>   	} else {


-- 
Thanks,
Anatoly

  reply	other threads:[~2026-10-23  9:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-23  6:05 [PATCH] mem: fix fbarray placement colliding with PCI BARs Aarnav JP
2026-10-23  9:48 ` Burakov, Anatoly [this message]
2026-10-27  5:21 ` [PATCH v2] " Aarnav JP

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=666ccda0-f613-4df1-b97a-ae470736f419@intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=ajp@marvell.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=jerinj@marvell.com \
    --cc=ndabilpuram@marvell.com \
    --cc=rbhansali@marvell.com \
    --cc=stable@dpdk.org \
    /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