lkml.org 
[lkml]   [2012]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/2] Refine mutex and rcu method in module.c, kernel<3.2.9>
    On 03/09/2012 12:13 AM, Chen, Dennis (SRDC SW) wrote:
    > Hi Rusty,
    >
    > Pls notice the following change in the patch (in set_all_modules_text_ro function):
    >
    > /* Iterate through all modules and set each module's text as RO */
    > @@ -1693,7 +1699,7 @@
    > {
    > struct module *mod;
    >
    > - mutex_lock(&module_mutex);
    > + rcu_read_lock();
    > list_for_each_entry_rcu(mod,&modules, list) {
    > if ((mod->module_core)&& (mod->core_text_size)) {
    > set_page_attributes(mod->module_core,
    > @@ -1706,7 +1712,7 @@
    > set_memory_ro);
    > }
    > }
    > - mutex_unlock(&module_mutex);
    > + rcu_read_unlock();
    > }
    >
    > This function just needs to iterate the modules list, but now it holds a unnecessary lock when it does that,
    > The other module can't be inserted during this operation, also can you make sure the set_page_attributes will
    > run smoothly all the time, if not it's a risk action to hold a lock.
    > So summary--
    > I think the idea for kernel module protection is simple:
    > Writers to modules, use mutex_lock
    > Readers, use rcu. __ALL__ codes here should be with a unified style! This will make our kernel gracefully.
    >
    > PS: my comments in the patch " /* Concurrent writers for the global modules list are protected by RCU*/" is not right, RCU
    > Should be mutex lock.

    I think your change makes sense, I don't know why preempt_disable() was
    used, git blame told me the related two commits are 4 years-old...

    cb2a5205 2008-01-14 00:55:03 -0800 3180
    d72b3751 2008-08-30 10:09:00 +0200 3181

    maybe at that time rcu was not what it is today... Cc'ing Paul.


    \
     
     \ /
      Last update: 2012-03-10 05:29    [W:0.022 / U:29.680 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site