[PATCH] utrace: remove exports

[PATCH] utrace: remove exports

[Christoph Hellwig]

All the exports in utrace are totally unused, and not really something
I'd want modules to use anyway :)


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/arch/i386/kernel/ptrace.c
===================================================================
--- linux-2.6.orig/arch/i386/kernel/ptrace.c	2007-04-13 15:56:17.000000000 +0200
+++ linux-2.6/arch/i386/kernel/ptrace.c	2007-04-13 15:56:20.000000000 +0200
@@ -729,7 +729,6 @@ const struct utrace_regset_view utrace_i
 	.name = "i386", .e_machine = EM_386,
 	.regsets = native_regsets, .n = ARRAY_SIZE(native_regsets)
 };
-EXPORT_SYMBOL_GPL(utrace_i386_native);
 
 #ifdef CONFIG_PTRACE
 static const struct ptrace_layout_segment i386_uarea[] = {
Index: linux-2.6/arch/ia64/ia32/sys_ia32.c
===================================================================
--- linux-2.6.orig/arch/ia64/ia32/sys_ia32.c	2007-04-13 15:55:26.000000000 +0200
+++ linux-2.6/arch/ia64/ia32/sys_ia32.c	2007-04-13 15:55:29.000000000 +0200
@@ -2337,7 +2337,6 @@ const struct utrace_regset_view utrace_i
 	.name = "i386", .e_machine = EM_386,
 	.regsets = ia32_regsets, .n = ARRAY_SIZE(ia32_regsets)
 };
-EXPORT_SYMBOL_GPL(utrace_ia32_view);
 
 #ifdef CONFIG_PTRACE
 /*
Index: linux-2.6/arch/ia64/kernel/ptrace.c
===================================================================
--- linux-2.6.orig/arch/ia64/kernel/ptrace.c	2007-04-13 15:55:33.000000000 +0200
+++ linux-2.6/arch/ia64/kernel/ptrace.c	2007-04-13 15:55:35.000000000 +0200
@@ -1547,7 +1547,6 @@ const struct utrace_regset_view utrace_i
 	.e_machine = EM_IA_64,
 	.regsets = native_regsets, .n = ARRAY_SIZE(native_regsets)
 };
-EXPORT_SYMBOL_GPL(utrace_ia64_native);
 
 #ifdef CONFIG_PTRACE
 
Index: linux-2.6/arch/powerpc/kernel/ptrace.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/ptrace.c	2007-04-13 15:56:03.000000000 +0200
+++ linux-2.6/arch/powerpc/kernel/ptrace.c	2007-04-13 15:56:07.000000000 +0200
@@ -346,8 +346,6 @@ const struct utrace_regset_view utrace_p
 	.name = UTS_MACHINE, .e_machine = ELF_ARCH,
 	.regsets = native_regsets, .n = ARRAY_SIZE(native_regsets)
 };
-EXPORT_SYMBOL_GPL(utrace_ppc_native_view);
-
 
 #ifdef CONFIG_PPC64
 #include <linux/compat.h>
@@ -458,7 +456,6 @@ const struct utrace_regset_view utrace_p
 	.name = "ppc", .e_machine = EM_PPC,
 	.regsets = ppc32_regsets, .n = ARRAY_SIZE(ppc32_regsets)
 };
-EXPORT_SYMBOL_GPL(utrace_ppc32_view);
 #endif
 
 
Index: linux-2.6/arch/s390/kernel/ptrace.c
===================================================================
--- linux-2.6.orig/arch/s390/kernel/ptrace.c	2007-04-13 15:55:07.000000000 +0200
+++ linux-2.6/arch/s390/kernel/ptrace.c	2007-04-13 15:55:14.000000000 +0200
@@ -315,8 +315,6 @@ const struct utrace_regset_view utrace_s
 	.name = UTS_MACHINE, .e_machine = ELF_ARCH,
 	.regsets = native_regsets, .n = ARRAY_SIZE(native_regsets)
 };
-EXPORT_SYMBOL_GPL(utrace_s390_native_view);
-
 
 #ifdef CONFIG_COMPAT
 static int
@@ -564,7 +562,6 @@ const struct utrace_regset_view utrace_s
 	.name = "s390", .e_machine = EM_S390,
 	.regsets = s390_compat_regsets, .n = ARRAY_SIZE(s390_compat_regsets)
 };
-EXPORT_SYMBOL_GPL(utrace_s390_compat_view);
 #endif	/* CONFIG_COMPAT */
 
 
