Commit 5d505337 authored Jan 27, 2020 by Tomasz Zawadzki

lib/blob: fix inserting extent pages



ctx->extent_page signifies if page was allocated
for insertion.

1) It is possible for a thread to claim extent page
on its own thread, and put it in ctx->extent_page.
If conflicting thread allocates another ctx->extent_page,
then it should be freed. This does not mean failure
to insert cluster. As different threads could have
been trying to allocate different clusters,
so condition on line 6716 does not cover it.
If so then it shouldn't be an issue to release
the claimed ctx->extent_page and proceed with updating the
extent page which originally won the race.
NOTE: if clusters were conflicting, then extent_page is
freed in _spdk_blob_insert_cluster_cpl().

2) At this point of _spdk_blob_insert_cluster_msg()
we already verified that there already is
extent page allocated at "*extent_page".
In such case ctx->extent_page will be 0,
and should not be used.

Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: Id5b57c88248890eee60d2e7dbecbd984c98b561b
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/482867


Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>

parent 1dd8c57a

lib/blob/blobstore.c

+8 −2

Original line number	Diff line number	Diff line
		@@ -6737,10 +6737,16 @@ _spdk_blob_insert_cluster_msg(void *arg)
		ctx->blob->state = SPDK_BLOB_STATE_DIRTY;
		_spdk_blob_sync_md(ctx->blob, _spdk_blob_insert_cluster_msg_cb, ctx);
		} else {
		assert(ctx->extent_page == 0);
		/* It is possible for original thread to allocate extent page for
		* different cluster in the same extent page. In such case proceed with
		* updating the existing extent page, but release the additional one. */
		if (ctx->extent_page != 0) {
		assert(spdk_bit_array_get(ctx->blob->bs->used_md_pages, ctx->extent_page) == true);
		_spdk_bs_release_md_page(ctx->blob->bs, ctx->extent_page);
		}
		/* Extent page already allocated.
		* Every cluster allocation, requires just an update of single extent page. */
		_spdk_blob_insert_extent(ctx->blob, ctx->extent_page, ctx->cluster_num,
		_spdk_blob_insert_extent(ctx->blob, *extent_page, ctx->cluster_num,
		_spdk_blob_insert_cluster_msg_cb, ctx);
		}
		}