From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f52.google.com (mail-wm0-f52.google.com [74.125.82.52]) by dpdk.org (Postfix) with ESMTP id 3741529CF for ; Fri, 6 Jan 2017 15:55:50 +0100 (CET) Received: by mail-wm0-f52.google.com with SMTP id k184so32750169wme.1 for ; Fri, 06 Jan 2017 06:55:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:user-agent:in-reply-to :references:mime-version:content-transfer-encoding; bh=+/uQO9NeKgGcMrJDY4VC02SHItp2388tdxnKYwRUgb4=; b=zLRJAKKPtC2DYYWI0THO4YqaG0SaymNKV5ILiVUY/9SFlMgjywENhvfl+dbCuxm1IJ i8CW7vFtKLo7S1wWwIlKW0r4yCYTJDdV/UA1gcDVsOk6S/ZXAiK9g7cFJ4EecPHhJs1C byzJ+DvWpKZ2aTFN61ApCQNV1QLpUXlvPbyQ+cfA9Si77zzU5mnKGIB1H7IrkyFMfMYa godpt/IGdNao4Syckk0dpqe3oeGEkhQmLmSokdSiuJHFSREfny+Tw59+NHvTmtEo1hJn +Rv23keKoc+L1qeNhIG+CO+laHqDQnM7kHQzK9Pmnach7Iw757lyDi+ZzsbwunVAIyrD rYzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:user-agent :in-reply-to:references:mime-version:content-transfer-encoding; bh=+/uQO9NeKgGcMrJDY4VC02SHItp2388tdxnKYwRUgb4=; b=TdYRJY4iyMgpmVXum25DihDT4uKjTOiPAWHawMYx90Z1AnMVwxwaLbupdPyzBKLNv3 FA08/V2uXUrSsEy7+bUFoPUguuOV8ed2+DiWbXscJFeHPXkGnR3aTVNhgSYvep/Mde7c WZe2aPmgG2s4mqmAwEoXaZ/27lgEDbHL5h4nfgaIYko21QemFogQ4fkYt/v0RvXmHNGI eEad5ugVktOewzFcAS6VLGel2rMFHchdUhIJjKC8o1CbAdbqg2oqk1bR5WREnd/fTqPI E3It1Eb1Z5NxXu7utA6DqGNkpn2PQtxDg76saZHdpKOn1vCb2grK2xa3Rm75YhaeBBFk pPWg== X-Gm-Message-State: AIkVDXIMcPKufx5l+ytiGL9FOC9np+5/mU6Ns8UYs/aaslAxX+MfiKoqH4178Ta1WwqiwPld X-Received: by 10.223.161.69 with SMTP id r5mr1417714wrr.96.1483714549773; Fri, 06 Jan 2017 06:55:49 -0800 (PST) Received: from xps13.localnet (184.203.134.77.rev.sfr.net. [77.134.203.184]) by smtp.gmail.com with ESMTPSA id d184sm3792733wmd.8.2017.01.06.06.55.48 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 06 Jan 2017 06:55:49 -0800 (PST) From: Thomas Monjalon To: Shreyansh Jain Cc: david.marchand@6wind.com, dev@dpdk.org Date: Fri, 06 Jan 2017 15:55:48 +0100 Message-ID: <105987546.PAaz77144n@xps13> User-Agent: KMail/4.14.10 (Linux/4.5.4-1-ARCH; KDE/4.14.11; x86_64; ; ) In-Reply-To: References: <1482756644-13726-1-git-send-email-shreyansh.jain@nxp.com> <12579803.ZHBMo4GdSA@xps13> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v5 01/12] eal/bus: introduce bus abstraction 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: Fri, 06 Jan 2017 14:55:50 -0000 2017-01-06 16:01, Shreyansh Jain: > On Wednesday 04 January 2017 03:22 AM, Thomas Monjalon wrote: > > 2016-12-26 18:53, Shreyansh Jain: > >> +/** > >> + * A structure describing a generic bus. > >> + */ > >> +struct rte_bus { > >> + TAILQ_ENTRY(rte_bus) next; /**< Next bus object in linked list */ > >> + struct rte_driver_list driver_list; > >> + /**< List of all drivers on bus */ > >> + struct rte_device_list device_list; > >> + /**< List of all devices on bus */ > >> + const char *name; /**< Name of the bus */ > >> +}; > > > > I am not convinced we should link a generic bus to drivers and devices. > > What do you think of having rte_pci_bus being a rte_bus and linking > > with rte_pci_device and rte_pci_driver lists? > > This is different from what I had in mind. > You are saying: > > Class: rte_bus > `-> No object instantiated for this class > Class: rte_pci_bus inheriting rte_bus > `-> object instantiated for this class. > > Here, rte_bus is being treated as an abstract class which is only > inherited and rte_pci_bus is the base class which is instantiated. > > And I was thinking: > > Class: rte_bus > `-> Object: pci_bus (defined in */eal/eal_pci.c) > > Here, rte_bus is that base class which is instantiated. > > I agree that what you are suggesting is inline with current model: > rte_device -> abstract class (no one instantiates it) > `-> rte_pci_device -> Base class which inherits rte_device and > is instantiated. Yes > When I choose not to create rte_pci_bus, it was because I didn't want > another indirection in form of rte_bus->rte_pci_bus->object. > There were no 'non-generic' bus functions which were only applicable for > rte_pci_bus. Eventually, rte_pci_bus ended up being a direct inheritance > of rte_bus. > > > I'm thinking to something like that: > > > > struct rte_bus { > > TAILQ_ENTRY(rte_bus) next; > > const char *name; > > rte_bus_scan_t scan; > > rte_bus_match_t match; > > }; > > struct rte_pci_bus { > > struct rte_bus bus; > > struct rte_pci_driver_list pci_drivers; > > struct rte_pci_device_list pci_devices; > > }; > > if we go by rte_bus->rte_pci_bus->(instance of rte_pci_bus), above is > fine. Though, I am in favor of rte_bus->(instance of rte_bus for PCI) > because I don't see any 'non-generic' information in rte_pci_bus which > can't be put in rte_bus. The lists of drivers and devices are specific to the bus. Your proposal was to list them as generic rte_driver/rte_device and cast them. I'm just saying we can directly declare them with the right type, e.g. rte_pci_driver/rte_pci_device. In the same logic, the functions probe/remove are specifics for the bus and should be declared in rte_pci_driver instead of the generic rte_driver. > >> +/** Helper for Bus registration. The constructor has higher priority than > >> + * PMD constructors > >> + */ > >> +#define RTE_REGISTER_BUS(nm, bus) \ > >> +static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \ > >> +{\ > >> + (bus).name = RTE_STR(nm);\ > >> + rte_eal_bus_register(&bus); \ > >> +} > > > > By removing the lists from rte_bus as suggested above, do you still need > > a priority for this constructor? > > I think yes. > Even if we have rte_pci_bus as a class, object of rte_bus should be part > of Bus list *before* registration of a driver (because, driver > registration searches for bus by name). > > (This is assuming that no global PCI/VDEV/XXX bus object is created > which is directly used within all PCI specific bus operations). > > There was another suggestion on list which was to check for existence of > bus _within_ each driver registration and create/instantiate an object > in case no bus is registered. I didn't like the approach so I didn't use > it. From David [1], and me [2] > > [1] http://dpdk.org/ml/archives/dev/2016-December/051689.html > [2] http://dpdk.org/ml/archives/dev/2016-December/051698.html OK, we can keep your approach of prioritize bus registrations. If we see an issue later, we could switch to a bus registration done when a first driver is registered on the bus. > >> struct rte_device { > >> TAILQ_ENTRY(rte_device) next; /**< Next device */ > >> + struct rte_bus *bus; /**< Device connected to this bus */ > >> const struct rte_driver *driver;/**< Associated driver */ > >> int numa_node; /**< NUMA node connection */ > >> struct rte_devargs *devargs; /**< Device user arguments */ > >> @@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev); > >> */ > >> struct rte_driver { > >> TAILQ_ENTRY(rte_driver) next; /**< Next in list. */ > >> + struct rte_bus *bus; /**< Bus serviced by this driver */ > >> const char *name; /**< Driver name. */ > >> const char *alias; /**< Driver alias. */ > >> }; > > > > Do we need to know the bus associated to a driver in rte_driver? > > Bus and driver are already associated in rte_device. > > Two reasons: > 1/ A driver should be associated with a bus so that if required, all bus > can be directly extracted - even when probing has not been done. I do not understand this need. > 2/ device->driver would only be updated after probe. device->driver->bus > would not be valid in such cases, if required. We can update device->driver on match. Please let's do not over-engineer if not needed. In this case, I think we can skip rte_driver->bus. > Overall, I don't have objections for rte_bus->rte_pci_bus=>object as > compared to rte_bus=>PCI-object. But, I would still like to get a final > confirmation of a more preferred way. > > Meanwhile, I will make changes to accommodate this change to save time > in case rte_pci_bus class is final/preferred method. It looks more natural to me to avoid class casting and use specialized classes when possible. So yes I prefer instantiating rte_pci_bus. However, I could be wrong, and will consider any argument. Thanks