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 23CE0A0C4C for ; Mon, 12 Jul 2021 15:15:00 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 076BA4069E; Mon, 12 Jul 2021 15:15:00 +0200 (CEST) Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com [209.85.221.46]) by mails.dpdk.org (Postfix) with ESMTP id 9ABAE40685 for ; Mon, 12 Jul 2021 15:14:58 +0200 (CEST) Received: by mail-wr1-f46.google.com with SMTP id f17so25577436wrt.6 for ; Mon, 12 Jul 2021 06:14:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=GCbkY/W5abs8FxKYj0AtUBE1VGVW6hHXi0ObCO+sjOU=; b=FffSJO2BaWI6Vo59oPSOcFSVlUv2cwv78wKvOz/NdLFxPAV1FvsDaHfSFaNrq2dgN/ gANN5jLMf0KfapJFpY5zVAKQeo0eBXZLVyxt3pTRRRO14meeqisn09609jhX8a2Axk6W veumFc38JB6J7O/ET4255szz3HRkUgVNjQZn/YcmmfdTglOi/D3vxM+TK33Q7Pw5Hdu6 9Vf1yP5/HWZIpHDhJpmU8I6JEHFlKKNdliw48ohqT4YQNCKHS6DhC2heyY5Z4fIYqIE9 X7nLE51JZr0Trk42JXKiHGbQ9Y+QknIJmtGr1c/sF4djwpKQEBoSyFnNrmAhLF87fGPI O8mw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=GCbkY/W5abs8FxKYj0AtUBE1VGVW6hHXi0ObCO+sjOU=; b=oN+qZFeppXaFG97oH+lLiT91EFKLZBZr4FTP2Ny8mH2iziY5fUUcvN6oGlDH/Oqnfe 0bLBPpumFUMD4avFtasppQ3vvu7zCbdx0TapJ5Nf8TQP26YAKc4x36RAUsi9RinSEKUh dUE8J0/nOUVCPrcepbrCqoq1QgjG2e2il65mn1eAzztXmbwTdrtD+iEITMbjKLNudKhT su8T/i6nlJ+D5YkqVOgmhvJo5BnkfJUJfsTg8zYsME+CusMfS8JclCnnx+npS1RYjvqC mbDskcWFuaDeFOTOpiZ/ocm8vCAKaPTROiyCV/kHaMZGh+EwNPYNpU/XidPjs981xWwf a8xA== X-Gm-Message-State: AOAM533K4s+KQ4KuoY9mz8bPwfPqUThtIrp/0FJkL3dfCfUecOIdJxev ABdNUBSSd3pSE5CVc6zlMz7gAJWs4ye82wL4 X-Google-Smtp-Source: ABdhPJzfDeLmzSpYSspUYZpTWnYAaQsazChgHd5B/Q4lc/u08xFaqzTrwM9Z2GFpWg4YO8gacOPntA== X-Received: by 2002:a05:6000:2a1:: with SMTP id l1mr14735246wry.128.1626095698333; Mon, 12 Jul 2021 06:14:58 -0700 (PDT) Received: from localhost ([137.220.125.106]) by smtp.gmail.com with ESMTPSA id f5sm15546674wrg.67.2021.07.12.06.14.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 12 Jul 2021 06:14:57 -0700 (PDT) From: luca.boccassi@gmail.com To: Michael Baum Cc: Matan Azrad , dpdk stable Date: Mon, 12 Jul 2021 14:05:28 +0100 Message-Id: <20210712130551.2462159-93-luca.boccassi@gmail.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210712130551.2462159-1-luca.boccassi@gmail.com> References: <20210712130551.2462159-1-luca.boccassi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-stable] patch 'common/mlx5: fix memory region leak' has been queued to stable release 20.11.3 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi, FYI, your patch has been queued to stable release 20.11.3 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 07/14/21. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. This will indicate if there was any rebasing needed to apply to the stable branch. If there were code changes for rebasing (ie: not only metadata diffs), please double check that the rebase was correctly done. Queued patches are on a temporary branch at: https://github.com/bluca/dpdk-stable This queued commit can be viewed at: https://github.com/bluca/dpdk-stable/commit/d7ecda57fbbd8a715be4bcac9bdefacb7a12db3a Thanks. Luca Boccassi --- >From d7ecda57fbbd8a715be4bcac9bdefacb7a12db3a Mon Sep 17 00:00:00 2001 From: Michael Baum Date: Mon, 28 Jun 2021 18:06:14 +0300 Subject: [PATCH] common/mlx5: fix memory region leak [ upstream commit 2f6c2adbe550ea95a0f73c4f9a9cc5da890b9bf2 ] All the mlx5 drivers using MRs for data-path must unregister the mapped memory when it is freed by the dpdk process. Currently, only the net/eth driver unregisters MRs in free event. Move the net callback handler from net driver to common. Signed-off-by: Michael Baum Acked-by: Matan Azrad --- drivers/common/mlx5/mlx5_common_mr.c | 89 ++++++++++++++++++++++++++ drivers/common/mlx5/mlx5_common_mr.h | 3 + drivers/common/mlx5/version.map | 1 + drivers/net/mlx5/mlx5_mr.c | 95 +--------------------------- 4 files changed, 95 insertions(+), 93 deletions(-) diff --git a/drivers/common/mlx5/mlx5_common_mr.c b/drivers/common/mlx5/mlx5_common_mr.c index 7c25541dc4..d01f86837d 100644 --- a/drivers/common/mlx5/mlx5_common_mr.c +++ b/drivers/common/mlx5/mlx5_common_mr.c @@ -1060,6 +1060,95 @@ mlx5_create_mr_ext(void *pd, uintptr_t addr, size_t len, int socket_id, return mr; } +/** + * Callback for memory free event. Iterate freed memsegs and check whether it + * belongs to an existing MR. If found, clear the bit from bitmap of MR. As a + * result, the MR would be fragmented. If it becomes empty, the MR will be freed + * later by mlx5_mr_garbage_collect(). Even if this callback is called from a + * secondary process, the garbage collector will be called in primary process + * as the secondary process can't call mlx5_mr_create(). + * + * The global cache must be rebuilt if there's any change and this event has to + * be propagated to dataplane threads to flush the local caches. + * + * @param share_cache + * Pointer to a global shared MR cache. + * @param ibdev_name + * Name of ibv device. + * @param addr + * Address of freed memory. + * @param len + * Size of freed memory. + */ +void +mlx5_free_mr_by_addr(struct mlx5_mr_share_cache *share_cache, + const char *ibdev_name, const void *addr, size_t len) +{ + const struct rte_memseg_list *msl; + struct mlx5_mr *mr; + int ms_n; + int i; + int rebuild = 0; + + DRV_LOG(DEBUG, "device %s free callback: addr=%p, len=%zu", + ibdev_name, addr, len); + msl = rte_mem_virt2memseg_list(addr); + /* addr and len must be page-aligned. */ + MLX5_ASSERT((uintptr_t)addr == + RTE_ALIGN((uintptr_t)addr, msl->page_sz)); + MLX5_ASSERT(len == RTE_ALIGN(len, msl->page_sz)); + ms_n = len / msl->page_sz; + rte_rwlock_write_lock(&share_cache->rwlock); + /* Clear bits of freed memsegs from MR. */ + for (i = 0; i < ms_n; ++i) { + const struct rte_memseg *ms; + struct mr_cache_entry entry; + uintptr_t start; + int ms_idx; + uint32_t pos; + + /* Find MR having this memseg. */ + start = (uintptr_t)addr + i * msl->page_sz; + mr = mlx5_mr_lookup_list(share_cache, &entry, start); + if (mr == NULL) + continue; + MLX5_ASSERT(mr->msl); /* Can't be external memory. */ + ms = rte_mem_virt2memseg((void *)start, msl); + MLX5_ASSERT(ms != NULL); + MLX5_ASSERT(msl->page_sz == ms->hugepage_sz); + ms_idx = rte_fbarray_find_idx(&msl->memseg_arr, ms); + pos = ms_idx - mr->ms_base_idx; + MLX5_ASSERT(rte_bitmap_get(mr->ms_bmp, pos)); + MLX5_ASSERT(pos < mr->ms_bmp_n); + DRV_LOG(DEBUG, "device %s MR(%p): clear bitmap[%u] for addr %p", + ibdev_name, (void *)mr, pos, (void *)start); + rte_bitmap_clear(mr->ms_bmp, pos); + if (--mr->ms_n == 0) { + LIST_REMOVE(mr, mr); + LIST_INSERT_HEAD(&share_cache->mr_free_list, mr, mr); + DRV_LOG(DEBUG, "device %s remove MR(%p) from list", + ibdev_name, (void *)mr); + } + /* + * MR is fragmented or will be freed. the global cache must be + * rebuilt. + */ + rebuild = 1; + } + if (rebuild) { + mlx5_mr_rebuild_cache(share_cache); + /* + * No explicit wmb is needed after updating dev_gen due to + * store-release ordering in unlock that provides the + * implicit barrier at the software visible level. + */ + ++share_cache->dev_gen; + DRV_LOG(DEBUG, "broadcasting local cache flush, gen=%d", + share_cache->dev_gen); + } + rte_rwlock_write_unlock(&share_cache->rwlock); +} + /** * Dump all the created MRs and the global cache entries. * diff --git a/drivers/common/mlx5/mlx5_common_mr.h b/drivers/common/mlx5/mlx5_common_mr.h index da0a0f0c79..09d39ddb5b 100644 --- a/drivers/common/mlx5/mlx5_common_mr.h +++ b/drivers/common/mlx5/mlx5_common_mr.h @@ -143,6 +143,9 @@ void mlx5_mr_rebuild_cache(struct mlx5_mr_share_cache *share_cache); __rte_internal void mlx5_mr_flush_local_cache(struct mlx5_mr_ctrl *mr_ctrl); __rte_internal +void mlx5_free_mr_by_addr(struct mlx5_mr_share_cache *share_cache, + const char *ibdev_name, const void *addr, size_t len); +__rte_internal int mlx5_mr_insert_cache(struct mlx5_mr_share_cache *share_cache, struct mlx5_mr *mr); diff --git a/drivers/common/mlx5/version.map b/drivers/common/mlx5/version.map index fd6019bd2b..02aedbc431 100644 --- a/drivers/common/mlx5/version.map +++ b/drivers/common/mlx5/version.map @@ -69,6 +69,7 @@ INTERNAL { mlx5_mr_create_primary; mlx5_mr_flush_local_cache; mlx5_mr_free; + mlx5_free_mr_by_addr; mlx5_nl_allmulti; mlx5_nl_devlink_family_id_get; diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c index da4e91fc24..2cce3b9fe8 100644 --- a/drivers/net/mlx5/mlx5_mr.c +++ b/drivers/net/mlx5/mlx5_mr.c @@ -29,98 +29,6 @@ struct mr_update_mp_data { int ret; }; -/** - * Callback for memory free event. Iterate freed memsegs and check whether it - * belongs to an existing MR. If found, clear the bit from bitmap of MR. As a - * result, the MR would be fragmented. If it becomes empty, the MR will be freed - * later by mlx5_mr_garbage_collect(). Even if this callback is called from a - * secondary process, the garbage collector will be called in primary process - * as the secondary process can't call mlx5_mr_create(). - * - * The global cache must be rebuilt if there's any change and this event has to - * be propagated to dataplane threads to flush the local caches. - * - * @param sh - * Pointer to the Ethernet device shared context. - * @param addr - * Address of freed memory. - * @param len - * Size of freed memory. - */ -static void -mlx5_mr_mem_event_free_cb(struct mlx5_dev_ctx_shared *sh, - const void *addr, size_t len) -{ - const struct rte_memseg_list *msl; - struct mlx5_mr *mr; - int ms_n; - int i; - int rebuild = 0; - - DEBUG("device %s free callback: addr=%p, len=%zu", - sh->ibdev_name, addr, len); - msl = rte_mem_virt2memseg_list(addr); - /* addr and len must be page-aligned. */ - MLX5_ASSERT((uintptr_t)addr == - RTE_ALIGN((uintptr_t)addr, msl->page_sz)); - MLX5_ASSERT(len == RTE_ALIGN(len, msl->page_sz)); - ms_n = len / msl->page_sz; - rte_rwlock_write_lock(&sh->share_cache.rwlock); - /* Clear bits of freed memsegs from MR. */ - for (i = 0; i < ms_n; ++i) { - const struct rte_memseg *ms; - struct mr_cache_entry entry; - uintptr_t start; - int ms_idx; - uint32_t pos; - - /* Find MR having this memseg. */ - start = (uintptr_t)addr + i * msl->page_sz; - mr = mlx5_mr_lookup_list(&sh->share_cache, &entry, start); - if (mr == NULL) - continue; - MLX5_ASSERT(mr->msl); /* Can't be external memory. */ - ms = rte_mem_virt2memseg((void *)start, msl); - MLX5_ASSERT(ms != NULL); - MLX5_ASSERT(msl->page_sz == ms->hugepage_sz); - ms_idx = rte_fbarray_find_idx(&msl->memseg_arr, ms); - pos = ms_idx - mr->ms_base_idx; - MLX5_ASSERT(rte_bitmap_get(mr->ms_bmp, pos)); - MLX5_ASSERT(pos < mr->ms_bmp_n); - DEBUG("device %s MR(%p): clear bitmap[%u] for addr %p", - sh->ibdev_name, (void *)mr, pos, (void *)start); - rte_bitmap_clear(mr->ms_bmp, pos); - if (--mr->ms_n == 0) { - LIST_REMOVE(mr, mr); - LIST_INSERT_HEAD(&sh->share_cache.mr_free_list, mr, mr); - DEBUG("device %s remove MR(%p) from list", - sh->ibdev_name, (void *)mr); - } - /* - * MR is fragmented or will be freed. the global cache must be - * rebuilt. - */ - rebuild = 1; - } - if (rebuild) { - mlx5_mr_rebuild_cache(&sh->share_cache); - /* - * Flush local caches by propagating invalidation across cores. - * rte_smp_wmb() is enough to synchronize this event. If one of - * freed memsegs is seen by other core, that means the memseg - * has been allocated by allocator, which will come after this - * free call. Therefore, this store instruction (incrementing - * generation below) will be guaranteed to be seen by other core - * before the core sees the newly allocated memory. - */ - ++sh->share_cache.dev_gen; - DEBUG("broadcasting local cache flush, gen=%d", - sh->share_cache.dev_gen); - rte_smp_wmb(); - } - rte_rwlock_write_unlock(&sh->share_cache.rwlock); -} - /** * Callback for memory event. This can be called from both primary and secondary * process. @@ -146,7 +54,8 @@ mlx5_mr_mem_event_cb(enum rte_mem_event event_type, const void *addr, rte_rwlock_write_lock(&mlx5_shared_data->mem_event_rwlock); /* Iterate all the existing mlx5 devices. */ LIST_FOREACH(sh, dev_list, mem_event_cb) - mlx5_mr_mem_event_free_cb(sh, addr, len); + mlx5_free_mr_by_addr(&sh->share_cache, + sh->ibdev_name, addr, len); rte_rwlock_write_unlock(&mlx5_shared_data->mem_event_rwlock); break; case RTE_MEM_EVENT_ALLOC: -- 2.30.2 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2021-07-12 13:41:41.388316195 +0100 +++ 0093-common-mlx5-fix-memory-region-leak.patch 2021-07-12 13:41:36.782128693 +0100 @@ -1 +1 @@ -From 2f6c2adbe550ea95a0f73c4f9a9cc5da890b9bf2 Mon Sep 17 00:00:00 2001 +From d7ecda57fbbd8a715be4bcac9bdefacb7a12db3a Mon Sep 17 00:00:00 2001 @@ -5,0 +6,2 @@ +[ upstream commit 2f6c2adbe550ea95a0f73c4f9a9cc5da890b9bf2 ] + @@ -13,2 +14,0 @@ -Cc: stable@dpdk.org - @@ -18 +18 @@ - drivers/common/mlx5/mlx5_common_mr.c | 89 +++++++++++++++++++++++++++ + drivers/common/mlx5/mlx5_common_mr.c | 89 ++++++++++++++++++++++++++ @@ -21,2 +21,2 @@ - drivers/net/mlx5/mlx5_mr.c | 90 +--------------------------- - 4 files changed, 95 insertions(+), 88 deletions(-) + drivers/net/mlx5/mlx5_mr.c | 95 +--------------------------- + 4 files changed, 95 insertions(+), 93 deletions(-) @@ -25 +25 @@ -index afb5b3d0a7..98fe8698e2 100644 +index 7c25541dc4..d01f86837d 100644 @@ -28 +28 @@ -@@ -1062,6 +1062,95 @@ mlx5_create_mr_ext(void *pd, uintptr_t addr, size_t len, int socket_id, +@@ -1060,6 +1060,95 @@ mlx5_create_mr_ext(void *pd, uintptr_t addr, size_t len, int socket_id, @@ -125 +125 @@ -index 5cc3f097c2..6e465a05e9 100644 +index da0a0f0c79..09d39ddb5b 100644 @@ -128 +128 @@ -@@ -144,6 +144,9 @@ void mlx5_mr_rebuild_cache(struct mlx5_mr_share_cache *share_cache); +@@ -143,6 +143,9 @@ void mlx5_mr_rebuild_cache(struct mlx5_mr_share_cache *share_cache); @@ -139 +139 @@ -index db4f13f1f7..b8be73a77b 100644 +index fd6019bd2b..02aedbc431 100644 @@ -142,4 +142,4 @@ -@@ -103,6 +103,7 @@ INTERNAL { - mlx5_mr_insert_cache; - mlx5_mr_lookup_cache; - mlx5_mr_lookup_list; +@@ -69,6 +69,7 @@ INTERNAL { + mlx5_mr_create_primary; + mlx5_mr_flush_local_cache; + mlx5_mr_free; @@ -147,2 +146,0 @@ - mlx5_mr_rebuild_cache; - mlx5_mr_release_cache; @@ -149,0 +148,2 @@ + mlx5_nl_allmulti; + mlx5_nl_devlink_family_id_get; @@ -151 +151 @@ -index 0c5403e493..0b6cfc8cb9 100644 +index da4e91fc24..2cce3b9fe8 100644 @@ -154 +154 @@ -@@ -31,93 +31,6 @@ struct mr_update_mp_data { +@@ -29,98 +29,6 @@ struct mr_update_mp_data { @@ -186 +186 @@ -- DRV_LOG(DEBUG, "device %s free callback: addr=%p, len=%zu", +- DEBUG("device %s free callback: addr=%p, len=%zu", @@ -216 +216 @@ -- DRV_LOG(DEBUG, "device %s MR(%p): clear bitmap[%u] for addr %p", +- DEBUG("device %s MR(%p): clear bitmap[%u] for addr %p", @@ -222 +222 @@ -- DRV_LOG(DEBUG, "device %s remove MR(%p) from list", +- DEBUG("device %s remove MR(%p) from list", @@ -234,3 +234,7 @@ -- * No explicit wmb is needed after updating dev_gen due to -- * store-release ordering in unlock that provides the -- * implicit barrier at the software visible level. +- * Flush local caches by propagating invalidation across cores. +- * rte_smp_wmb() is enough to synchronize this event. If one of +- * freed memsegs is seen by other core, that means the memseg +- * has been allocated by allocator, which will come after this +- * free call. Therefore, this store instruction (incrementing +- * generation below) will be guaranteed to be seen by other core +- * before the core sees the newly allocated memory. @@ -239 +243 @@ -- DRV_LOG(DEBUG, "broadcasting local cache flush, gen=%d", +- DEBUG("broadcasting local cache flush, gen=%d", @@ -240,0 +245 @@ +- rte_smp_wmb(); @@ -248 +253 @@ -@@ -143,7 +56,8 @@ mlx5_mr_mem_event_cb(enum rte_mem_event event_type, const void *addr, +@@ -146,7 +54,8 @@ mlx5_mr_mem_event_cb(enum rte_mem_event event_type, const void *addr,