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 4327542AF1; Thu, 18 May 2023 17:40:57 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 33B5242B71; Thu, 18 May 2023 17:40:57 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 9489C40E25 for ; Thu, 18 May 2023 17:40:56 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 3A1342B74 for ; Thu, 18 May 2023 17:40:56 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 1A4A6302A; Thu, 18 May 2023 17:40:56 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-2.8 required=5.0 tests=ALL_TRUSTED, AWL, NICE_REPLY_A, T_SCC_BODY_TEXT_LINE autolearn=disabled version=3.4.6 X-Spam-Score: -2.8 Received: from [192.168.1.59] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id 01C5F309E; Thu, 18 May 2023 17:40:52 +0200 (CEST) Message-ID: <8dce3bac-f6f7-ba83-795f-abcbba6a94c3@lysator.liu.se> Date: Thu, 18 May 2023 17:40:51 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH] lib/mempool : rte_mempool_avail_count, fixing return bigger than mempool size Content-Language: en-US To: =?UTF-8?Q?Morten_Br=c3=b8rup?= , Stephen Hemminger , Yasin CANER Cc: dev@dpdk.org, Yasin CANER , Olivier Matz , Andrew Rybchenko References: <20230516134146.480047-1-yasinncaner@gmail.com> <20230516082349.041c0e68@hermes.local> <98CBD80474FA8B44BF855DF32C47DC35D87923@smartserver.smartshare.dk> From: =?UTF-8?Q?Mattias_R=c3=b6nnblom?= In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D87923@smartserver.smartshare.dk> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Virus-Scanned: ClamAV using ClamSMTP 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 On 2023-05-16 18:03, Morten Brørup wrote: >> From: Stephen Hemminger [mailto:stephen@networkplumber.org] >> Sent: Tuesday, 16 May 2023 17.24 >> >> On Tue, 16 May 2023 13:41:46 +0000 >> Yasin CANER wrote: >> >>> From: Yasin CANER >>> >>> after a while working rte_mempool_avail_count function returns bigger >>> than mempool size that cause miscalculation rte_mempool_in_use_count. >>> >>> it helps to avoid miscalculation rte_mempool_in_use_count. >>> >>> Bugzilla ID: 1229 >>> >>> Signed-off-by: Yasin CANER >> >> An alternative that avoids some code duplication. >> >> diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c >> index cf5dea2304a7..2406b112e7b0 100644 >> --- a/lib/mempool/rte_mempool.c >> +++ b/lib/mempool/rte_mempool.c >> @@ -1010,7 +1010,7 @@ rte_mempool_avail_count(const struct rte_mempool >> *mp) >> count = rte_mempool_ops_get_count(mp); >> >> if (mp->cache_size == 0) >> - return count; >> + goto exit; > > This bug can only occur here (i.e. with cache_size==0) if rte_mempool_ops_get_count() returns an incorrect value. The bug should be fixed there instead. > >> >> for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) >> count += mp->local_cache[lcore_id].len; >> @@ -1019,6 +1019,7 @@ rte_mempool_avail_count(const struct rte_mempool >> *mp) This loop may result in the same symptoms (i.e., count > mp->size). For example, the LF stack has a relaxed atomic load to retrieve the usage count, which may well be reordered (by the CPU and/or compiler) in relation to these non-atomic loads. The same issue may well exist on the writer side, with the order between the global and the cache counts not being ordered. So while sanity checking the count without caching is not needed, with, you can argue it is. >> * due to race condition (access to len is not locked), the >> * total can be greater than size... so fix the result >> */ >> +exit: >> if (count > mp->size) >> return mp->size; >> return count;