On Mon, Apr 25, 2011 at 04:20:37PM +0800, Lai Jiangshan wrote: > 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]>
Good catch!!! Acked-by: Paul E. McKenney <[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