Index: linux-2.6/arch/sparc64/kernel/ptrace.c
===================================================================
--- linux-2.6.orig/arch/sparc64/kernel/ptrace.c	2007-04-13 15:55:45.000000000 +0200
+++ linux-2.6/arch/sparc64/kernel/ptrace.c	2007-04-13 15:55:53.000000000 +0200
@@ -258,7 +258,6 @@ const struct utrace_regset_view utrace_s
 	.name = UTS_MACHINE, .e_machine = ELF_ARCH,
 	.regsets = native_regsets, .n = ARRAY_SIZE(native_regsets)
 };
-EXPORT_SYMBOL_GPL(utrace_sparc64_native_view);
 
 #ifdef CONFIG_COMPAT
 
@@ -596,7 +595,6 @@ const struct utrace_regset_view utrace_s
 	.name = "sparc", .e_machine = EM_SPARC,
 	.regsets = sparc32_regsets, .n = ARRAY_SIZE(sparc32_regsets)
 };
-EXPORT_SYMBOL_GPL(utrace_sparc32_view);
 
 #endif	/* CONFIG_COMPAT */
 
Index: linux-2.6/arch/x86_64/ia32/ptrace32.c
===================================================================
--- linux-2.6.orig/arch/x86_64/ia32/ptrace32.c	2007-04-13 15:54:42.000000000 +0200
+++ linux-2.6/arch/x86_64/ia32/ptrace32.c	2007-04-13 15:54:46.000000000 +0200
@@ -596,8 +596,6 @@ const struct utrace_regset_view utrace_i
 	.name = "i386", .e_machine = EM_386,
 	.regsets = ia32_regsets, .n = ARRAY_SIZE(ia32_regsets)
 };
