Re: [dpdk-dev] [PATCH 18.05-RC3] mem: add argument to mem event callbacks

Thu, 03 May 2018 01:12:07 -0700 

On 03-May-18 8:51 AM, Maxime Coquelin wrote:

Hi Anatoly,

On 05/03/2018 09:47 AM, Anatoly Burakov wrote:

It may be useful to pass arbitrary data to the callback (such
as device pointers), so add this to the mem event callback API.

Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com>
Suggested-by: Maxime Coquelin <maxime.coque...@redhat.com>
---
  drivers/bus/fslmc/fslmc_vfio.c              |  5 +++--
  lib/librte_eal/common/eal_common_memalloc.c | 26 +++++++++++++++----------- 
  lib/librte_eal/common/eal_common_memory.c   |  9 +++++----
  lib/librte_eal/common/eal_memalloc.h        |  4 ++--
  lib/librte_eal/common/include/rte_memory.h  | 13 ++++++++++---
  lib/librte_eal/linuxapp/eal/eal_vfio.c      |  8 +++++---
  6 files changed, 40 insertions(+), 25 deletions(-)
diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c 
index 749d92d..56c36a0 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -194,7 +194,8 @@ static int fslmc_map_dma(uint64_t vaddr, rte_iova_t iovaddr, size_t len);   static int fslmc_unmap_dma(uint64_t vaddr, rte_iova_t iovaddr, size_t len); 
  static void
-fslmc_memevent_cb(enum rte_mem_event type, const void *addr, size_t len)
+fslmc_memevent_cb(enum rte_mem_event type, const void *addr, size_t len,
+        void *arg __rte_unused)
  {
      struct rte_memseg_list *msl;
      struct rte_memseg *ms;
@@ -347,7 +348,7 @@ int rte_fslmc_vfio_dmamap(void)
      }
      ret = rte_mem_event_callback_register("fslmc_memevent_clb",
-                          fslmc_memevent_cb);
+            fslmc_memevent_cb, NULL);
      if (ret && rte_errno == ENOTSUP)
          DPAA2_BUS_DEBUG("Memory event callbacks not supported");
      else if (ret)
