I read the code and comments written by Nick Piggin(mainly from e286781d),
page_cache_get_speculative() is protected by rcu_read_lock(),
it may be preempted when preemptible RCU, so we must use get_page_unless_zero()
in this situation.

In the days of e286781d, we only have CLASSIC_RCU and (old)PREEMPT_RCU,
so "defined(CONFIG_CLASSIC_RCU)" means non-preemptible RCU and the code is 
correct.

In the days of b560d8ad, we only have TREE_RCU and TREE_PREEMPT_RCU,
so "defined(CONFIG_TREE_RCU)" means non-preemptible RCU and the code is correct.

But in nowadays, we have TREE_RCU, TREE_PREEMPT_RCU, TINY_RCU and 
TINY_PREEMPT_RCU,
so the "defined(CONFIG_TREE_RCU)" for non-preemptible RCU is incorrect, and it 
may causes bugs.
we should use "!defined(CONFIG_PREEMPT_RCU)" for non-preemptible RCU code block.

CC: Nick Piggin <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Reported-by: WANG Cong <[email protected]>
Signed-off-by: Lai Jiangshan <[email protected]>
---
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index c119506..a0e8070 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -133,7 +133,7 @@ static inline int page_cache_get_speculative(struct page 
*page)
 {
        VM_BUG_ON(in_interrupt());
 
-#if !defined(CONFIG_SMP) && defined(CONFIG_TREE_RCU)
+#if !defined(CONFIG_SMP) && !defined(CONFIG_PREEMPT_RCU)
 # ifdef CONFIG_PREEMPT
        VM_BUG_ON(!in_atomic());
 # endif
@@ -171,7 +171,7 @@ static inline int page_cache_add_speculative(struct page 
*page, int count)
 {
        VM_BUG_ON(in_interrupt());
 
-#if !defined(CONFIG_SMP) && defined(CONFIG_TREE_RCU)
+#if !defined(CONFIG_SMP) && !defined(CONFIG_PREEMPT_RCU)
 # ifdef CONFIG_PREEMPT
        VM_BUG_ON(!in_atomic());
 # endif

_______________________________________________
stable mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/stable

Reply via email to