From mboxrd@z Thu Jan 1 00:00:00 1970 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) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 14 Sep 2016 08:32:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,334,1470726000"; d="scan'208";a="760662913" Received: from sie-lab-212-151.ir.intel.com (HELO [10.237.212.151]) ([10.237.212.151]) by FMSMGA003.fm.intel.com with ESMTP; 14 Sep 2016 08:32:48 -0700 To: Yuanhan Liu References: <1472725298-8455-1-git-send-email-john.griffin@intel.com> <20160905032356.GH30752@yliu-dev.sh.intel.com> Cc: dev@dpdk.org, eoin.breen@intel.com, pablo.de.lara.guarch@intel.com From: John Griffin Message-ID: <57D96D9E.8040301@intel.com> Date: Wed, 14 Sep 2016 16:32:46 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <20160905032356.GH30752@yliu-dev.sh.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] crypto/qat: fix memzone creation to use a fixed size string X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Sep 2016 15:33:11 -0000 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 >