diff mbox

[dpdk-dev,v5,10/20] pci: avoid inlining functions

Message ID 35d2cbe2bb9d8c03d0564256a85cce54b8331eee.1507804944.git.gaetan.rivet@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail apply patch file failure

Commit Message

Gaëtan Rivet Oct. 12, 2017, 10:45 a.m. UTC 
  Parsing operations should not happen in performance critical sections.
Headers should not propose implementations unless duly required.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_pci/include/rte_pci.h   | 69 ++++----------------------------------
 lib/librte_pci/rte_pci.c           | 65 +++++++++++++++++++++++++++++++++++
 lib/librte_pci/rte_pci_version.map |  4 +++
 3 files changed, 75 insertions(+), 63 deletions(-)

Comments

Aaron Conole Oct. 17, 2017, 6:20 p.m. UTC | #1 
Gaetan Rivet <gaetan.rivet@6wind.com> writes:

> Parsing operations should not happen in performance critical sections.
> Headers should not propose implementations unless duly required.
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---

Can these cleanups be done before the move in patch 3?
Gaëtan Rivet Oct. 18, 2017, 8:54 a.m. UTC | #2 
On Tue, Oct 17, 2017 at 02:20:45PM -0400, Aaron Conole wrote:
> Gaetan Rivet <gaetan.rivet@6wind.com> writes:
> 
> > Parsing operations should not happen in performance critical sections.
> > Headers should not propose implementations unless duly required.
> >
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> 
> Can these cleanups be done before the move in patch 3?

Sure.

I wanted to minimize the changes before moving the code. If I had rework
to do on these patches, I wanted to avoid having to redo the move and
verify the changes were properly propagated.

But if it is necessary, these changes can happen before moving the whole
lot.
Aaron Conole Oct. 18, 2017, 2:30 p.m. UTC | #3 
Gaëtan Rivet <gaetan.rivet@6wind.com> writes:

> On Tue, Oct 17, 2017 at 02:20:45PM -0400, Aaron Conole wrote:
>> Gaetan Rivet <gaetan.rivet@6wind.com> writes:
>> 
>> > Parsing operations should not happen in performance critical sections.
>> > Headers should not propose implementations unless duly required.
>> >
>> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
>> > ---
>> 
>> Can these cleanups be done before the move in patch 3?
>
> Sure.
>
> I wanted to minimize the changes before moving the code. If I had rework
> to do on these patches, I wanted to avoid having to redo the move and
> verify the changes were properly propagated.

I use 'git rebase -i' to do changes; it can take care of moving through
a stack of changes.

> But if it is necessary, these changes can happen before moving the whole
> lot.

I think it's better to do all the changes before hand.  If there is a
change that could be part of a bug fix, it can be easily backported to
stable branches.
diff mbox

Patch

