From 7f139e549b3d20f69af0eb37192cc9fecded4dc8 Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Thu, 17 Sep 2020 12:58:45 -0400 Subject: [PATCH] lib/blob: extent_page shall never occupy md page 0 Search for md_page to be used as extent page started from 0, which is completely valid md_page. This page can be free when for example blob with id 0 was deleted and some other requested a new page for extent. There are already existing blobs that have extents pointing to 0, which means unallocated. Unfortunetly it means 0 can never mean md page 0. If that already occured for someone, this extent page was already lost during blob/bs reload and nothing can be done. With this in mind following assumptions are made for extent pages: - 0 means unallocated extent page - UINT32_MAX means we ran out of md pages, and should not be persisted - [NEW] extent page can never occupy md page 0 That last one is new addition in this patch. bs_allocate_cluster will now always try to find md page from 1 or higher. Signed-off-by: Tomasz Zawadzki Change-Id: Ia17ce5bbca2fab4fb4487e4e263f3a0aa120bf17 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/4314 Tested-by: SPDK CI Jenkins Community-CI: Mellanox Build Bot Reviewed-by: Ben Walker Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto --- lib/blob/blobstore.c | 4 ++++ test/unit/lib/blob/blob.c/blob_ut.c | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index 9c063fc1b..1bd2843fa 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -165,6 +165,10 @@ bs_allocate_cluster(struct spdk_blob *blob, uint32_t cluster_num, if (blob->use_extent_table) { extent_page = bs_cluster_to_extent_page(blob, cluster_num); if (*extent_page == 0) { + /* Extent page shall never occupy md_page so start the search from 1 */ + if (*lowest_free_md_page == 0) { + *lowest_free_md_page = 1; + } /* No extent_page is allocated for the cluster */ *lowest_free_md_page = spdk_bit_array_find_first_clear(blob->bs->used_md_pages, *lowest_free_md_page); diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index dfcf2a3c2..95c1a5453 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c +++ b/test/unit/lib/blob/blob.c/blob_ut.c @@ -3769,7 +3769,7 @@ blob_thin_prov_rw(void) { static const uint8_t zero[10 * 4096] = { 0 }; struct spdk_blob_store *bs = g_bs; - struct spdk_blob *blob; + struct spdk_blob *blob, *blob_id0; struct spdk_io_channel *channel, *channel_thread1; struct spdk_blob_opts opts; uint64_t free_clusters; @@ -3788,7 +3788,11 @@ blob_thin_prov_rw(void) ut_spdk_blob_opts_init(&opts); opts.thin_provision = true; + /* Create and delete blob at md page 0, so that next md page allocation + * for extent will use that. */ + blob_id0 = ut_blob_create_and_open(bs, &opts); blob = ut_blob_create_and_open(bs, &opts); + ut_blob_close_and_delete(bs, blob_id0); CU_ASSERT(free_clusters == spdk_bs_free_cluster_count(bs)); CU_ASSERT(blob->active.num_clusters == 0);