-EXPORT_SYMBOL_GPL(utrace_ia32_view);
-
 
 #ifdef CONFIG_PTRACE
 /*
Index: linux-2.6/arch/x86_64/kernel/ptrace.c
===================================================================
--- linux-2.6.orig/arch/x86_64/kernel/ptrace.c	2007-04-13 15:54:50.000000000 +0200
+++ linux-2.6/arch/x86_64/kernel/ptrace.c	2007-04-13 15:54:54.000000000 +0200
@@ -703,8 +703,6 @@ const struct utrace_regset_view utrace_x
 	.name = "x86-64", .e_machine = EM_X86_64,
 	.regsets = native_regsets, .n = ARRAY_SIZE(native_regsets)
 };
-EXPORT_SYMBOL_GPL(utrace_x86_64_native);
-
 
 #ifdef CONFIG_PTRACE
 static const struct ptrace_layout_segment x86_64_uarea[] = {
Index: linux-2.6/kernel/utrace.c
===================================================================
--- linux-2.6.orig/kernel/utrace.c	2007-04-13 15:56:28.000000000 +0200
+++ linux-2.6/kernel/utrace.c	2007-04-13 15:56:39.000000000 +0200
@@ -490,7 +490,6 @@ restart:
 
 	return engine;
 }
-EXPORT_SYMBOL_GPL(utrace_attach);
 
 /*
  * When an engine is detached, the target thread may still see it and make
@@ -700,8 +699,6 @@ utrace_detach(struct task_struct *target
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(utrace_detach);
-
 
 /*
  * Called with utrace->lock held.
@@ -900,8 +897,7 @@ restart:			/* See below. */
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(utrace_set_flags);
-\f
+
 /*
  * While running an engine callback, no locks are held.
  * If a callback updates its engine's action state, then
@@ -1930,8 +1926,6 @@ utrace_inject_signal(struct task_struct 
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(utrace_inject_signal);
-
 
 const struct utrace_regset *
 utrace_regset(struct task_struct *target,
@@ -1946,8 +1940,6 @@ utrace_regset(struct task_struct *target
 
 	return &view->regsets[which];
 }
-EXPORT_SYMBOL_GPL(utrace_regset);
-
 
 /*
  * Return the task_struct for the task using ptrace on this one, or NULL.

Re: [PATCH] utrace: remove exports

[Jim Keniston]

Christoph Hellwig wrote:
> 
> All the exports in utrace are totally unused, and not really something
> I'd want modules to use anyway :)
> 

Please leave the exports in place.

Very early in Documentation/utrace.txt, it says:
"The UTRACE is infrastructure code for tracing and controlling user
threads.  This is the foundation for writing tracing engines, which
can be loadable kernel modules."

If we can't use utrace to write ad hoc instrumentation modules (i.e.,
because utrace_attach(), utrace_detach(), etc. are no longer exported),
then utrace's usefulness is greatly reduced.

Jim Keniston
IBM LTC

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
...
> Index: linux-2.6/kernel/utrace.c
> ===================================================================
> --- linux-2.6.orig/kernel/utrace.c	2007-04-13 15:56:28.000000000 +0200
> +++ linux-2.6/kernel/utrace.c	2007-04-13 15:56:39.000000000 +0200
> @@ -490,7 +490,6 @@ restart:
> 
>  	return engine;
>  }
> -EXPORT_SYMBOL_GPL(utrace_attach);
> 
>  /*
>   * When an engine is detached, the target thread may still see it and make
> @@ -700,8 +699,6 @@ utrace_detach(struct task_struct *target
> 
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(utrace_detach);
> -
> 
>  /*
>   * Called with utrace->lock held.
> @@ -900,8 +897,7 @@ restart:			/* See below. */
> 
>  	return ret;
>  }
> -EXPORT_SYMBOL_GPL(utrace_set_flags);
> -
> +
>  /*
>   * While running an engine callback, no locks are held.
>   * If a callback updates its engine's action state, then
> @@ -1930,8 +1926,6 @@ utrace_inject_signal(struct task_struct 
> 
>  	return ret;
>  }
> -EXPORT_SYMBOL_GPL(utrace_inject_signal);
> -
> 
>  const struct utrace_regset *
>  utrace_regset(struct task_struct *target,
> @@ -1946,8 +1940,6 @@ utrace_regset(struct task_struct *target
> 
>  	return &view->regsets[which];
>  }
> -EXPORT_SYMBOL_GPL(utrace_regset);
> -
> 
>  /*
>   * Return the task_struct for the task using ptrace on this one, or NULL.
> -

Re: [PATCH] utrace: remove exports

[Roland McGrath]

> All the exports in utrace are totally unused, and not really something
> I'd want modules to use anyway :)

Previously you said:

> EXPORT_SYMBOL_GPL(utrace_attach);
> 
> 	There is not modular user of this, so this and the other utrace_
> 	functions should not be exported.  Nor do I think that exporting
> 	such a low-level process control is nessecary a good idea, but
> 	we'll have to evaluate that if patches to add users show up.

If you remove the exports, just throw the whole thing away.
The reason utrace exists is to be used by modules.

It's true that the only users around are some simple test and demo
modules I've done, and some unfinished and experimental work such as
uprobes.  When useful things built on utrace get hashed out and in a
functional state ready for general consumption, then it may be time to
put them in the kernel tree.  (All the work I'm aware of is pretty well
public already, just not in any condition to be submitted.)

There is something of a chicken and egg problem here.  In the current
kernel, most people consider adding or enhancing a user debugging
facility to be intractable.  The intent of utrace is to make
experimenting and developing in this area feasible.  If it's not there,
people can't play with it.  If someone has to have a serious use before
it can be there, it's never going to be easy for anyone to get their
feet wet so they might wind up producing something serious.  

I hope there settles to be one main, nice, new thing at a higher level
with a sane user-level interface, that most everyone wants to use.
Manifestly, I have been taking my sweet time coming up with something
worth talking about so far.  Even when I do produce something like that,
I don't expect everyone to like everything about it that I like.
Someone else might produce something better or sooner or both--but noone
else seems to want to do something else new at the lowest layer.  The
basic utrace interface is about as straightforward in the core kernel
implementation context as it can be while allowing multiple unrelated
higher-level agents to coexist, and cleaning up the interfaces into
machine-dependent code.  The complexities it does have stem from efforts
to make it easier not to write bugs in the higher layers and to make it
harder to write bugs that badly derail user processes or the system
overall.  The hope is to give people a chance to write interesting
things and mitigate some of the most onerous debugging of such things.
Perhaps one day everything anyone wants to do will be based around a
higher-level thing by me or someone else, and the need for the low-level
utrace platform will fall away.  But I tend to think there will always
be some specialized uses for which people want a lower-level base to
work on.  I think of utrace like the block device layer: most people
don't care about that layer, they just want to use a filesystem and
other things above that; but there are always people who want to do some
special thing directly on the lower level, or want to start over
building an entirely different style of filesystem.


Thanks,
Roland

Re: [PATCH] utrace: remove exports

[Christoph Hellwig]

On Sun, Apr 29, 2007 at 09:02:32PM -0700, Roland McGrath wrote:
> > EXPORT_SYMBOL_GPL(utrace_attach);
> > 
> > 	There is not modular user of this, so this and the other utrace_
> > 	functions should not be exported.  Nor do I think that exporting
> > 	such a low-level process control is nessecary a good idea, but
> > 	we'll have to evaluate that if patches to add users show up.
> 
> If you remove the exports, just throw the whole thing away.
> The reason utrace exists is to be used by modules.

used != used by modules.  Even without things exported you can use
things in the kernel.  Once we have a valid modular use case we can
add exports.

Re: [PATCH] utrace: remove exports

[Alexey Dobriyan]

On Mon, Apr 30, 2007 at 10:12:44AM +0100, Christoph Hellwig wrote:
> On Sun, Apr 29, 2007 at 09:02:32PM -0700, Roland McGrath wrote:
> > > EXPORT_SYMBOL_GPL(utrace_attach);
> > > 
> > > 	There is not modular user of this, so this and the other utrace_
> > > 	functions should not be exported.  Nor do I think that exporting
> > > 	such a low-level process control is nessecary a good idea, but
> > > 	we'll have to evaluate that if patches to add users show up.
> > 
> > If you remove the exports, just throw the whole thing away.
> > The reason utrace exists is to be used by modules.
> 
> used != used by modules.  Even without things exported you can use
> things in the kernel.  Once we have a valid modular use case we can
> add exports.

Christoph, it's at
http://people.redhat.com/roland/utrace/crash-suspend.c

The file crash-suspend.c is a simple example of a loadable kernel module
using the new infrastructure.  It implements the old GNU feature of doing a
job suspend on crash, so you can attach a debugger right then instead of
just looking at the core file afterwards.  Try for example:
	$ sudo /sbin/insmod crash-suspend.ko pid=$$
to load the module and attach it to your shell.  It then attaches to any
children of your shell.  Now run something that crashes and would
ordinarily dump core--you can just run cat or sleep and hit ^\ to crash it
with a SIGQUIT.  See it suspend.  Try "jobs -l" and see it stopped with a
non-stop signal.  If you resume it with "fg" it will go ahead and dump
core.  Or, you can attach a debugger like gdb first, and then debug it.



Most certainly, it should be included into utrace patchset to pin some
exports.