blobfs, tree: fix a bug in spdk_tree_insert_buffer

From the exising code, the comparison should + 1.
Suppose offset = 2^19, we still can be fit into
tree with level =0, since there will be 64 buffer,
each with size 2^18, so we do not need to
increse the tree level.

Also a unit test is added to demonstrate this.

Change-Id: I95d3542b0881aa7bb661bc57bc789cc4ef4e7509
Signed-off-by: Ziye Yang <optimistyzy@gmail.com>
Reviewed-on: https://review.gerrithub.io/372396
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
This commit is contained in:
Ziye Yang 2017-08-03 22:54:41 +08:00 committed by Daniel Verkamp
parent 58ce0711eb
commit 9ea96234d8
7 changed files with 261 additions and 2 deletions

View File

@ -82,7 +82,7 @@ spdk_tree_insert_buffer(struct cache_tree *root, struct cache_buffer *buffer)
uint64_t index, offset;
offset = buffer->offset;
while (offset >= CACHE_TREE_LEVEL_SIZE(root->level)) {
while (offset >= CACHE_TREE_LEVEL_SIZE(root->level + 1)) {
if (root->present_mask != 0) {
tree = calloc(1, sizeof(*tree));
tree->level = root->level + 1;

View File

@ -34,7 +34,7 @@
SPDK_ROOT_DIR := $(abspath $(CURDIR)/../../..)
include $(SPDK_ROOT_DIR)/mk/spdk.common.mk
DIRS-y = bdev blob ioat iscsi json jsonrpc log nvme nvmf scsi util
DIRS-y = bdev blob blobfs ioat iscsi json jsonrpc log nvme nvmf scsi util
ifeq ($(OS),Linux)
DIRS-$(CONFIG_VHOST) += vhost
endif

View File

@ -0,0 +1,44 @@
#
# BSD LICENSE
#
# Copyright (c) Intel Corporation.
# All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions
# are met:
#
# * Redistributions of source code must retain the above copyright
# notice, this list of conditions and the following disclaimer.
# * Redistributions in binary form must reproduce the above copyright
# notice, this list of conditions and the following disclaimer in
# the documentation and/or other materials provided with the
# distribution.
# * Neither the name of Intel Corporation nor the names of its
# contributors may be used to endorse or promote products derived
# from this software without specific prior written permission.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#
SPDK_ROOT_DIR := $(abspath $(CURDIR)/../../../..)
include $(SPDK_ROOT_DIR)/mk/spdk.common.mk
DIRS-y = tree.c
.PHONY: all clean $(DIRS-y)
all: $(DIRS-y)
clean: $(DIRS-y)
include $(SPDK_ROOT_DIR)/mk/spdk.subdirs.mk

View File

@ -0,0 +1 @@
tree_ut

View File

@ -0,0 +1,53 @@
#
# BSD LICENSE
#
# Copyright (c) Intel Corporation.
# All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions
# are met:
#
# * Redistributions of source code must retain the above copyright
# notice, this list of conditions and the following disclaimer.
# * Redistributions in binary form must reproduce the above copyright
# notice, this list of conditions and the following disclaimer in
# the documentation and/or other materials provided with the
# distribution.
# * Neither the name of Intel Corporation nor the names of its
# contributors may be used to endorse or promote products derived
# from this software without specific prior written permission.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#
SPDK_ROOT_DIR := $(abspath $(CURDIR)/../../../../..)
include $(SPDK_ROOT_DIR)/mk/spdk.common.mk
include $(SPDK_ROOT_DIR)/mk/spdk.app.mk
APP = tree_ut
C_SRCS := tree_ut.c
CFLAGS += -I$(SPDK_ROOT_DIR)/lib/blobfs -I$(SPDK_ROOT_DIR)/test
LIBS += $(SPDK_LIB_LINKER_ARGS) -lcunit
all : $(APP)
$(APP) : $(OBJS) $(SPDK_LIB_FILES)
$(LINK_C)
clean :
$(CLEAN_C) $(APP)
include $(SPDK_ROOT_DIR)/mk/spdk.deps.mk

View File

@ -0,0 +1,160 @@
/*-
* BSD LICENSE
*
* Copyright (c) Intel Corporation.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
*
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in
* the documentation and/or other materials provided with the
* distribution.
* * Neither the name of Intel Corporation nor the names of its
* contributors may be used to endorse or promote products derived
* from this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include "spdk_cunit.h"
#include "tree.h"
#include "tree.c"
void
spdk_cache_buffer_free(struct cache_buffer *cache_buffer)
{
free(cache_buffer);
}
static void
blobfs_tree_op_test(void)
{
struct cache_tree *tree;
struct cache_buffer *buffer[5];
struct cache_buffer *tmp_buffer;
int i;
for (i = 0; i < 5; i ++) {
buffer[i] = calloc(1, sizeof(struct cache_buffer));
SPDK_CU_ASSERT_FATAL(buffer[i]);
}
tree = calloc(1, sizeof(*tree));
SPDK_CU_ASSERT_FATAL(tree != NULL);
/* insert buffer[0] */
buffer[0]->offset = 0;
tree = spdk_tree_insert_buffer(tree, buffer[0]);
SPDK_CU_ASSERT_FATAL(tree != NULL);
CU_ASSERT(tree->level == 0);
tmp_buffer = spdk_tree_find_buffer(tree, buffer[0]->offset);
CU_ASSERT(tmp_buffer == buffer[0]);
/* insert buffer[1] */
buffer[1]->offset = CACHE_BUFFER_SIZE;
/* set the bytes_filled equal = bytes_filled with same non zero value, e.g., 32 */
buffer[1]->bytes_filled = buffer[1]->bytes_flushed = 32;
tree = spdk_tree_insert_buffer(tree, buffer[1]);
SPDK_CU_ASSERT_FATAL(tree != NULL);
CU_ASSERT(tree->level == 0);
tmp_buffer = spdk_tree_find_filled_buffer(tree, buffer[1]->offset);
CU_ASSERT(tmp_buffer == buffer[1]);
/* insert buffer[2] */
buffer[2]->offset = (CACHE_TREE_WIDTH - 1) * CACHE_BUFFER_SIZE;
tree = spdk_tree_insert_buffer(tree, buffer[2]);
SPDK_CU_ASSERT_FATAL(tree != NULL);
CU_ASSERT(tree->level == 0);
tmp_buffer = spdk_tree_find_buffer(tree, buffer[2]->offset);
CU_ASSERT(tmp_buffer == buffer[2]);
tmp_buffer = spdk_tree_find_filled_buffer(tree, buffer[2]->offset);
CU_ASSERT(tmp_buffer == NULL);
/* insert buffer[3], set an offset which can not be fit level 0 */
buffer[3]->offset = CACHE_TREE_LEVEL_SIZE(1);
tree = spdk_tree_insert_buffer(tree, buffer[3]);
SPDK_CU_ASSERT_FATAL(tree != NULL);
CU_ASSERT(tree->level == 1);
tmp_buffer = spdk_tree_find_buffer(tree, buffer[3]->offset);
CU_ASSERT(tmp_buffer == buffer[3]);
/* insert buffer[4], set an offset which can not be fit level 1 */
buffer[4]->offset = CACHE_TREE_LEVEL_SIZE(2);
tree = spdk_tree_insert_buffer(tree, buffer[4]);
SPDK_CU_ASSERT_FATAL(tree != NULL);
CU_ASSERT(tree->level == 2);
tmp_buffer = spdk_tree_find_buffer(tree, buffer[4]->offset);
CU_ASSERT(tmp_buffer == buffer[4]);
/* delete buffer[0] */
spdk_tree_remove_buffer(tree, buffer[0]);
/* check whether buffer[0] is still existed or not */
tmp_buffer = spdk_tree_find_buffer(tree, 0);
CU_ASSERT(tmp_buffer == NULL);
/* delete buffer[3] */
spdk_tree_remove_buffer(tree, buffer[3]);
/* check whether buffer[3] is still existed or not */
tmp_buffer = spdk_tree_find_buffer(tree, CACHE_TREE_LEVEL_SIZE(1));
CU_ASSERT(tmp_buffer == NULL);
/* free all buffers in the tree */
spdk_tree_free_buffers(tree);
/* check whether buffer[1] is still existed or not */
tmp_buffer = spdk_tree_find_buffer(tree, CACHE_BUFFER_SIZE);
CU_ASSERT(tmp_buffer == NULL);
/* check whether buffer[2] is still existed or not */
tmp_buffer = spdk_tree_find_buffer(tree, (CACHE_TREE_WIDTH - 1) * CACHE_BUFFER_SIZE);
CU_ASSERT(tmp_buffer == NULL);
/* check whether buffer[4] is still existed or not */
tmp_buffer = spdk_tree_find_buffer(tree, CACHE_TREE_LEVEL_SIZE(2));
CU_ASSERT(tmp_buffer == NULL);
/* According to spdk_tree_free_buffers, root will not be freed */
free(tree);
}
int main(int argc, char **argv)
{
CU_pSuite suite = NULL;
unsigned int num_failures;
if (CU_initialize_registry() != CUE_SUCCESS) {
return CU_get_error();
}
suite = CU_add_suite("tree", NULL, NULL);
if (suite == NULL) {
CU_cleanup_registry();
return CU_get_error();
}
if (CU_add_test(suite, "blobfs_tree_op_test", blobfs_tree_op_test) == NULL) {
CU_cleanup_registry();
return CU_get_error();
}
CU_basic_set_mode(CU_BRM_VERBOSE);
CU_basic_run_tests();
num_failures = CU_get_number_of_failures();
CU_cleanup_registry();
return num_failures;
}

View File

@ -46,6 +46,7 @@ $valgrind test/unit/lib/bdev/scsi_nvme.c/scsi_nvme_ut
$valgrind test/unit/lib/bdev/gpt/gpt.c/gpt_ut
$valgrind test/unit/lib/blob/blob.c/blob_ut
$valgrind test/unit/lib/blobfs/tree.c/tree_ut
$valgrind test/lib/blobfs/blobfs_async_ut/blobfs_async_ut
# blobfs_sync_ut hangs when run under valgrind, so don't use $valgrind