path: root/list.h
diff options
authorJeff King <>2017-09-05 12:15:00 (GMT)
committerJunio C Hamano <>2017-09-06 08:19:54 (GMT)
commit24d82185d267daafc29ca61af1ed1dc746ba3463 (patch)
tree0b941a604c811a5a9e6bb6a421aec930d7dd829e /list.h
parent102cf7a6aa1f09177c09536a81dbaef44e13e038 (diff)
tempfile: use list.h for linked list
The tempfile API keeps to-be-cleaned tempfiles in a singly-linked list and never removes items from the list. A future patch would like to start removing items, but removal from a singly linked list is O(n), as we have to walk the list to find the predecessor element. This means that a process which takes "n" simultaneous lockfiles (for example, an atomic transaction on "n" refs) may end up quadratic in "n". Before we start allowing items to be removed, it would be nice to have a way to cover this case in linear time. The simplest solution is to make an assumption about the order in which tempfiles are added and removed from the list. If both operations iterate over the tempfiles in the same order, then by putting new items at the end of the list our removal search will always find its items at the beginning of the list. And indeed, that would work for the case of refs. But it creates a hidden dependency between unrelated parts of the code. If anybody changes the ref code (or if we add a new caller that opens multiple simultaneous tempfiles) they may unknowingly introduce a performance regression. Another solution is to use a better data structure. A doubly-linked list works fine, and we already have an implementation in list.h. But there's one snag: the elements of "struct tempfile" are all marked as "volatile", since a signal handler may interrupt us and iterate over the list at any moment (even if we were in the middle of adding a new entry). We can declare a "volatile struct list_head", but we can't actually use it with the normal list functions. The compiler complains about passing a pointer-to-volatile via a regular pointer argument. And rightfully so, as the sub-function would potentially need different code to deal with the volatile case. That leaves us with a few options: 1. Drop the "volatile" modifier for the list items. This is probably a bad idea. I checked the assembly output from "gcc -O2", and the "volatile" really does impact the order in which it updates memory. 2. Use macros instead of inline functions. The irony here is that list.h is entirely implemented as trivial inline functions. So we basically are already generating custom code for each call. But sadly there's no way in C to declare the inline function to take a more generic type. We could do so by switching the inline functions to macros, but it does make the end result harder to read. And it doesn't fully solve the problem (for instance, the declaration of list_head needs to change so that its "prev" and "next" pointers point to other volatile structs). 3. Don't use list.h, and just make our own ad-hoc doubly-linked list. It's not that much code to implement the basics that we need here. But if we're going to do so, why not add the few extra lines required to model it after the actual list.h interface? We can even reuse a few of the macro helpers. So this patch takes option 3, but actually implements a parallel "volatile list" interface in list.h, where it could potentially be reused by other code. This implements just enough for tempfile.c's use, though we could easily port other functions later if need be. Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'list.h')
1 files changed, 38 insertions, 0 deletions
diff --git a/list.h b/list.h
index a226a87..eb60119 100644
--- a/list.h
+++ b/list.h
@@ -163,4 +163,42 @@ static inline void list_replace_init(struct list_head *old,
+ * This is exactly the same as a normal list_head, except that it can be
+ * declared volatile (e.g., if you have a list that may be accessed from signal
+ * handlers).
+ */
+struct volatile_list_head {
+ volatile struct volatile_list_head *next, *prev;
+#define VOLATILE_LIST_HEAD(name) \
+ volatile struct volatile_list_head name = { &(name), &(name) }
+static inline void __volatile_list_del(volatile struct volatile_list_head *prev,
+ volatile struct volatile_list_head *next)
+ next->prev = prev;
+ prev->next = next;
+static inline void volatile_list_del(volatile struct volatile_list_head *elem)
+ __volatile_list_del(elem->prev, elem->next);
+static inline int volatile_list_empty(volatile struct volatile_list_head *head)
+ return head == head->next;
+static inline void volatile_list_add(volatile struct volatile_list_head *newp,
+ volatile struct volatile_list_head *head)
+ head->next->prev = newp;
+ newp->next = head->next;
+ newp->prev = head;
+ head->next = newp;
#endif /* LIST_H */