[patch 0/3] kallsyms: Add gate page and all symbols support

[patch 0/3] kallsyms: Add gate page and all symbols support

[Keith Owens]

Three patches with the overall aim of improving kernel backtraces using
kallsyms.

1 Clean up the special casing of in_gate_area().

2 Add in_gate_area_no_task() for use from places where no task is valid.

3 Treat the gate page as part of the kernel.  Honour CONFIG_KALLSYMS_ALL.

[patch 1/3] kallsyms: Clean up x86-64 special casing of in_gate_area()

[Keith Owens]

x86-64 has special case code for in_gate_area(), but it is not clean.

* Replace CONFIG_ARCH_GATE_AREA with __HAVE_ARCH_GATE_AREA.
  ARCH_GATE_AREA is not a config option.

* The definitions of get_gate_vma() and in_gate_area() are identical in
  include/asm-x86_64/page.h and include/linux/mm.h.  Fold the duplicate
  definitions into include/linux/mm.h.

Does not affect kallsyms directly, this patch just creates a clean base
for patch 2.

Signed-off-by: Keith Owens <kaos@ocs.com.au>

Index: 2.6.10-rc2-bk11/include/asm-x86_64/page.h
===================================================================
--- 2.6.10-rc2-bk11.orig/include/asm-x86_64/page.h	2004-11-15 14:26:40.778123375 +1100
+++ 2.6.10-rc2-bk11/include/asm-x86_64/page.h	2004-11-28 14:42:13.850863382 +1100
@@ -134,13 +134,7 @@ extern __inline__ int get_order(unsigned
 	(((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0 ) | \
 	 VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
 
-#define CONFIG_ARCH_GATE_AREA 1	
-
-#ifndef __ASSEMBLY__
-struct task_struct;
-struct vm_area_struct *get_gate_vma(struct task_struct *tsk);
-int in_gate_area(struct task_struct *task, unsigned long addr);
-#endif
+#define __HAVE_ARCH_GATE_AREA 1	
 
 #endif /* __KERNEL__ */
 
Index: 2.6.10-rc2-bk11/include/linux/mm.h
===================================================================
--- 2.6.10-rc2-bk11.orig/include/linux/mm.h	2004-11-28 14:41:04.390462272 +1100
+++ 2.6.10-rc2-bk11/include/linux/mm.h	2004-11-28 14:42:13.868860636 +1100
@@ -799,10 +799,8 @@ kernel_map_pages(struct page *page, int 
 }
 #endif
 
-#ifndef CONFIG_ARCH_GATE_AREA
 extern struct vm_area_struct *get_gate_vma(struct task_struct *tsk);
 int in_gate_area(struct task_struct *task, unsigned long addr);
-#endif
 
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
Index: 2.6.10-rc2-bk11/mm/memory.c
===================================================================
--- 2.6.10-rc2-bk11.orig/mm/memory.c	2004-11-28 14:41:04.802399415 +1100
+++ 2.6.10-rc2-bk11/mm/memory.c	2004-11-28 14:42:13.870860331 +1100
@@ -1811,7 +1811,7 @@ unsigned long vmalloc_to_pfn(void * vmal
 
 EXPORT_SYMBOL(vmalloc_to_pfn);
 
-#if !defined(CONFIG_ARCH_GATE_AREA)
+#if !defined(__HAVE_ARCH_GATE_AREA)
 
 #if defined(AT_SYSINFO_EHDR)
 struct vm_area_struct gate_vma;
@@ -1846,4 +1846,4 @@ int in_gate_area(struct task_struct *tas
 	return 0;
 }
 
-#endif
+#endif	/* __HAVE_ARCH_GATE_AREA */

[patch 2/3] kallsyms: Add in_gate_area_no_task()

[Keith Owens]

Add in_gate_area_no_task() for use in places where no task is valid
(e.g. kallsyms).  If you have a valid task, use in_gate_area() as
before.

Signed-off-by: Keith Owens <kaos@ocs.com.au>

Index: 2.6.10-rc2-bk11/arch/x86_64/mm/init.c
===================================================================
--- 2.6.10-rc2-bk11.orig/arch/x86_64/mm/init.c	2004-11-15 14:25:52.154487837 +1100
+++ 2.6.10-rc2-bk11/arch/x86_64/mm/init.c	2004-11-28 14:42:19.525997420 +1100
@@ -623,3 +623,13 @@ int in_gate_area(struct task_struct *tas
 	struct vm_area_struct *vma = get_gate_vma(task);
 	return (addr >= vma->vm_start) && (addr < vma->vm_end);
 }
+
+/* Use this when you have no reliable task/vma, typically from interrupt
+ * context.  It is less reliable than using the task's vma and may give
+ * false positives.
+ */
+int in_gate_area_no_task(unsigned long addr)
+{
+	return ((addr >= VSYSCALL_START) && (addr < VSYSCALL_END) ||
+		(addr >= VSYSCALL32_BASE) && (addr < VSYSCALL32_END));
+}
Index: 2.6.10-rc2-bk11/include/linux/mm.h
===================================================================
--- 2.6.10-rc2-bk11.orig/include/linux/mm.h	2004-11-28 14:42:13.868860636 +1100
+++ 2.6.10-rc2-bk11/include/linux/mm.h	2004-11-28 14:42:19.526997267 +1100
@@ -800,7 +800,13 @@ kernel_map_pages(struct page *page, int 
 #endif
 
 extern struct vm_area_struct *get_gate_vma(struct task_struct *tsk);
+#ifdef	__HAVE_ARCH_GATE_AREA
+int in_gate_area_no_task(unsigned long addr);
 int in_gate_area(struct task_struct *task, unsigned long addr);
+#else
+int in_gate_area_no_task(unsigned long addr);
+#define in_gate_area(task, addr) ({(void)task; in_gate_area_no_task(addr);})
+#endif	/* __HAVE_ARCH_GATE_AREA */
 
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
Index: 2.6.10-rc2-bk11/mm/memory.c
===================================================================
--- 2.6.10-rc2-bk11.orig/mm/memory.c	2004-11-28 14:42:13.870860331 +1100
+++ 2.6.10-rc2-bk11/mm/memory.c	2004-11-28 14:42:19.527997114 +1100
@@ -1837,7 +1837,7 @@ struct vm_area_struct *get_gate_vma(stru
 #endif
 }
 
-int in_gate_area(struct task_struct *task, unsigned long addr)
+int in_gate_area_no_task(unsigned long addr)
 {
 #ifdef AT_SYSINFO_EHDR
 	if ((addr >= FIXADDR_USER_START) && (addr < FIXADDR_USER_END))

[patch 3/3] kallsyms: gate page is part of the kernel, honour CONFIG_KALLSYMS_ALL

[Keith Owens]

* Treat the gate page as part of the kernel, to improve kernel backtraces.

* Honour CONFIG_KALLSYMS_ALL, all symbols are valid, not just text.

Signed-off-by: Keith Owens <kaos@ocs.com.au>

Index: 2.6.10-rc2-bk11/kernel/kallsyms.c
===================================================================
--- 2.6.10-rc2-bk11.orig/kernel/kallsyms.c	2004-11-15 14:26:49.080865854 +1100
+++ 2.6.10-rc2-bk11/kernel/kallsyms.c	2004-11-28 14:42:23.755352067 +1100
@@ -18,6 +18,13 @@
 #include <linux/fs.h>
 #include <linux/err.h>
 #include <linux/proc_fs.h>
+#include <linux/mm.h>
+
+#ifdef CONFIG_KALLSYMS_ALL
+#define all_var 1
+#else
+#define all_var 0
+#endif
 
 /* These will be re-linked against their real values during the second link stage */
 extern unsigned long kallsyms_addresses[] __attribute__((weak));
@@ -30,7 +37,7 @@ extern u16 kallsyms_token_index[] __attr
 extern unsigned long kallsyms_markers[] __attribute__((weak));
 
 /* Defined by the linker script. */
-extern char _stext[], _etext[], _sinittext[], _einittext[];
+extern char _stext[], _etext[], _sinittext[], _einittext[], _end[];
 
 static inline int is_kernel_inittext(unsigned long addr)
 {
@@ -44,7 +51,7 @@ static inline int is_kernel_text(unsigne
 {
 	if (addr >= (unsigned long)_stext && addr <= (unsigned long)_etext)
 		return 1;
-	return 0;
+	return in_gate_area_no_task(addr);
 }
 
 /* expand a compressed symbol data into the resulting uncompressed string,
@@ -147,7 +154,7 @@ const char *kallsyms_lookup(unsigned lon
 	namebuf[KSYM_NAME_LEN] = 0;
 	namebuf[0] = 0;
 
-	if (is_kernel_text(addr) || is_kernel_inittext(addr)) {
+	if (all_var || is_kernel_text(addr) || is_kernel_inittext(addr)) {
 		unsigned long symbol_end=0;
 
 		/* do a binary search on the sorted kallsyms_addresses array */
@@ -181,7 +188,7 @@ const char *kallsyms_lookup(unsigned lon
 			if (is_kernel_inittext(addr))
 				symbol_end = (unsigned long)_einittext;
 			else
-				symbol_end = (unsigned long)_etext;
+				symbol_end = all_var ? (unsigned long)_end : (unsigned long)_etext;
 		}
 
 		*symbolsize = symbol_end - kallsyms_addresses[low];

Re: [patch 0/3] kallsyms: Add gate page and all symbols support

[Sam Ravnborg]

On Sun, Nov 28, 2004 at 02:38:47PM +1100, Keith Owens wrote:
> Three patches with the overall aim of improving kernel backtraces using
> kallsyms.
> 
> 1 Clean up the special casing of in_gate_area().
> 
> 2 Add in_gate_area_no_task() for use from places where no task is valid.
> 
> 3 Treat the gate page as part of the kernel.  Honour CONFIG_KALLSYMS_ALL.
> 

Applied all three patches - thanks.

	Sam

Re: [patch 0/3] kallsyms: Add gate page and all symbols support

[Benjamin Herrenschmidt]

On Tue, 2004-12-28 at 22:17 +0100, Sam Ravnborg wrote:

> > 2 Add in_gate_area_no_task() for use from places where no task is valid.

Can you back that out ? Or at least explain why you need to add this
"no_task" thing and not just use "current" where no task is available ?
I think the above is bogus for 3 reasons:

 - I tend to dislike adding functions
"foo_with_that_parameter_instead_of_that" ... We do it here or there,
but the less we do it, the happier I am :)

 - Since you unconditionally #define in_gate_area() to use
in_gate_area_no_task(), what is the point of having in_gate_area() at
all ? Which rather means, what is the point of adding that "_no_task"
version and not just change in_gate_area to not take a task ?

 - I dislike the fact that you now define the prototype of the function
in the __HAVE_ARCH_GATE_AREA case. I want my arch .h to be the one doing
so, since i want to inline it (to nothing in the ppc64 case since the
vDSO I'm implementing doesn't need any special treatement of the gate
area, it's a normal VMA added to the mm's at exec time).

Ben.

Re: [patch 0/3] kallsyms: Add gate page and all symbols support

[Keith Owens]

On Tue, 18 Jan 2005 18:52:55 +1100, 
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>On Tue, 2004-12-28 at 22:17 +0100, Sam Ravnborg wrote:
>
>> > 2 Add in_gate_area_no_task() for use from places where no task is valid.
>
>Can you back that out ? Or at least explain why you need to add this
>"no_task" thing and not just use "current" where no task is available ?

kallsyms is used to look up a symbol for any task, e.g. to do a
backtrace with symbol lookup of all running tasks, not just the current
one.  None of the kallsym interfaces allow you to specify which task
you are making the query against, and the change to do so is too messy
and intrusive for far too little return.  The no_task variant asks "is
this a possible gate address for any task?", at the small risk of
getting false positives in kallsyms lookup.

> - Since you unconditionally #define in_gate_area() to use
>in_gate_area_no_task(), what is the point of having in_gate_area() at
>all ? Which rather means, what is the point of adding that "_no_task"
>version and not just change in_gate_area to not take a task ?

x86-64 needs both variants.  in_gate_area() is sometimes called in a
context where you know the required task (mm/memory.c), sometimes when
any task is implied (kernel/kallsyms.c).  x86-64 makes it more
complicated by using different gate pages depending on whether the
specified task is in 32 bit emulation mode or not.

> - I dislike the fact that you now define the prototype of the function
>in the __HAVE_ARCH_GATE_AREA case. I want my arch .h to be the one doing
>so, since i want to inline it

Maybe.  I dislike copying definitions to multiple asm headers.  If you
think that the win of inlining the ppc64 version of these functions
outweighs the header duplication then send a patch.  Don't forget to
duplicate the definition in include/asm-x86_64 as well.

>(to nothing in the ppc64 case since the
>vDSO I'm implementing doesn't need any special treatement of the gate
>area, it's a normal VMA added to the mm's at exec time).

Added to specific task's mm or to all tasks?  If the gate VMA varies
according to the task then you have to support the kallsyms "is this a
possible gate address for any task?" question, like x86-64.

Re: [patch 0/3] kallsyms: Add gate page and all symbols support

[Benjamin Herrenschmidt]

> > - I dislike the fact that you now define the prototype of the function
> >in the __HAVE_ARCH_GATE_AREA case. I want my arch .h to be the one doing
> >so, since i want to inline it
> 
> Maybe.  I dislike copying definitions to multiple asm headers.  If you
> think that the win of inlining the ppc64 version of these functions
> outweighs the header duplication then send a patch.  Don't forget to
> duplicate the definition in include/asm-x86_64 as well.

Well, I just want to inline it as en empty function ... 

>(to nothing in the ppc64 case since the
> >vDSO I'm implementing doesn't need any special treatement of the gate
> >area, it's a normal VMA added to the mm's at exec time).
> 
> Added to specific task's mm or to all tasks?  If the gate VMA varies
> according to the task then you have to support the kallsyms "is this a
> possible gate address for any task?" question, like x86-64.

each task mm can have it differently. why would I need kallsym support ?
the ppc64 vDSO support will not be part of the kernel address ranges,
but will be somewhere in the normal userland areas.

Ben.