[v1,7/8] common/mlx5: exclude OS dependency in devx commands

Message ID 20200610093233.23902-8-ophirmu@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Raslan Darawsheh
Headers
Series mlx5 PMD multi OS support - part #2 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ophir Munk June 10, 2020, 9:32 a.m. UTC
  Shared function mlx5_devx_cmd_mkey_create() reads the OS pagesize by
calling a Linux API: 'sysconf(_SC_PAGESIZE)'. Wrap this call with a
shared API 'mlx5_os_get_page_size()' which contains the specific OS
implementation.

Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
 drivers/common/mlx5/linux/mlx5_common_os.c | 12 ++++++++++++
 drivers/common/mlx5/mlx5_common.h          |  1 +
 drivers/common/mlx5/mlx5_devx_cmds.c       |  2 +-
 3 files changed, 14 insertions(+), 1 deletion(-)
  

Comments

Thomas Monjalon June 18, 2020, 10:47 p.m. UTC | #1
10/06/2020 11:32, Ophir Munk:
> Shared function mlx5_devx_cmd_mkey_create() reads the OS pagesize by
> calling a Linux API: 'sysconf(_SC_PAGESIZE)'. Wrap this call with a
> shared API 'mlx5_os_get_page_size()' which contains the specific OS
> implementation.
> 
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>

Sorry, I drop this patch while pulling next-net.

> +/**
> + * Get OS page size
> + *
> + * @return
> + *   OS pagesize
> + */
> +size_t
> +mlx5_os_get_page_size(void)
> +{
> +	return sysconf(_SC_PAGESIZE);
> +}

The same purpose is achieved with rte_mem_page_size(),
which was added in EAL recently for Windows memory management.

In general, such basic need should not be implemented in a PMD.
  
Thomas Monjalon June 18, 2020, 10:48 p.m. UTC | #2
+Cc Ferruh for info

19/06/2020 00:47, Thomas Monjalon:
> 10/06/2020 11:32, Ophir Munk:
> > Shared function mlx5_devx_cmd_mkey_create() reads the OS pagesize by
> > calling a Linux API: 'sysconf(_SC_PAGESIZE)'. Wrap this call with a
> > shared API 'mlx5_os_get_page_size()' which contains the specific OS
> > implementation.
> > 
> > Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> 
> Sorry, I drop this patch while pulling next-net.
> 
> > +/**
> > + * Get OS page size
> > + *
> > + * @return
> > + *   OS pagesize
> > + */
> > +size_t
> > +mlx5_os_get_page_size(void)
> > +{
> > +	return sysconf(_SC_PAGESIZE);
> > +}
> 
> The same purpose is achieved with rte_mem_page_size(),
> which was added in EAL recently for Windows memory management.
> 
> In general, such basic need should not be implemented in a PMD.
  

Patch

diff --git a/drivers/common/mlx5/linux/mlx5_common_os.c b/drivers/common/mlx5/linux/mlx5_common_os.c
index 4e04d70..1b71347 100644
--- a/drivers/common/mlx5/linux/mlx5_common_os.c
+++ b/drivers/common/mlx5/linux/mlx5_common_os.c
@@ -125,6 +125,18 @@  mlx5_translate_port_name(const char *port_name_in,
 	port_info_out->name_type = MLX5_PHYS_PORT_NAME_TYPE_UNKNOWN;
 }
 
+/**
+ * Get OS page size
+ *
+ * @return
+ *   OS pagesize
+ */
+size_t
+mlx5_os_get_page_size(void)
+{
+	return sysconf(_SC_PAGESIZE);
+}
+
 #ifdef MLX5_GLUE
 
 /**
diff --git a/drivers/common/mlx5/mlx5_common.h b/drivers/common/mlx5/mlx5_common.h
index 8e679c6..77f10e6 100644
--- a/drivers/common/mlx5/mlx5_common.h
+++ b/drivers/common/mlx5/mlx5_common.h
@@ -213,6 +213,7 @@  __rte_internal
 void mlx5_translate_port_name(const char *port_name_in,
 			      struct mlx5_switch_info *port_info_out);
 void mlx5_glue_constructor(void);
+size_t mlx5_os_get_page_size(void);
 
 extern uint8_t haswell_broadwell_cpu;
 
diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c b/drivers/common/mlx5/mlx5_devx_cmds.c
index 091a825..ccba1c1 100644
--- a/drivers/common/mlx5/mlx5_devx_cmds.c
+++ b/drivers/common/mlx5/mlx5_devx_cmds.c
@@ -158,7 +158,7 @@  mlx5_devx_cmd_mkey_create(void *ctx,
 		return NULL;
 	}
 	memset(in, 0, in_size_dw * 4);
-	pgsize = sysconf(_SC_PAGESIZE);
+	pgsize = mlx5_os_get_page_size();
 	MLX5_SET(create_mkey_in, in, opcode, MLX5_CMD_OP_CREATE_MKEY);
 	mkc = MLX5_ADDR_OF(create_mkey_in, in, memory_key_mkey_entry);
 	if (klm_num > 0) {