2005-04-03 04:41:52

by Paul E. McKenney

[permalink] [raw]
Subject: [RFC, PATCH] add rcu_read_lock in ItLpQueue_process()

Hello!

I believe that the synchronize_kernel() needs a matching
rcu_read_lock() and rcu_read_unlock() pair as shown below,
along with an rcu_dereference(). Without these, I believe
that the following sequence of events could occur:

o CPU 0 in ItLpQueue_process() tests the lpEventHandler
element, and finds it non-NULL, proceeding into the
"then" clause.

o CPU 1 in HvLpEvent_unregisterHandler() sets the element
to NULL.

o CPU 0 picks up the lpEventHandler once more, and does
a function call through the now-NULL pointer.

That said, there might be some higher-level locking that I missed
that prevents this...

Thanx, Paul

Signed-off-by: <[email protected]>

diff -urpN -X dontdiff linux-2.6.12-rc1/arch/ppc64/kernel/ItLpQueue.c linux-2.6.12-rc1-ppcfix/arch/ppc64/kernel/ItLpQueue.c
--- linux-2.6.12-rc1/arch/ppc64/kernel/ItLpQueue.c Tue Mar 1 23:37:48 2005
+++ linux-2.6.12-rc1-ppcfix/arch/ppc64/kernel/ItLpQueue.c Sat Apr 2 20:36:16 2005
@@ -107,6 +107,7 @@ unsigned ItLpQueue_process( struct ItLpQ
{
unsigned numIntsProcessed = 0;
struct HvLpEvent * nextLpEvent;
+ LpEventHandler func;

/* If we have recursed, just return */
if ( !set_inUse( lpQueue ) )
@@ -140,9 +141,12 @@ unsigned ItLpQueue_process( struct ItLpQ
*/
if ( nextLpEvent->xType < HvLpEvent_Type_NumTypes )
lpQueue->xLpIntCountByType[nextLpEvent->xType]++;
- if ( nextLpEvent->xType < HvLpEvent_Type_NumTypes &&
- lpEventHandler[nextLpEvent->xType] )
- lpEventHandler[nextLpEvent->xType](nextLpEvent, regs);
+ if ( nextLpEvent->xType < HvLpEvent_Type_NumTypes )
+ rcu_read_lock();
+ func = rcu_dereference(lpEventHandler[nextLpEvent->xType]);
+ if (func)
+ func(nextLpEvent, regs);
+ rcu_read_unlock();
else
printk(KERN_INFO "Unexpected Lp Event type=%d\n", nextLpEvent->xType );