From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 057805323 for ; Fri, 7 Dec 2018 15:47:28 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Dec 2018 06:47:27 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,326,1539673200"; d="scan'208";a="116833412" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.252.12.22]) ([10.252.12.22]) by FMSMGA003.fm.intel.com with ESMTP; 07 Dec 2018 06:47:26 -0800 To: Jakub Grajciar , dev@dpdk.org References: <20181207114048.2324-1-jgrajcia@cisco.com> From: "Burakov, Anatoly" Message-ID: <327df61b-0ce5-492b-87b9-28fcdc035ec3@intel.com> Date: Fri, 7 Dec 2018 14:47:23 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.3.2 MIME-Version: 1.0 In-Reply-To: <20181207114048.2324-1-jgrajcia@cisco.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] eal_interrupts: add option for pending callback unregister 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: Fri, 07 Dec 2018 14:47:29 -0000 On 07-Dec-18 11:40 AM, Jakub Grajciar wrote: > 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 | 30 +++++++ > lib/librte_eal/linuxapp/eal/eal_interrupts.c | 83 ++++++++++++++++++- > 2 files changed, 111 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_eal/common/include/rte_interrupts.h b/lib/librte_eal/common/include/rte_interrupts.h > index d751a6378..3946742ad 100644 > --- a/lib/librte_eal/common/include/rte_interrupts.h > +++ b/lib/librte_eal/common/include/rte_interrupts.h > @@ -24,6 +24,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 +68,29 @@ 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_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..144ac4c22 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_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) { > + return -EAGAIN; Forgot unlock? > + > + } 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,33 @@ 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) > + return -EPIPE; Forgot unlock? > + > rte_spinlock_unlock(&intr_lock); > } > > -- Thanks, Anatoly