Commit e3494a36 authored by Jim Harris's avatar Jim Harris Committed by Konrad Sztyber
Browse files

test/unit/bdev: remove bdev_multi_allocation test 



This test is not really useful. The intended purposes
I think were:

1) Validate that the height of the tree is never too
   high.
2) Capture the time to insert a bdev's names into the
   rb tree.

But #1 isn't being tested correctly.  It only tests
the height of a name that was *just* inserted.  RB
trees always insert a new node at the leaf, followed
by some rotations as necessary to maintain the RB
tree properties.

#2 doesn't help with anything.  The NOTICELOGs don't
get printed, and even if they did, we aren't validating
them at all.

So just remove this test altogether.  We don't need
to validate the RB tree code itself, and we shouldn't
try to validate performance with a unit test.  The way
the test is constructed is way too expensive too - it
takes up the vast majority of the time to run the
entire unit test suite. Removing just this one test cut
the time for unittest.sh from 28 seconds to less than
3 seconds on my system.

Fixes issue #3013.

Signed-off-by: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: If2253d4927ca5f1184fb165ed3f2c25befe9f4a4
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17980


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
parent f3e7a8f2

test/unit/lib/bdev/bdev.c/bdev_ut.c

+0 −67
Original line number Diff line number Diff line
@@ -5462,72 +5462,6 @@ bdev_set_options_test(void) 
	CU_ASSERT(rc == 0);

}



static uint64_t

get_ns_time(void)

{

	int rc;

	struct timespec ts;



	rc = clock_gettime(CLOCK_MONOTONIC, &ts);

	CU_ASSERT(rc == 0);

	return ts.tv_sec * 1000 * 1000 * 1000 + ts.tv_nsec;

}



static int

rb_tree_get_height(struct spdk_bdev_name *bdev_name)

{

	int h1, h2;



	if (bdev_name == NULL) {

		return -1;

	} else {

		h1 = rb_tree_get_height(RB_LEFT(bdev_name, node));

		h2 = rb_tree_get_height(RB_RIGHT(bdev_name, node));



		return spdk_max(h1, h2) + 1;

	}

}



static void

bdev_multi_allocation(void)

{

	const int max_bdev_num = 1024 * 16;

	char name[max_bdev_num][16];

	char noexist_name[] = "invalid_bdev";

	struct spdk_bdev *bdev[max_bdev_num];

	int i, j;

	uint64_t last_time;

	int bdev_num;

	int height;



	for (j = 0; j < max_bdev_num; j++) {

		snprintf(name[j], sizeof(name[j]), "bdev%d", j);

	}



	for (i = 0; i < 16; i++) {

		last_time = get_ns_time();

		bdev_num = 1024 * (i + 1);

		for (j = 0; j < bdev_num; j++) {

			bdev[j] = allocate_bdev(name[j]);

			height = rb_tree_get_height(&bdev[j]->internal.bdev_name);

			CU_ASSERT(height <= (int)(spdk_u32log2(2 * j + 2)));

		}

		SPDK_NOTICELOG("alloc bdev num %d takes %" PRIu64 " ms\n", bdev_num,

			       (get_ns_time() - last_time) / 1000 / 1000);

		for (j = 0; j < bdev_num; j++) {

			CU_ASSERT(spdk_bdev_get_by_name(name[j]) != NULL);

		}

		CU_ASSERT(spdk_bdev_get_by_name(noexist_name) == NULL);



		for (j = 0; j < bdev_num; j++) {

			free_bdev(bdev[j]);

		}

		for (j = 0; j < bdev_num; j++) {

			CU_ASSERT(spdk_bdev_get_by_name(name[j]) == NULL);

		}

	}

}



static struct spdk_memory_domain *g_bdev_memory_domain = (struct spdk_memory_domain *) 0xf00df00d;



static int
@@ -7237,7 +7171,6 @@ main(int argc, char **argv) 
	CU_ADD_TEST(suite, bdev_unmap);

	CU_ADD_TEST(suite, bdev_write_zeroes_split_test);

	CU_ADD_TEST(suite, bdev_set_options_test);

	CU_ADD_TEST(suite, bdev_multi_allocation);

	CU_ADD_TEST(suite, bdev_get_memory_domains);

	CU_ADD_TEST(suite, bdev_io_ext);

	CU_ADD_TEST(suite, bdev_io_ext_no_opts);