public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] lib/igt_list: Update, clean-up and document igt_list
@ 2019-09-17  9:54 Arkadiusz Hiler
  2019-09-17 10:22 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
  2019-09-17 16:32 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  0 siblings, 2 replies; 3+ messages in thread
From: Arkadiusz Hiler @ 2019-09-17  9:54 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

We have diverged from how Wayland implements list both in parameter
ordering and function naming.

Let's make our side consistent to ease the cognitive dissonance for
developers working on both projects.

This patch:
 * mimics the current Wayland implementation
 * separates IGT helpers in the source files
 * adds brief explanation and code example for igt-doc
 * introduces igt_list.c as static inlines are not visible in the docs

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 benchmarks/gem_wsim.c                         |   6 +-
 .../igt-gpu-tools/igt-gpu-tools-docs.xml      |   1 +
 lib/Makefile.sources                          |   2 +
 lib/igt_chamelium.c                           |   2 +-
 lib/igt_core.c                                |   4 +-
 lib/igt_dummyload.c                           |   4 +-
 lib/igt_kmod.c                                |   2 +-
 lib/igt_list.c                                |  66 +++++++
 lib/igt_list.h                                | 165 +++++++++---------
 lib/meson.build                               |   1 +
 tests/vc4_purgeable_bo.c                      |   6 +-
 11 files changed, 163 insertions(+), 96 deletions(-)
 create mode 100644 lib/igt_list.c

diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
index 87f873b0..44745d06 100644
--- a/benchmarks/gem_wsim.c
+++ b/benchmarks/gem_wsim.c
@@ -2814,11 +2814,11 @@ static void *run_workload(void *data)
 			do_eb(wrk, w, engine, wrk->flags);
 
 			if (w->request != -1) {
-				igt_list_del(&w->rq_link);
+				igt_list_remove(&w->rq_link);
 				wrk->nrequest[w->request]--;
 			}
 			w->request = engine;
-			igt_list_add_tail(&w->rq_link, &wrk->requests[engine]);
+			igt_list_insert_tail(&wrk->requests[engine], &w->rq_link);
 			wrk->nrequest[engine]++;
 
 			if (!wrk->run)
@@ -2840,7 +2840,7 @@ static void *run_workload(void *data)
 					last_sync = true;
 
 					s->request = -1;
-					igt_list_del(&s->rq_link);
+					igt_list_remove(&s->rq_link);
 					wrk->nrequest[engine]--;
 				}
 			}
diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
index ac83272f..3f14d7be 100644
--- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
+++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
@@ -31,6 +31,7 @@
     <xi:include href="xml/igt_gvt.xml"/>
     <xi:include href="xml/igt_kmod.xml"/>
     <xi:include href="xml/igt_kms.xml"/>
+    <xi:include href="xml/igt_list.xml"/>
     <xi:include href="xml/igt_pm.xml"/>
     <xi:include href="xml/igt_primes.xml"/>
     <xi:include href="xml/igt_rand.xml"/>
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index cf094ab8..4a7c54cd 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -43,6 +43,8 @@ lib_source_list =	 	\
 	igt_halffloat.h		\
 	igt_infoframe.c		\
 	igt_infoframe.h		\
+	igt_list.c		\
+	igt_list.h		\
 	igt_matrix.c		\
 	igt_matrix.h		\
 	igt_primes.c		\
diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 1b03a103..7de8ab07 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -597,7 +597,7 @@ struct chamelium_edid *chamelium_new_edid(struct chamelium *chamelium,
 	chamelium_edid->chamelium = chamelium;
 	chamelium_edid->base = malloc(edid_size);
 	memcpy(chamelium_edid->base, edid, edid_size);
-	igt_list_add(&chamelium_edid->link, &chamelium->edids);
+	igt_list_insert(&chamelium->edids, &chamelium_edid->link);
 
 	return chamelium_edid;
 }
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 1cbb09f9..d5dca618 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1138,7 +1138,7 @@ void __igt_subtest_group_save(int *save, int *desc)
 	if (__current_description[0] != '\0') {
 		struct description_node *new = calloc(1, sizeof(*new));
 		memcpy(new->desc, __current_description, sizeof(__current_description));
-		igt_list_add_tail(&new->link, &subgroup_descriptions);
+		igt_list_insert_tail(&subgroup_descriptions, &new->link);
 		_clear_current_description();
 		*desc = true;
 	}
