From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Burakov, Anatoly" Subject: Re: [PATCH] ethdev: fix DMA zone reserve not honoring size Date: Mon, 1 Apr 2019 10:40:38 +0100 Message-ID: <8bce9dd4-4879-db22-41ad-cc584c7afee8@intel.com> References: <20190331162437.13048-1-pbhagavatula@marvell.com> <105f221f-0c64-e950-df9a-2c9eaa0991c3@solarflare.com> <2319df1d-3e51-4554-10b5-b99bd0955ab5@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: "dev@dpdk.org" , "stable@dpdk.org" To: Andrew Rybchenko , Pavan Nikhilesh Bhagavatula , Jerin Jacob Kollanukkaran , "thomas@monjalon.net" , "ferruh.yigit@intel.com" , "stephen@networkplumber.org" Return-path: In-Reply-To: <2319df1d-3e51-4554-10b5-b99bd0955ab5@intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 01-Apr-19 10:28 AM, Burakov, Anatoly wrote: > On 01-Apr-19 8:30 AM, Andrew Rybchenko wrote: >> On 3/31/19 7:25 PM, Pavan Nikhilesh Bhagavatula wrote: >>> From: Pavan Nikhilesh >>> >>> The `rte_eth_dma_zone_reserve()` is generally used to create HW rings. >>> In some scenarios when a driver needs to reconfigure the ring size >>> since the named memzone already exists it returns the previous memzone >>> without checking if a different sized ring is requested. >>> >>> Introduce a check to see if the ring size requested is different from >>> the >>> previously created memzone length. >>> >>> Fixes: 719dbebceb81 ("xen: allow determining DOM0 at runtime") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Pavan Nikhilesh >>> --- >>>   lib/librte_ethdev/rte_ethdev.c | 5 ++++- >>>   1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/librte_ethdev/rte_ethdev.c >>> b/lib/librte_ethdev/rte_ethdev.c >>> index 12b66b68c..4ae12e43b 100644 >>> --- a/lib/librte_ethdev/rte_ethdev.c >>> +++ b/lib/librte_ethdev/rte_ethdev.c >>> @@ -3604,9 +3604,12 @@ rte_eth_dma_zone_reserve(const struct >>> rte_eth_dev *dev, const char *ring_name, >>>       } >>>       mz = rte_memzone_lookup(z_name); >>> -    if (mz) >>> +    if (mz && (mz->len == size)) >>>           return mz; >>> +    if (mz) >>> +        rte_memzone_free(mz); >> >> NACK >> I really don't like that API which should reserve does free if requested >> size does not match previously allocated. >> I understand the motivation, but I don't think the solution is correct. > > Why does size change in the first place? Never mind, i forgot that NICs can be reconfigured :) Currently, there is no way to resize memzones, so freeing and reallocating is the only option. Since memzones are backed by regular malloc elements, we could add a memzone_resize API. That would help, because all of the references to the memzone itself will still be valid, even if memory ends up being reallocated. > >> >>> + >>>       return rte_memzone_reserve_aligned(z_name, size, socket_id, >>>               RTE_MEMZONE_IOVA_CONTIG, align); >>>   } >> >> > > -- Thanks, Anatoly