Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/117747/?format=api
{ "id": 117747, "url": "https://patches.dpdk.org/api/patches/117747/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/patch/20221009133737.795377-4-andrew.rybchenko@oktetlabs.ru/", "project": { "id": 1, "url": "https://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<20221009133737.795377-4-andrew.rybchenko@oktetlabs.ru>", "list_archive_url": "https://inbox.dpdk.org/dev/20221009133737.795377-4-andrew.rybchenko@oktetlabs.ru", "date": "2022-10-09T13:37:36", "name": "[v6,3/4] mempool: fix cache flushing algorithm", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "de42786b0cce9d5ff76911f67dbaf6c870300919", "submitter": { "id": 2013, "url": "https://patches.dpdk.org/api/people/2013/?format=api", "name": "Andrew Rybchenko", "email": "Andrew.Rybchenko@oktetlabs.ru" }, "delegate": { "id": 1, "url": "https://patches.dpdk.org/api/users/1/?format=api", "username": "tmonjalo", "first_name": "Thomas", "last_name": "Monjalon", "email": "thomas@monjalon.net" }, "mbox": "https://patches.dpdk.org/project/dpdk/patch/20221009133737.795377-4-andrew.rybchenko@oktetlabs.ru/mbox/", "series": [ { "id": 25063, "url": "https://patches.dpdk.org/api/series/25063/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/list/?series=25063", "date": "2022-10-09T13:37:34", "name": "mempool: fix mempool cache flushing algorithm", "version": 6, "mbox": "https://patches.dpdk.org/series/25063/mbox/" } ], "comments": "https://patches.dpdk.org/api/patches/117747/comments/", "check": "success", "checks": "https://patches.dpdk.org/api/patches/117747/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from mails.dpdk.org (mails.dpdk.org [217.70.189.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id 04F45A0542;\n\tSun, 9 Oct 2022 15:38:00 +0200 (CEST)", "from [217.70.189.124] (localhost [127.0.0.1])\n\tby mails.dpdk.org (Postfix) with ESMTP id D399D42802;\n\tSun, 9 Oct 2022 15:37:47 +0200 (CEST)", "from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113])\n by mails.dpdk.org (Postfix) with ESMTP id E20E040042\n for <dev@dpdk.org>; Sun, 9 Oct 2022 15:37:44 +0200 (CEST)", "by shelob.oktetlabs.ru (Postfix, from userid 115)\n id 7F39098; Sun, 9 Oct 2022 16:37:44 +0300 (MSK)", "from aros.oktetlabs.ru (aros.oktetlabs.ru [192.168.38.17])\n by shelob.oktetlabs.ru (Postfix) with ESMTP id B850592;\n Sun, 9 Oct 2022 16:37:42 +0300 (MSK)" ], "X-Spam-Checker-Version": "SpamAssassin 3.4.6 (2021-04-09) on mail1.oktetlabs.ru", "X-Spam-Level": "", "X-Spam-Status": "No, score=0.8 required=5.0 tests=ALL_TRUSTED, DKIM_ADSP_DISCARD\n autolearn=no autolearn_force=no version=3.4.6", "DKIM-Filter": "OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru B850592", "Authentication-Results": "shelob.oktetlabs.ru/B850592; dkim=none;\n dkim-atps=neutral", "From": "Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>", "To": "Olivier Matz <olivier.matz@6wind.com>", "Cc": "dev@dpdk.org, =?utf-8?q?Morten_Br=C3=B8rup?= <mb@smartsharesystems.com>,\n Bruce Richardson <bruce.richardson@intel.com>", "Subject": "[PATCH v6 3/4] mempool: fix cache flushing algorithm", "Date": "Sun, 9 Oct 2022 16:37:36 +0300", "Message-Id": "<20221009133737.795377-4-andrew.rybchenko@oktetlabs.ru>", "X-Mailer": "git-send-email 2.30.2", "In-Reply-To": "<20221009133737.795377-1-andrew.rybchenko@oktetlabs.ru>", "References": "<98CBD80474FA8B44BF855DF32C47DC35D86DB2@smartserver.smartshare.dk>\n <20221009133737.795377-1-andrew.rybchenko@oktetlabs.ru>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org" }, "content": "From: Morten Brørup <mb@smartsharesystems.com>\n\nFix the rte_mempool_do_generic_put() caching flushing algorithm to\nkeep hot objects in cache instead of cold ones.\n\nThe algorithm was:\n 1. Add the objects to the cache.\n 2. Anything greater than the cache size (if it crosses the cache flush\n threshold) is flushed to the backend.\n\nPlease note that the description in the source code said that it kept\n\"cache min value\" objects after flushing, but the function actually kept\nthe cache full after flushing, which the above description reflects.\n\nNow, the algorithm is:\n 1. If the objects cannot be added to the cache without crossing the\n flush threshold, flush some cached objects to the backend to\n free up required space.\n 2. Add the objects to the cache.\n\nThe most recent (hot) objects were flushed, leaving the oldest (cold)\nobjects in the mempool cache. The bug degraded performance, because\nflushing prevented immediate reuse of the (hot) objects already in\nthe CPU cache. Now, the existing (cold) objects in the mempool cache\nare flushed before the new (hot) objects are added the to the mempool\ncache.\n\nSince nearby code is touched anyway fix flush threshold comparison\nto do flushing if the threshold is really exceed, not just reached.\nI.e. it must be \"len > flushthresh\", not \"len >= flushthresh\".\nConsider a flush multiplier of 1 instead of 1.5; the cache would be\nflushed already when reaching size objects, not when exceeding size\nobjects. In other words, the cache would not be able to hold \"size\"\nobjects, which is clearly a bug. The bug could degraded performance\ndue to premature flushing.\n\nSince we never exceed flush threshold now, cache size in the mempool\nmay be decreased from RTE_MEMPOOL_CACHE_MAX_SIZE * 3 to\nRTE_MEMPOOL_CACHE_MAX_SIZE * 2. In fact it could be\nCALC_CACHE_FLUSHTHRESH(RTE_MEMPOOL_CACHE_MAX_SIZE), but flush\nthreshold multiplier is internal.\n\nSigned-off-by: Morten Brørup <mb@smartsharesystems.com>\nSigned-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>\n---\n lib/mempool/rte_mempool.c | 5 +++++\n lib/mempool/rte_mempool.h | 43 +++++++++++++++++++++++----------------\n 2 files changed, 31 insertions(+), 17 deletions(-)", "diff": "diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c\nindex de59009baf..4ba8ab7b63 100644\n--- a/lib/mempool/rte_mempool.c\n+++ b/lib/mempool/rte_mempool.c\n@@ -746,6 +746,11 @@ rte_mempool_free(struct rte_mempool *mp)\n static void\n mempool_cache_init(struct rte_mempool_cache *cache, uint32_t size)\n {\n+\t/* Check that cache have enough space for flush threshold */\n+\tRTE_BUILD_BUG_ON(CALC_CACHE_FLUSHTHRESH(RTE_MEMPOOL_CACHE_MAX_SIZE) >\n+\t\t\t RTE_SIZEOF_FIELD(struct rte_mempool_cache, objs) /\n+\t\t\t RTE_SIZEOF_FIELD(struct rte_mempool_cache, objs[0]));\n+\n \tcache->size = size;\n \tcache->flushthresh = CALC_CACHE_FLUSHTHRESH(size);\n \tcache->len = 0;\ndiff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h\nindex a072e5554b..e3364ed7b8 100644\n--- a/lib/mempool/rte_mempool.h\n+++ b/lib/mempool/rte_mempool.h\n@@ -90,7 +90,7 @@ struct rte_mempool_cache {\n \t * Cache is allocated to this size to allow it to overflow in certain\n \t * cases to avoid needless emptying of cache.\n \t */\n-\tvoid *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 3]; /**< Cache objects */\n+\tvoid *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 2]; /**< Cache objects */\n } __rte_cache_aligned;\n \n /**\n@@ -1329,30 +1329,39 @@ rte_mempool_do_generic_put(struct rte_mempool *mp, void * const *obj_table,\n \tRTE_MEMPOOL_STAT_ADD(mp, put_bulk, 1);\n \tRTE_MEMPOOL_STAT_ADD(mp, put_objs, n);\n \n-\t/* No cache provided or if put would overflow mem allocated for cache */\n-\tif (unlikely(cache == NULL || n > RTE_MEMPOOL_CACHE_MAX_SIZE))\n+\t/* No cache provided or the request itself is too big for the cache */\n+\tif (unlikely(cache == NULL || n > cache->flushthresh))\n \t\tgoto driver_enqueue;\n \n-\tcache_objs = &cache->objs[cache->len];\n-\n \t/*\n-\t * The cache follows the following algorithm\n-\t * 1. Add the objects to the cache\n-\t * 2. Anything greater than the cache min value (if it crosses the\n-\t * cache flush threshold) is flushed to the backend.\n+\t * The cache follows the following algorithm:\n+\t * 1. If the objects cannot be added to the cache without crossing\n+\t * the flush threshold, flush the cache to the backend.\n+\t * 2. Add the objects to the cache.\n \t */\n \n-\t/* Add elements back into the cache */\n-\trte_memcpy(&cache_objs[0], obj_table, sizeof(void *) * n);\n-\n-\tcache->len += n;\n+\tif (cache->len + n <= cache->flushthresh) {\n+\t\tcache_objs = &cache->objs[cache->len];\n+\t\tcache->len += n;\n+\t} else {\n+\t\tunsigned int keep = (n >= cache->size) ? 0 : (cache->size - n);\n \n-\tif (cache->len >= cache->flushthresh) {\n-\t\trte_mempool_ops_enqueue_bulk(mp, &cache->objs[cache->size],\n-\t\t\t\tcache->len - cache->size);\n-\t\tcache->len = cache->size;\n+\t\t/*\n+\t\t * If number of object to keep in the cache is positive:\n+\t\t * keep = cache->size - n < cache->flushthresh - n < cache->len\n+\t\t * since cache->flushthresh > cache->size.\n+\t\t * If keep is 0, cache->len cannot be 0 anyway since\n+\t\t * n <= cache->flushthresh and we'd no be here with\n+\t\t * cache->len == 0.\n+\t\t */\n+\t\tcache_objs = &cache->objs[keep];\n+\t\trte_mempool_ops_enqueue_bulk(mp, cache_objs, cache->len - keep);\n+\t\tcache->len = keep + n;\n \t}\n \n+\t/* Add the objects to the cache. */\n+\trte_memcpy(cache_objs, obj_table, sizeof(void *) * n);\n+\n \treturn;\n \n driver_enqueue:\n", "prefixes": [ "v6", "3/4" ] }