From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 5F0201D7 for ; Thu, 3 May 2018 10:10:56 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 May 2018 01:10:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,357,1520924400"; d="scan'208";a="51156799" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.252.18.186]) ([10.252.18.186]) by fmsmga004.fm.intel.com with ESMTP; 03 May 2018 01:10:52 -0700 To: Maxime Coquelin , dev@dpdk.org Cc: Hemant Agrawal , Shreyansh Jain References: <0016ba7532becb9168ec1f53bf578fffa2bd206b.1525333611.git.anatoly.burakov@intel.com> <8c9c700f-3b2a-b416-e117-ba9d50e1425f@redhat.com> From: "Burakov, Anatoly" Message-ID: <45cee0e8-2d5c-46b0-ab71-c237fc6231b4@intel.com> Date: Thu, 3 May 2018 09:10:52 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <8c9c700f-3b2a-b416-e117-ba9d50e1425f@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 18.05-RC3] mem: add argument to mem event callbacks 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: Thu, 03 May 2018 08:10:57 -0000 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 >> Suggested-by: Maxime Coquelin >> --- >>   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