From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 325401B504 for ; Thu, 11 Oct 2018 17:29:26 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 9899220B78; Thu, 11 Oct 2018 11:29:25 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Thu, 11 Oct 2018 11:29:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=+zqyuSHFcasFSQyNpP1900xnnIKsv0QjfyYI6VsqVog=; b=EzF6OVhkdtLZ +Ku/BWyK64Z2WhXz584ogwgCCUMZFQnn9JP2SDYGfAd6+MYweR53rHWxe2mMffW4 ZOK0Nz2MCeTZ3SyoHSfv56pXEMZN9BAZhV+zpXEQ3ZLHRJkHTkbNNAmLM8ICAn2/ hB18ruBB+l+C+DNGT2CtFQKvM+Nt7hw= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=+zqyuSHFcasFSQyNpP1900xnnIKsv0QjfyYI6VsqV og=; b=laWuI0waKr+saUFvuf3jEePADaPS5NHcb6Al/BCHmKlG8Iv9dizyrvyHV woi1uB63PCKtKP/i+zyvwbL93LMXSr6ICOymfm31codfR2sbs80FYKOV/7ARfHYq HPgn3rv53+KSdERU44Lv+mvUHmO+idcKQoYD5Oqy9OqprSxh80LONC//9wiK1EOq POrhBB4LG/w24vVub0M8O5dK3KCFcI/gDfpB65I9o7EwCAh5zwBPN9UgPCpsqgNg uFZL6Frdi8U9oqGDVvb+xZHgWiY8bNTgecHktmugF7HHdegYbCDS1AqY6bApxigd S9zg/G0RVi9Ag4mBO36sxxnCSHUkw== X-ME-Sender: X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 30336E4625; Thu, 11 Oct 2018 11:29:24 -0400 (EDT) From: Thomas Monjalon To: Andrew Rybchenko Cc: dev@dpdk.org, gaetan.rivet@6wind.com, ophirmu@mellanox.com, qi.z.zhang@intel.com, ferruh.yigit@intel.com Date: Thu, 11 Oct 2018 17:29:22 +0200 Message-ID: <1830556.armPUsWzaN@xps> In-Reply-To: <7c3bd6cb-95c8-c5a0-d69f-a7c45ba598e6@solarflare.com> References: <20180907230958.21402-1-thomas@monjalon.net> <2193492.h8Rn68zyUV@xps> <7c3bd6cb-95c8-c5a0-d69f-a7c45ba598e6@solarflare.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v3 1/3] drivers/bus: move driver assignment to end of probing X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Oct 2018 15:29:26 -0000 11/10/2018 15:15, Andrew Rybchenko: > On 10/11/18 3:59 PM, Thomas Monjalon wrote: > > 11/10/2018 13:54, Andrew Rybchenko: > >> On 10/11/18 2:45 PM, Thomas Monjalon wrote: > >>> 11/10/2018 12:53, Andrew Rybchenko: > >>>> On 10/8/18 1:09 AM, Thomas Monjalon wrote: > >>>>> The PCI mapping requires to know the PCI driver to use, > >>>>> even before the probing is done. That's why the PCI driver is > >>>>> referenced early inside the PCI device structure. See > >>>>> 1d20a073fa5e ("bus/pci: reference driver structure before mapping") > >>>>> > >>>>> However the rte_driver does not need to be referenced in rte_device > >>>>> before the device probing is done. > >>>>> By moving back this assignment at the end of the device probing, > >>>>> it becomes possible to make clear the status of a rte_device. > >>>>> > >>>>> Signed-off-by: Thomas Monjalon > >>>>> --- > >>>>> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c > >>>>> index c7695d108..d63e68045 100644 > >>>>> --- a/drivers/bus/pci/pci_common.c > >>>>> +++ b/drivers/bus/pci/pci_common.c > >>>>> @@ -160,14 +160,12 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr, > >>>>> * driver flags for adjusting configuration. > >>>>> */ > >>>>> dev->driver = dr; > >>>>> - dev->device.driver = &dr->driver; > >>>> It breaks net/sfc and I guess other drivers which use > >>>> rte_eth_dma_zone_reserve() > >>>> from probe. The function makes zone name using dev->device->driver->name. > >>> Please, can you show code line where we does such access? > >>> > >>> I checked such access before and did not find some. > >>> Anyway, it can be fixed by accessing rte_pci_driver->driver->name. > >>> Note that rte_pci_driver is referenced in rte_pci_device. > >> Below in snprintf(), in theory it can be called for vdev as well. > >> > >> const struct rte_memzone * > >> rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char > >> *ring_name, > >> uint16_t queue_id, size_t size, unsigned align, > >> int socket_id) > >> { > >> char z_name[RTE_MEMZONE_NAMESIZE]; > >> const struct rte_memzone *mz; > >> > >> snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d", > >> dev->device->driver->name, ring_name, > >> dev->data->port_id, queue_id); > > I see, I missed it. > > > > I think it's strange to use rte_device name for ethdev memory. > > Should we use the ethdev name instead? > > > > snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d", > > - dev->device->driver->name, ring_name, > > + dev->data->name, ring_name, > > dev->data->port_id, queue_id); > > data->name could be update to 63 characters (RTE_DEV_NAME_MAX_LEN=64). > RTE_MEMZONE_NAMESIZE is 32. Sounds like a problem. > It is especially a problem if name may be specified/set by user. > > Right now device driver writer knows the driver name, choose ring name and > have limits on port and queue ID. So, the writer at least has possibility to > be sure that the results will always fit z_name. What about removing the device name from the memzone name? It is already unique thanks to port_id, queue_id and ring_name.