We're sorry, the patch sent in the last e-mail contains some debug stuff inside. This new version is clean. Maybe on Wed, 17 Dec 2003 14:10:08 -0300 Juliano F Silva wrote: | Hi Rusty, | | Tiago (sapuglha@yahoo.com.br) and I (julianofs@pop.com.br) fixed the patch | as you suggested. | | We would like to comment your citation below. | | >This code is incorrect. The init section could be before or after the | > core section, meaning that all the init-related exceptions will be at | > the start, or at the end. So, the code would look like this: | | In this patch we classify the exception table by an increasing order. So to | verify if an exception belongs to the init section, we consider the address | of the init section (module_init) until module_init + init_size. In this | case, we believe that the address of the core section doesn't intervenes on | the code. | | Thanks a lot, | | Juliano F. Silva and Tiago S. Silva | | ----- Original Message ----- | From: "Rusty Russell" | To: "Tiago Sant' Anna" | Cc: ; ; | | Sent: Monday, December 01, 2003 3:03 AM | Subject: Re: New feature: Removal of the exceptions wich belongs to the init | section | | | > In message <20031128155853.33283fec.sapuglha@yahoo.com.br> you write: | > > Hello Rusty, | > | > Hi, | > | > > We, Juliano (julianofs@pop.com.br) and I (Tiago Sant' Anna | > > (sapuglha@yahoo.com.br) developed this patch. | > | > I've commented on your patch: I hope you find my feedback | > useful. | > | > > diff -Nur linux-2.6.0-test9-vanilla/arch/alpha/mm/extable.c | linux-2.6.0-test9-modified/arch/alpha/mm/extable.c | > > --- linux-2.6.0-test9-vanilla/arch/alpha/mm/extable.c 2003-10-25 | 16:42:52.000000000 -0200 | > > +++ linux-2.6.0-test9-modified/arch/alpha/mm/extable.c 2003-11-28 | 10:43:18.000000000 -0200 | > > @@ -27,3 +27,18 @@ | > > | > > return NULL; | > > } | > | > It's generally not a good idea to write code for architectures which | > you don't have access to, but to leave them for arch maintainers to | > sort out. | > | > > +/* Exception_table_entry Comparison. Only the field insn is considered. | > > + Results: | > > + equal: 0 | > > + ex1 less than ex2: -1 | > > + ex1 major than ex2: 1 | > > +*/ | > | > This comment is a little verbose: I would simply say: | > | > /* Compare two exception table entries: < 0 if ex1 comes before ex2. */ | > | > > +int extable_cmp(const struct exception_table_entry ex1, const struct | exception_table_entry ex2) { | > > + | > > + if (ex1.insn < ex2.insn) | > > + return(-1); | > > + else if (ex1.insn > ex2.insn) | > > + return(1); | > > + return(0); | > > +} | > | > And it can be implemented as "return (long)ex1.insn - ex2.insn;", | > making it a candidate for an inline function in the asm/uaccess.h | > header. | > | > > +/* Verify if the addr belongs to the init section */ | > > +static inline int within_mod_init_section(unsigned long addr, | > > + void *start, unsigned long | size) | > > +{ | > > + return ((void *)addr >= start && (void *)addr < start + size); | > > +} | > > + | > > +/* Remove exception table entries that point to init area. | > > + * It will be used in the unload of init section. | > > + */ | > > +void remove_init_exceptions(struct module *mod) { | > | > This comment is not quite true. The exception entries themselves are | > in the __ex_table section, which is in the module code. Some of the | > "insn" pointers are into the module init section, which is to be | > discarded. | > | > This function should be in kernel/extable.c. | > | > > + | > > + static spinlock_t init_ex_remove = SPIN_LOCK_UNLOCKED; | > | > Do not invent your own lock. You only need to protect against the | > exception table walk by other CPUs, for which modlist_lock is | > sufficient. | > | > > + | > > + const struct exception_table_entry *local; | > > + unsigned int i; | > > + int num_init_ex=0; | > > + | > > + | > > + local = mod->extable; | > > + i = 1; | > > + | > > + while (i <= mod->num_exentries) { | > > + if (within_mod_init_section((unsigned long) local->insn, | mod->module_init, mod->init_size)) { | > > + num_init_ex++; | > > + } | > > + else break; | > > + local = local+1; | > > + i++; | > > + } | > > + | > > + local = mod->extable; | > | > This code is incorrect. The init section could be before or after the | > core section, meaning that all the init-related exceptions will be at | > the start, or at the end. So, the code would look like this: | > | > void remove_init_exceptions(struct module *mod) | > { | > int i; | > | > if (!mod->module_init) | > return; | > | > spin_lock(&modlist_lock); | > if (mod->module_init > mod->module_core) { | > /* init exception entries at the end. Trim */ | > for (i = mod->num_exentries-1; i >= 0; i--) | > if (within(mod->extable[i].insn, | > mod->module_init, mod->init_size)) | > mod->num_exentries--; | > } else { | > /* init exception entries at the start. Move ptr. */ | > for (i = 0; i < mod->num_exentries; i++) { | > if (!within(mod->extable[i].insn, | > mod->module_init, mod->init_size)) | > break; | > } | > mod->extable += i; | > mod->num_exentries -= i; | > } | > spin_nulock(&modlist_lock); | > } | > | > > + /* Unload the init exceptions */ | > > + for(i=0; i < num_init_ex; ++i) | > > + kfree(local+i); | > | > This doesn't make sense: these are not pointers, but are part of | > module->core! They will be freed in the normal way. | > | > > /* Free memory returned from module_alloc */ | > > void module_free(struct module *mod, void *module_region) | > > -{ | > > +{ | > > + /* Remove exception entries of the init section */ | > > + if (module_region == mod->module_init) { | > > + remove_init_exceptions(mod); | > > + } | > > + | > | > Call this from kernel/module.c directly. | > | > > +++ linux-2.6.0-test9-modified/include/linux/extable.h 2003-11-28 10:4 | > 0:57.000000000 -0200 | > > @@ -0,0 +1,51 @@ | > > +#ifndef _EXTABLE_H | > > +#define _EXTABLE_H | > > + | > > +/* | > > + * Functions regarding to exception's table. | > > + * | > > + * * Written by Juliano Silva and Tiago Silva, 2003 | > > + * | > > + **/ | > > + | > > + | > > +#include | > > +#include | > > +#include | > > +#include | > > + | > > +/* Exception_table_entry Comparison. Only the field insn is considered. | > > + * Results: | > > + * equal: 0 | > > + * ex1 less than ex2: -1 | > > + * ex1 major than ex2: 1 | > > + * */ | > > +extern int extable_cmp(struct exception_table_entry ex1, | > > + struct exception_table_entry ex2); | > > + | > > +/* This code sorts an exception table. It is very used with modules | code | > > + * void __init_or_module sort_ex_table(struct exception_table_entry | *start, | > > + * struct exception_table_entry *finish); | > > + */ | > > +void __init_or_module sort_ex_table(struct exception_table_entry | *start, struct exception_table_entry *finish) | > | > Never put non-inline functions in a header file. And never inline a | > function this large. | > | > Since this is only called from module.c, perhaps put it in | > include/linux/moduleloader.h and the code in extable.c. | > | > Hope that helps, | > Rusty. | > -- | > Anyone who quotes me in their sig is an idiot. -- Rusty Russell. | > | -- => Tiago Sant' Anna - UIN: 14252973 - Linux user #136940 [.. ..]