Re: [PATCH 1/2] module: use rcu to protect module list read

From: Cong Wang
Date: Tue Mar 13 2012 - 06:10:16 EST


On 03/13/2012 08:37 AM, Rusty Russell wrote:
On Sat, 10 Mar 2012 07:25:57 -0800, Eric Dumazet<eric.dumazet@xxxxxxxxx> wrote:
Le samedi 10 mars 2012 Ã 22:20 +0800, Cong Wang a Ãcrit :
Now the read of module list is protected by preempt disable + *_rcu
list operations, this is odd, as RCU read lock should be able to
protect it directly. This patch makes the read of module list
protected by RCU read lock and the write still protected by
module_mutex.


Problem is that your patch does more than that.

In set_all_modules_text_rw() and set_all_modules_text_ro() you removed
the mutex in favor of rcu_read_lock()

Also, module code uses synchronize_sched(), not synchronize_rcu()

Yes, but only for paranoia. Really, it's vs. stop_machine().

Take a look at Documentation/RCU/whatisRCU.txt and see that
preempt_disable() / preempt_enable() are documented as a right protect
code, in line 333.

You added races in /proc/modules as well.

I'm surprised that patch didn't warn... CONFIG_DEBUG_ATOMIC_SLEEP=y
might help here....

Ok I will enable it for testing.

Thanks.
--
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/