@@ -1151,7 +1151,7 @@ void __igt_subtest_group_restore(int save, int desc)
 	if (desc) {
 		struct description_node *last =
 			igt_list_last_entry(&subgroup_descriptions, last, link);
-		igt_list_del(&last->link);
+		igt_list_remove(&last->link);
 		free(last);
 	}
 
diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 0e06276a..ec0ab969 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -284,7 +284,7 @@ spin_create(int fd, const struct igt_spin_factory *opts)
 	spin->out_fence = emit_recursive_batch(spin, fd, opts);
 
 	pthread_mutex_lock(&list_lock);
-	igt_list_add(&spin->link, &spin_list);
+	igt_list_insert(&spin_list, &spin->link);
 	pthread_mutex_unlock(&list_lock);
 
 	return spin;
@@ -430,7 +430,7 @@ void igt_spin_free(int fd, igt_spin_t *spin)
 		return;
 
 	pthread_mutex_lock(&list_lock);
-	igt_list_del(&spin->link);
+	igt_list_remove(&spin->link);
 	pthread_mutex_unlock(&list_lock);
 
 	if (spin->timer)
diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 91662eb3..8f66d1dd 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -404,7 +404,7 @@ static void tests_add(struct igt_kselftest_list *tl, struct igt_list *list)
 		if (pos->number > tl->number)
 			break;
 
