From 6c1c5d9ef0be6eca1861f01f2d015f04b6e229bc Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Tue, 10 Jan 2023 01:10:48 +0300 Subject: [PATCH] util/disk_cache: Fix rw cache lookup when using combined ro+rw caches When combining of ro+rw caches is enabled, at first the ro cache should be looked up and if data isn't found there then rw cache should be checked. The rw cache checking got lost by accident after the code rebase and there was no unit test covering this condition. Fix the rw cache looking up and add the unit test case. Fixes: 32fe60e8c429 ("util/disk_cache: Support combined foz ro and non-foz rw caches") Reviewed-by: Timothy Arceri Reviewed-by: Juston Li Signed-off-by: Dmitry Osipenko Part-of: --- src/util/disk_cache.c | 29 +++++++++--------- src/util/tests/cache_test.cpp | 55 +++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c index 2280f53ac97..8382efc360a 100644 --- a/src/util/disk_cache.c +++ b/src/util/disk_cache.c @@ -578,25 +578,26 @@ disk_cache_put_nocopy(struct disk_cache *cache, const cache_key key, void * disk_cache_get(struct disk_cache *cache, const cache_key key, size_t *size) { - void *buf; + void *buf = NULL; if (size) *size = 0; - if (cache->blob_get_cb) { - buf = blob_get_compressed(cache, key, size); - } else if (cache->foz_ro_cache) { + if (cache->foz_ro_cache) buf = disk_cache_load_item_foz(cache->foz_ro_cache, key, size); - } else if (cache->type == DISK_CACHE_SINGLE_FILE) { - buf = disk_cache_load_item_foz(cache, key, size); - } else if (cache->type == DISK_CACHE_DATABASE) { - buf = disk_cache_db_load_item(cache, key, size); - } else { - char *filename = disk_cache_get_cache_filename(cache, key); - if (filename == NULL) - buf = NULL; - else - buf = disk_cache_load_item(cache, filename, size); + + if (!buf) { + if (cache->blob_get_cb) { + buf = blob_get_compressed(cache, key, size); + } else if (cache->type == DISK_CACHE_SINGLE_FILE) { + buf = disk_cache_load_item_foz(cache, key, size); + } else if (cache->type == DISK_CACHE_DATABASE) { + buf = disk_cache_db_load_item(cache, key, size); + } else { + char *filename = disk_cache_get_cache_filename(cache, key); + if (filename) + buf = disk_cache_load_item(cache, filename, size); + } } if (unlikely(cache->stats.enabled)) { diff --git a/src/util/tests/cache_test.cpp b/src/util/tests/cache_test.cpp index 5d0e7d9b293..60e8627c754 100644 --- a/src/util/tests/cache_test.cpp +++ b/src/util/tests/cache_test.cpp @@ -778,8 +778,10 @@ TEST_F(Cache, Combined) { const char *driver_id = "make_check"; char blob[] = "This is a RO blob"; + char blob2[] = "This is a RW blob"; uint8_t dummy_key[20] = { 0 }; uint8_t blob_key[20]; + uint8_t blob_key2[20]; char foz_rw_idx_file[1024]; char foz_ro_idx_file[1024]; char foz_rw_file[1024]; @@ -807,6 +809,7 @@ TEST_F(Cache, Combined) driver_id, 0); disk_cache_compute_key(cache_sf_wr, blob, sizeof(blob), blob_key); + disk_cache_compute_key(cache_sf_wr, blob2, sizeof(blob2), blob_key2); /* Ensure that disk_cache_get returns nothing before anything is added. */ result = (char *) disk_cache_get(cache_sf_wr, blob_key, &size); @@ -882,11 +885,40 @@ TEST_F(Cache, Combined) EXPECT_EQ(size, sizeof(blob)) << "disk_cache_get of existing item (size)"; free(result); + /* Blob2 entry must not present in any of the caches. */ + result = (char *) disk_cache_get(cache_mesa_db, blob_key2, &size); + EXPECT_EQ(result, nullptr) << "disk_cache_get with non-existent item (pointer)"; + EXPECT_EQ(size, 0) << "disk_cache_get with non-existent item (size)"; + + /* Put blob2 entry to the cache. */ + disk_cache_put(cache_mesa_db, blob_key2, blob2, sizeof(blob2), NULL); + disk_cache_wait_for_idle(cache_mesa_db); + + /* Blob2 entry must present because it shall be retrieved from the + * read-write cache. */ + result = (char *) disk_cache_get(cache_mesa_db, blob_key2, &size); + EXPECT_STREQ(blob2, result) << "disk_cache_get of existing item (pointer)"; + EXPECT_EQ(size, sizeof(blob2)) << "disk_cache_get of existing item (size)"; + free(result); + disk_cache_destroy(cache_mesa_db); /* Disable read-only cache. */ setenv("MESA_DISK_CACHE_COMBINE_RW_WITH_RO_FOZ", "false", 1); + /* Create MESA-DB cache with disabled retrieval from the + * read-only cache. */ + cache_mesa_db = disk_cache_create("combined_test", driver_id, 0); + + /* Blob2 entry must present because it shall be retrieved from the + * MESA-DB cache. */ + result = (char *) disk_cache_get(cache_mesa_db, blob_key2, &size); + EXPECT_STREQ(blob2, result) << "disk_cache_get of existing item (pointer)"; + EXPECT_EQ(size, sizeof(blob2)) << "disk_cache_get of existing item (size)"; + free(result); + + disk_cache_destroy(cache_mesa_db); + /* Create MESA-DB cache with disabled retrieval from the read-only * cache. */ cache_mesa_db = disk_cache_create("combined_test", driver_id, 0); @@ -917,6 +949,22 @@ TEST_F(Cache, Combined) EXPECT_EQ(size, sizeof(blob)) << "disk_cache_get of existing item (size)"; free(result); + /* Blob2 entry must not present in any of the caches. */ + result = (char *) disk_cache_get(cache_multifile, blob_key2, &size); + EXPECT_EQ(result, nullptr) << "disk_cache_get with non-existent item (pointer)"; + EXPECT_EQ(size, 0) << "disk_cache_get with non-existent item (size)"; + + /* Put blob2 entry to the cache. */ + disk_cache_put(cache_multifile, blob_key2, blob2, sizeof(blob2), NULL); + disk_cache_wait_for_idle(cache_multifile); + + /* Blob2 entry must present because it shall be retrieved from the + * read-write cache. */ + result = (char *) disk_cache_get(cache_multifile, blob_key2, &size); + EXPECT_STREQ(blob2, result) << "disk_cache_get of existing item (pointer)"; + EXPECT_EQ(size, sizeof(blob2)) << "disk_cache_get of existing item (size)"; + free(result); + disk_cache_destroy(cache_multifile); /* Disable read-only cache. */ @@ -933,6 +981,13 @@ TEST_F(Cache, Combined) EXPECT_EQ(result, nullptr) << "disk_cache_get with non-existent item (pointer)"; EXPECT_EQ(size, 0) << "disk_cache_get with non-existent item (size)"; + /* Blob2 entry must present because it shall be retrieved from the + * read-write cache. */ + result = (char *) disk_cache_get(cache_multifile, blob_key2, &size); + EXPECT_STREQ(blob2, result) << "disk_cache_get of existing item (pointer)"; + EXPECT_EQ(size, sizeof(blob2)) << "disk_cache_get of existing item (size)"; + free(result); + disk_cache_destroy(cache_multifile); int err = rmrf_local(CACHE_TEST_TMP);