From beaf69617ab4166c5b40ed83c549d15148d5386f Mon Sep 17 00:00:00 2001 From: Changpeng Liu Date: Tue, 18 Feb 2020 21:42:29 -0500 Subject: [PATCH] blobfs: try to free a valid file cache buffer when allocating new caches When allocating a new cache buffer, we will pick up a file and try to free cache buffers with this file, but sometimes the file lock maybe hold by other thread, so we use pthread_spin_trylock() here, when pthread_spin_trylock() return a negative value, we will break the loop, this is not efficient as the orginal logic, which may result cache buffer allocation failure, so we will break the loop only when there is a valid cache free. Change-Id: I7be2fddb33c38dba466c2fcc2584ccf7ac16ea7a Signed-off-by: Changpeng Liu Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/950 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto --- lib/blobfs/blobfs.c | 60 +++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/lib/blobfs/blobfs.c b/lib/blobfs/blobfs.c index 366665c66..28a9d4349 100644 --- a/lib/blobfs/blobfs.c +++ b/lib/blobfs/blobfs.c @@ -2019,7 +2019,7 @@ static void __file_flush(void *ctx); /* Try to free some cache buffers of this file, this function must * be called while holding g_caches_lock. */ -static void +static int reclaim_cache_buffers(struct spdk_file *file) { int rc; @@ -2032,12 +2032,12 @@ reclaim_cache_buffers(struct spdk_file *file) */ rc = pthread_spin_trylock(&file->lock); if (rc != 0) { - return; + return -1; } if (file->tree->present_mask == 0) { pthread_spin_unlock(&file->lock); - return; + return -1; } spdk_tree_free_buffers(file->tree); @@ -2049,6 +2049,8 @@ reclaim_cache_buffers(struct spdk_file *file) file->last = NULL; } pthread_spin_unlock(&file->lock); + + return 0; } static void * @@ -2056,6 +2058,7 @@ alloc_cache_memory_buffer(struct spdk_file *context) { struct spdk_file *file, *tmp; void *buf; + int rc; buf = spdk_mempool_get(g_cache_pool); if (buf != NULL) { @@ -2067,39 +2070,48 @@ alloc_cache_memory_buffer(struct spdk_file *context) if (!file->open_for_writing && file->priority == SPDK_FILE_PRIORITY_LOW && file != context) { - reclaim_cache_buffers(file); + rc = reclaim_cache_buffers(file); + if (rc < 0) { + continue; + } + buf = spdk_mempool_get(g_cache_pool); + if (buf != NULL) { + pthread_spin_unlock(&g_caches_lock); + return buf; + } + break; } - buf = spdk_mempool_get(g_cache_pool); - if (buf != NULL) { - pthread_spin_unlock(&g_caches_lock); - return buf; - } - break; } TAILQ_FOREACH_SAFE(file, &g_caches, cache_tailq, tmp) { if (!file->open_for_writing && file != context) { - reclaim_cache_buffers(file); + rc = reclaim_cache_buffers(file); + if (rc < 0) { + continue; + } + buf = spdk_mempool_get(g_cache_pool); + if (buf != NULL) { + pthread_spin_unlock(&g_caches_lock); + return buf; + } + break; } - buf = spdk_mempool_get(g_cache_pool); - if (buf != NULL) { - pthread_spin_unlock(&g_caches_lock); - return buf; - } - break; } TAILQ_FOREACH_SAFE(file, &g_caches, cache_tailq, tmp) { if (file != context) { - reclaim_cache_buffers(file); + rc = reclaim_cache_buffers(file); + if (rc < 0) { + continue; + } + buf = spdk_mempool_get(g_cache_pool); + if (buf != NULL) { + pthread_spin_unlock(&g_caches_lock); + return buf; + } + break; } - buf = spdk_mempool_get(g_cache_pool); - if (buf != NULL) { - pthread_spin_unlock(&g_caches_lock); - return buf; - } - break; } pthread_spin_unlock(&g_caches_lock);