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 F0C9F1B398 for ; Thu, 11 Oct 2018 15:15:57 +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 366FF100063; Thu, 11 Oct 2018 13:15:56 +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 14:15:49 +0100 To: Thomas Monjalon CC: , , , , References: <20180907230958.21402-1-thomas@monjalon.net> <2290243.n1gutIJ8el@xps> <58501734-4a92-2ce6-55b9-43f30c1f12ce@solarflare.com> <2193492.h8Rn68zyUV@xps> From: Andrew Rybchenko Message-ID: <7c3bd6cb-95c8-c5a0-d69f-a7c45ba598e6@solarflare.com> Date: Thu, 11 Oct 2018 16:15:08 +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: <2193492.h8Rn68zyUV@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-21.370900-8.000000-10 X-TMASE-MatchedRID: xcONGPdDH5oOwH4pD14DsPHkpkyUphL9uLwbhNl9B5X4JyR+b5tvoOln +pgUTqXBuIA/hwcjvFZuTK43U2r81q23CuWprcjSA9lly13c/gHUqhJbkmLVe/002DXYmoa1Wtc VmlHQcwoFIQK4YsYDXkajIAl7Mi8mQRHl12CcQN79vE/QVDV5IZSL3AH2CZxKw01zN1c0miIwKY tIU69m3bGsW17r4fjXuVb2rQ9hC5w7OkSLg0nnE9jko+KiQPUGQ95F2IiVUkTwQK0wGZW4tD5oR BVnV41m636zVphkNNC0NItx1A9XzDblc6Gei4nlIj0zFI5DoJJeCrB32KOS0KMLUT/MIQivCvE6 8u8U9k8R4d+Uvhl0qv1VprTjLOsR3lmiQx+Vqg0NEGOVZ0MgDZ03C0fPk8xKeI8u4tqjXL3dKUS BW7I322+5ieh24ZYRkZOl7WKIImrvXOvQVlExsEY41YX/o/8KdMrO7yc0I/x5ILrkKK3IDEIVUs fuu+8UfCbT11ptJrgLK3Qpal5+46EEZMonZo7MftwZ3X11IV0= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--21.370900-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24148.003 X-MDID: 1539263757-R0XYe3XhAE96 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 13:15:58 -0000 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.