From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Griffin Subject: Re: [PATCH] crypto/qat: fix memzone creation to use a fixed size string Date: Wed, 14 Sep 2016 16:32:46 +0100 Message-ID: <57D96D9E.8040301@intel.com> References: <1472725298-8455-1-git-send-email-john.griffin@intel.com> <20160905032356.GH30752@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, eoin.breen@intel.com, pablo.de.lara.guarch@intel.com To: Yuanhan Liu Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 526545685 for ; Wed, 14 Sep 2016 17:33:11 +0200 (CEST) In-Reply-To: <20160905032356.GH30752@yliu-dev.sh.intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Liu, Comments embedded. Rgds, John. On 05/09/16 04:23, Yuanhan Liu wrote: > On Thu, Sep 01, 2016 at 11:21:38AM +0100, John Griffin wrote: >> Remove the dependency on dev->driver->pci_drv.name when >> creating the memzone for the qat hardware queues. >> The pci_drv.name may grow too large for RTE_MEMZONE_NAMESIZE. > > Will the "may grow too large" cause any issues? If so, state it here. If > not, marking this patch as a "fix" patch doesn't make sense to me then. We discovered this when applying a future patch (2141c21966) and it exposed this issue. Problem is we create a memzone per hardware queue pair and if the memzone name is too large, then this code will not produce a unique name and two qps will end using the same memzone. In retrospect, I'll wait for Pablo to apply that future patch and then re-base a v2. > >> >> Fixes: 1703e94ac5ce ("qat: add driver for QuickAssist devices") >> >> Signed-off-by: John Griffin >> --- >> drivers/crypto/qat/qat_qp.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/crypto/qat/qat_qp.c b/drivers/crypto/qat/qat_qp.c >> index 5de47e3..a29ed66 100644 >> --- a/drivers/crypto/qat/qat_qp.c >> +++ b/drivers/crypto/qat/qat_qp.c >> @@ -300,7 +300,7 @@ qat_queue_create(struct rte_cryptodev *dev, struct qat_queue *queue, >> * Allocate a memzone for the queue - create a unique name. >> */ >> snprintf(queue->memz_name, sizeof(queue->memz_name), "%s_%s_%d_%d_%d", >> - dev->driver->pci_drv.name, "qp_mem", dev->data->dev_id, >> + "qat_pmd", "qp_mem", dev->data->dev_id, > > Besides that, why not putting "qat_pmd" and "qp_mem" inside the format > string? Fair point will send in the v2. > > --yliu >