Some random x86 patches that should all go into git-x86
Subsystems (4)
cover Some random x86 patches that should all go into git-x86
Some are reposts, some are not. See patch descriptions for details. I believe I addressed all feedback that made sense in the reposted patches.
-Andi
PATCH 1/7 i386: Move MWAIT idle check to generic CPU initialization
Previously it was only run for Intel CPUs, but AMD Fam10h implements MWAIT too.
This matches 64bit behaviour.
--- arch/x86/kernel/cpu/common.c | 2 ++ arch/x86/kernel/cpu/intel.c | 1 - 2 files changed, 2 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/common.c +2 -0
@@ -499,6 +499,8 @@ void __cpuinit identify_cpu(struct cpuin/* Init Machine Check Exception if available. */ mcheck_init (c); + +select_idle_routine (c); }void __init identify_boot_cpu (void )
arch/x86/kernel/cpu/intel.c +0 -1
@@ -137,7 +137,6 @@ static void __cpuinit init_intel(struct }#endif -select_idle_routine (c); l2 =init_intel_cacheinfo (c);if (c->cpuid_level >9 ) {unsigned eax =cpuid_eax (10 );
PATCH 2/7 Use the correct cpuid method to detect MWAIT support for C states
In reply to Andi KleenPreviously there was a AMD specific quirk to handle the case of AMD Fam10h MWAIT not supporting any C states. But it turns out that CPUID already has ways to detectly detect that without using special quirks.
The new code simply checks if MWAIT supports at least C1 and doesn't use it if it doesn't. No more vendor specific code.
Note this is does not simply clear MWAIT because MWAIT can be still useful even without C states.
Credit goes to Ben Serebrin for pointing out the (nearly) obvious.
--- arch/x86/kernel/cpu/amd.c | 3 --- arch/x86/kernel/process_32.c | 10 +++++++++- arch/x86/kernel/process_64.c | 11 ++++++++++- arch/x86/kernel/setup_64.c | 4 ---- 4 files changed, 19 insertions(+), 9 deletions(-)
arch/x86/kernel/cpu/amd.c +0 -3
@@ -300,9 +300,6 @@ static void __cpuinit init_amd(struct cp local_apic_timer_disabled =1 ;#endif -if (c->x86 ==0x10 && !force_mwait) -clear_bit (X86_FEATURE_MWAIT , c->x86_capability); -/* K6s reports MCEs but don't actually have all the MSRs */ if (c->x86 <6 )clear_bit (X86_FEATURE_MCE , c->x86_capability);
arch/x86/kernel/process_32.c +9 -1
@@ -285,9 +285,17 @@ static void mwait_idle(void)mwait_idle_with_hints (0 ,0 ); } +static int mwait_usable (const struct cpuinfo_x86 *c) +{ +if (force_mwait) +return 1 ; +/* Any C1 states supported? */ +return c->cpuid_level >=5 && ((cpuid_edx (5 ) >>4 ) &0xf ) >0 ; +} +void __cpuinit select_idle_routine (const struct cpuinfo_x86 *c) { -if (cpu_has (c,X86_FEATURE_MWAIT )) { +if (cpu_has (c,X86_FEATURE_MWAIT ) &&mwait_usable (c)) {printk ("monitor/mwait feature present.\n" );/* * Skip, if setup has overridden idle.
arch/x86/kernel/process_64.c +10 -1
@@ -280,10 +280,19 @@ static void mwait_idle(void) } } + +static int mwait_usable (const struct cpuinfo_x86 *c) +{ +if (force_mwait) +return 1 ; +/* Any C1 states supported? */ +return c->cpuid_level >=5 && ((cpuid_edx (5 ) >>4 ) &0xf ) >0 ; +} +void __cpuinit select_idle_routine (const struct cpuinfo_x86 *c) {static int printed; -if (cpu_has (c,X86_FEATURE_MWAIT )) { +if (cpu_has (c,X86_FEATURE_MWAIT ) &&mwait_usable (c)) {/* * Skip, if setup has overridden idle. * One CPU supports mwait => All CPUs supports mwait
arch/x86/kernel/setup_64.c +0 -4
@@ -778,10 +778,6 @@ static void __cpuinit init_amd(struct cp/* MFENCE stops RDTSC speculation */ set_cpu_cap (c,X86_FEATURE_MFENCE_RDTSC ); -/* Family 10 doesn't support C states in MWAIT so don't use it */ -if (c->x86 ==0x10 && !force_mwait) -clear_cpu_cap (c,X86_FEATURE_MWAIT ); -if (amd_apic_timer_broken ()) disable_apic_timer =1 ; }
PATCH 3/7 Use shorter addresses in i386 segfault printks
In reply to Andi Kleen--- arch/x86/mm/fault_32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/mm/fault_32.c +1 -1
@@ -516,7 +516,7 @@ bad_area_nosemaphore: printk_ratelimit()) { printk(#ifdef CONFIG_X86_32 - "%s%s[%d]: segfault at %08lx ip %08lx sp %08lx error %lx\n", + "%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx\n", #else "%s%s[%d]: segfault at %lx ip %lx sp %lx error %lx\n",#endif
PATCH 4/7 Print which shared library/executable faulted in segfault etc. messages
In reply to Andi KleenThey now look like
hal-resmgr[13791]: segfault at 3c rip 2b9c8caec182 rsp 7fff1e825d30 error 4 in libacl.so.1.1.0[2b9c8caea000+6000]
This makes it easier to pinpoint bugs to specific libraries.
And printing the offset into a mapping also always allows to find the correct fault point in a library even with randomized mappings. Previously there was no way to actually find the correct code address inside the randomized mapping.
Relies on earlier patch to shorten the printk formats.
They are often now longer than 80 characters, but I think that's worth it.
Patch for i386 and x86-64.
[includes fix from Eric Dumazet to check d_path error value]
--- arch/x86/kernel/signal_32.c | 7 +++++-- arch/x86/kernel/signal_64.c | 7 +++++-- arch/x86/kernel/traps_32.c | 7 +++++-- arch/x86/mm/fault_32.c | 4 +++- include/linux/mm.h | 1 + mm/memory.c | 31 +++++++++++++++++++++++++++++++ 6 files changed, 50 insertions(+), 7 deletions(-)
include/linux/mm.h +1 -0
@@ -1145,6 +1145,7 @@ extern int randomize_va_space;#endif const char *arch_vma_name (struct vm_area_struct *vma); +void print_vma_addr (char *prefix,unsigned long rip);struct page *sparse_mem_map_populate (unsigned long pnum,int nid);pgd_t *vmemmap_pgd_populate (unsigned long addr,int node);
mm/memory.c +31 -0
@@ -2746,3 +2746,34 @@ int access_process_vm(struct task_structreturn buf - old_buf; } + +/* +* Print the name of a VMA. +*/ +void print_vma_addr (char *prefix,unsigned long ip) +{ +struct mm_struct *mm = current->mm; +struct vm_area_struct *vma; + +down_read (&mm->mmap_sem); + vma =find_vma (mm, ip); +if (vma && vma->vm_file) { +struct file *f = vma->vm_file; +char *buf = (char *)__get_free_page (GFP_KERNEL ); +if (buf) { +char *p, *s; + + p =d_path (f->f_dentry, f->f_vfsmnt, buf,PAGE_SIZE ); +if (IS_ERR (p)) + p ="?" ; + s =strrchr (p,'/' ); +if (s) + p = s+1 ; +printk ("%s%s[%lx+%lx]" , prefix, p, + vma->vm_start, + vma->vm_end - vma->vm_start); +free_page ((unsigned long )buf); + } + } +up_read (¤t->mm->mmap_sem); +}
arch/x86/kernel/signal_32.c +5 -2
@@ -198,12 +198,15 @@ asmlinkage int sys_sigreturn(unsigned loreturn ax; badframe: -if (show_unhandled_signals &&printk_ratelimit ()) +if (show_unhandled_signals &&printk_ratelimit ()) {printk ("%s%s[%d] bad frame in sigreturn frame:%p ip:%lx" -" sp:%lx oeax:%lx\n" , +" sp:%lx oeax:%lx" ,task_pid_nr (current) >1 ?KERN_INFO :KERN_EMERG , current->comm,task_pid_nr (current), frame, regs->ip, regs->sp, regs->orig_ax); +print_vma_addr (" in " , regs->ip); +printk ("\n" ); + }force_sig (SIGSEGV , current);return 0 ;
arch/x86/kernel/signal_64.c +5 -2
@@ -481,9 +481,12 @@ do_notify_resume(struct pt_regs *regs, vvoid signal_fault (struct pt_regs *regs,void __user *frame,char *where) {struct task_struct *me = current; -if (show_unhandled_signals &&printk_ratelimit ()) -printk ("%s[%d] bad frame in %s frame:%p ip:%lx sp:%lx orax:%lx\n" , +if (show_unhandled_signals &&printk_ratelimit ()) { +printk ("%s[%d] bad frame in %s frame:%p ip:%lx sp:%lx orax:%lx" , me->comm,me->pid,where,frame,regs->ip,regs->sp,regs->orig_ax); +print_vma_addr (" in " , regs->ip); +printk ("\n" ); + }force_sig (SIGSEGV , me); }
arch/x86/kernel/traps_32.c +5 -2
@@ -608,11 +608,14 @@ void __kprobes do_general_protection(str current->thread.error_code = error_code; current->thread.trap_no =13 ;if (show_unhandled_signals &&unhandled_signal (current,SIGSEGV ) && -printk_ratelimit ()) +printk_ratelimit ()) {printk (KERN_INFO -"%s[%d] general protection ip:%lx sp:%lx error:%lx\n" , +"%s[%d] general protection ip:%lx sp:%lx error:%lx" , current->comm,task_pid_nr (current), regs->ip, regs->sp, error_code); +print_vma_addr (" in " , regs->ip); +printk ("\n" ); + }force_sig (SIGSEGV , current);return ;
arch/x86/mm/fault_32.c +3 -1
@@ -518,11 +518,13 @@ bad_area_nosemaphore:#ifdef CONFIG_X86_32 "%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx\n" , #else -"%s%s[%d]: segfault at %lx ip %lx sp %lx error %lx\n" , +"%s%s[%d]: segfault at %lx ip %lx sp %lx error %lx" , #endiftask_pid_nr (tsk) >1 ?KERN_INFO :KERN_EMERG , tsk->comm,task_pid_nr (tsk), address, regs->ip, regs->sp, error_code); +print_vma_addr (" in " , regs->ip); + printk("\n"); } tsk->thread.cr2 = address;/* Kernel addresses are always protection faults */
PATCH 6/7 Optimize lock prefix switching to run less frequently v2
In reply to Andi KleenOn VMs implemented using JITs that cache translated code changing the lock prefixes is a quite costly operation that forces the JIT to throw away and retranslate a lot of code.
Previously a SMP kernel would rewrite the locks once for each CPU which is quite unnecessary. This patch changes the code to never switch at boot in
the normal case (SMP kernel booting with >1 CPU) or only once for SMP kernel
on UP.
This makes a significant difference in boot up performance on AMD SimNow! Also I expect it to be a little faster on native systems too because a smp switch does a lot of text_poke()s which each synchronize the pipeline.
v1->v2: Rename max_cpusv1->v2: Fix off by one in UP check (Thomas Gleixner)
--- arch/x86/kernel/alternative.c | 16 ++++++++++++++-- include/linux/smp.h | 2 ++ init/main.c | 16 ++++++++-------- 3 files changed, 24 insertions(+), 10 deletions(-)
arch/x86/kernel/alternative.c +14 -2
@@ -273,6 +273,7 @@ struct smp_alt_module { };static LIST_HEAD (smp_alt_modules);static DEFINE_SPINLOCK (smp_alt); +static int smp_mode =1 ;/* protected by smp_alt */ void alternatives_smp_module_add(struct module *mod, char *name, void *locks, void *locks_end,
@@ -354,7 +355,14 @@ void alternatives_smp_switch(int smp)BUG_ON (!smp && (num_online_cpus () >1 ));spin_lock_irqsave (&smp_alt, flags); -if (smp) { + +/* +* Avoid unnecessary switches because it forces JIT based VMs to +* throw away all cached translations, which can be quite costly. +*/ +if (smp == smp_mode) { +/* nothing */ + }else if (smp) {printk (KERN_INFO "SMP alternatives: switching to SMP code\n" );clear_cpu_cap (&boot_cpu_data,X86_FEATURE_UP );clear_cpu_cap (&cpu_data (0 ),X86_FEATURE_UP );
@@ -369,6 +377,7 @@ void alternatives_smp_switch(int smp)alternatives_smp_unlock (mod->locks, mod->locks_end, mod->text, mod->text_end); } + smp_mode = smp;spin_unlock_irqrestore (&smp_alt, flags); }
@@ -441,7 +450,10 @@ void __init alternative_instructions(voialternatives_smp_module_add (NULL ,"core kernel" , __smp_locks, __smp_locks_end, _text, _etext); - alternatives_smp_switch(0); + +/* Only switch to UP mode if we don't immediately boot others */ +if (num_possible_cpus () ==1 || setup_max_cpus <=1 ) + alternatives_smp_switch(0); }#endif apply_paravirt (__parainstructions, __parainstructions_end);
include/linux/smp.h +2 -0
@@ -78,6 +78,8 @@ int on_each_cpu(void (*func) (void *info*/ void smp_prepare_boot_cpu (void ); +extern unsigned int setup_max_cpus; +#else /* !SMP */ /*
init/main.c +8 -8
@@ -128,7 +128,7 @@ static char *ramdisk_execute_command;#ifdef CONFIG_SMP /* Setup configured maximum number of CPUs to activate */ -static unsigned int __initdata max_cpus =NR_CPUS ; +unsigned int __initdata setup_max_cpus =NR_CPUS ;/* * Setup routine for controlling SMP activation
@@ -146,7 +146,7 @@ static inline void disable_ioapic_setup(static int __init nosmp (char *str) { - max_cpus =0 ; + setup_max_cpus =0 ;disable_ioapic_setup ();return 0 ; }
@@ -155,8 +155,8 @@ early_param("nosmp", nosmp);static int __init maxcpus (char *str) { -get_option (&str, &max_cpus); -if (max_cpus ==0 ) +get_option (&str, &setup_max_cpus); +if (setup_max_cpus ==0 )disable_ioapic_setup ();return 0 ;
@@ -164,7 +164,7 @@ static int __init maxcpus(char *str)early_param ("maxcpus" , maxcpus);#else -#define max_cpus NR_CPUS +#define setup_max_cpus NR_CPUS#endif /*
@@ -393,7 +393,7 @@ static void __init smp_init(void)/* FIXME: This should be done in userspace --RR */ for_each_present_cpu (cpu) { -if (num_online_cpus () >= max_cpus) +if (num_online_cpus () >= setup_max_cpus)break ;if (!cpu_online (cpu))cpu_up (cpu);
@@ -401,7 +401,7 @@ static void __init smp_init(void)/* Any cleanup work */ printk (KERN_INFO "Brought up %ld CPUs\n" , (long )num_online_cpus ()); - smp_cpus_done(max_cpus); + smp_cpus_done(setup_max_cpus); }#endif
@@ -823,7 +823,7 @@ static int __init kernel_init(void * unu__set_special_pids (1 ,1 ); cad_pid =task_pid (current); -smp_prepare_cpus (max_cpus); +smp_prepare_cpus (setup_max_cpus);do_pre_smp_initcalls ();
PATCH 7/7 Don't disable the APIC if it hasn't been mapped yet
In reply to Andi KleenWhen the kernel panics early for some unrelated reason there would be eventually an early exception inside panic because clear_local_APIC tried to disable the not yet mapped APIC. Check for that explicitely.
--- arch/x86/kernel/apic_32.c | 11 ++++++++--- arch/x86/kernel/apic_64.c | 9 +++++++-- 2 files changed, 15 insertions(+), 5 deletions(-)
arch/x86/kernel/apic_32.c +8 -3
@@ -99,6 +99,8 @@ static DEFINE_PER_CPU(struct clock_event/* Local APIC was disabled by the BIOS and enabled by the kernel */ static int enabled_via_apicbase; +static unsigned long apic_phys; +/* * Get the LAPIC version */
@@ -616,9 +618,14 @@ int setup_profiling_timer(unsigned int m*/ void clear_local_APIC (void ) { -int maxlvt =lapic_get_maxlvt (); +int maxlvt;u32 v; +/* APIC hasn't been mapped yet */ +if (!apic_phys) +return ; + + maxlvt =lapic_get_maxlvt ();/* * Masking an LVT entry can trigger a local APIC error * if the vector is zero. Mask LVTERR first to prevent this.
@@ -1105,8 +1112,6 @@ no_apic:*/ void __init init_apic_mappings (void ) { -unsigned long apic_phys; -/* * If no local APIC can be found then set up a fake all * zeroes page to simulate the local APIC and another
arch/x86/kernel/apic_64.c +7 -2
@@ -81,6 +81,8 @@ static struct clock_event_device lapic_c }; static DEFINE_PER_CPU(struct clock_event_device, lapic_events); +static unsigned long apic_phys; +/* * Get the LAPIC version */
@@ -516,6 +518,11 @@ void clear_local_APIC(void)int maxlvt =lapic_get_maxlvt ();u32 v; +/* APIC hasn't been mapped yet */ +if (!apic_phys) +return ; + + maxlvt =lapic_get_maxlvt ();/* * Masking an LVT entry can trigger a local APIC error * if the vector is zero. Mask LVTERR first to prevent this.
@@ -850,8 +857,6 @@ static int __init detect_init_APIC(void)*/ void __init init_apic_mappings (void ) { -unsigned long apic_phys; -/* * If no local APIC can be found then set up a fake all * zeroes page to simulate the local APIC and another
PATCH 5/7 Replace hard coded reservations in x86-64 early boot code with dynamic table v2
In reply to Andi KleenOn x86-64 there are several memory allocations before bootmem. To avoid them stomping on each other they used to be all hard coded in bad_area(). Replace this with an array that is filled as needed.
This cleans up the code considerably and allows to expand its use.
v1->v2: add one tab
--- arch/x86/kernel/e820_64.c | 95 ++++++++++++++++++++++++--------------------- arch/x86/kernel/head64.c | 48 ++++++++++++++++++++++ arch/x86/kernel/setup_64.c | 72 +--------------------------------- arch/x86/mm/init_64.c | 5 +- arch/x86/mm/numa_64.c | 1 include/asm-x86/e820_64.h | 4 - include/asm-x86/proto.h | 2 7 files changed, 110 insertions(+), 117 deletions(-)
arch/x86/kernel/e820_64.c +52 -43
@@ -47,56 +47,65 @@ unsigned long end_pfn_map;*/ static unsigned long __initdata end_user_pfn =MAXMEM >>PAGE_SHIFT ; -/* Check for some hardcoded bad areas that early boot is not allowed to touch */ -static inline int bad_addr (unsigned long *addrp,unsigned long size) -{ -unsigned long addr = *addrp, last = addr + size; - -/* various gunk below that needed for SMP startup */ -if (addr <0x8000 ) { - *addrp =PAGE_ALIGN (0x8000 ); -return 1 ; - } - -/* direct mapping tables of the kernel */ -if (last >= table_start<<PAGE_SHIFT && addr < table_end<<PAGE_SHIFT ) { - *addrp =PAGE_ALIGN (table_end <<PAGE_SHIFT ); -return 1 ; - } - -/* initrd */ -#ifdef CONFIG_BLK_DEV_INITRD -if (boot_params.hdr.type_of_loader && boot_params.hdr.ramdisk_image) { -unsigned long ramdisk_image = boot_params.hdr.ramdisk_image; -unsigned long ramdisk_size = boot_params.hdr.ramdisk_size; -unsigned long ramdisk_end = ramdisk_image+ramdisk_size; +/* +* Early reserved memory areas. +*/ +#define MAX_EARLY_RES 20 -if (last >= ramdisk_image && addr < ramdisk_end) { - *addrp =PAGE_ALIGN (ramdisk_end); -return 1 ; - } - } +struct early_res { +unsigned long start, end; +}; +static struct early_res early_res[MAX_EARLY_RES ]__initdata = { + {0 ,PAGE_SIZE },/* BIOS data page */ +#ifdef CONFIG_SMP + {SMP_TRAMPOLINE_BASE ,SMP_TRAMPOLINE_BASE +2 *PAGE_SIZE }, #endif -/* kernel code */ -if (last >=__pa_symbol (&_text) && addr <__pa_symbol (&_end)) { - *addrp =PAGE_ALIGN (__pa_symbol (&_end)); -return 1 ; + {} +}; + +void __init reserve_early (unsigned long start,unsigned long end) +{ +int i; +struct early_res *r; +for (i =0 ; i <MAX_EARLY_RES && early_res[i].end; i++) { + r = &early_res[i]; +if (end > r->start && start < r->end) +panic ("Duplicated early reservation %lx-%lx\n" , + start, end); } +if (i >=MAX_EARLY_RES ) +panic ("Too many early reservations" ); + r = &early_res[i]; + r->start = start; + r->end = end; +} -if (last >= ebda_addr && addr < ebda_addr + ebda_size) { - *addrp =PAGE_ALIGN (ebda_addr + ebda_size); -return 1 ; +void __init early_res_to_bootmem (void ) +{ +int i; +for (i =0 ; i <MAX_EARLY_RES && early_res[i].end; i++) { +struct early_res *r = &early_res[i]; +reserve_bootmem_generic (r->start, r->end - r->start); } +} -#ifdef CONFIG_NUMA -/* NUMA memory to node map */ -if (last >= nodemap_addr && addr < nodemap_addr + nodemap_size) { - *addrp = nodemap_addr + nodemap_size; -return 1 ; +/* Check for already reserved areas */ +static inline int bad_addr (unsigned long *addrp,unsigned long size) +{ +int i; +unsigned long addr = *addrp, last; +int changed =0 ; +again: + last = addr + size; +for (i =0 ; i <MAX_EARLY_RES && early_res[i].end; i++) { +struct early_res *r = &early_res[i]; +if (last >= r->start && addr < r->end) { + *addrp = addr = r->end; + changed =1 ; + goto again; + } } -#endif -/* XXX ramdisk image here? */ -return 0 ; +return changed; }/*
arch/x86/kernel/head64.c +48 -0
65 lines not on this page · raw message
arch/x86/kernel/setup_64.c +3 -69
96 lines not on this page · raw message
arch/x86/mm/numa_64.c +1 -0
@@ -104,6 +104,7 @@ static int __init allocate_cachealigned_ } pad_addr = (nodemap_addr + pad) & ~pad; memnodemap =phys_to_virt (pad_addr); +reserve_early (nodemap_addr, nodemap_addr + nodemap_size);printk (KERN_DEBUG "NUMA: Allocated memnodemap from %lx - %lx\n" , nodemap_addr, nodemap_addr + nodemap_size);
include/asm-x86/e820_64.h +2 -2
@@ -41,8 +41,8 @@ extern void finish_e820_parsing(void);extern struct e820map e820;extern void update_e820 (void ); -extern unsigned ebda_addr, ebda_size; -extern unsigned long nodemap_addr, nodemap_size; +extern void reserve_early (unsigned long start,unsigned long end); +extern void early_res_to_bootmem (void );#endif /*!__ASSEMBLY__*/
arch/x86/mm/init_64.c +4 -1
@@ -176,7 +176,8 @@ __set_fixmap (enum fixed_addresses idx,set_pte_phys (address, phys, prot); } -unsigned long __meminitdata table_start, table_end; +static unsigned long __initdata table_start; +static unsigned long __meminitdata table_end;static __meminit void *alloc_low_page (unsigned long *phys) {
@@ -391,6 +392,8 @@ void __init_refok init_memory_mapping(unif (!after_bootmem) mmu_cr4_features =read_cr4 ();__flush_tlb_all (); + +reserve_early (table_start <<PAGE_SHIFT , table_end <<PAGE_SHIFT ); }#ifndef CONFIG_NUMA
include/asm-x86/proto.h +0 -2
@@ -22,8 +22,6 @@ extern void syscall32_cpu_init(void);extern void check_efer (void ); -extern unsigned long table_start, table_end; -extern int reboot_force;long do_arch_prctl (struct task_struct *task,int code,unsigned long addr);
Re: [PATCH] [3/7] Use shorter addresses in i386 segfault printks
In reply to Andi KleenQuoted 4 lines · Andi Kleen
Signed-off-by Andi Kleen <[email protected]>--- arch/x86/mm/fault_32.c | 2 +-
Could use exactly the same in fault_64.c
arch/x86/mm/fault_32.c bad_area_nosemaphore: · line 522
Quoted 6 lines
#ifdef
CONFIG_X86_32arch/x86/mm/fault_32.c +1
-1- "%s%s[%d]: segfault at %08lx ip %08lx sp %08lx error %lx\n", + "%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx\n", #else "%s%s[%d]: segfault at %lx ip %lx sp %lx error %lx\n",#endif
With the ongoing unification work, it would be nice if we could come up with a way to unify printks like this. Anyone have any bright ideas on a format that will keep the current alignment on 32 and 64 bit with the same syntax, or will these tiny ifdefs keep sprouting?
Harvey
H. Peter Anvin
reviewerRe: [PATCH] [3/7] Use shorter addresses in i386 segfault printks
Quoted 8 lines · Harvey Harrison
Quoted 4 lines · Andi Kleen
Signed-off-by Andi Kleen <[email protected]>--- arch/x86/mm/fault_32.c | 2 +-Could use exactly the same in
fault_64.c
Quoted 12 lines
arch/x86/mm/fault_32.c bad_area_nosemaphore: · line 522
Quoted 6 lines
#ifdef
CONFIG_X86_32arch/x86/mm/fault_32.c +1
-1- "%s%s[%d]: segfault at %08lx ip %08lx sp %08lx error %lx\n", + "%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx\n", #else "%s%s[%d]: segfault at %lx ip %lx sp %lx error %lx\n",#endif With the ongoing unification work, it would be nice if we could come up with a way to unify printks like this. Anyone have any bright ideas on a format that will keep the current alignment on 32 and 64 bit with the same syntax, or will these tiny ifdefs keep sprouting?
Casting to (void *) and using %p is probably your best bet. That's what it really is anyway.
Note: in the kernel right now, %p doesn't have the leading 0x prefix, which it probably should...
-hpa
Re: [PATCH] [3/7] Use shorter addresses in i386 segfault printks
Quoted 22 lines · H. Peter Anvin
Quoted 8 lines · Harvey Harrison
Quoted 4 lines · Andi Kleen
Signed-off-by Andi Kleen <[email protected]>--- arch/x86/mm/fault_32.c | 2 +-Could use exactly the same in
fault_64.cQuoted 12 lines
arch/x86/mm/fault_32.c bad_area_nosemaphore: · line 522
Quoted 6 lines
#ifdef
CONFIG_X86_32arch/x86/mm/fault_32.c +1
-1- "%s%s[%d]: segfault at %08lx ip %08lx sp %08lx error %lx\n", + "%s%s[%d]: segfault at %lx ip %08lx sp %08lx error %lx\n", #else "%s%s[%d]: segfault at %lx ip %lx sp %lx error %lx\n",#endif With the ongoing unification work, it would be nice if we could come up with a way to unify printks like this. Anyone have any bright ideas on a format that will keep the current alignment on 32 and 64 bit with the same syntax, or will these tiny ifdefs keep sprouting?
Quoted 5 lines
Casting to (void *) and using %p is probably your best bet. That's what it really is anyway.
Note: in the kernel right now, %p doesn't have the leading 0x prefix, which it probably should...
Well, that won't exactly be the nicest looking solution in places, maybe a shorthand could be developed for this, or could another format specifier be added that implicitly does the (void *) cast? (%P perhaps)
Harvey
H. Peter Anvin
reviewerRe: [PATCH] [3/7] Use shorter addresses in i386 segfault printks
Quoted 9 lines · Harvey Harrison
Quoted 4 lines
Quoted 1 line
Casting to (void *) and using %p is probably your best bet. That's what it really is anyway.
Note: in the kernel right now, %p doesn't have the leading 0x prefix, which it probably should...
Well, that won't exactly be the nicest looking solution in places, maybe a shorthand could be developed for this, or could another format specifier be added that implicitly does the (void *) cast? (%P perhaps)
Not without losing the ability of gcc to type-check printk arguments.
-hpa
Re: [PATCH] [3/7] Use shorter addresses in i386 segfault printks
In reply to Harvey HarrisonQuoted 7 lines · On Thursday 17 January 2008 03:58:58 Harvey Harrison wrote:
Quoted 4 lines · Andi Kleen
Signed-off-by Andi Kleen <[email protected]>--- arch/x86/mm/fault_32.c | 2 +-Could use exactly the same in
fault_64.c
Hmm, I had this somewhere but it might have gotten lost during some merge step again.
-Andi
Quoted 3 lines · Andi Kleen
Some are reposts, some are not. See patch descriptions for details. I believe I addressed all feedback that made sense in the reposted patches.
thanks Andi, i've picked them up.
Ingo