Re: 2.6.25-git2: BUG: unable to handle kernel paging request atffffffffffffffff

From: Linus Torvalds
Date: Sun Apr 20 2008 - 17:33:46 EST




On Sun, 20 Apr 2008, Rafael J. Wysocki wrote:
>
> I've just got the following traces from 2.6.25-git2 on HP nx6325 (64-bit).
> I think they are related to the hang I described yesterday:
>
> [12844.066757] BUG: unable to handle kernel paging request at ffffffffffffffff

Something has added a dentry pointer that has the value -1 to the dentry
hash list. The access that oopses seems to be the

prefetch(pos->next)

which is part of hlist_for_each_entry_rcu(), where "pos" is -1.

I suspect it's an RCU error, ie somebody has released a dentry entry, and
free'd it without waiting for the RCU grace period.

Talking about RCU I also think that whoever did those "rcu_dereference()"
macros in <linux/list.h> was insane. It's totally pointless to do
"rcu_dereference()" on a local variable. It simply *cannot* make sense.
Herbert, Paul, you guys should look at it.

As far as I can tell, rcu_dereference() should _always_ be done when we
access the "next" pointer (except for when prefetching, where we simply
don't care).

Paul? Herbert? Totally untested patch appended.

NOTE! I do not expect this patch to matter for this oops. There's
something else going on there.

Linus

---
include/linux/list.h | 34 +++++++++++++++++-----------------
1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/include/linux/list.h b/include/linux/list.h
index 75ce2cb..4a851ba 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -631,14 +631,14 @@ static inline void list_splice_init_rcu(struct list_head *list,
* as long as the traversal is guarded by rcu_read_lock().
*/
#define list_for_each_rcu(pos, head) \
- for (pos = (head)->next; \
- prefetch(rcu_dereference(pos)->next), pos != (head); \
- pos = pos->next)
+ for (pos = rcu_dereference((head)->next); \
+ prefetch(pos->next), pos != (head); \
+ pos = rcu_dereference(pos->next))

#define __list_for_each_rcu(pos, head) \
- for (pos = (head)->next; \
- rcu_dereference(pos) != (head); \
- pos = pos->next)
+ for (pos = rcu_dereference((head)->next); \
+ pos != (head); \
+ pos = rcu_dereference(pos->next))

/**
* list_for_each_safe_rcu
@@ -653,8 +653,8 @@ static inline void list_splice_init_rcu(struct list_head *list,
* as long as the traversal is guarded by rcu_read_lock().
*/
#define list_for_each_safe_rcu(pos, n, head) \
- for (pos = (head)->next; \
- n = rcu_dereference(pos)->next, pos != (head); \
+ for (pos = rcu_dereference((head)->next); \
+ n = rcu_dereference((pos)->next), pos != (head); \
pos = n)

/**
@@ -668,10 +668,10 @@ static inline void list_splice_init_rcu(struct list_head *list,
* as long as the traversal is guarded by rcu_read_lock().
*/
#define list_for_each_entry_rcu(pos, head, member) \
- for (pos = list_entry((head)->next, typeof(*pos), member); \
- prefetch(rcu_dereference(pos)->member.next), \
+ for (pos = list_entry(rcu_dereference((head)->next), typeof(*pos), member); \
+ prefetch(pos->member.next), \
&pos->member != (head); \
- pos = list_entry(pos->member.next, typeof(*pos), member))
+ pos = list_entry(rcu_dereference(pos->member.next), typeof(*pos), member))


/**
@@ -686,9 +686,9 @@ static inline void list_splice_init_rcu(struct list_head *list,
* as long as the traversal is guarded by rcu_read_lock().
*/
#define list_for_each_continue_rcu(pos, head) \
- for ((pos) = (pos)->next; \
- prefetch(rcu_dereference((pos))->next), (pos) != (head); \
- (pos) = (pos)->next)
+ for ((pos) = rcu_dereference((pos)->next); \
+ prefetch((pos)->next), (pos) != (head); \
+ (pos) = rcu_dereference((pos)->next))

/*
* Double linked lists with a single pointer list head.
@@ -986,10 +986,10 @@ static inline void hlist_add_after_rcu(struct hlist_node *prev,
* as long as the traversal is guarded by rcu_read_lock().
*/
#define hlist_for_each_entry_rcu(tpos, pos, head, member) \
- for (pos = (head)->first; \
- rcu_dereference(pos) && ({ prefetch(pos->next); 1;}) && \
+ for (pos = rcu_dereference((head)->first); \
+ ({ prefetch(pos->next); 1;}) && \
({ tpos = hlist_entry(pos, typeof(*tpos), member); 1;}); \
- pos = pos->next)
+ pos = rcu_dereference(pos->next))

#else
#warning "don't include kernel headers in userspace"
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/