-	igt_list_add_tail(&tl->link, &pos->link);
+	igt_list_insert_tail(&pos->link, &tl->link);
 }
 
 void igt_kselftest_get_tests(struct kmod_module *kmod,
diff --git a/lib/igt_list.c b/lib/igt_list.c
new file mode 100644
index 00000000..af615124
--- /dev/null
+++ b/lib/igt_list.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "igt_list.h"
+
+void igt_list_init(struct igt_list *list)
+{
+	list->prev = list;
+	list->next = list;
+}
+
+void igt_list_insert(struct igt_list *list, struct igt_list *elm)
+{
+	elm->prev = list;
+	elm->next = list->next;
+	list->next = elm;
+	elm->next->prev = elm;
+}
+
+
+void igt_list_remove(struct igt_list *elm)
+{
+	elm->prev->next = elm->next;
+	elm->next->prev = elm->prev;
+	elm->next = NULL;
+	elm->prev = NULL;
+}
+
+int igt_list_length(const struct igt_list *list)
+{
+	struct  igt_list *e = list->next;
+	int count = 0;
+
+	while (e != list) {
+		e = e->next;
+		count++;
+	}
+
+	return count;
+}
+
+bool igt_list_empty(const struct igt_list *list)
+{
+	return list->next == list;
+}
diff --git a/lib/igt_list.h b/lib/igt_list.h
index cadf9dd3..c73a98f1 100644
--- a/lib/igt_list.h
+++ b/lib/igt_list.h
@@ -28,9 +28,42 @@
 #include <stdbool.h>
 #include <stddef.h>
 
-/*
- * This list data structure is a verbatim copy from wayland-util.h from the
- * Wayland project; except that wl_ prefix has been removed.
+/**
+ * SECTION:igt_list
+ * @short_description: a list implementation borrowed from the Wayland project
+ * @title: IGT List
+ * @include: igt_list.h
+ *
+ * This list data structure mimics the one from wayland-util.h from the Wayland
+ * project. A few bonus helpers are provided.
+ *
+ * igt_list is a doubly-linked list where an instance of igt_list is a head
+ * sentinel and has to be initalized. See Wayland documentation for more
+ * details.
+ *
+ * Example usage:
+ *
+ * |[<!-- language="C" -->
+ * struct igt_list foo_list;
+ *
+ * struct element {
+ *         int foo;
+ *         struct igt_list link;
+ * };
+ *
+ * struct element e1, e2, e3;
+ *
+ * igt_list_init(&foo_list);
+ * igt_list_insert(&foo_list, &e1.link);   // e1 is the first element
+ * igt_list_insert(&foo_list, &e2.link);   // e2 is now the first element
+ * igt_list_insert(&e2.link, &e3.link);    // insert e3 after e2
+ *
+ * struct igt_spin *iter;
+ * igt_list_for_each(iter, &foo_list, link) {
+ * 	printf("%d\n", iter->foo);
+ * }
+ * ]|
+ *
  */
 
 struct igt_list {
@@ -38,92 +71,56 @@ struct igt_list {
 	struct igt_list *next;
 };
 
-#define __IGT_INIT_LIST(name) { &(name), &(name) }
-#define IGT_LIST(name) struct igt_list name = __IGT_INIT_LIST(name)
-
-static inline void igt_list_init(struct igt_list *list)
-{
-	list->prev = list;
-	list->next = list;
-}
-
-static inline void __igt_list_add(struct igt_list *list,
-				  struct igt_list *prev,
-				  struct igt_list *next)
-{
-	next->prev = list;
-	list->next = next;
-	list->prev = prev;
-	prev->next = list;
-}
-
-static inline void igt_list_add(struct igt_list *elm, struct igt_list *list)
-{
-	__igt_list_add(elm, list, list->next);
-}
-
-static inline void igt_list_add_tail(struct igt_list *elm,
-				     struct igt_list *list)
-{
-	__igt_list_add(elm, list->prev, list);
-}
-
-static inline void __igt_list_del(struct igt_list *prev, struct igt_list *next)
-{
-	next->prev = prev;
-	prev->next = next;
-}
-
-static inline void igt_list_del(struct igt_list *elm)
-{
-	__igt_list_del(elm->prev, elm->next);
-}
-
-static inline void igt_list_move(struct igt_list *elm, struct igt_list *list)
-{
-	igt_list_del(elm);
-	igt_list_add(elm, list);
-}
-
-static inline void igt_list_move_tail(struct igt_list *elm,
-				      struct igt_list *list)
-{
-	igt_list_del(elm);
-	igt_list_add_tail(elm, list);
-}
-
-static inline bool igt_list_empty(const struct igt_list *list)
-{
-	return list->next == list;
-}
-
-#define container_of(ptr, sample, member)				\
-	(typeof(sample))((char *)(ptr) - offsetof(typeof(*sample), member))
-
-#define igt_list_first_entry(head, pos, member) \
-	container_of((head)->next, (pos), member)
-#define igt_list_last_entry(head, pos, member) \
-	container_of((head)->prev, (pos), member)
-
-#define igt_list_next_entry(pos, member) \
-	container_of((pos)->member.next, (pos), member)
-#define igt_list_prev_entry(pos, member) \
-	container_of((pos)->member.prev, (pos), member)
+void igt_list_init(struct igt_list *list);
+void igt_list_insert(struct igt_list *list, struct igt_list *elm);
+void igt_list_remove(struct igt_list *elm);
+int igt_list_length(const struct igt_list *list);
+bool igt_list_empty(const struct igt_list *list);
+
+#define igt_container_of(ptr, sample, member)				\
+	(__typeof__(sample))((char *)(ptr) -				\
+				offsetof(__typeof__(*sample), member))
 
 #define igt_list_for_each(pos, head, member)				\
-	for (pos = igt_list_first_entry(head, pos, member);		\
+	for (pos = igt_container_of((head)->next, pos, member);		\
 	     &pos->member != (head);					\
-	     pos = igt_list_next_entry(pos, member))
-
-#define igt_list_for_each_reverse(pos, head, member)			\
-	for (pos = igt_list_last_entry(head, pos, member);		\
-	     &pos->member != (head);					\
-	     pos = igt_list_prev_entry(pos, member))
+	     pos = igt_container_of((pos)->member.next, pos, member))
 
+/**
+ * igt_list_for_each_safe
+ *
+ * Safe against removel of the *current* list element. To achive this it
+ * requires an extra helper variable `tmp` with the same type as `pos`.
+ */
 #define igt_list_for_each_safe(pos, tmp, head, member)			\
-	for (pos = igt_list_first_entry(head, pos, member),		\
-	     tmp = igt_list_next_entry(pos, member);			\
+	for (pos = igt_container_of((head)->next, pos, member),		\
+	     tmp = igt_container_of((pos)->member.next, tmp, member); 	\
 	     &pos->member != (head);					\
-	     pos = tmp, tmp = igt_list_next_entry(pos, member))
+	     pos = tmp,							\
+	     tmp = igt_container_of((pos)->member.next, tmp, member))
+
+#define igt_list_for_each_reverse(pos, head, member)			\
+	for (pos = igt_container_of((head)->prev, pos, member);		\
+	     &pos->member != (head);					\
+	     pos = igt_container_of((pos)->member.prev, pos, member))
+
+
+/* IGT custom helpers */
+
+/**
+ * IGT_LIST
+ *
+ * Instead of having to use `igt_list_init()` list can be defined using
+ * this helper, e.g. `static IGT_LIST(list);`
+ */
+#define IGT_LIST(name) struct igt_list name = { &(name), &(name) }
+
+#define igt_list_insert_tail(list, elem) igt_list_insert((list)->prev, elem)
+
+#define igt_list_first_entry(head, pos, member) \
+	igt_container_of((head)->next, (pos), member)
+
+#define igt_list_last_entry(head, pos, member) \
+	igt_container_of((head)->prev, (pos), member)
 
 #endif /* IGT_LIST_H */
diff --git a/lib/meson.build b/lib/meson.build
index 221ae28c..26d5db6f 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -50,6 +50,7 @@ lib_sources = [
 	'igt_fb.c',
 	'igt_core.c',
 	'igt_draw.c',
+	'igt_list.c',
 	'igt_pm.c',
 	'igt_dummyload.c',
 	'uwildmat/uwildmat.c',
diff --git a/tests/vc4_purgeable_bo.c b/tests/vc4_purgeable_bo.c
index cfe14e70..030b1d1b 100644
--- a/tests/vc4_purgeable_bo.c
+++ b/tests/vc4_purgeable_bo.c
@@ -67,7 +67,7 @@ static void igt_vc4_alloc_mmap_max_bo(int fd, struct igt_list *list,
 		bo->size = create.size;
 		bo->map = igt_vc4_mmap_bo(fd, bo->handle, bo->size,
 					  PROT_READ | PROT_WRITE);
-		igt_list_add_tail(&bo->node, list);
+		igt_list_insert_tail(list, &bo->node);
 	}
 }
 
@@ -78,7 +78,7 @@ static void igt_vc4_unmap_free_bo_pool(int fd, struct igt_list *list)
 	while (!igt_list_empty(list)) {
 		bo = igt_list_first_entry(list, bo, node);
 		igt_assert(bo);
-		igt_list_del(&bo->node);
+		igt_list_remove(&bo->node);
 		munmap(bo->map, bo->size);
 		gem_close(fd, bo->handle);
 		free(bo);
@@ -253,7 +253,7 @@ igt_main
 		/* Trigger a purge. */
 		igt_vc4_trigger_purge(fd);
 
-		igt_list_del(&bo->node);
+		igt_list_remove(&bo->node);
 		munmap(bo->map, bo->size);
 		gem_close(fd, bo->handle);
 		free(bo);
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-17 16:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-17  9:54 [igt-dev] [PATCH i-g-t] lib/igt_list: Update, clean-up and document igt_list Arkadiusz Hiler
2019-09-17 10:22 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2019-09-17 16:32 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox