diff mbox series

[RESEND,v5,04/16] lib min_heap: Add type safe interface

Message ID 20240514084724.557100-5-visitorckw@gmail.com (mailing list archive)
State Not Applicable, archived
Delegated to: Mike Snitzer
Headers show
Series treewide: Refactor heap related implementation | expand

Commit Message

Kuan-Wei Chiu May 14, 2024, 8:47 a.m. UTC
Implement a type-safe interface for min_heap using strong type
pointers instead of void * in the data field. This change includes
adding small macro wrappers around functions, enabling the use of
__minheap_cast and __minheap_obj_size macros for type casting and
obtaining element size. This implementation removes the necessity of
passing element size in min_heap_callbacks. Additionally, introduce the
MIN_HEAP_PREALLOCATED macro for preallocating some elements.

Link: https://lkml.kernel.org/ioyfizrzq7w7mjrqcadtzsfgpuntowtjdw5pgn4qhvsdp4mqqg@nrlek5vmisbu
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
Reviewed-by: Ian Rogers <irogers@google.com>
---
 drivers/md/dm-vdo/repair.c     |  9 ++--
 drivers/md/dm-vdo/slab-depot.c |  5 +--
 include/linux/min_heap.h       | 79 ++++++++++++++++++++++------------
 kernel/events/core.c           | 11 ++---
 lib/test_min_heap.c            | 13 +++---
 5 files changed, 70 insertions(+), 47 deletions(-)

Comments

Andrew Morton May 22, 2024, 11:10 p.m. UTC | #1
On Tue, 14 May 2024 16:47:12 +0800 Kuan-Wei Chiu <visitorckw@gmail.com> wrote:

> Implement a type-safe interface for min_heap using strong type
> pointers instead of void * in the data field. This change includes
> adding small macro wrappers around functions, enabling the use of
> __minheap_cast and __minheap_obj_size macros for type casting and
> obtaining element size. This implementation removes the necessity of
> passing element size in min_heap_callbacks. Additionally, introduce the
> MIN_HEAP_PREALLOCATED macro for preallocating some elements.

I guess it's a nice change, although it's unclear how valuable all this
is.  Being able to remove the local implementations in bcache and
bcachefs is good.

> --- a/drivers/md/dm-vdo/repair.c
> +++ b/drivers/md/dm-vdo/repair.c
> @@ -51,6 +51,8 @@ struct recovery_point {
>  	bool increment_applied;
>  };
>  
> +MIN_HEAP(struct numbered_block_mapping, replay_heap);
>
> ...
>
> -struct min_heap {
> -	void *data;
> -	int nr;
> -	int size;
> -};
> +#define MIN_HEAP_PREALLOCATED(_type, _name, _nr)	\
> +struct _name {	\
> +	int nr;	\
> +	int size;	\
> +	_type *data;	\
> +	_type preallocated[_nr];	\
> +}

I think that the MIN_HEAP() macro would be better named
DEFINE_MIN_HEAP().  There's a lot of precedent for this and it's
unclear whether "MIN_HEAP" actually implements storage.  I looked at
the repair.c addition and thought "hm, shouldn't that be static"!

