Commit d148fba3 authored by Mateusz Kozlowski's avatar Mateusz Kozlowski Committed by Tomasz Zawadzki
Browse files

lib/blob: Handling of concurrent unmaps to a cluster 



Only release clusters for the first unmap.

Fixes #3358

Change-Id: I79810a6611c57df1d0c280616d5666d2f7487d33
Signed-off-by: default avatarMateusz Kozlowski <mateusz.kozlowski@solidigm.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/23014


Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
parent 1826c4dc

lib/blob/blobstore.c

+7 −0
Original line number Diff line number Diff line
@@ -8478,6 +8478,13 @@ blob_free_cluster_msg(void *arg) 
	size_t i;



	ctx->cluster = ctx->blob->active.clusters[ctx->cluster_num];



	/* There were concurrent unmaps to the same cluster, only release the cluster on the first one */

	if (ctx->cluster == 0) {

		blob_op_cluster_msg_cb(ctx, 0);

		return;

	}



	ctx->blob->active.clusters[ctx->cluster_num] = 0;

	if (ctx->cluster != 0) {

		ctx->blob->active.num_allocated_clusters--;

test/unit/lib/blob/blob.c/blob_ut.c

+21 −0
Original line number Diff line number Diff line
@@ -4678,6 +4678,7 @@ blob_thin_prov_unmap_cluster(void) 
	const uint32_t CLUSTER_COUNT = 3;

	uint32_t pages_per_cluster;

	uint32_t i;

	int err;



	/* Use a very large cluster size for this test. Check how the unmap/release cluster code path behaves when

	 * clusters are fully used.
@@ -4815,6 +4816,26 @@ blob_thin_prov_unmap_cluster(void) 
	CU_ASSERT(g_bserrno == 0);

	CU_ASSERT(1 == spdk_bs_free_cluster_count(bs));



	/* Issue #3358 had a bug with concurrent trims to the same cluster causing an assert, check for regressions.

	 * Send three concurrent unmaps to the same cluster.

	 */

	g_bserrno = -1;

	memset(payload_write, 7, sizeof(payload_write));

	spdk_blob_io_write(blob, ch, payload_write, 0, 1, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	CU_ASSERT(0 == spdk_bs_free_cluster_count(bs));



	g_bserrno = -1;

	err = -1;

	spdk_blob_io_unmap(blob, ch, 0, pages_per_cluster, blob_op_complete, NULL);

	spdk_blob_io_unmap(blob, ch, 0, pages_per_cluster, blob_op_complete, NULL);

	spdk_blob_io_unmap(blob, ch, 0, pages_per_cluster, blob_op_complete, &err);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	CU_ASSERT(err == 0);

	CU_ASSERT(1 == spdk_bs_free_cluster_count(bs));



	ut_blob_close_and_delete(bs, blob);

	CU_ASSERT(free_clusters == spdk_bs_free_cluster_count(bs));