From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v8] eal_interrupts: add option for pending callback unregister Date: Mon, 14 Jan 2019 15:01:34 +0100 Message-ID: <7206339.pO2ALYZzR5@xps> References: <20181217123009.23501-1-jgrajcia@cisco.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: Jakub Grajciar , anatoly.burakov@intel.com, david.marchand@redhat.com, bruce.richardson@intel.com, arybchenko@solarflare.com, ferruh.yigit@intel.com, olivier.matz@6wind.com, konstantin.ananyev@intel.com, jerin.jacob@caviumnetworks.com To: dev@dpdk.org Return-path: Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id 5AA1C1B215 for ; Mon, 14 Jan 2019 15:01:41 +0100 (CET) In-Reply-To: <20181217123009.23501-1-jgrajcia@cisco.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Someone for a review please? 17/12/2018 13:30, Jakub Grajciar: > use case: if callback is used to receive message form socket, > and the message received is disconnect/error, this callback needs > to be unregistered, but cannot because it is still active. > > With this patch it is possible to mark the callback to be > unregistered once the interrupt process is done with this > interrupt source. > > Signed-off-by: Jakub Grajciar > --- > .../common/include/rte_interrupts.h | 32 +++++++ > lib/librte_eal/linuxapp/eal/eal_interrupts.c | 85 ++++++++++++++++++- > lib/librte_eal/rte_eal_version.map | 1 + > 3 files changed, 116 insertions(+), 2 deletions(-) > > v2: > - fix coding syle > > v3: > - fix locks > > v4: > - update version map > > v6: > - add _rte_experimental to rte_intr_callback_unregister_pending > > v8: > - fix compilation errors > > diff --git a/lib/librte_eal/common/include/rte_interrupts.h b/lib/librte_eal/common/include/rte_interrupts.h > index d751a6378..13d73a137 100644 > --- a/lib/librte_eal/common/include/rte_interrupts.h > +++ b/lib/librte_eal/common/include/rte_interrupts.h > @@ -6,6 +6,7 @@ > #define _RTE_INTERRUPTS_H_ > > #include > +#include > > /** > * @file > @@ -24,6 +25,13 @@ struct rte_intr_handle; > /** Function to be registered for the specific interrupt */ > typedef void (*rte_intr_callback_fn)(void *cb_arg); > > +/** > + * Function to call after a callback is unregistered. > + * Can be used to close fd and free cb_arg. > + */ > +typedef void (*rte_intr_unregister_callback_fn)(struct rte_intr_handle *intr_handle, > + void *cb_arg); > + > #include "rte_eal_interrupts.h" > > /** > @@ -61,6 +69,30 @@ int rte_intr_callback_register(const struct rte_intr_handle *intr_handle, > int rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle, > rte_intr_callback_fn cb, void *cb_arg); > > +/** > + * It unregisters the callback according to the specified interrupt handle, > + * after it's no longer acive. Failes if source is not active. > + * > + * @param intr_handle > + * pointer to the interrupt handle. > + * @param cb > + * callback address. > + * @param cb_arg > + * address of parameter for callback, (void *)-1 means to remove all > + * registered which has the same callback address. > + * @param ucb_fn > + * callback to call before cb is unregistered (optional). > + * can be used to close fd and free cb_arg. > + * > + * @return > + * - On success, return the number of callback entities marked for remove. > + * - On failure, a negative value. > + */ > +int __rte_experimental > +rte_intr_callback_unregister_pending(const struct rte_intr_handle *intr_handle, > + rte_intr_callback_fn cb_fn, void *cb_arg, > + rte_intr_unregister_callback_fn ucb_fn); > + > /** > * It enables the interrupt for the specified handle. > * > diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c > index cbac451e1..be0dd4490 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c > +++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c > @@ -76,6 +76,8 @@ struct rte_intr_callback { > TAILQ_ENTRY(rte_intr_callback) next; > rte_intr_callback_fn cb_fn; /**< callback address */ > void *cb_arg; /**< parameter for callback */ > + uint8_t pending_delete; /**< delete after callback is called */ > + rte_intr_unregister_callback_fn ucb_fn; /**< fn to call before cb is deleted */ > }; > > struct rte_intr_source { > @@ -472,6 +474,8 @@ rte_intr_callback_register(const struct rte_intr_handle *intr_handle, > } > callback->cb_fn = cb; > callback->cb_arg = cb_arg; > + callback->pending_delete = 0; > + callback->ucb_fn = NULL; > > rte_spinlock_lock(&intr_lock); > > @@ -518,6 +522,57 @@ rte_intr_callback_register(const struct rte_intr_handle *intr_handle, > return ret; > } > > +int __rte_experimental > +rte_intr_callback_unregister_pending(const struct rte_intr_handle *intr_handle, > + rte_intr_callback_fn cb_fn, void *cb_arg, > + rte_intr_unregister_callback_fn ucb_fn) > +{ > + int ret; > + struct rte_intr_source *src; > + struct rte_intr_callback *cb, *next; > + > + /* do parameter checking first */ > + if (intr_handle == NULL || intr_handle->fd < 0) { > + RTE_LOG(ERR, EAL, > + "Unregistering with invalid input parameter\n"); > + return -EINVAL; > + } > + > + rte_spinlock_lock(&intr_lock); > + > + /* check if the insterrupt source for the fd is existent */ > + TAILQ_FOREACH(src, &intr_sources, next) > + if (src->intr_handle.fd == intr_handle->fd) > + break; > + > + /* No interrupt source registered for the fd */ > + if (src == NULL) { > + ret = -ENOENT; > + > + /* only usable if the source is active */ > + } else if (src->active == 0) { > + ret = -EAGAIN; > + > + } else { > + ret = 0; > + > + /* walk through the callbacks and mark all that match. */ > + for (cb = TAILQ_FIRST(&src->callbacks); cb != NULL; cb = next) { > + next = TAILQ_NEXT(cb, next); > + if (cb->cb_fn == cb_fn && (cb_arg == (void *)-1 || > + cb->cb_arg == cb_arg)) { > + cb->pending_delete = 1; > + cb->ucb_fn = ucb_fn; > + ret++; > + } > + } > + } > + > + rte_spinlock_unlock(&intr_lock); > + > + return ret; > +} > + > int > rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle, > rte_intr_callback_fn cb_fn, void *cb_arg) > @@ -698,7 +753,7 @@ static int > eal_intr_process_interrupts(struct epoll_event *events, int nfds) > { > bool call = false; > - int n, bytes_read; > + int n, bytes_read, rv; > struct rte_intr_source *src; > struct rte_intr_callback *cb, *next; > union rte_intr_read_buffer buf; > @@ -823,9 +878,35 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds) > rte_spinlock_lock(&intr_lock); > } > } > - > /* we done with that interrupt source, release it. */ > src->active = 0; > + > + rv = 0; > + > + /* check if any callback are supposed to be removed */ > + for (cb = TAILQ_FIRST(&src->callbacks); cb != NULL; cb = next) { > + next = TAILQ_NEXT(cb, next); > + if (cb->pending_delete) { > + TAILQ_REMOVE(&src->callbacks, cb, next); > + if (cb->ucb_fn) > + cb->ucb_fn(&src->intr_handle, cb->cb_arg); > + free(cb); > + rv++; > + } > + } > + > + /* all callbacks for that source are removed. */ > + if (TAILQ_EMPTY(&src->callbacks)) { > + TAILQ_REMOVE(&intr_sources, src, next); > + free(src); > + } > + > + /* notify the pipe fd waited by epoll_wait to rebuild the wait list */ > + if (rv >= 0 && write(intr_pipe.writefd, "1", 1) < 0) { > + rte_spinlock_unlock(&intr_lock); > + return -EPIPE; > + } > + > rte_spinlock_unlock(&intr_lock); > } > > diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map > index 3fe78260d..989e4d72a 100644 > --- a/lib/librte_eal/rte_eal_version.map > +++ b/lib/librte_eal/rte_eal_version.map > @@ -318,6 +318,7 @@ EXPERIMENTAL { > rte_fbarray_is_used; > rte_fbarray_set_free; > rte_fbarray_set_used; > + rte_intr_callback_unregister_pending; > rte_log_register_type_and_pick_level; > rte_malloc_dump_heaps; > rte_malloc_heap_create; >