diff --git a/lib/librte_pci/include/rte_pci.h b/lib/librte_pci/include/rte_pci.h
index 2647568..fe4d411 100644
--- a/lib/librte_pci/include/rte_pci.h
+++ b/lib/librte_pci/include/rte_pci.h
@@ -134,19 +134,6 @@  struct mapped_pci_resource {
 /** mapped pci device list */
 TAILQ_HEAD(mapped_pci_res_list, mapped_pci_resource);
 
-/**< Internal use only - Macro used by pci addr parsing functions **/
-#define GET_PCIADDR_FIELD(in, fd, lim, dlm)                   \
-do {                                                               \
-	unsigned long val;                                      \
-	char *end;                                              \
-	errno = 0;                                              \
-	val = strtoul((in), &end, 16);                          \
-	if (errno != 0 || end[0] != (dlm) || val > (lim))       \
-		return -EINVAL;                                 \
-	(fd) = (typeof (fd))val;                                \
-	(in) = end + 1;                                         \
-} while(0)
-
 /**
  * Utility function to produce a PCI Bus-Device-Function value
  * given a string representation. Assumes that the BDF is provided without
@@ -160,15 +147,7 @@  do {                                                               \
  * @return
  *  0 on success, negative on error.
  */
-static inline int
-eal_parse_pci_BDF(const char *input, struct rte_pci_addr *dev_addr)
-{
-	dev_addr->domain = 0;
-	GET_PCIADDR_FIELD(input, dev_addr->bus, UINT8_MAX, ':');
-	GET_PCIADDR_FIELD(input, dev_addr->devid, UINT8_MAX, '.');
-	GET_PCIADDR_FIELD(input, dev_addr->function, UINT8_MAX, 0);
-	return 0;
-}
+int eal_parse_pci_BDF(const char *input, struct rte_pci_addr *dev_addr);
 
 /**
  * Utility function to produce a PCI Bus-Device-Function value
@@ -182,16 +161,7 @@  eal_parse_pci_BDF(const char *input, struct rte_pci_addr *dev_addr)
  * @return
  *  0 on success, negative on error.
  */
-static inline int
-eal_parse_pci_DomBDF(const char *input, struct rte_pci_addr *dev_addr)
-{
-	GET_PCIADDR_FIELD(input, dev_addr->domain, UINT16_MAX, ':');
-	GET_PCIADDR_FIELD(input, dev_addr->bus, UINT8_MAX, ':');
-	GET_PCIADDR_FIELD(input, dev_addr->devid, UINT8_MAX, '.');
-	GET_PCIADDR_FIELD(input, dev_addr->function, UINT8_MAX, 0);
-	return 0;
-}
-#undef GET_PCIADDR_FIELD
+int eal_parse_pci_DomBDF(const char *input, struct rte_pci_addr *dev_addr);
 
 /**
  * Utility function to write a pci device name, this device name can later be
@@ -205,17 +175,9 @@  eal_parse_pci_DomBDF(const char *input, struct rte_pci_addr *dev_addr)
  * @param size
  *	The output buffer size
  */
-static inline void
-rte_pci_device_name(const struct rte_pci_addr *addr,
-		char *output, size_t size)
-{
-	RTE_VERIFY(size >= PCI_PRI_STR_SIZE);
-	RTE_VERIFY(snprintf(output, size, PCI_PRI_FMT,
-			    addr->domain, addr->bus,
-			    addr->devid, addr->function) >= 0);
-}
+void rte_pci_device_name(const struct rte_pci_addr *addr,
+			 char *output, size_t size);
 
-/* Compare two PCI device addresses. */
 /**
  * Utility function to compare two PCI device addresses.
  *
@@ -228,27 +190,8 @@  rte_pci_device_name(const struct rte_pci_addr *addr,
  *	Positive on addr is greater than addr2.
  *	Negative on addr is less than addr2, or error.
  */
-static inline int
-rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
-			 const struct rte_pci_addr *addr2)
-{
-	uint64_t dev_addr, dev_addr2;
-
-	if ((addr == NULL) || (addr2 == NULL))
-		return -1;
-
-	dev_addr = ((uint64_t)addr->domain << 24) |
-		(addr->bus << 16) | (addr->devid << 8) | addr->function;
-	dev_addr2 = ((uint64_t)addr2->domain << 24) |
-		(addr2->bus << 16) | (addr2->devid << 8) | addr2->function;
-
-	if (dev_addr > dev_addr2)
-		return 1;
-	else if (dev_addr < dev_addr2)
-		return -1;
-	else
-		return 0;
-}
+int rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
+			     const struct rte_pci_addr *addr2);
 
 /**
  * Map a particular resource from a file.
diff --git a/lib/librte_pci/rte_pci.c b/lib/librte_pci/rte_pci.c
index 9dfdd3f..8584b55 100644
--- a/lib/librte_pci/rte_pci.c
+++ b/lib/librte_pci/rte_pci.c
@@ -53,6 +53,71 @@ 
 
 #include "rte_pci.h"
 
+/* Macro used by pci addr parsing functions. **/
+#define GET_PCIADDR_FIELD(in, fd, lim, dlm)                     \
+do {                                                            \
+	unsigned long val;                                      \
+	char *end;                                              \
+	errno = 0;                                              \
+	val = strtoul((in), &end, 16);                          \
+	if (errno != 0 || end[0] != (dlm) || val > (lim))       \
+		return -EINVAL;                                 \
+	(fd) = (typeof (fd))val;                                \
+	(in) = end + 1;                                         \
+} while(0)
+
+int
+eal_parse_pci_BDF(const char *input, struct rte_pci_addr *dev_addr)
+{
+	dev_addr->domain = 0;
+	GET_PCIADDR_FIELD(input, dev_addr->bus, UINT8_MAX, ':');
+	GET_PCIADDR_FIELD(input, dev_addr->devid, UINT8_MAX, '.');
+	GET_PCIADDR_FIELD(input, dev_addr->function, UINT8_MAX, 0);
+	return 0;
+}
+
+int
+eal_parse_pci_DomBDF(const char *input, struct rte_pci_addr *dev_addr)
+{
+	GET_PCIADDR_FIELD(input, dev_addr->domain, UINT16_MAX, ':');
+	GET_PCIADDR_FIELD(input, dev_addr->bus, UINT8_MAX, ':');
+	GET_PCIADDR_FIELD(input, dev_addr->devid, UINT8_MAX, '.');
+	GET_PCIADDR_FIELD(input, dev_addr->function, UINT8_MAX, 0);
+	return 0;
+}
+
+void
+rte_pci_device_name(const struct rte_pci_addr *addr,
+		    char *output, size_t size)
+{
+	RTE_VERIFY(size >= PCI_PRI_STR_SIZE);
+	RTE_VERIFY(snprintf(output, size, PCI_PRI_FMT,
+			    addr->domain, addr->bus,
+			    addr->devid, addr->function) >= 0);
+}
+
+int
+rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
+			 const struct rte_pci_addr *addr2)
+{
+	uint64_t dev_addr, dev_addr2;
+
+	if ((addr == NULL) || (addr2 == NULL))
+		return -1;
+
+	dev_addr = ((uint64_t)addr->domain << 24) |
+		(addr->bus << 16) | (addr->devid << 8) | addr->function;
+	dev_addr2 = ((uint64_t)addr2->domain << 24) |
+		(addr2->bus << 16) | (addr2->devid << 8) | addr2->function;
+
+	if (dev_addr > dev_addr2)
+		return 1;
+	else if (dev_addr < dev_addr2)
+		return -1;
+	else
+		return 0;
+}
+
 /* map a particular resource from a file */
 void *
 pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size,
diff --git a/lib/librte_pci/rte_pci_version.map b/lib/librte_pci/rte_pci_version.map
index 64dec54..a940259 100644
--- a/lib/librte_pci/rte_pci_version.map
+++ b/lib/librte_pci/rte_pci_version.map
@@ -1,8 +1,12 @@ 
 DPDK_17.11 {
 	global:
 
+	eal_parse_pci_BDF;
+	eal_parse_pci_DomBDF;
 	pci_map_resource;
 	pci_unmap_resource;
+	rte_eal_compare_pci_addr;
+	rte_pci_device_name;
 
 	local: *;
 };