DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: dev@dpdk.org, gaetan.rivet@6wind.com, ophirmu@mellanox.com,
	qi.z.zhang@intel.com, ferruh.yigit@intel.com
Subject: Re: [dpdk-dev] [PATCH v3 1/3] drivers/bus: move driver assignment to end of probing
Date: Thu, 11 Oct 2018 17:29:22 +0200	[thread overview]
Message-ID: <1830556.armPUsWzaN@xps> (raw)
In-Reply-To: <7c3bd6cb-95c8-c5a0-d69f-a7c45ba598e6@solarflare.com>

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 <thomas@monjalon.net>
> >>>>> ---
> >>>>> 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.

  reply	other threads:[~2018-10-11 15:29 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-07 23:09 [dpdk-dev] [RFC] eal: allow hotplug to skip an already probed device Thomas Monjalon
2018-09-13  6:29 ` Ophir Munk
2018-09-16 10:14   ` Ophir Munk
2018-09-28 16:40 ` [dpdk-dev] [PATCH v2 0/3] " Thomas Monjalon
2018-09-28 16:40   ` [dpdk-dev] [PATCH v2 1/3] drivers/bus: move driver assignment to end of probing Thomas Monjalon
2018-09-28 16:40   ` [dpdk-dev] [PATCH v2 2/3] eal: add function to query device status Thomas Monjalon
2018-09-28 16:40   ` [dpdk-dev] [PATCH v2 3/3] eal: allow probing a device again Thomas Monjalon
2018-10-04  9:44     ` Doherty, Declan
2018-10-04 14:25       ` Thomas Monjalon
2018-10-07 22:09 ` [dpdk-dev] [PATCH v3 0/3] eal: allow hotplug to skip an already probed device Thomas Monjalon
2018-10-07 22:09   ` [dpdk-dev] [PATCH v3 1/3] drivers/bus: move driver assignment to end of probing Thomas Monjalon
2018-10-08  8:05     ` Andrew Rybchenko
2018-10-11 10:53     ` Andrew Rybchenko
2018-10-11 11:45       ` Thomas Monjalon
2018-10-11 11:54         ` Andrew Rybchenko
2018-10-11 12:59           ` Thomas Monjalon
2018-10-11 13:15             ` Andrew Rybchenko
2018-10-11 15:29               ` Thomas Monjalon [this message]
2018-10-11 15:41                 ` Andrew Rybchenko
2018-10-11 16:00                   ` Thomas Monjalon
2018-10-07 22:09   ` [dpdk-dev] [PATCH v3 2/3] eal: add function to query device status Thomas Monjalon
2018-10-08  8:05     ` Andrew Rybchenko
2018-10-07 22:09   ` [dpdk-dev] [PATCH v3 3/3] eal: allow probing a device again Thomas Monjalon
2018-10-08  8:05     ` Andrew Rybchenko
2018-10-11 21:02 ` [dpdk-dev] [PATCH v4 0/4] eal: allow hotplug to skip an already probed device Thomas Monjalon
2018-10-11 21:02   ` [dpdk-dev] [PATCH v4 1/4] ethdev: rename memzones allocated for DMA Thomas Monjalon
2018-10-12  7:53     ` Andrew Rybchenko
2018-10-12 16:40       ` Thomas Monjalon
2018-10-12 16:42         ` Andrew Rybchenko
2018-10-12 16:46           ` Andrew Rybchenko
2018-10-12 17:18             ` Thomas Monjalon
2018-10-12 17:21               ` Thomas Monjalon
2018-10-12 17:51                 ` Andrew Rybchenko
2018-10-11 21:02   ` [dpdk-dev] [PATCH v4 2/4] drivers/bus: move driver assignment to end of probing Thomas Monjalon
2018-10-12  7:44     ` Andrew Rybchenko
2018-10-12  8:32       ` Jan Remeš
2018-10-12 10:45         ` Thomas Monjalon
2018-10-12 15:50       ` Thomas Monjalon
2018-10-11 21:02   ` [dpdk-dev] [PATCH v4 3/4] eal: add function to query device status Thomas Monjalon
2018-10-11 21:02   ` [dpdk-dev] [PATCH v4 4/4] eal: allow probing a device again Thomas Monjalon
2018-10-12  9:26   ` [dpdk-dev] [PATCH v4 0/4] eal: allow hotplug to skip an already probed device Andrew Rybchenko
2018-10-14 20:47 ` [dpdk-dev] [PATCH v5 0/7] " Thomas Monjalon
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 1/7] net/mlx5: remove useless driver name comparison Thomas Monjalon
2018-10-14 20:49     ` Thomas Monjalon
2018-10-15  5:53       ` Shahaf Shuler
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 2/7] ethdev: rename memzones allocated for DMA Thomas Monjalon
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 3/7] cryptodev: remove driver name from logs Thomas Monjalon
2018-10-15  8:51     ` Thomas Monjalon
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 4/7] compressdev: " Thomas Monjalon
2018-10-15  8:51     ` Thomas Monjalon
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 5/7] drivers/bus: move driver assignment to end of probing Thomas Monjalon
2018-10-14 20:53     ` Thomas Monjalon
2018-10-15  6:11       ` Xu, Rosen
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 6/7] eal: add function to query device status Thomas Monjalon
2018-10-14 20:47   ` [dpdk-dev] [PATCH v5 7/7] eal: allow probing a device again Thomas Monjalon
2018-10-16 10:40     ` Shreyansh Jain
2018-10-17 11:37   ` [dpdk-dev] [PATCH v5 0/7] allow hotplug to skip an already probed device Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1830556.armPUsWzaN@xps \
    --to=thomas@monjalon.net \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=gaetan.rivet@6wind.com \
    --cc=ophirmu@mellanox.com \
    --cc=qi.z.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).