From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 27272A0C41; Sun, 24 Oct 2021 22:06:02 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 15DDE410F1; Sun, 24 Oct 2021 22:06:02 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 6EB71410F1 for ; Sun, 24 Oct 2021 22:06:00 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635105960; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Caql2ciRay6rNOwb8in4EdkgCiMLP71ZcB2PMzAgk4I=; b=XkGPFCaGUdk27qiuBLHJz8MI/j6q2kjLN69q7OTKyfHu1sW4zJeGPQKLreOtZ8/LuxzPt7 MzVqksJRe8rmwR8ySc4Krzd3wZ1X4Y4QYUaqSaPja/RzX9s3kJPMJ7GeG7sykVzhtcj0xi HRnZHRAGpESKdW7JebG7RsCK09hTdj8= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-542-vSMJN-vzOAKEDiAUs-QhaA-1; Sun, 24 Oct 2021 16:05:58 -0400 X-MC-Unique: vSMJN-vzOAKEDiAUs-QhaA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4A5B880668A; Sun, 24 Oct 2021 20:05:57 +0000 (UTC) Received: from dmarchan.remote.csb (unknown [10.40.192.64]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6D4095D9D5; Sun, 24 Oct 2021 20:05:54 +0000 (UTC) From: David Marchand To: hkalra@marvell.com, dev@dpdk.org Cc: dmitry.kozliuk@gmail.com, Bruce Richardson Date: Sun, 24 Oct 2021 22:04:44 +0200 Message-Id: <20211024200449.12024-5-david.marchand@redhat.com> In-Reply-To: <20211024200449.12024-1-david.marchand@redhat.com> References: <20211022204934.132186-1-hkalra@marvell.com> <20211024200449.12024-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david.marchand@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" Subject: [dpdk-dev] [PATCH v6 4/9] alarm: remove direct access to interrupt handle X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Harman Kalra Removing direct access to interrupt handle structure fields, rather use respective get set APIs for the same. Making changes to all the libraries access the interrupt handle fields. Implementing alarm cleanup routine, where the memory allocated for interrupt instance can be freed. Signed-off-by: Harman Kalra Signed-off-by: David Marchand --- Changes since v5: - split from patch4, - merged patch6, - renamed rte_eal_alarm_fini as rte_eal_alarm_cleanup, --- lib/eal/common/eal_private.h | 10 ++++++++ lib/eal/freebsd/eal.c | 1 + lib/eal/freebsd/eal_alarm.c | 44 +++++++++++++++++++++++++++++++----- lib/eal/linux/eal.c | 1 + lib/eal/linux/eal_alarm.c | 32 ++++++++++++++++++++------ 5 files changed, 75 insertions(+), 13 deletions(-) diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h index 86dab1f057..36bcc0b5a4 100644 --- a/lib/eal/common/eal_private.h +++ b/lib/eal/common/eal_private.h @@ -163,6 +163,16 @@ int rte_eal_intr_init(void); */ int rte_eal_alarm_init(void); +/** + * Alarm mechanism cleanup. + * + * This function is private to EAL. + * + * @return + * 0 on success, negative on error + */ +void rte_eal_alarm_cleanup(void); + /** * Function is to check if the kernel module(like, vfio, vfio_iommu_type1, * etc.) loaded. diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c index 56a60f13e9..9935356ed4 100644 --- a/lib/eal/freebsd/eal.c +++ b/lib/eal/freebsd/eal.c @@ -975,6 +975,7 @@ rte_eal_cleanup(void) rte_mp_channel_cleanup(); /* after this point, any DPDK pointers will become dangling */ rte_eal_memory_detach(); + rte_eal_alarm_cleanup(); rte_trace_save(); eal_trace_fini(); eal_cleanup_config(internal_conf); diff --git a/lib/eal/freebsd/eal_alarm.c b/lib/eal/freebsd/eal_alarm.c index c38b2e04f8..1a8fcf24c5 100644 --- a/lib/eal/freebsd/eal_alarm.c +++ b/lib/eal/freebsd/eal_alarm.c @@ -32,7 +32,7 @@ struct alarm_entry { LIST_ENTRY(alarm_entry) next; - struct rte_intr_handle handle; + struct rte_intr_handle *handle; struct timespec time; rte_eal_alarm_callback cb_fn; void *cb_arg; @@ -43,22 +43,46 @@ struct alarm_entry { static LIST_HEAD(alarm_list, alarm_entry) alarm_list = LIST_HEAD_INITIALIZER(); static rte_spinlock_t alarm_list_lk = RTE_SPINLOCK_INITIALIZER; -static struct rte_intr_handle intr_handle = {.fd = -1 }; +static struct rte_intr_handle *intr_handle; static void eal_alarm_callback(void *arg); +void +rte_eal_alarm_cleanup(void) +{ + rte_intr_instance_free(intr_handle); +} + int rte_eal_alarm_init(void) { - intr_handle.type = RTE_INTR_HANDLE_ALARM; + int fd; + + intr_handle = rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE); + if (intr_handle == NULL) { + RTE_LOG(ERR, EAL, "Fail to allocate intr_handle\n"); + goto error; + } + + if (rte_intr_type_set(intr_handle, RTE_INTR_HANDLE_ALARM)) + goto error; + + if (rte_intr_fd_set(intr_handle, -1)) + goto error; /* on FreeBSD, timers don't use fd's, and their identifiers are stored * in separate namespace from fd's, so using any value is OK. however, * EAL interrupts handler expects fd's to be unique, so use an actual fd * to guarantee unique timer identifier. */ - intr_handle.fd = open("/dev/zero", O_RDONLY); + fd = open("/dev/zero", O_RDONLY); + + if (rte_intr_fd_set(intr_handle, fd)) + goto error; return 0; +error: + rte_intr_instance_free(intr_handle); + return -1; } static inline int @@ -118,7 +142,7 @@ unregister_current_callback(void) ap = LIST_FIRST(&alarm_list); do { - ret = rte_intr_callback_unregister(&intr_handle, + ret = rte_intr_callback_unregister(intr_handle, eal_alarm_callback, &ap->time); } while (ret == -EAGAIN); } @@ -136,7 +160,7 @@ register_first_callback(void) ap = LIST_FIRST(&alarm_list); /* register a new callback */ - ret = rte_intr_callback_register(&intr_handle, + ret = rte_intr_callback_register(intr_handle, eal_alarm_callback, &ap->time); } return ret; @@ -164,6 +188,7 @@ eal_alarm_callback(void *arg __rte_unused) rte_spinlock_lock(&alarm_list_lk); LIST_REMOVE(ap, next); + rte_intr_instance_free(ap->handle); free(ap); ap = LIST_FIRST(&alarm_list); @@ -202,6 +227,11 @@ rte_eal_alarm_set(uint64_t us, rte_eal_alarm_callback cb_fn, void *cb_arg) new_alarm->time.tv_nsec = (now.tv_nsec + ns) % NS_PER_S; new_alarm->time.tv_sec = now.tv_sec + ((now.tv_nsec + ns) / NS_PER_S); + new_alarm->handle = + rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE); + if (new_alarm->handle == NULL) + return -ENOMEM; + rte_spinlock_lock(&alarm_list_lk); if (LIST_EMPTY(&alarm_list)) @@ -255,6 +285,7 @@ rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg) break; if (ap->executing == 0) { LIST_REMOVE(ap, next); + rte_intr_instance_free(ap->handle); free(ap); count++; } else { @@ -282,6 +313,7 @@ rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg) cb_arg == ap->cb_arg)) { if (ap->executing == 0) { LIST_REMOVE(ap, next); + rte_intr_instance_free(ap->handle); free(ap); count++; ap = ap_prev; diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c index 0d0fc66668..81fdebc6a0 100644 --- a/lib/eal/linux/eal.c +++ b/lib/eal/linux/eal.c @@ -1368,6 +1368,7 @@ rte_eal_cleanup(void) rte_mp_channel_cleanup(); /* after this point, any DPDK pointers will become dangling */ rte_eal_memory_detach(); + rte_eal_alarm_cleanup(); rte_trace_save(); eal_trace_fini(); eal_cleanup_config(internal_conf); diff --git a/lib/eal/linux/eal_alarm.c b/lib/eal/linux/eal_alarm.c index 3252c6fa59..3b5e894595 100644 --- a/lib/eal/linux/eal_alarm.c +++ b/lib/eal/linux/eal_alarm.c @@ -54,22 +54,40 @@ struct alarm_entry { static LIST_HEAD(alarm_list, alarm_entry) alarm_list = LIST_HEAD_INITIALIZER(); static rte_spinlock_t alarm_list_lk = RTE_SPINLOCK_INITIALIZER; -static struct rte_intr_handle intr_handle = {.fd = -1 }; +static struct rte_intr_handle *intr_handle; static int handler_registered = 0; static void eal_alarm_callback(void *arg); +void +rte_eal_alarm_cleanup(void) +{ + rte_intr_instance_free(intr_handle); +} + int rte_eal_alarm_init(void) { - intr_handle.type = RTE_INTR_HANDLE_ALARM; + + intr_handle = rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE); + if (intr_handle == NULL) { + RTE_LOG(ERR, EAL, "Fail to allocate intr_handle\n"); + goto error; + } + + if (rte_intr_type_set(intr_handle, RTE_INTR_HANDLE_ALARM)) + goto error; + /* create a timerfd file descriptor */ - intr_handle.fd = timerfd_create(CLOCK_MONOTONIC, TFD_NONBLOCK); - if (intr_handle.fd == -1) + if (rte_intr_fd_set(intr_handle, + timerfd_create(CLOCK_MONOTONIC, TFD_NONBLOCK))) goto error; + if (rte_intr_fd_get(intr_handle) == -1) + goto error; return 0; error: + rte_intr_instance_free(intr_handle); rte_errno = errno; return -1; } @@ -109,7 +127,7 @@ eal_alarm_callback(void *arg __rte_unused) atime.it_value.tv_sec -= now.tv_sec; atime.it_value.tv_nsec -= now.tv_nsec; - timerfd_settime(intr_handle.fd, 0, &atime, NULL); + timerfd_settime(rte_intr_fd_get(intr_handle), 0, &atime, NULL); } rte_spinlock_unlock(&alarm_list_lk); } @@ -140,7 +158,7 @@ rte_eal_alarm_set(uint64_t us, rte_eal_alarm_callback cb_fn, void *cb_arg) rte_spinlock_lock(&alarm_list_lk); if (!handler_registered) { /* registration can fail, callback can be registered later */ - if (rte_intr_callback_register(&intr_handle, + if (rte_intr_callback_register(intr_handle, eal_alarm_callback, NULL) == 0) handler_registered = 1; } @@ -170,7 +188,7 @@ rte_eal_alarm_set(uint64_t us, rte_eal_alarm_callback cb_fn, void *cb_arg) .tv_nsec = (us % US_PER_S) * NS_PER_US, }, }; - ret |= timerfd_settime(intr_handle.fd, 0, &alarm_time, NULL); + ret |= timerfd_settime(rte_intr_fd_get(intr_handle), 0, &alarm_time, NULL); } rte_spinlock_unlock(&alarm_list_lk); -- 2.23.0