Re: 2.5.70-bk16: nfs crash

[Linus Torvalds]

On Thu, 12 Jun 2003, Linus Torvalds wrote:
> 
> If you depend on not re-initializing the pointers, you should not use the 
> "xxx_del()" function, and you should document it.

Besides, the code doesn't actually depend on not re-initializing the 
pointers, it depends on the _forward_ pointers still being walkable in 
case some other CPU is traversing the list just as we remove the entry.

Which means that I think the proper patch is to (a) document this and also
(b) poison the back pointer.

A patch like the attached, in short.

		Linus

---
===== include/linux/dcache.h 1.32 vs edited =====
--- 1.32/include/linux/dcache.h	Tue Jun 10 14:56:43 2003
+++ edited/include/linux/dcache.h	Thu Jun 12 09:12:27 2003
@@ -174,8 +174,10 @@
 
 static inline void __d_drop(struct dentry *dentry)
 {
-	dentry->d_vfs_flags |= DCACHE_UNHASHED;
-	hlist_del_rcu_init(&dentry->d_hash);
+	if (!(dentry->d_vfs_flags & DCACHE_UNHASHED)) {
+		dentry->d_vfs_flags |= DCACHE_UNHASHED;
+		hlist_del_rcu(&dentry->d_hash);
+	}
 }
 
 static inline void d_drop(struct dentry *dentry)
===== include/linux/list.h 1.32 vs edited =====
--- 1.32/include/linux/list.h	Tue Jun 10 15:46:31 2003
+++ edited/include/linux/list.h	Thu Jun 12 08:59:31 2003
@@ -152,14 +152,17 @@
 /**
  * list_del_rcu - deletes entry from list without re-initialization
  * @entry: the element to delete from the list.
+ *
  * Note: list_empty on entry does not return true after this, 
  * the entry is in an undefined state. It is useful for RCU based
  * lockfree traversal.
+ *
+ * In particular, it means that we can not poison the forward 
+ * pointers that may still be used for path walking.
  */
 static inline void list_del_rcu(struct list_head *entry)
 {
 	__list_del(entry->prev, entry->next);
-	entry->next = LIST_POISON1;
 	entry->prev = LIST_POISON2;
 }
 
@@ -431,7 +434,22 @@
 	n->pprev = LIST_POISON2;
 }
 
-#define hlist_del_rcu hlist_del  /* list_del_rcu is identical too? */
+/**
+ * hlist_del_rcu - deletes entry from hash list without re-initialization
+ * @entry: the element to delete from the list.
+ *
+ * Note: list_empty on entry does not return true after this, 
+ * the entry is in an undefined state. It is useful for RCU based
+ * lockfree traversal.
+ *
+ * In particular, it means that we can not poison the forward
+ * pointers that may still be used for path walking.
+ */
+static inline void hlist_del_rcu(struct hlist_node *n)
+{
+	__hlist_del(n);
+	n->pprev = LIST_POISON2;
+}
 
 static __inline__ void hlist_del_init(struct hlist_node *n) 
 {