diff --git a/lib/librte_eal/common/eal_common_memalloc.c b/lib/librte_eal/common/eal_common_memalloc.c 
index e983688..3005ef9 100644
--- a/lib/librte_eal/common/eal_common_memalloc.c
+++ b/lib/librte_eal/common/eal_common_memalloc.c
@@ -21,6 +21,7 @@ struct mem_event_callback_entry {
      TAILQ_ENTRY(mem_event_callback_entry) next;
      char name[RTE_MEM_EVENT_CALLBACK_NAME_LEN];
      rte_mem_event_callback_t clb;
+    void *arg;
  };
  struct mem_alloc_validator_entry {
@@ -44,12 +45,12 @@ static struct mem_alloc_validator_entry_list mem_alloc_validator_list =   static rte_rwlock_t mem_alloc_validator_rwlock = RTE_RWLOCK_INITIALIZER; 
  static struct mem_event_callback_entry *
-find_mem_event_callback(const char *name)
+find_mem_event_callback(const char *name, void *arg)
  {
      struct mem_event_callback_entry *r;
      TAILQ_FOREACH(r, &mem_event_callback_list, next) {
-        if (!strcmp(r->name, name))
+        if (!strcmp(r->name, name) && r->arg == arg)
              break;
      }
      return r;
@@ -146,7 +147,7 @@ eal_memalloc_is_contig(const struct rte_memseg_list *msl, void *start, 
  int
  eal_memalloc_mem_event_callback_register(const char *name,
-        rte_mem_event_callback_t clb)
+        rte_mem_event_callback_t clb, void *arg)
  {
      struct mem_event_callback_entry *entry;
      int ret, len;
@@ -164,7 +165,7 @@ eal_memalloc_mem_event_callback_register(const char *name, 
      }
      rte_rwlock_write_lock(&mem_event_rwlock);
-    entry = find_mem_event_callback(name);
+    entry = find_mem_event_callback(name, arg);
      if (entry != NULL) {
          rte_errno = EEXIST;
          ret = -1;
@@ -180,12 +181,14 @@ eal_memalloc_mem_event_callback_register(const char *name,       /* callback successfully created and is valid, add it to the list */ 
      entry->clb = clb;
+    entry->arg = arg;
      strlcpy(entry->name, name, RTE_MEM_EVENT_CALLBACK_NAME_LEN);
      TAILQ_INSERT_TAIL(&mem_event_callback_list, entry, next);
      ret = 0;
-    RTE_LOG(DEBUG, EAL, "Mem event callback '%s' registered\n", name);
+    RTE_LOG(DEBUG, EAL, "Mem event callback '%s:%p' registered\n",
+            name, arg);
  unlock:
      rte_rwlock_write_unlock(&mem_event_rwlock);
@@ -193,7 +196,7 @@ eal_memalloc_mem_event_callback_register(const char *name, 
  }
  int
-eal_memalloc_mem_event_callback_unregister(const char *name)
+eal_memalloc_mem_event_callback_unregister(const char *name, void *arg)
  {
      struct mem_event_callback_entry *entry;
      int ret, len;
@@ -212,7 +215,7 @@ eal_memalloc_mem_event_callback_unregister(const char *name) 
      }
      rte_rwlock_write_lock(&mem_event_rwlock);
-    entry = find_mem_event_callback(name);
+    entry = find_mem_event_callback(name, arg);
      if (entry == NULL) {
          rte_errno = ENOENT;
          ret = -1;
@@ -223,7 +226,8 @@ eal_memalloc_mem_event_callback_unregister(const char *name) 
      ret = 0;
-    RTE_LOG(DEBUG, EAL, "Mem event callback '%s' unregistered\n", name);
+    RTE_LOG(DEBUG, EAL, "Mem event callback '%s:%p' unregistered\n",
+            name, arg);
  unlock:
      rte_rwlock_write_unlock(&mem_event_rwlock);
@@ -239,9 +243,9 @@ eal_memalloc_mem_event_notify(enum rte_mem_event event, const void *start, 
      rte_rwlock_read_lock(&mem_event_rwlock);
      TAILQ_FOREACH(entry, &mem_event_callback_list, next) {
-        RTE_LOG(DEBUG, EAL, "Calling mem event callback %s",
-            entry->name);
-        entry->clb(event, start, len);
+        RTE_LOG(DEBUG, EAL, "Calling mem event callback '%s:%p'",
+            entry->name, entry->arg);
+        entry->clb(event, start, len, entry->arg);
      }
      rte_rwlock_read_unlock(&mem_event_rwlock);
diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c 
index 4c943b0..f080132 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -661,7 +661,8 @@ dump_memseg(const struct rte_memseg_list *msl, const struct rte_memseg *ms, 
   * is in eal_common_memalloc.c, like all other memalloc internals.
   */
  int __rte_experimental
-rte_mem_event_callback_register(const char *name, rte_mem_event_callback_t clb) +rte_mem_event_callback_register(const char *name, rte_mem_event_callback_t clb, 
+        void *arg)
  {
      /* FreeBSD boots with legacy mem enabled by default */
      if (internal_config.legacy_mem) {
@@ -669,11 +670,11 @@ rte_mem_event_callback_register(const char *name, rte_mem_event_callback_t clb) 
          rte_errno = ENOTSUP;
          return -1;
      }
-    return eal_memalloc_mem_event_callback_register(name, clb);
+    return eal_memalloc_mem_event_callback_register(name, clb, arg);
  }
  int __rte_experimental
-rte_mem_event_callback_unregister(const char *name)
+rte_mem_event_callback_unregister(const char *name, void *arg)
  {
      /* FreeBSD boots with legacy mem enabled by default */
      if (internal_config.legacy_mem) {
@@ -681,7 +682,7 @@ rte_mem_event_callback_unregister(const char *name)
          rte_errno = ENOTSUP;
          return -1;
      }
-    return eal_memalloc_mem_event_callback_unregister(name);
+    return eal_memalloc_mem_event_callback_unregister(name, arg);
  }
  int __rte_experimental
diff --git a/lib/librte_eal/common/eal_memalloc.h b/lib/librte_eal/common/eal_memalloc.h 
index 662b3b5..36bb1a0 100644
--- a/lib/librte_eal/common/eal_memalloc.h
+++ b/lib/librte_eal/common/eal_memalloc.h
@@ -57,10 +57,10 @@ eal_memalloc_sync_with_primary(void);
  int
  eal_memalloc_mem_event_callback_register(const char *name,
-        rte_mem_event_callback_t clb);
+        rte_mem_event_callback_t clb, void *arg);
  int
-eal_memalloc_mem_event_callback_unregister(const char *name);
+eal_memalloc_mem_event_callback_unregister(const char *name, void *arg);
  void
  eal_memalloc_mem_event_notify(enum rte_mem_event event, const void *start, diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h 
index 1e35cb2..857773c 100644
--- a/lib/librte_eal/common/include/rte_memory.h
+++ b/lib/librte_eal/common/include/rte_memory.h
@@ -330,7 +330,7 @@ enum rte_mem_event {
   * Function typedef used to register callbacks for memory events.
   */
  typedef void (*rte_mem_event_callback_t)(enum rte_mem_event event_type,
-        const void *addr, size_t len);
+        const void *addr, size_t len, void *arg);
  /**
   * Function used to register callbacks for memory events.
@@ -345,13 +345,17 @@ typedef void (*rte_mem_event_callback_t)(enum rte_mem_event event_type, 
   * @param clb
   *   Callback function pointer.
   *
+ * @param arg
+ *   Argument to pass to the callback.
+ *
   * @return
   *   0 on successful callback register
   *   -1 on unsuccessful callback register, with rte_errno value indicating 
   *   reason for failure.
   */
  int __rte_experimental
-rte_mem_event_callback_register(const char *name, rte_mem_event_callback_t clb); +rte_mem_event_callback_register(const char *name, rte_mem_event_callback_t clb, 
+        void *arg);
  /**
   * Function used to unregister callbacks for memory events.
@@ -359,13 +363,16 @@ rte_mem_event_callback_register(const char *name, rte_mem_event_callback_t clb); 
   * @param name
   *   Name associated with specified callback to be removed from the list. 
   *
+ * @param arg
+ *   Argument to look for among callbacks with specified callback name.
+ *
   * @return
   *   0 on successful callback unregister
   *   -1 on unsuccessful callback unregister, with rte_errno value indicating 
   *   reason for failure.
   */
  int __rte_experimental
-rte_mem_event_callback_unregister(const char *name);
+rte_mem_event_callback_unregister(const char *name, void *arg);
  #define RTE_MEM_ALLOC_VALIDATOR_NAME_LEN 64
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c 
index 7afa33d..db2a111 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
@@ -535,7 +535,8 @@ vfio_group_device_count(int vfio_group_fd)
  }
  static void
-vfio_mem_event_callback(enum rte_mem_event type, const void *addr, size_t len) +vfio_mem_event_callback(enum rte_mem_event type, const void *addr, size_t len, 
+        void *arg)


You need to tag arg as unused:
lib/librte_eal/linuxapp/eal/eal_vfio.c: In function ‘vfio_mem_event_callback’: lib/librte_eal/linuxapp/eal/eal_vfio.c:539:9: error: unused parameter ‘arg’ [-Werror=unused-parameter] 
    void *arg)
          ^~~



Yep, thanks!


Thanks,
Maxime

  {
      struct rte_memseg_list *msl;
      struct rte_memseg *ms;
@@ -780,7 +781,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr, 
              if (vfio_cfg == default_vfio_cfg)
                  ret = rte_mem_event_callback_register(
                      VFIO_MEM_EVENT_CLB_NAME,
-                    vfio_mem_event_callback);
+                    vfio_mem_event_callback, NULL);
              else
                  ret = 0;
              /* unlock memory hotplug */
@@ -908,7 +909,8 @@ rte_vfio_release_device(const char *sysfs_base, const char *dev_addr, 
       * avoid spurious attempts to map/unmap memory from VFIO.
       */
      if (vfio_cfg == default_vfio_cfg && vfio_cfg->vfio_active_groups == 0) 
-        rte_mem_event_callback_unregister(VFIO_MEM_EVENT_CLB_NAME);
+        rte_mem_event_callback_unregister(VFIO_MEM_EVENT_CLB_NAME,
+                NULL);
      /* success */
      ret = 0;






--
Thanks,
Anatoly

Reply via email to