Messages in this thread Patch in this message | | | Date | Thu, 11 May 2006 23:07:27 +0200 | From | Rene Herman <> | Subject | Re: [patch] SMP alternatives: skip with UP kernels. |
| |
Gerd Hoffmann wrote:
[ depending on CONFIG_SMP ]
>> Okay, thanks. Yes, I agree such would make sense. > > Patch below. It simply returns in case the tables are empty and nothing > is do to, thus avoids printing the confusing message.
It does avoid the message but well, that's also _all_ it avoids. Why do you want to do it this way? If I apply the attached, things compile and work fine which seems to confirm that, yes, all this smp_alternatives code is simply dead baggage on !CONFIG_SMP.
Yes, the #ifdef in arch/i386/kernel/module.c is a bit clumsy. Just proof of concept, might be abstracted out a bit better. It does lose most of alternatives.c which would seem to be good thing.
Unless I'm missing some points, I believe this code should not be compiled in for !CONFIG_SMP.
One other point, although probably not a relevant one. Your changes in module_finalize() potentially change behaviour. It used to be:
for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) { void *seg; if (strcmp(".altinstructions", secstrings + s->sh_name)) continue; seg = (void *)s->sh_addr; apply_alternatives(seg, seg + s->sh_size); }
which means that any .altinstructions section would be patched. It's now:
for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) { ... if (!strcmp(".altinstructions", secstrings + s->sh_name)) alt = s; ... } if (alt) { /* patch .altinstructions */ void *aseg = (void *)alt->sh_addr; apply_alternatives(aseg, aseg + alt->sh_size); }
which means that only the last such section would. I suppose there's only one such section anyway, but not seeing a break in the original does make me wonder if this was originally done on purpose.
Rene.
Index: local/arch/i386/kernel/alternative.c =================================================================== --- local.orig/arch/i386/kernel/alternative.c 2006-05-11 20:28:56.000000000 +0200 +++ local/arch/i386/kernel/alternative.c 2006-05-11 20:32:24.000000000 +0200 @@ -118,6 +118,8 @@ void apply_alternatives(struct alt_instr } } +#ifdef CONFIG_SMP + static void alternatives_smp_save(struct alt_instr *start, struct alt_instr *end) { struct alt_instr *a; @@ -283,10 +285,13 @@ void alternatives_smp_switch(int smp) spin_unlock_irqrestore(&smp_alt, flags); } +#endif /* CONFIG_SMP */ + void __init alternative_instructions(void) { apply_alternatives(__alt_instructions, __alt_instructions_end); +#ifdef CONFIG_SMP /* switch to patch-once-at-boottime-only mode and free the * tables in case we know the number of CPUs will never ever * change */ @@ -318,4 +323,5 @@ void __init alternative_instructions(voi _text, _etext); alternatives_smp_switch(0); } +#endif } Index: local/arch/i386/kernel/module.c =================================================================== --- local.orig/arch/i386/kernel/module.c 2006-05-11 20:28:56.000000000 +0200 +++ local/arch/i386/kernel/module.c 2006-05-11 20:29:00.000000000 +0200 @@ -112,12 +112,14 @@ int module_finalize(const Elf_Ehdr *hdr, char *secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset; for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++) { - if (!strcmp(".text", secstrings + s->sh_name)) - text = s; if (!strcmp(".altinstructions", secstrings + s->sh_name)) alt = s; +#ifdef CONFIG_SMP if (!strcmp(".smp_locks", secstrings + s->sh_name)) locks= s; + if (!strcmp(".text", secstrings + s->sh_name)) + text = s; +#endif } if (alt) { @@ -126,16 +128,20 @@ int module_finalize(const Elf_Ehdr *hdr, apply_alternatives(aseg, aseg + alt->sh_size); } if (locks && text) { +#ifdef CONFIG_SMP void *lseg = (void *)locks->sh_addr; void *tseg = (void *)text->sh_addr; alternatives_smp_module_add(me, me->name, lseg, lseg + locks->sh_size, tseg, tseg + text->sh_size); +#endif } return 0; } void module_arch_cleanup(struct module *mod) { +#ifdef CONFIG_SMP alternatives_smp_module_del(mod); +#endif } | |