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:28:39 +0100 Message-ID: <2319df1d-3e51-4554-10b5-b99bd0955ab5@intel.com> References: <20190331162437.13048-1-pbhagavatula@marvell.com> <105f221f-0c64-e950-df9a-2c9eaa0991c3@solarflare.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: <105f221f-0c64-e950-df9a-2c9eaa0991c3@solarflare.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 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? > >> + >>       return rte_memzone_reserve_aligned(z_name, size, socket_id, >>               RTE_MEMZONE_IOVA_CONTIG, align); >>   } > > -- Thanks, Anatoly