Re: [PATCH] x86: make SMP locks handling interact properly with CONFIG_DEBUG_RODATA (2nd try)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andi,

I guess the patch I proposed there:
"Text Edit Lock"
http://www.ussg.iu.edu/hypermail/linux/kernel/0707.1/2980.html

solves the problem more generally than the patch discussed in this
thread. It provides locking (architecture independent) and correct
page protection (architecture dependent) for paravirt, alternatives
kprobes and, eventually, immediate values.

Mathieu

* Andi Kleen ([email protected]) wrote:
> On Wednesday 04 July 2007 08:33:24 Jan Beulich wrote:
> 
> Sorry for late feedback.
> 
> 
> > +#ifdef CONFIG_DEBUG_RODATA
> > +
> > +#ifdef CONFIG_X86_32
> > +#include <asm/highmem.h>
> > +#define MODULES_VADDR VMALLOC_START
> > +#endif
> > +
> > +static inline void make_writable(const void *instr, unsigned int len)
> > +{
> > +	unsigned long va = (unsigned long)instr;
> > +
> > +	if (va < MODULES_VADDR) {
> > +		change_page_attr(virt_to_page(instr),
> > +				 PFN_UP(va + len) - PFN_DOWN(va),
> > +				 PAGE_KERNEL_EXEC);
> > +		global_flush_tlb();
> > +	}
> > +}
> > +
> > +static inline void make_readonly(const void *instr, unsigned int len)
> > +{
> > +	unsigned long va = (unsigned long)instr;
> > +
> > +	if (va < MODULES_VADDR) {
> > +		change_page_attr(virt_to_page(instr),
> > +				 PFN_UP(va + len) - PFN_DOWN(va),
> > +#ifdef CONFIG_X86_64
> > +				 PAGE_KERNEL_RO);
> > +#else
> > +				 PAGE_KERNEL_RX);
> > +#endif
> > +		global_flush_tlb();
> > +	}
> > +}
> 
> Can you please move those into the respective pageattr.cs without ifdefs? 
> Also the va < MODULES_VADDR test seems overly magic; while it may work right now
> it seems fragile. It would be better to test the exact ranges in the respective
> architectures
> 
> >  void alternatives_smp_module_del(struct module *mod)
> >  {
> >  	struct smp_alt_module *item;
> > -	unsigned long flags;
> >  
> >  	if (smp_alt_once || noreplace_smp)
> >  		return;
> >  
> > -	spin_lock_irqsave(&smp_alt, flags);
> > +	spin_lock(&smp_alt);
> 
> Unrelated change?  Why? Should probably be separate patch.
> 
> > --- linux-2.6.22-rc7/arch/x86_64/kernel/vmlinux.lds.S	2007-07-03 10:57:39.000000000 +0200
> > +++ 2.6.22-rc7-x86-alt-page-attr/arch/x86_64/kernel/vmlinux.lds.S	2007-07-02 14:40:15.000000000 +0200
> > @@ -131,20 +131,11 @@ SECTIONS
> >    /* might get freed after init */
> >    . = ALIGN(4096);
> >    __smp_alt_begin = .;
> > -  __smp_alt_instructions = .;
> > -  .smp_altinstructions : AT(ADDR(.smp_altinstructions) - LOAD_OFFSET) {
> > -	*(.smp_altinstructions)
> > -  }
> > -  __smp_alt_instructions_end = .;
> > -  . = ALIGN(8);
> >    __smp_locks = .;
> >    .smp_locks : AT(ADDR(.smp_locks) - LOAD_OFFSET) {
> >  	*(.smp_locks)
> >    }
> >    __smp_locks_end = .;
> > -  .smp_altinstr_replacement : AT(ADDR(.smp_altinstr_replacement) - LOAD_OFFSET) {
> > -	*(.smp_altinstr_replacement)
> > -  }
> 
> While ok too it should be also separate please.
> 
> -Andi
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux