From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 0D3081B554 for ; Thu, 11 Oct 2018 17:42:44 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id CAA3DB4007F; Thu, 11 Oct 2018 15:42:41 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Thu, 11 Oct 2018 16:42:34 +0100 To: Thomas Monjalon CC: , , , , References: <20180907230958.21402-1-thomas@monjalon.net> <2193492.h8Rn68zyUV@xps> <7c3bd6cb-95c8-c5a0-d69f-a7c45ba598e6@solarflare.com> <1830556.armPUsWzaN@xps> From: Andrew Rybchenko Message-ID: Date: Thu, 11 Oct 2018 18:41:52 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: <1830556.armPUsWzaN@xps> Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24148.003 X-TM-AS-Result: No-22.113600-8.000000-10 X-TMASE-MatchedRID: C/snMIRQLS0OwH4pD14DsPHkpkyUphL9uLwbhNl9B5X4JyR+b5tvoOln +pgUTqXBuIA/hwcjvFZuTK43U2r81q23CuWprcjSA9lly13c/gHUqhJbkmLVe/002DXYmoa1Wtc VmlHQcwoFIQK4YsYDXkajIAl7Mi8mQRHl12CcQN79vE/QVDV5IZSL3AH2CZxKw01zN1c0miIwKY tIU69m3bGsW17r4fjXuVb2rQ9hC5w7OkSLg0nnE9jko+KiQPUGQ95F2IiVUkTwQK0wGZW4tD5oR BVnV41m636zVphkNNBN2YvGsd0NGRBqKmn50bNvf01qcJQDhV6LGZZVMT5DdgeLCIX046iBSrJT O1VGhMF2+AZuxakJy89fuxSQqqSyyA1ihY42R28vj6wHfIGxyQhRCJFb9cuseVCAkHLwReM8q3P oecYM1OeBnUZSgni9YLaaleRz6NvfW6mMWTtZdOQYBHVKqgDUfS0Ip2eEHny+qryzYw2E8LLn+0 Vm71LcGua1AIchQ1AgBwKKRHe+r2qAroFzGXOCrP5r7ieZ12mlYcAMjFW2HZGbU0MhJ404vHCcp aQCnn4= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--22.113600-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24148.003 X-MDID: 1539272562-32dan360FCAI Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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:42:44 -0000 On 10/11/18 6:29 PM, Thomas Monjalon wrote: > 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. Driver name is nice since it simplify buggy code identification, but not that critical. Maybe we should highlight that it is ethdev (not other port/queue), i.e. ethdev_%s_%d_%d, to be sure that port_id and queue_id uniquely identify it.