It is currently Sun, 09 Aug 2020 02:35:44 GMT



 
Author Message
 list_head debugging?
A friend recently was bitten by passing a list_head from list_for_each
to a code path that later moved the list_head around.  Does the attached
patch re-create some debugging wheel that's hiding off in a corner
somewhere?

Beyond catching the obvious use of uninitialized things, it also seems
to catch double adds, double deletes, and simple deletes of 'pos' in
list_for_each.  it even seems to bark about the seemingly hard-to-detect
movement of 'pos' from the iterating list to another, but probably only
in the rare case where you're unconditionally moving all 'pos' in the
loop body.  (you eventually iterate into the second list's head and try
to add it to itself)

I've only played with this in userspace but am glad to pretty it up with
CONFIG_s and bring it into 2.5 if people care.  its against 2.4.mumble.

--
 zach

--- ./list.h.debug      Thu Sep 19 15:58:47 2002
+++ ./list.h    Fri Sep 20 13:43:21 2002
@@ -21,6 +21,25 @@

 typedef struct list_head list_t;

+#define LIST_HEAD_DEBUGGING
+#ifdef LIST_HEAD_DEBUGGING
+
+static inline void __list_valid(struct list_head *list)
+{
+       BUG_ON(list == NULL);
+       BUG_ON(list->next == NULL);
+       BUG_ON(list->prev == NULL);
+       BUG_ON(list->next->prev != list);
+       BUG_ON(list->prev->next != list);
+       BUG_ON((list->next == list) && (list->prev != list));
+       BUG_ON((list->prev == list) && (list->next != list));
+}
+#else
+
+#define __list_valid(args...)
+
+#endif
+
 #define LIST_HEAD_INIT(name) { &(name), &(name) }

 #define LIST_HEAD(name) \
@@ -56,7 +75,9 @@
  */
 static __inline__ void list_add(struct list_head *new, struct list_head *head)
 {
+       __list_valid(head);
        __list_add(new, head, head->next);
+       __list_valid(new);
 }

 /**
@@ -69,7 +90,9 @@
  */
 static __inline__ void list_add_tail(struct list_head *new, struct list_head *head)
 {
+       __list_valid(head);
        __list_add(new, head->prev, head);
+       __list_valid(new);
 }

 /*
@@ -93,6 +116,7 @@
  */
 static __inline__ void list_del(struct list_head *entry)
 {
+       __list_valid(entry);
        __list_del(entry->prev, entry->next);
 }

@@ -102,6 +126,7 @@
  */
 static __inline__ void list_del_init(struct list_head *entry)
 {
+       __list_valid(entry);
        __list_del(entry->prev, entry->next);
        INIT_LIST_HEAD(entry);
 }
@@ -112,6 +137,7 @@
  */
 static __inline__ int list_empty(struct list_head *head)
 {
+       __list_valid(head);
        return head->next == head;
 }

@@ -124,6 +150,9 @@
 {
        struct list_head *first = list->next;

+       __list_valid(list);
+       __list_valid(head);
+
        if (first != list) {
                struct list_head *last = list->prev;
                struct list_head *at = head->next;
@@ -150,9 +179,10 @@
  * @pos:       the &struct list_head to use as a loop counter.
  * @head:      the head for your list.
  */
-#define list_for_each(pos, head) \
-       for (pos = (head)->next, prefetch(pos->next); pos != (head); \
-               pos = pos->next, prefetch(pos->next))
+#define list_for_each(pos, head)                       \
+       for (pos = (head)->next, prefetch(pos->next); \
+               __list_valid(pos), pos != (head);       \
+               __list_valid(pos), pos = pos->next, prefetch(pos->next))

 /**
  * list_for_each_safe  -       iterate over a list safe against removal of list entry
@@ -170,8 +200,9 @@
  * @head:      the head for your list.
  */
 #define list_for_each_prev(pos, head) \
-       for (pos = (head)->prev, prefetch(pos->prev); pos != (head); \
-               pos = pos->prev, prefetch(pos->prev))
+       for (pos = (head)->prev, prefetch(pos->prev); \
+               __list_valid(pos), pos != (head); \
+               __list_valid(pos), pos = pos->prev, prefetch(pos->prev))

 #endif /* __KERNEL__ || _LVM_H_INCLUDE */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at   http://www.**-**.com/
Please read the FAQ at   http://www.**-**.com/



 Wed, 09 Mar 2005 05:00:10 GMT   
 list_head debugging?

Hey, cool!

Yes, I think a lot of people would be all for something like this as a
CONFIG_DEBUG_LISTS or such.  Very nice.

        Robert Love

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Wed, 09 Mar 2005 05:40:07 GMT   
 list_head debugging?
Hi,

Before Fri, 20 Sep 2002, Zach Brown wrote:

--- ./list.h.debug      Thu Sep 19 15:58:47 2002
+++ ./list.h    Fri Sep 20 13:43:21 2002
@@ -21,6 +21,25 @@

 typedef struct list_head list_t;

+#define LIST_HEAD_DEBUGGING
+#ifdef LIST_HEAD_DEBUGGING
+
+static inline void __list_valid(struct list_head *list)
+{
+       BUG_ON(list == NULL);
+       BUG_ON(list->next == NULL);
+       BUG_ON(list->prev == NULL);
+       BUG_ON(list->next->prev != list);
+       BUG_ON(list->prev->next != list);
+       BUG_ON((list->next == list) && (list->prev != list));
+       BUG_ON((list->prev == list) && (list->next != list));
+}
+#else

It's all cool, but I'm not entirely convinced why it must be a BUG macro.
I'd rather have something said via printk here. If whatever we did was
bad, it will show up with a BUG() just too soon.

I'd describe a macro.

#define list_assert(cond)                               \
        if (cond) printk(KERN_ERR "%s failed!\n", #cond)

Or the like. BTW, I'd define LIST_HEAD_DEBUGGING as 1.

                        Thunder
--
assert(typeof((fool)->next) == typeof(fool));        /* wrong */

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Wed, 09 Mar 2005 08:40:04 GMT   
 list_head debugging?
Em Fri, Sep 20, 2002 at 04:53:04PM -0400, Zach Brown escreveu:

Cool! I'll be always using this while developing, thanks!

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Wed, 09 Mar 2005 12:40:03 GMT   
 
   [ 4 post ] 

Similar Threads

1. list_head debugging patch

2. 2.5.39 list_head debugging

3. list_head debugging

4. list_head debugging

5. convert tty_drivers to list_heads

6. vma->shared list_head initializations

7. move the mempool list_head out of the managed elements

8. list_head makes me crazy

9. Help with kernel source (struct list_head)

10. : Kernel debugging (saving debug info)


 
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group.
Designed by ST Software