>  /* Sift the element at pos down the heap. */
>  static __always_inline
> -void min_heapify(struct min_heap *heap, int pos,
> +void __min_heapify(min_heap_char *heap, int pos, size_t elem_size,
>  		const struct min_heap_callbacks *func)
>  {

It's a separate project, but min_heap.h has some crazily huge inlined
functions.  I expect we'd have a smaller and faster kernel if those
were moved into a new lib/min_heap.c.
Kent Overstreet May 22, 2024, 11:33 p.m. UTC | #2
On Wed, May 22, 2024 at 04:10:48PM -0700, Andrew Morton wrote:
> On Tue, 14 May 2024 16:47:12 +0800 Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
> 
> > Implement a type-safe interface for min_heap using strong type
> > pointers instead of void * in the data field. This change includes
> > adding small macro wrappers around functions, enabling the use of
> > __minheap_cast and __minheap_obj_size macros for type casting and
> > obtaining element size. This implementation removes the necessity of
> > passing element size in min_heap_callbacks. Additionally, introduce the
> > MIN_HEAP_PREALLOCATED macro for preallocating some elements.
> 
> I guess it's a nice change, although it's unclear how valuable all this
> is.  Being able to remove the local implementations in bcache and
> bcachefs is good.

Yes, and now the standard kernel version is type safe. I suggested it to
Kuan as a good starting project, I find (well picked) refactorings and
cleanups are a great place for people to start.

> 
> > --- a/drivers/md/dm-vdo/repair.c
> > +++ b/drivers/md/dm-vdo/repair.c
> > @@ -51,6 +51,8 @@ struct recovery_point {
> >  	bool increment_applied;
> >  };
> >  
> > +MIN_HEAP(struct numbered_block_mapping, replay_heap);
> >
> > ...
> >
> > -struct min_heap {
> > -	void *data;
> > -	int nr;
> > -	int size;
> > -};
> > +#define MIN_HEAP_PREALLOCATED(_type, _name, _nr)	\
> > +struct _name {	\
> > +	int nr;	\
> > +	int size;	\
> > +	_type *data;	\
> > +	_type preallocated[_nr];	\
> > +}
> 
> I think that the MIN_HEAP() macro would be better named
> DEFINE_MIN_HEAP().  There's a lot of precedent for this and it's
> unclear whether "MIN_HEAP" actually implements storage.  I looked at
> the repair.c addition and thought "hm, shouldn't that be static"!

Agreed

> >  /* Sift the element at pos down the heap. */
> >  static __always_inline
> > -void min_heapify(struct min_heap *heap, int pos,
> > +void __min_heapify(min_heap_char *heap, int pos, size_t elem_size,
> >  		const struct min_heap_callbacks *func)
> >  {
> 
> It's a separate project, but min_heap.h has some crazily huge inlined
> functions.  I expect we'd have a smaller and faster kernel if those
> were moved into a new lib/min_heap.c.

good thought - Kuan, want to do that too? Since we missed the merge
window, may as well :)
Ian Rogers May 23, 2024, 3:51 a.m. UTC | #3
On Wed, May 22, 2024 at 4:33 PM Kent Overstreet
<kent.overstreet@linux.dev> wrote:
>
> On Wed, May 22, 2024 at 04:10:48PM -0700, Andrew Morton wrote:
> > On Tue, 14 May 2024 16:47:12 +0800 Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
> >
> > > Implement a type-safe interface for min_heap using strong type
> > > pointers instead of void * in the data field. This change includes
> > > adding small macro wrappers around functions, enabling the use of
> > > __minheap_cast and __minheap_obj_size macros for type casting and
> > > obtaining element size. This implementation removes the necessity of
> > > passing element size in min_heap_callbacks. Additionally, introduce the
> > > MIN_HEAP_PREALLOCATED macro for preallocating some elements.
> >
> > I guess it's a nice change, although it's unclear how valuable all this
> > is.  Being able to remove the local implementations in bcache and
> > bcachefs is good.
>
> Yes, and now the standard kernel version is type safe. I suggested it to
> Kuan as a good starting project, I find (well picked) refactorings and
> cleanups are a great place for people to start.
>
> >
> > > --- a/drivers/md/dm-vdo/repair.c
> > > +++ b/drivers/md/dm-vdo/repair.c
> > > @@ -51,6 +51,8 @@ struct recovery_point {
> > >     bool increment_applied;
> > >  };
> > >
> > > +MIN_HEAP(struct numbered_block_mapping, replay_heap);
> > >
> > > ...
> > >
> > > -struct min_heap {
> > > -   void *data;
> > > -   int nr;
> > > -   int size;
> > > -};
> > > +#define MIN_HEAP_PREALLOCATED(_type, _name, _nr)   \
> > > +struct _name {     \
> > > +   int nr; \
> > > +   int size;       \
> > > +   _type *data;    \
> > > +   _type preallocated[_nr];        \
> > > +}
> >
> > I think that the MIN_HEAP() macro would be better named
> > DEFINE_MIN_HEAP().  There's a lot of precedent for this and it's
> > unclear whether "MIN_HEAP" actually implements storage.  I looked at
> > the repair.c addition and thought "hm, shouldn't that be static"!
>
> Agreed
>
> > >  /* Sift the element at pos down the heap. */
> > >  static __always_inline
> > > -void min_heapify(struct min_heap *heap, int pos,
> > > +void __min_heapify(min_heap_char *heap, int pos, size_t elem_size,
> > >             const struct min_heap_callbacks *func)
> > >  {
> >
> > It's a separate project, but min_heap.h has some crazily huge inlined
> > functions.  I expect we'd have a smaller and faster kernel if those
> > were moved into a new lib/min_heap.c.
>
> good thought - Kuan, want to do that too? Since we missed the merge
> window, may as well :)

Just to mention that the perf events use of the min heap is during a
context switch, so it is somewhat performance sensitive for people
context switching with perf events opened/enabled.

Thanks,
Ian
Kuan-Wei Chiu May 23, 2024, 3:35 p.m. UTC | #4
On Wed, May 22, 2024 at 04:10:48PM -0700, Andrew Morton wrote:
> On Tue, 14 May 2024 16:47:12 +0800 Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
> 
> > Implement a type-safe interface for min_heap using strong type
> > pointers instead of void * in the data field. This change includes
> > adding small macro wrappers around functions, enabling the use of
> > __minheap_cast and __minheap_obj_size macros for type casting and
> > obtaining element size. This implementation removes the necessity of
> > passing element size in min_heap_callbacks. Additionally, introduce the
> > MIN_HEAP_PREALLOCATED macro for preallocating some elements.
> 
> I guess it's a nice change, although it's unclear how valuable all this
> is.  Being able to remove the local implementations in bcache and
> bcachefs is good.
> 
> > --- a/drivers/md/dm-vdo/repair.c
> > +++ b/drivers/md/dm-vdo/repair.c
> > @@ -51,6 +51,8 @@ struct recovery_point {
> >  	bool increment_applied;
> >  };
> >  
> > +MIN_HEAP(struct numbered_block_mapping, replay_heap);
> >
> > ...
> >
> > -struct min_heap {
> > -	void *data;
> > -	int nr;
> > -	int size;
> > -};
> > +#define MIN_HEAP_PREALLOCATED(_type, _name, _nr)	\
> > +struct _name {	\
> > +	int nr;	\
> > +	int size;	\
> > +	_type *data;	\
> > +	_type preallocated[_nr];	\
> > +}
> 
> I think that the MIN_HEAP() macro would be better named
> DEFINE_MIN_HEAP().  There's a lot of precedent for this and it's
> unclear whether "MIN_HEAP" actually implements storage.  I looked at
> the repair.c addition and thought "hm, shouldn't that be static"!
>
I will update this in the next version.

> >  /* Sift the element at pos down the heap. */
> >  static __always_inline
> > -void min_heapify(struct min_heap *heap, int pos,
> > +void __min_heapify(min_heap_char *heap, int pos, size_t elem_size,
> >  		const struct min_heap_callbacks *func)
> >  {
> 
> It's a separate project, but min_heap.h has some crazily huge inlined
> functions.  I expect we'd have a smaller and faster kernel if those
> were moved into a new lib/min_heap.c.
> 
Moving these inline functions to lib/min_heap.c to reduce kernel size
seems reasonable. However, I saw Ian mention that perf event's min_heap
is used during context switches. Since I'm unsure about the performance
impact of making these functions non-inline, I'd like to discuss this
further before actually moving them to lib/min_heap.c.
diff mbox series

Patch

diff --git a/drivers/md/dm-vdo/repair.c b/drivers/md/dm-vdo/repair.c
index defc9359f10e..48c266fa07b6 100644
--- a/drivers/md/dm-vdo/repair.c
+++ b/drivers/md/dm-vdo/repair.c
@@ -51,6 +51,8 @@  struct recovery_point {
 	bool increment_applied;
 };
 
+MIN_HEAP(struct numbered_block_mapping, replay_heap);
+
 struct repair_completion {
 	/* The completion header */
 	struct vdo_completion completion;
@@ -97,7 +99,7 @@  struct repair_completion {
 	 * order, then original journal order. This permits efficient iteration over the journal
 	 * entries in order.
 	 */
-	struct min_heap replay_heap;
+	struct replay_heap replay_heap;
 	/* Fields tracking progress through the journal entries. */
 	struct numbered_block_mapping *current_entry;
 	struct numbered_block_mapping *current_unfetched_entry;
@@ -163,14 +165,13 @@  static void swap_mappings(void *item1, void *item2)
 }
 
 static const struct min_heap_callbacks repair_min_heap = {
-	.elem_size = sizeof(struct numbered_block_mapping),
 	.less = mapping_is_less_than,
 	.swp = swap_mappings,
 };
 
 static struct numbered_block_mapping *sort_next_heap_element(struct repair_completion *repair)
 {
-	struct min_heap *heap = &repair->replay_heap;
+	struct replay_heap *heap = &repair->replay_heap;
 	struct numbered_block_mapping *last;
 
 	if (heap->nr == 0)
@@ -1117,7 +1118,7 @@  static void recover_block_map(struct vdo_completion *completion)
 	 * Organize the journal entries into a binary heap so we can iterate over them in sorted
 	 * order incrementally, avoiding an expensive sort call.
 	 */
-	repair->replay_heap = (struct min_heap) {
+	repair->replay_heap = (struct replay_heap) {
 		.data = repair->entries,
 		.nr = repair->block_map_entry_count,
 		.size = repair->block_map_entry_count,
diff --git a/drivers/md/dm-vdo/slab-depot.c b/drivers/md/dm-vdo/slab-depot.c
index 46e4721e5b4f..f6ae6956f321 100644
--- a/drivers/md/dm-vdo/slab-depot.c
+++ b/drivers/md/dm-vdo/slab-depot.c
@@ -3309,7 +3309,6 @@  static void swap_slab_statuses(void *item1, void *item2)
 }
 
 static const struct min_heap_callbacks slab_status_min_heap = {
-	.elem_size = sizeof(struct slab_status),
 	.less = slab_status_is_less_than,
 	.swp = swap_slab_statuses,
 };
@@ -3509,7 +3508,7 @@  static int get_slab_statuses(struct block_allocator *allocator,
 static int __must_check vdo_prepare_slabs_for_allocation(struct block_allocator *allocator)
 {
 	struct slab_status current_slab_status;
-	struct min_heap heap;
+	MIN_HEAP(struct slab_status, heap) heap;
 	int result;
 	struct slab_status *slab_statuses;
 	struct slab_depot *depot = allocator->depot;
@@ -3521,7 +3520,7 @@  static int __must_check vdo_prepare_slabs_for_allocation(struct block_allocator
 		return result;
 
 	/* Sort the slabs by cleanliness, then by emptiness hint. */
-	heap = (struct min_heap) {
+	heap = (struct heap) {
 		.data = slab_statuses,
 		.nr = allocator->slab_count,
 		.size = allocator->slab_count,
diff --git a/include/linux/min_heap.h b/include/linux/min_heap.h
index d52daf45861b..87737cadb9a5 100644
--- a/include/linux/min_heap.h
+++ b/include/linux/min_heap.h
@@ -7,45 +7,53 @@ 
 #include <linux/types.h>
 
 /**
- * struct min_heap - Data structure to hold a min-heap.
- * @data: Start of array holding the heap elements.
+ * Data structure to hold a min-heap.
  * @nr: Number of elements currently in the heap.
  * @size: Maximum number of elements that can be held in current storage.
+ * @data: Pointer to the start of array holding the heap elements.
+ * @preallocated: Start of the static preallocated array holding the heap elements.
  */
-struct min_heap {
-	void *data;
-	int nr;
-	int size;
-};
+#define MIN_HEAP_PREALLOCATED(_type, _name, _nr)	\
+struct _name {	\
+	int nr;	\
+	int size;	\
+	_type *data;	\
+	_type preallocated[_nr];	\
+}
+
+#define MIN_HEAP(_type, _name) MIN_HEAP_PREALLOCATED(_type, _name, 0)
+
+typedef MIN_HEAP(char, min_heap_char) min_heap_char;
+
+#define __minheap_cast(_heap)		(typeof((_heap)->data[0]) *)
+#define __minheap_obj_size(_heap)	sizeof((_heap)->data[0])
 
 /**
  * struct min_heap_callbacks - Data/functions to customise the min_heap.
- * @elem_size: The nr of each element in bytes.
  * @less: Partial order function for this heap.
  * @swp: Swap elements function.
  */
 struct min_heap_callbacks {
-	int elem_size;
 	bool (*less)(const void *lhs, const void *rhs);
 	void (*swp)(void *lhs, void *rhs);
 };
 
 /* Sift the element at pos down the heap. */
 static __always_inline
-void min_heapify(struct min_heap *heap, int pos,
+void __min_heapify(min_heap_char *heap, int pos, size_t elem_size,
 		const struct min_heap_callbacks *func)
 {
 	void *left, *right;
 	void *data = heap->data;
-	void *root = data + pos * func->elem_size;
+	void *root = data + pos * elem_size;
 	int i = pos, j;
 
 	/* Find the sift-down path all the way to the leaves. */
 	for (;;) {
 		if (i * 2 + 2 >= heap->nr)
 			break;
-		left = data + (i * 2 + 1) * func->elem_size;
-		right = data + (i * 2 + 2) * func->elem_size;
+		left = data + (i * 2 + 1) * elem_size;
+		right = data + (i * 2 + 2) * elem_size;
 		i = func->less(left, right) ? i * 2 + 1 : i * 2 + 2;
 	}
 
@@ -54,31 +62,37 @@  void min_heapify(struct min_heap *heap, int pos,
 		i = i * 2 + 1;
 
 	/* Backtrack to the correct location. */
-	while (i != pos && func->less(root, data + i * func->elem_size))
+	while (i != pos && func->less(root, data + i * elem_size))
 		i = (i - 1) / 2;
 
 	/* Shift the element into its correct place. */
 	j = i;
 	while (i != pos) {
 		i = (i - 1) / 2;
-		func->swp(data + i * func->elem_size, data + j * func->elem_size);
+		func->swp(data + i * elem_size, data + j * elem_size);
 	}
 }
 
+#define min_heapify(_heap, _pos, _func)	\
+	__min_heapify((min_heap_char *)_heap, _pos, __minheap_obj_size(_heap), _func)
+
 /* Floyd's approach to heapification that is O(nr). */
 static __always_inline
-void min_heapify_all(struct min_heap *heap,
+void __min_heapify_all(min_heap_char *heap, size_t elem_size,
 		const struct min_heap_callbacks *func)
 {
 	int i;
 
 	for (i = heap->nr / 2 - 1; i >= 0; i--)
-		min_heapify(heap, i, func);
+		__min_heapify(heap, i, elem_size, func);
 }
 
+#define min_heapify_all(_heap, _func)	\
+	__min_heapify_all((min_heap_char *)_heap, __minheap_obj_size(_heap), _func)
+
 /* Remove minimum element from the heap, O(log2(nr)). */
 static __always_inline
-void min_heap_pop(struct min_heap *heap,
+void __min_heap_pop(min_heap_char *heap, size_t elem_size,
 		const struct min_heap_callbacks *func)
 {
 	void *data = heap->data;
@@ -88,27 +102,33 @@  void min_heap_pop(struct min_heap *heap,
 
 	/* Place last element at the root (position 0) and then sift down. */
 	heap->nr--;
-	memcpy(data, data + (heap->nr * func->elem_size), func->elem_size);
-	min_heapify(heap, 0, func);
+	memcpy(data, data + (heap->nr * elem_size), elem_size);
+	__min_heapify(heap, 0, elem_size, func);
 }
 
+#define min_heap_pop(_heap, _func)	\
+	__min_heap_pop((min_heap_char *)_heap, __minheap_obj_size(_heap), _func)
+
 /*
  * Remove the minimum element and then push the given element. The
  * implementation performs 1 sift (O(log2(nr))) and is therefore more
  * efficient than a pop followed by a push that does 2.
  */
 static __always_inline
-void min_heap_pop_push(struct min_heap *heap,
-		const void *element,
+void __min_heap_pop_push(min_heap_char *heap,
+		const void *element, size_t elem_size,
 		const struct min_heap_callbacks *func)
 {
-	memcpy(heap->data, element, func->elem_size);
-	min_heapify(heap, 0, func);
+	memcpy(heap->data, element, elem_size);
+	__min_heapify(heap, 0, elem_size, func);
 }
 
+#define min_heap_pop_push(_heap, _element, _func)	\
+	__min_heap_pop_push((min_heap_char *)_heap, _element, __minheap_obj_size(_heap), _func)
+
 /* Push an element on to the heap, O(log2(nr)). */
 static __always_inline
-void min_heap_push(struct min_heap *heap, const void *element,
+void __min_heap_push(min_heap_char *heap, const void *element, size_t elem_size,
 		const struct min_heap_callbacks *func)
 {
 	void *data = heap->data;
@@ -120,17 +140,20 @@  void min_heap_push(struct min_heap *heap, const void *element,
 
 	/* Place at the end of data. */
 	pos = heap->nr;
-	memcpy(data + (pos * func->elem_size), element, func->elem_size);
+	memcpy(data + (pos * elem_size), element, elem_size);
 	heap->nr++;
 
 	/* Sift child at pos up. */
 	for (; pos > 0; pos = (pos - 1) / 2) {
-		child = data + (pos * func->elem_size);
-		parent = data + ((pos - 1) / 2) * func->elem_size;
+		child = data + (pos * elem_size);
+		parent = data + ((pos - 1) / 2) * elem_size;
 		if (func->less(parent, child))
 			break;
 		func->swp(parent, child);
 	}
 }
 
+#define min_heap_push(_heap, _element, _func)	\
+	__min_heap_push((min_heap_char *)_heap, _element, __minheap_obj_size(_heap), _func)
+
 #endif /* _LINUX_MIN_HEAP_H */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 10ac2db83f14..55930479599b 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3698,13 +3698,14 @@  static void swap_ptr(void *l, void *r)
 	swap(*lp, *rp);
 }
 
+MIN_HEAP(struct perf_event *, perf_event_min_heap);
+
 static const struct min_heap_callbacks perf_min_heap = {
-	.elem_size = sizeof(struct perf_event *),
 	.less = perf_less_group_idx,
 	.swp = swap_ptr,
 };
 
-static void __heap_add(struct min_heap *heap, struct perf_event *event)
+static void __heap_add(struct perf_event_min_heap *heap, struct perf_event *event)
 {
 	struct perf_event **itrs = heap->data;
 
@@ -3738,7 +3739,7 @@  static noinline int visit_groups_merge(struct perf_event_context *ctx,
 	struct perf_cpu_context *cpuctx = NULL;
 	/* Space for per CPU and/or any CPU event iterators. */
 	struct perf_event *itrs[2];
-	struct min_heap event_heap;
+	struct perf_event_min_heap event_heap;
 	struct perf_event **evt;
 	int ret;
 
@@ -3747,7 +3748,7 @@  static noinline int visit_groups_merge(struct perf_event_context *ctx,
 
 	if (!ctx->task) {
 		cpuctx = this_cpu_ptr(&perf_cpu_context);
-		event_heap = (struct min_heap){
+		event_heap = (struct perf_event_min_heap){
 			.data = cpuctx->heap,
 			.nr = 0,
 			.size = cpuctx->heap_size,
@@ -3760,7 +3761,7 @@  static noinline int visit_groups_merge(struct perf_event_context *ctx,
 			css = &cpuctx->cgrp->css;
 #endif
 	} else {
-		event_heap = (struct min_heap){
+		event_heap = (struct perf_event_min_heap){
 			.data = itrs,
 			.nr = 0,
 			.size = ARRAY_SIZE(itrs),
diff --git a/lib/test_min_heap.c b/lib/test_min_heap.c
index 7b01b4387cfb..dd5c0fa862ab 100644
--- a/lib/test_min_heap.c
+++ b/lib/test_min_heap.c
@@ -11,6 +11,8 @@ 
 #include <linux/printk.h>
 #include <linux/random.h>
 
+MIN_HEAP(int, min_heap_test);
+
 static __init bool less_than(const void *lhs, const void *rhs)
 {
 	return *(int *)lhs < *(int *)rhs;
@@ -30,7 +32,7 @@  static __init void swap_ints(void *lhs, void *rhs)
 }
 
 static __init int pop_verify_heap(bool min_heap,
-				struct min_heap *heap,
+				struct min_heap_test *heap,
 				const struct min_heap_callbacks *funcs)
 {
 	int *values = heap->data;
@@ -63,13 +65,12 @@  static __init int test_heapify_all(bool min_heap)
 {
 	int values[] = { 3, 1, 2, 4, 0x8000000, 0x7FFFFFF, 0,
 			 -3, -1, -2, -4, 0x8000000, 0x7FFFFFF };
-	struct min_heap heap = {
+	struct min_heap_test heap = {
 		.data = values,
 		.nr = ARRAY_SIZE(values),
 		.size =  ARRAY_SIZE(values),
 	};
 	struct min_heap_callbacks funcs = {
-		.elem_size = sizeof(int),
 		.less = min_heap ? less_than : greater_than,
 		.swp = swap_ints,
 	};
@@ -96,13 +97,12 @@  static __init int test_heap_push(bool min_heap)
 	const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
 			     -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
 	int values[ARRAY_SIZE(data)];
-	struct min_heap heap = {
+	struct min_heap_test heap = {
 		.data = values,
 		.nr = 0,
 		.size =  ARRAY_SIZE(values),
 	};
 	struct min_heap_callbacks funcs = {
-		.elem_size = sizeof(int),
 		.less = min_heap ? less_than : greater_than,
 		.swp = swap_ints,
 	};
@@ -129,13 +129,12 @@  static __init int test_heap_pop_push(bool min_heap)
 	const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
 			     -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
 	int values[ARRAY_SIZE(data)];
-	struct min_heap heap = {
+	struct min_heap_test heap = {
 		.data = values,
 		.nr = 0,
 		.size =  ARRAY_SIZE(values),
 	};
 	struct min_heap_callbacks funcs = {
-		.elem_size = sizeof(int),
 		.less = min_heap ? less_than : greater_than,
 		.swp = swap_ints,
 	};