Messages in this thread | | | Date | Sat, 10 Mar 2012 12:26:30 +0800 | From | Cong Wang <> | Subject | Re: [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.
| |