Commit 3327bb43 authored by Jim Harris's avatar Jim Harris Committed by Tomasz Zawadzki
Browse files

histogram_data: check bucket_shift when merging 



When merging data from one spdk_histogram_data to
another, the merging is only valid if the bucket_shift
for each structure is the same.  Otherwise we are
combining data points that cover different ranges
of values.

So check that the bucket_shifts are the same before
merging. Change the return type to int to
return -EINVAL if structures with different
bucket_shifts are attempted to be merged.

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


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
parent 69bec87a

include/spdk/histogram_data.h

+11 −1
Original line number Diff line number Diff line
@@ -179,15 +179,25 @@ spdk_histogram_data_iterate(const struct spdk_histogram_data *histogram, 
	}

}



static inline void

static inline int

spdk_histogram_data_merge(const struct spdk_histogram_data *dst,

			  const struct spdk_histogram_data *src)

{

	uint64_t i;



	/* Histograms with different bucket_shift values cannot be simply

	 * merged, because the buckets represent different ranges of

	 * values.

	 */

	if (dst->bucket_shift != src->bucket_shift) {

		return -EINVAL;

	}



	for (i = 0; i < SPDK_HISTOGRAM_NUM_BUCKETS(dst); i++) {

		dst->bucket[i] += src->bucket[i];

	}



	return 0;

}



static inline struct spdk_histogram_data *

test/unit/include/spdk/histogram_data.h/histogram_ut.c

+12 −1
Original line number Diff line number Diff line
@@ -82,6 +82,7 @@ histogram_merge(void) 
	struct spdk_histogram_data *h1, *h2;

	uint64_t *values = g_values;

	uint32_t i;

	int rc;



	h1 = spdk_histogram_data_alloc();

	h2 = spdk_histogram_data_alloc();
@@ -91,7 +92,8 @@ histogram_merge(void) 
		spdk_histogram_data_tally(h2, g_values[i]);

	}



	spdk_histogram_data_merge(h1, h2);

	rc = spdk_histogram_data_merge(h1, h2);

	CU_ASSERT(rc == 0);



	g_total = 0;

	g_number_of_merged_histograms = 2;
@@ -99,6 +101,15 @@ histogram_merge(void) 


	spdk_histogram_data_free(h1);

	spdk_histogram_data_free(h2);



	h1 = spdk_histogram_data_alloc_sized(SPDK_HISTOGRAM_BUCKET_SHIFT_DEFAULT);

	h2 = spdk_histogram_data_alloc_sized(SPDK_HISTOGRAM_BUCKET_SHIFT_DEFAULT - 1);



	rc = spdk_histogram_data_merge(h1, h2);

	CU_ASSERT(rc == -EINVAL);



	spdk_histogram_data_free(h1);

	spdk_histogram_data_free(h2);

}



int